Public Access
DEVX-163: docs: add vikunja-tasks skill and skill validation tests, fix create-task docs
This commit was merged in pull request #323.
This commit is contained in:
@@ -2,11 +2,21 @@
|
||||
|
||||
Quick reference for devx tools when working on the devx repo itself.
|
||||
|
||||
## 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)
|
||||
|
||||
| Task | Command |
|
||||
|------|---------|
|
||||
| Create Vikunja task | `make create-task -- --title "..." --description "..."` |
|
||||
| Create Vikunja task | `.venv/bin/python -m devx.tools.create_task --title "..." --description "..."` (make target doesn't forward args) |
|
||||
| Create PR | `make create-pr` |
|
||||
| Push + create PR | `make push-with-pr` |
|
||||
| Check CI status | `make devx-pr-status` or `make devx-pr-status PR=42 WAIT=1` |
|
||||
|
||||
@@ -1,5 +1,14 @@
|
||||
# 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 (`DEVX-N`) — see `vikunja-tasks` skill
|
||||
|
||||
## Overview
|
||||
|
||||
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
|
||||
instead of raw `pytest`, `ruff`, or `actionlint` commands.**
|
||||
|
||||
## When to Invoke
|
||||
|
||||
Invoke this skill when running tests, investigating CI failures, or
|
||||
linting before push. Also invoke when asked to "run tests", "check
|
||||
coverage", or "debug a failure".
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- `.venv` exists (run `make setup` if not)
|
||||
- Tools installed (run `make install-tools` for actionlint/act_runner)
|
||||
|
||||
## Why Make Targets
|
||||
|
||||
Make targets encapsulate the correct venv activation, PYTHONPATH, env
|
||||
|
||||
@@ -0,0 +1,74 @@
|
||||
# vikunja-tasks
|
||||
|
||||
Vikunja task lifecycle beyond `create`: querying status, closing, and
|
||||
recovering when the tracker is unreachable.
|
||||
|
||||
## When to Invoke
|
||||
|
||||
- Creating, closing, or checking a Vikunja task
|
||||
- A spec workflow step needs the task ID or done state
|
||||
- `vikunja.oblachno.oblachno.fyi` fails to resolve / times out
|
||||
|
||||
## Prerequisites
|
||||
|
||||
- `.env` with `VIKUNJA_TOKEN`
|
||||
- Project ID comes from `[tool.devx]` in `pyproject.toml`
|
||||
(`DEVX_VIKUNJA_PROJECT_ID`)
|
||||
|
||||
## Create
|
||||
|
||||
`make create-task` does **not** forward arguments — call the module:
|
||||
|
||||
```bash
|
||||
.venv/bin/python -m devx.tools.create_task \
|
||||
--title "Task title (no DEVX-N prefix)" \
|
||||
--description "<h2>Context</h2><p>...</p>"
|
||||
```
|
||||
|
||||
Prints `DEVX-N` + next steps. Title must not include the task-ID
|
||||
prefix (auto-merge prepends it; a manual prefix double-prefixes the
|
||||
PR title and fails validation).
|
||||
|
||||
## Query / Close
|
||||
|
||||
```bash
|
||||
# Task details (ID = numeric part of DEVX-N)
|
||||
curl -sf -H "Authorization: Bearer $VIKUNJA_TOKEN" \
|
||||
"https://vikunja.oblachno.oblachno.fyi/api/v1/tasks/<N>"
|
||||
|
||||
# Close: mark done
|
||||
curl -sf -X POST -H "Authorization: Bearer $VIKUNJA_TOKEN" \
|
||||
-H "Content-Type: application/json" -d '{"done":true}' \
|
||||
"https://vikunja.oblachno.oblachno.fyi/api/v1/tasks/<N>"
|
||||
```
|
||||
|
||||
Post-merge automation marks the task done when the PR squash-merges —
|
||||
manual close is only needed for abandoned/superseded tasks.
|
||||
|
||||
## Task-ID / Spec Collisions
|
||||
|
||||
Vikunja IDs can collide with historical spec files (an old task reused
|
||||
the number). Convention: preserve the old file as
|
||||
`docs/specs/<ID>-<topic>-historical.md`, then write the new spec at
|
||||
`docs/specs/<ID>.md`. Check `git log` on the existing spec before
|
||||
moving it.
|
||||
|
||||
## Tracker Unreachable
|
||||
|
||||
If the Vikunja host fails DNS/TLS:
|
||||
|
||||
1. Don't block the whole workflow — record the intended task title in
|
||||
the spec draft and retry `create_task` before branching.
|
||||
2. Never invent an ID — branch/PR titles must match a real task or
|
||||
`pre_push_check` / auto-merge validation fails.
|
||||
3. DNS failures observed so far were transient; retry after a few
|
||||
minutes before escalating.
|
||||
|
||||
## Common Mistakes
|
||||
|
||||
- `make create-task -- --title ...` — args are dropped; use the module
|
||||
call above (forwarding fix is S11 scope).
|
||||
- Including `DEVX-N:` in the task title — double prefix breaks
|
||||
auto-merge.
|
||||
- Closing a task whose PR is still open — auto-merge's post-merge
|
||||
step handles the close; manual close confuses the audit trail.
|
||||
@@ -0,0 +1,33 @@
|
||||
# DEVX-163: Fix _run_push to check stdout for HTTP 500
|
||||
|
||||
## Problem
|
||||
`_run_push` only checked `result.stderr` for HTTP 500, but docker push
|
||||
sends the "received unexpected HTTP status: 500 Internal Server Error"
|
||||
message to **stdout**, not stderr. This means the tenacity retry logic
|
||||
added in DEVX-162 never triggered — the push failed immediately without
|
||||
retrying.
|
||||
|
||||
## Approach
|
||||
Check both `result.stdout` and `result.stderr` for the "500" status code.
|
||||
Also update the "already exists" check in `push_image` to check both
|
||||
streams, since docker may send that message to stdout as well.
|
||||
|
||||
REQ-1: _run_push checks both stdout and stderr for HTTP 500
|
||||
REQ-2: push_image "already exists" check uses combined stdout+stderr
|
||||
REQ-3: All existing tests pass with 100% coverage
|
||||
|
||||
## Test Plan
|
||||
- Unit tests for stdout 500 detection
|
||||
- Unit tests for stderr 500 detection
|
||||
- Manual: trigger build-images workflow and verify retry works
|
||||
|
||||
## Deploy Plan
|
||||
- Merge to master
|
||||
|
||||
## Rollback Plan
|
||||
- Revert the merge commit
|
||||
|
||||
## Acceptance Criteria
|
||||
- [x] REQ-1: _run_push checks both stdout and stderr for HTTP 500
|
||||
- [x] REQ-2: push_image "already exists" check uses combined stdout+stderr
|
||||
- [x] REQ-3: All existing tests pass with 100% coverage
|
||||
+33
-20
@@ -1,33 +1,46 @@
|
||||
# DEVX-163: Fix _run_push to check stdout for HTTP 500
|
||||
# DEVX-163: Add vikunja-tasks skill and skill validation tests, fix create-task docs
|
||||
|
||||
## Problem
|
||||
`_run_push` only checked `result.stderr` for HTTP 500, but docker push
|
||||
sends the "received unexpected HTTP status: 500 Internal Server Error"
|
||||
message to **stdout**, not stderr. This means the tenacity retry logic
|
||||
added in DEVX-162 never triggered — the push failed immediately without
|
||||
retrying.
|
||||
|
||||
The OBL-INFRA-548 programme audit found devx lacks a Vikunja
|
||||
task-lifecycle skill and has no skill validation tests (infra and
|
||||
sso-bridge have them; grm gained them under GRM-171).
|
||||
`devx-workflow` documents `make create-task -- --title`, which fails
|
||||
because `devx-create-task` forwards no arguments.
|
||||
|
||||
## Approach
|
||||
Check both `result.stdout` and `result.stderr` for the "500" status code.
|
||||
Also update the "already exists" check in `push_image` to check both
|
||||
streams, since docker may send that message to stdout as well.
|
||||
|
||||
REQ-1: _run_push checks both stdout and stderr for HTTP 500
|
||||
REQ-2: push_image "already exists" check uses combined stdout+stderr
|
||||
REQ-3: All existing tests pass with 100% coverage
|
||||
REQ-1: Add `vikunja-tasks` skill: create via module call, query,
|
||||
close, spec-collision convention, unreachable-tracker handling.
|
||||
REQ-2: Add `tests/unit/test_skills_validation.py` covering
|
||||
structure, make-target, file-ref checks + existence tests for all
|
||||
skills.
|
||||
REQ-3: Fix broken `make create-task -- --title` documentation in
|
||||
`devx-workflow` skill; add missing When to Invoke / Prerequisites
|
||||
sections to older-format skills.
|
||||
|
||||
Preserve the colliding spec as
|
||||
[DEVX-163-run-push-stdout-historical](DEVX-163-run-push-stdout-historical.md).
|
||||
|
||||
## Test Plan
|
||||
- Unit tests for stdout 500 detection
|
||||
- Unit tests for stderr 500 detection
|
||||
- Manual: trigger build-images workflow and verify retry works
|
||||
|
||||
- `pytest tests/unit/test_skills_validation.py` passes (10 tests).
|
||||
|
||||
## Deploy Plan
|
||||
- Merge to master
|
||||
|
||||
Documentation/skills only — auto-merge to master; no runtime deploy.
|
||||
|
||||
## Rollback Plan
|
||||
- Revert the merge commit
|
||||
|
||||
Revert the squash-merge commit; skills are inert documentation.
|
||||
|
||||
## Acceptance Criteria
|
||||
- [x] REQ-1: _run_push checks both stdout and stderr for HTTP 500
|
||||
- [x] REQ-2: push_image "already exists" check uses combined stdout+stderr
|
||||
- [x] REQ-3: All existing tests pass with 100% coverage
|
||||
|
||||
- [x] REQ-1: `vikunja-tasks` skill exists.
|
||||
- [x] REQ-2: `tests/unit/test_skills_validation.py` exists and passes.
|
||||
- [x] REQ-3: create-task docs corrected.
|
||||
|
||||
## Out of Scope
|
||||
|
||||
- Fixing `devx-create-task` argument forwarding (S11 backlog: the
|
||||
devx.mak target takes no args; needs env-var or arg forwarding).
|
||||
|
||||
@@ -0,0 +1,121 @@
|
||||
"""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",
|
||||
"pr-review",
|
||||
"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))
|
||||
# devx repo: devx.mak lives in the package source (editable install)
|
||||
for mak in REPO_ROOT.glob("src/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()) >= 7, "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