test: add skill validation tests; fix stale make-target refs in skills
CI / validate (pull_request) Failing after 2m1s
CI / molecule-tests (1) (pull_request) Skipped
CI / molecule-tests (2) (pull_request) Skipped
CI / molecule-tests (3) (pull_request) Skipped
CI / molecule-tests (4) (pull_request) Skipped
CI / auto-merge (pull_request) Skipped
CI / validate (pull_request) Failing after 2m1s
CI / molecule-tests (1) (pull_request) Skipped
CI / molecule-tests (2) (pull_request) Skipped
CI / molecule-tests (3) (pull_request) Skipped
CI / molecule-tests (4) (pull_request) Skipped
CI / auto-merge (pull_request) Skipped
- New tests/unit/test_skills_validation.py: structure, make-target, file-ref checks + existence tests for all 10 skills - testing-and-debugging: fix molecule count (6->7), drop molecule-all-parallel/pre-push/lint-ci refs (not grm targets), add When to Invoke + Prerequisites - devx-workflow: fix rebase targets (devx-rebase/devx-pr-rebase), add When to Invoke + Prerequisites - spec-driven-development: add When to Invoke + Prerequisites
This commit is contained in:
@@ -2,6 +2,16 @@
|
|||||||
|
|
||||||
Quick reference for devx tools when working on this repo.
|
Quick reference for devx tools when working on this repo.
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when creating PRs, checking CI status, adding
|
||||||
|
labels, rebasing branches, or performing any PR lifecycle operation.
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.venv` exists (run `make setup` if not)
|
||||||
|
- `.env` with `DEVELOPER_GITEA_API_TOKEN`, `VIKUNJA_TOKEN`
|
||||||
|
|
||||||
## PR Workflow (use these, not raw git/tea/MCP)
|
## PR Workflow (use these, not raw git/tea/MCP)
|
||||||
|
|
||||||
| Task | Command |
|
| Task | Command |
|
||||||
@@ -12,8 +22,8 @@ Quick reference for devx tools when working on this repo.
|
|||||||
| Check CI status | `make devx-pr-status` or `make devx-pr-status PR=42 WAIT=1` |
|
| Check CI status | `make devx-pr-status` or `make devx-pr-status PR=42 WAIT=1` |
|
||||||
| Fetch CI failure logs | `make devx-pr-logs` or `make devx-pr-logs PR=42 JOB=quality TAIL=50` |
|
| Fetch CI failure logs | `make devx-pr-logs` or `make devx-pr-logs PR=42 JOB=quality TAIL=50` |
|
||||||
| Add ready-to-merge label | `make devx-pr-label` or `make devx-pr-label PR=42` |
|
| Add ready-to-merge label | `make devx-pr-label` or `make devx-pr-label PR=42` |
|
||||||
| Rebase current branch | `make rebase` |
|
| Rebase current branch | `make devx-rebase` |
|
||||||
| Rebase PR via API | `make pr-rebase` or `make pr-rebase PR=42` |
|
| Rebase PR via API | `make devx-pr-rebase` or `make pr-rebase PR=42` |
|
||||||
|
|
||||||
## Auto-merge Behavior
|
## Auto-merge Behavior
|
||||||
|
|
||||||
|
|||||||
@@ -1,5 +1,14 @@
|
|||||||
# Spec-Driven Development
|
# Spec-Driven Development
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when starting any change — every PR requires a spec
|
||||||
|
at `docs/specs/<TASK-ID>.md` that CI validates before merge.
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- A Vikunja task ID (`GRM-N`) — see `vikunja-tasks` skill
|
||||||
|
|
||||||
## Overview
|
## Overview
|
||||||
|
|
||||||
Every change starts with a spec. No spec, no code. No code, no PR.
|
Every change starts with a spec. No spec, no code. No code, no PR.
|
||||||
|
|||||||
@@ -3,6 +3,17 @@
|
|||||||
Make targets for testing, debugging, and CI investigation. **Use these
|
Make targets for testing, debugging, and CI investigation. **Use these
|
||||||
instead of raw `pytest`, `ruff`, or `molecule` commands.**
|
instead of raw `pytest`, `ruff`, or `molecule` commands.**
|
||||||
|
|
||||||
|
## When to Invoke
|
||||||
|
|
||||||
|
Invoke this skill when running tests, investigating CI failures, or
|
||||||
|
debugging molecule scenarios. Also invoke when asked to "run tests",
|
||||||
|
"check coverage", or "debug a failure".
|
||||||
|
|
||||||
|
## Prerequisites
|
||||||
|
|
||||||
|
- `.venv` exists (run `make setup` if not)
|
||||||
|
- For molecule tests: Docker is running
|
||||||
|
|
||||||
## Why Make Targets
|
## Why Make Targets
|
||||||
|
|
||||||
Make targets encapsulate the correct venv activation, PYTHONPATH, env
|
Make targets encapsulate the correct venv activation, PYTHONPATH, env
|
||||||
@@ -31,9 +42,8 @@ produces false failures (missing dependencies, wrong Python version).
|
|||||||
|
|
||||||
| Task | Command | Notes |
|
| Task | Command | Notes |
|
||||||
|------|---------|-------|
|
|------|---------|-------|
|
||||||
| All scenarios | `make molecule` | All 6 scenarios on Ubuntu 22.04 |
|
| All scenarios | `make molecule` | All 7 scenarios on Ubuntu 22.04 |
|
||||||
| All platforms | `make molecule-all` | All 6 scenarios on all 4 OSes |
|
| All platforms | `make molecule-all` | All 7 scenarios on all 4 OSes |
|
||||||
| Parallel | `make molecule-all-parallel` | MOLECULE_JOBS=4 |
|
|
||||||
|
|
||||||
### Spec-Driven Workflow
|
### Spec-Driven Workflow
|
||||||
|
|
||||||
@@ -46,12 +56,12 @@ CI validates the spec before running expensive jobs.
|
|||||||
**Before pushing any branch:**
|
**Before pushing any branch:**
|
||||||
|
|
||||||
```bash
|
```bash
|
||||||
make pre-push
|
make lint-all && make pytest-cov
|
||||||
```
|
```
|
||||||
|
|
||||||
This runs `lint-all` + `pytest-cov`. The pre-push git hook only
|
This runs all linters + unit tests with coverage. The pre-push git
|
||||||
validates the Vikunja task exists — it does NOT run tests. You must
|
hook only validates the Vikunja task exists — it does NOT run tests.
|
||||||
run `make pre-push` manually.
|
Run the checks manually (there is no `pre-push` target here).
|
||||||
|
|
||||||
## CI Failure Investigation
|
## CI Failure Investigation
|
||||||
|
|
||||||
@@ -59,7 +69,7 @@ When investigating a CI failure:
|
|||||||
|
|
||||||
1. **Fetch logs via MCP** — use `mcp_call_tool` with gitea server,
|
1. **Fetch logs via MCP** — use `mcp_call_tool` with gitea server,
|
||||||
`actions_run_read` method, `download_job_log` tool
|
`actions_run_read` method, `download_job_log` tool
|
||||||
2. **Reproduce locally** — use `make pytest-cov` or `make lint-ci`
|
2. **Reproduce locally** — use `make pytest-cov` or `make lint-all`
|
||||||
depending on which CI job failed
|
depending on which CI job failed
|
||||||
3. **Never run raw pytest** — always use the make target
|
3. **Never run raw pytest** — always use the make target
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,120 @@
|
|||||||
|
"""Pytest tests for Devin skill validation.
|
||||||
|
|
||||||
|
Validates that all skills in .devin/skills/ are well-formed: H1 title,
|
||||||
|
"when to invoke" section, prerequisites when commands are referenced,
|
||||||
|
make-target references that exist, and file references that exist.
|
||||||
|
|
||||||
|
Run with: make pytest TEST=tests/test_skills_validation.py
|
||||||
|
"""
|
||||||
|
|
||||||
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import re
|
||||||
|
from pathlib import Path
|
||||||
|
|
||||||
|
import pytest
|
||||||
|
|
||||||
|
REPO_ROOT = Path(__file__).resolve().parents[2]
|
||||||
|
|
||||||
|
# Sections required for every skill
|
||||||
|
REQUIRED_SECTIONS = ["when to invoke"]
|
||||||
|
|
||||||
|
# Sections required only for skills that reference commands/tools
|
||||||
|
COMMAND_REQUIRED_SECTIONS = ["prerequisites"]
|
||||||
|
|
||||||
|
# Markers indicating a skill references commands/tools
|
||||||
|
COMMAND_MARKERS = ("`make ", "```bash", "```sh", "curl ", "python ", "python3 ", "ssh ")
|
||||||
|
|
||||||
|
EXPECTED_SKILLS = [
|
||||||
|
"dependency-graph",
|
||||||
|
"deployment-coordination",
|
||||||
|
"devx-workflow",
|
||||||
|
"molecule-testing",
|
||||||
|
"pr-review",
|
||||||
|
"runner-ops",
|
||||||
|
"skill-creation",
|
||||||
|
"spec-driven-development",
|
||||||
|
"testing-and-debugging",
|
||||||
|
"vikunja-tasks",
|
||||||
|
]
|
||||||
|
|
||||||
|
|
||||||
|
def _find_skills() -> dict[str, Path]:
|
||||||
|
skills_dir = REPO_ROOT / ".devin" / "skills"
|
||||||
|
assert skills_dir.exists(), ".devin/skills/ directory not found"
|
||||||
|
return {d.name: d / "SKILL.md" for d in skills_dir.iterdir() if d.is_dir() and (d / "SKILL.md").exists()}
|
||||||
|
|
||||||
|
|
||||||
|
# Skills shared with other repos — file-path references are only checked
|
||||||
|
# in the owning repo (infra), where the referenced files live.
|
||||||
|
SHARED_SKILLS = {"cross-repo-sync", "branch-hygiene", "dependency-graph", "skill-creation"}
|
||||||
|
|
||||||
|
|
||||||
|
def _make_targets() -> set[str]:
|
||||||
|
"""Collect make targets from Makefile plus included devx .mak files."""
|
||||||
|
targets: set[str] = set()
|
||||||
|
makefile = REPO_ROOT / "Makefile"
|
||||||
|
if makefile.exists():
|
||||||
|
targets.update(re.findall(r"^([a-zA-Z][a-zA-Z0-9_-]*):", makefile.read_text(), re.MULTILINE))
|
||||||
|
for mak in REPO_ROOT.glob(".venv/lib/python*/site-packages/devx/make/*.mak"):
|
||||||
|
targets.update(re.findall(r"^([a-zA-Z][a-zA-Z0-9_-]*):", mak.read_text(), re.MULTILINE))
|
||||||
|
return targets
|
||||||
|
|
||||||
|
|
||||||
|
def _validate_skill(skill_name: str, skill_path: Path, make_targets: set[str]) -> list[str]:
|
||||||
|
"""Validate a single skill file. Returns list of error messages."""
|
||||||
|
errors: list[str] = []
|
||||||
|
content = skill_path.read_text()
|
||||||
|
|
||||||
|
if not re.search(r"^# ", content, re.MULTILINE):
|
||||||
|
errors.append(f"{skill_name}: missing H1 title")
|
||||||
|
|
||||||
|
lower = content.lower()
|
||||||
|
for section in REQUIRED_SECTIONS:
|
||||||
|
if f"## {section}" not in lower:
|
||||||
|
errors.append(f"{skill_name}: missing '## {section.title()}' section")
|
||||||
|
|
||||||
|
references_commands = any(marker in content for marker in COMMAND_MARKERS)
|
||||||
|
if references_commands:
|
||||||
|
for section in COMMAND_REQUIRED_SECTIONS:
|
||||||
|
if f"## {section}" not in lower:
|
||||||
|
errors.append(
|
||||||
|
f"{skill_name}: missing '## {section.title()}' section "
|
||||||
|
"(required because skill references commands/tools)"
|
||||||
|
)
|
||||||
|
|
||||||
|
for target in re.findall(r"`make ([a-zA-Z][a-zA-Z0-9_-]*)`", content):
|
||||||
|
if target not in make_targets:
|
||||||
|
errors.append(f"{skill_name}: references `make {target}` but target does not exist")
|
||||||
|
|
||||||
|
# File-path checks: skip shared skills (checked in infra) and
|
||||||
|
# placeholder paths containing <...> templates.
|
||||||
|
if skill_name not in SHARED_SKILLS:
|
||||||
|
for match in re.findall(r"`((?:scripts|src|ansible|docs|tests|environments)/[^`\s]+)`", content):
|
||||||
|
if "<" in match:
|
||||||
|
continue
|
||||||
|
if not (REPO_ROOT / match).exists():
|
||||||
|
errors.append(f"{skill_name}: references `{match}` but file does not exist")
|
||||||
|
|
||||||
|
return errors
|
||||||
|
|
||||||
|
|
||||||
|
@pytest.mark.parametrize("skill_name", EXPECTED_SKILLS)
|
||||||
|
def test_skill_exists(skill_name: str) -> None:
|
||||||
|
"""Each expected skill must have a SKILL.md."""
|
||||||
|
skill = REPO_ROOT / ".devin" / "skills" / skill_name / "SKILL.md"
|
||||||
|
assert skill.exists(), f"{skill_name}/SKILL.md not found"
|
||||||
|
|
||||||
|
|
||||||
|
def test_minimum_skill_count() -> None:
|
||||||
|
"""The repo should carry a working set of skills, not a stub."""
|
||||||
|
assert len(_find_skills()) >= 8, "expected >=10 skills"
|
||||||
|
|
||||||
|
|
||||||
|
def test_all_skills_validate() -> None:
|
||||||
|
"""All skills must pass structure/reference validation."""
|
||||||
|
make_targets = _make_targets()
|
||||||
|
errors: list[str] = []
|
||||||
|
for skill_name, skill_path in _find_skills().items():
|
||||||
|
errors.extend(_validate_skill(skill_name, skill_path, make_targets))
|
||||||
|
assert not errors, "Skill validation failed:\n" + "\n".join(f" - {e}" for e in errors)
|
||||||
Reference in New Issue
Block a user