From 346413cd7de5e7d03baea09eb935409cfe6eeaa9 Mon Sep 17 00:00:00 2001 From: alexion Date: Sat, 1 Aug 2026 12:12:07 -0400 Subject: [PATCH] Add implementation workflow skills --- AGENTS.md | 2 + skills/implement/SKILL.md | 116 ++++++++++++++++++++++ skills/review/SKILL.md | 119 +++++++++++++++++++++++ skills/review/reviewers/documentation.md | 30 ++++++ skills/review/reviewers/evidence.md | 38 ++++++++ skills/review/reviewers/intent.md | 33 +++++++ skills/review/reviewers/risk.md | 46 +++++++++ skills/review/reviewers/standards.md | 43 ++++++++ skills/test-driven-development/SKILL.md | 75 ++++++++++++++ 9 files changed, 502 insertions(+) create mode 100644 skills/implement/SKILL.md create mode 100644 skills/review/SKILL.md create mode 100644 skills/review/reviewers/documentation.md create mode 100644 skills/review/reviewers/evidence.md create mode 100644 skills/review/reviewers/intent.md create mode 100644 skills/review/reviewers/risk.md create mode 100644 skills/review/reviewers/standards.md create mode 100644 skills/test-driven-development/SKILL.md diff --git a/AGENTS.md b/AGENTS.md index b4ef9e6..34589ce 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,3 +32,5 @@ The domain glossary lives at `$(xdg-user-dir DOCUMENTS)/ai-artifacts/projects/sk Customize only the surfaces required to fit the local workflow rather than turning incidental choices into new contracts. - The installed `gitea-axi pr view` does not support `--fields`. Use plain `gitea-axi pr view `. +- Wayfinder `ticket/task/human` tickets can still be implementation work. + They are blocked only at the point where human input is required. diff --git a/skills/implement/SKILL.md b/skills/implement/SKILL.md new file mode 100644 index 0000000..3530da4 --- /dev/null +++ b/skills/implement/SKILL.md @@ -0,0 +1,116 @@ +--- +name: implement +description: Implement scoped work using the local implementation workflow, following Wayfinder ticket protocol when a task artifact is part of the selected work. +disable-model-invocation: true +--- + +# implement + +Implement scoped work and hand off working code. +This skill defines execution protocol rather than deciding what work the agent is allowed to implement. +When selected work has a Wayfinder task artifact, follow the Wayfinder artifact lifecycle. +When selected work has no Wayfinder artifact, implement normally and use the same review, commit, and handoff discipline without creating a tracking artifact. + +## 1. Establish the selected work + +Identify the work to implement from the user's request and current context. +Use a Wayfinder task artifact, issue, spec, direct request, branch context, or other clear source when that is what the agent has selected. +Do not require a Wayfinder artifact and do not create one just to satisfy this skill. +Old `.claude/tasks` files may be ordinary context, but their lifecycle semantics are not part of this workflow. + +When the selected work has a Wayfinder task artifact, re-read it before claiming. +Verify that blockers are satisfied by the artifact metadata and that no other session has claimed it. +Claim it before changing code by setting `status: claimed`, `claimed-by` to the current session identifier, and `claimed-at` to the current timestamp. +Use `PI_SESSION_ID` when available. + +Done when the selected work is clear, and any Wayfinder task artifact is freshly read and claimed when applicable. + +## 2. Verify the execution checkout + +Assume the current checkout is the assigned execution checkout. +Do not create, lease, clean up, or switch worktrees. +Report the checkout path and branch when useful for handoff. + +Check the working tree before changing code. +Stop on unrelated or ambiguous uncommitted changes. +Continue only when dirty state is clearly already part of the selected work. +Never auto-stash. + +For source-changing implementation work that will be committed, work on a non-default branch unless repository context explicitly directs otherwise. +If an appropriate non-default branch is already prepared, continue there. +If you are on the default branch, fast-forward the default branch first and create a task branch. +Stop if the default branch cannot fast-forward cleanly. +Follow repository, user, or orchestrator branch naming conventions. +If no convention is discoverable and you must create a branch, choose a clear short descriptive name and report it. + +Check declared implementation blockers where they exist. +Stop when prerequisite implementation work is not reachable from the current base, and report the likely unmerged prerequisite. +Do not automatically branch from, merge, or cherry-pick sibling task work. +Follow an explicit integration-branch or wide-refactor plan only when the selected work names that exception. + +Done when the checkout, branch, dirty state, and reachable prerequisites are safe for the selected implementation work. + +## 3. Implement the work + +Use `test-driven-development` as a strong default when behavior can usefully be specified and tested before implementation. +Skip it only when the work is mechanical, documentation-only, exploratory, or when test-first would not add value. +Skipping test-driven development is an agent judgment, not an omission. + +Build the selected work in the assigned checkout. +Run focused verification while working and run broader verification when the repository or change calls for it. +Treat working code and tests as the implementation artifact. +Do not create a separate process report for test-driven development. +Record only durable outcomes such as tests added or changed, verification run, and deviations worth noting. + +Stage created and modified files intentionally. +Do not use broad staging that sweeps unrelated files into the change. + +Done when the selected work is implemented as working code and locally verified to the level the change warrants. + +## 4. Review the final intended handoff state + +Run `review` before reporting final handoff or resolving a Wayfinder task. +Choose review timing by judgment. +Review may happen before commit, after commit, or both. +The review must cover the final intended handoff state, including a local uncommitted handoff when no commit is made. +Do not report final handoff until review has run or you have explained why review could not cover that state. + +Fix blocking review findings unless the agent or user explicitly accepts them. +When code changes are made in response to review, rerun `review` on the relevant change set. +Record accepted blocking findings in the final handoff or Wayfinder closeout when they affect a future reader's decision to trust or continue the work. +Handle non-blocking findings by judgment. +Fix them when cheap or high-value, and otherwise report or record only when useful. + +Done when review has covered the final intended handoff state and blocking findings are fixed or explicitly accepted. + +## 5. Commit and hand off + +Follow the repository's commit and PR conventions. +Discover those conventions from repository instructions or recent history rather than assuming them. +Commit only the selected work. +Use one commit when that is the natural shape, but follow the repository's convention when it expects a different history shape. + +Push and open a PR when the repository and selected workflow call for that handoff. +If there is no remote or no supported forge workflow, stop after the local handoff point and report what remains for a human. +Keep PR and handoff text focused on what was built, verification, review disposition, and any deviations that matter. + +Done when the implementation has a concrete handoff state: committed locally, pushed, opened as a PR, or stopped with the exact remaining human action reported. + +## 6. Close out Wayfinder artifacts when present + +When the selected work has a Wayfinder task artifact, re-read it before closeout. +Record durable coordination facts only. +Wayfinder `status` is the lifecycle authority. +Acceptance criteria checkboxes may be updated as useful detail with `[x]` for satisfied criteria and `[-]` for deliberately dropped criteria when practical. +Checkbox state is not the source of truth. + +Keep Implementation Notes concise. +Capture only what future agents or humans need: deviations, dropped or changed scope, verification, branch, commit, PR, or why the ticket remains unresolved. +Use judgment for partial or ambiguous outcomes, but keep lifecycle recording honest. +Do not imply completion when the work is not complete. + +When resolving a Wayfinder task, set `status: resolved` and repair the owning map's Frontier according to Wayfinder artifact rules. +Put detailed implementation records in the task ticket, commit, PR, and working code rather than in the map. +The parent map should receive only the concise outcome summary required by Wayfinder. + +Done when any Wayfinder task and parent map agree with the implementation's actual lifecycle state. diff --git a/skills/review/SKILL.md b/skills/review/SKILL.md new file mode 100644 index 0000000..5fb3073 --- /dev/null +++ b/skills/review/SKILL.md @@ -0,0 +1,119 @@ +--- +name: review +description: Produce a structured, report-only review of a context-selected change set across risk, standards, intent, evidence, and documentation. +--- + +# review + +Review a change set selected from context and produce a structured review report. +This skill is standalone and context-driven. +It does not know about implementation lifecycle, Wayfinder claiming, task resolution, PR creation, or ticket closeout. +It must not edit code. + +Reviewer prompts live under [`reviewers/`](reviewers/). +Load the relevant reviewer files before running independent dimension reviews. + +## 1. Select and capture the change set + +Select the relevant change set from current context. +The change set may be uncommitted changes, branch changes, a PR, explicit files, or another clear source. +State what change set you selected before reviewing. +Ask or stop only when scope ambiguity would make the review untrustworthy. + +Capture enough material for reviewers to inspect the selected change set. +This may include a diff, changed-file list, commit list, PR details, relevant intent source, available verification evidence, and repository instructions or standards sources. +Do not overprescribe mode selection or intent-source taxonomy. +Use agent judgment when the selected change set and intent source are clear enough to review. + +Done when the chosen change set is explicit and reviewers have enough input to judge it. + +## 2. Run dimension reviews + +Run independent dimension reviewers when the runtime supports subagents or isolated workers. +Use one reviewer per dimension when practical. +Fall back to a single-agent review when independent workers are unavailable, but preserve separate report sections. + +Use these reviewer files: + +- [`reviewers/risk.md`](reviewers/risk.md) +- [`reviewers/standards.md`](reviewers/standards.md) +- [`reviewers/intent.md`](reviewers/intent.md) +- [`reviewers/evidence.md`](reviewers/evidence.md) +- [`reviewers/documentation.md`](reviewers/documentation.md) + +Each reviewer should follow its own purpose, boundary, rubric, and output expectations. +The shared base finding shape is flexible: + +```markdown +- **Severity**: error | warning | info + **Blocking**: yes | no + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` + +Reviewers may add dimension-specific fields where useful. +Reviewers classify findings as blocking or non-blocking. +The caller or user decides final disposition. + +Done when each applicable dimension has returned findings or a clear statement that the dimension does not apply. + +## 3. Run targeted checks when useful + +Run targeted checks when they are needed to assess a dimension. +Prefer focused commands that clarify evidence, risk, or standards conformance. +Do not turn review into a full external validation pipeline. +If a full suite or expensive check is clearly needed, explain why before relying on it. + +Record commands and artifacts that materially influenced the review. +Do not present unrun checks as evidence. + +Done when the review has enough evidence for its findings and limitations are stated honestly. + +## 4. Aggregate the structured review report + +Produce a structured review report, not a gate artifact and not a pass/fail verdict. +Keep sections separate so one dimension does not mask another. +Deduplicate exact duplicates only. +Mention when separate dimensions independently flag the same issue. + +Use this report shape unless the selected change set calls for a small adaptation: + +```markdown +## Reviewed change set + + + +## Risk + + + +## Standards + + + +## Intent + + + +## Evidence + + + +## Documentation + + + +## Summary + +- Blocking findings: +- Non-blocking findings: +- Targeted checks run: +- Evidence limitations: +``` + +Do not invent a pass/fail verdict. +Use blocking findings present or absent as the review's actionable summary. + +Done when the report states the reviewed change set, preserves every applicable dimension, and summarizes blocking status and evidence limitations. diff --git a/skills/review/reviewers/documentation.md b/skills/review/reviewers/documentation.md new file mode 100644 index 0000000..2b2ffe8 --- /dev/null +++ b/skills/review/reviewers/documentation.md @@ -0,0 +1,30 @@ +# Documentation reviewer + +Review only documentation consequences of the selected change set. +Do not make Risk, Standards, Intent, or Evidence findings unless the issue directly affects documentation correctness. + +Look for user-facing, operator-facing, contributor-facing, and agent-facing documentation that should change because behavior, interfaces, commands, options, workflows, or constraints changed. +Also look for stale documentation introduced or left behind by the change. +Do not demand new documentation surfaces for every change. +Prefer updating the existing owner of the fact when one is discoverable. + +Documentation findings may be non-blocking when the change is internal and no durable reader would be misled. +They may be blocking when users, operators, future contributors, or agents would reasonably make a wrong decision from stale or missing documentation. + +## Output format + +```markdown +## Documentation + +### Findings + +Use `No findings.` when this reviewer has no findings. + +- **Severity**: + **Blocking**: + **Documentation owner**: + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` diff --git a/skills/review/reviewers/evidence.md b/skills/review/reviewers/evidence.md new file mode 100644 index 0000000..298b52e --- /dev/null +++ b/skills/review/reviewers/evidence.md @@ -0,0 +1,38 @@ +# Evidence reviewer + +Review only whether the selected change set has convincing evidence. +Do not make Risk, Standards, Intent, or Documentation findings unless the issue directly affects evidentiary value. + +Check tests, commands, artifacts, manual checks, and any evidence the change provides. +You may recommend or run targeted checks when the orchestrating review context allows it. +Do not present unrun checks as evidence. + +Reject source-grep pseudo-tests as proof of behavior. +A test whose only evidence is matching implementation source text, tokens, lines, syntax, prompt phrases, regexes, AST shapes, or incidental snapshots does not prove behavior. +Prefer evidence that demonstrates observable behavior, state, output, side effects, failure modes, or semantic meaning through a public or executable interface. + +For declarative artifacts, prefer invoking the real consumer or parsing into a typed or normalized semantic model. +Reading file contents is legitimate when the file itself is the owned output or serialized contract under test. + +## Output format + +```markdown +## Evidence + +Evidence checked: + +- + +### Findings + +Use `No findings.` when this reviewer has no findings. + +- **Severity**: + **Blocking**: + **Evidence checked**: + **Evidence gap**: + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` diff --git a/skills/review/reviewers/intent.md b/skills/review/reviewers/intent.md new file mode 100644 index 0000000..50db591 --- /dev/null +++ b/skills/review/reviewers/intent.md @@ -0,0 +1,33 @@ +# Intent reviewer + +Review only whether the selected change set satisfies the apparent intent. +Do not make Risk, Standards, Evidence, or Documentation findings unless the issue directly changes intent fidelity. + +Use the clearest available intent source from context. +Possible sources include a Wayfinder task, issue, spec, direct request, PR description, branch context, commit messages, or conversation context. +Do not invent requirements. +When intent is inferred rather than explicit, say so and treat it with appropriate caution. + +Look for missing requested behavior, partial implementation, behavior outside the selected scope, and implementations that appear to satisfy wording while violating the underlying request. +Do not require remote branch, PR, or CI outcomes when another workflow step owns those outcomes. + +## Output format + +```markdown +## Intent + +Intent source: +Intent confidence: + +### Findings + +Use `No findings.` when this reviewer has no findings. + +- **Severity**: + **Blocking**: + **Intent source**: + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` diff --git a/skills/review/reviewers/risk.md b/skills/review/reviewers/risk.md new file mode 100644 index 0000000..3aac131 --- /dev/null +++ b/skills/review/reviewers/risk.md @@ -0,0 +1,46 @@ +# Risk reviewer + +Review only risk. +Do not make Standards, Intent, Evidence, or Documentation findings unless the issue directly changes the risk assessment. + +Assess how much attention the selected change set warrants before handoff. +Use the worst-factor-wins rubric from the old local review workflow. +Rate each factor Low, Medium, or High with a concise reason: + +- Blast radius. +- Reversibility. +- Test coverage. +- Sensitive domain. +- Size and complexity. +- Runtime criticality. + +Overall risk is the highest factor. +Risk alone need not block. +Missing evidence for a risky change may be blocking when a future reader should not trust the handoff without more proof. + +## Output format + +```markdown +## Risk + +**Overall: ** + +- Blast radius: — +- Reversibility: — +- Test coverage: — +- Sensitive domain: — +- Size and complexity: — +- Runtime criticality: — + +### Findings + +Use `No findings.` when this reviewer has no findings. + +- **Severity**: + **Blocking**: + **Risk factor**: + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` diff --git a/skills/review/reviewers/standards.md b/skills/review/reviewers/standards.md new file mode 100644 index 0000000..5f32452 --- /dev/null +++ b/skills/review/reviewers/standards.md @@ -0,0 +1,43 @@ +# Standards reviewer + +Review only repository standards and maintainability. +Do not make Risk, Intent, Evidence, or Documentation findings unless the issue directly affects standards conformance. + +Use repository instructions and discovered standards sources such as `AGENTS.md`, `CONTRIBUTING.md`, coding standards, or nearby conventions. +When documented standards conflict with generic advice, the repository standard wins. + +Also apply this smell baseline when tooling or project standards do not already cover the issue: + +- Mysterious Name. +- Duplicated Code. +- Feature Envy. +- Data Clumps. +- Primitive Obsession. +- Repeated Switches. +- Shotgun Surgery. +- Divergent Change. +- Speculative Generality. +- Message Chains. +- Middle Man. +- Refused Bequest. + +Treat smell findings as judgment calls, not automatic hard violations. +Skip anything deterministic tooling already enforces unless the current review evidence shows the tool is not being run. + +## Output format + +```markdown +## Standards + +### Findings + +Use `No findings.` when this reviewer has no findings. + +- **Severity**: + **Blocking**: + **Standard source**: + **Location**: + **Finding**: + **Why it matters**: + **Suggested fix**: +``` diff --git a/skills/test-driven-development/SKILL.md b/skills/test-driven-development/SKILL.md new file mode 100644 index 0000000..fe1c3d2 --- /dev/null +++ b/skills/test-driven-development/SKILL.md @@ -0,0 +1,75 @@ +--- +name: test-driven-development +description: Drive red-green-refactor implementation with tests written by someone other than the implementation agent, using public behavior and independent expected values. +--- + +# test-driven-development + +Use test-driven development as process discipline for behavior-bearing changes. +The result is working behavior plus useful tests, not a separate report artifact. + +## 1. Establish roles and behavior backlog + +Name the two functional roles before writing tests. +The implementation agent changes production code and drives the loop. +The test writer writes and edits tests. +Tests are always written by someone other than the agent doing implementation. + +Use an independent test writer when the runtime can spawn an independent worker. +If the runtime truly cannot spawn an independent worker, state that limitation before writing tests and preserve role separation in-process as much as possible. +Do not silently skip the independent-test-writer requirement. + +The implementation agent may keep a private behavior backlog. +Keep that backlog isolated from the test writer. +The test writer receives one behavior at a time, not the whole backlog. + +Done when roles are explicit and the implementation agent has the next behavior ready without exposing the whole backlog to the test writer. + +## 2. Choose the public seam and source of truth + +Test public behavior through the interface the code exposes or the task requires. +When the seam is unclear, identify the smallest public seam that can prove the behavior. +Do not test internals just to make RED easy. + +Each test needs an independent source of truth for expected values. +Acceptable sources include a spec, task, intent excerpt, worked example, known-good literal, existing behavior being preserved, user clarification, or external standard. +The implementation agent's derived computation is not enough. + +Done when the next behavior has a public seam, enough context for the test writer, and an independent expected-value source. + +## 3. Run one red-green cycle + +Give the test writer exactly one behavior, the public seam and context, the independent expected-value source, and any relevant project test conventions. +The test writer writes or edits one test for that behavior and confirms it fails for the intended reason. +A failing test should fail because the behavior is missing or wrong, not because of import, syntax, fixture, or collection errors. + +The implementation agent writes the minimal production code needed to pass that test. +The implementation agent must not edit test-writer-authored tests. +If the test has a mechanical defect, send the error back to the test writer. +If the implementation agent believes the test asserts the wrong semantics, pause for user or intent clarification. + +Done when one behavior has a meaningful failing test and then passes through production-code changes made by the implementation agent. + +## 4. Repeat behavior by behavior + +Repeat the red-green cycle one behavior at a time. +Do not bulk-write tests before implementation. +Do not let the test writer see the behavior backlog. +Do not add speculative behavior while making the current test pass. + +Run focused verification as each behavior lands. +Use broader verification when the repository or change warrants it. + +Done when every selected behavior has passed through the one-behavior red-green loop or has been deliberately deferred by the implementation agent's judgment. + +## 5. Refactor only after green + +Never refactor while RED. +After tests pass, the implementation agent may refactor production code. +Refactoring means changing code structure without changing externally observable behavior. + +The test writer updates tests only for deliberate public seam changes or test defects. +A test failure during refactoring normally means production behavior broke. +Fix production code unless the public seam changed deliberately. + +Done when refactoring, if any, is complete and the relevant tests remain green.