201 lines
11 KiB
Markdown
201 lines
11 KiB
Markdown
---
|
||
parent: "[[038-implement-skill-map]]"
|
||
tags:
|
||
- spec
|
||
---
|
||
|
||
# Implement workflow implementation spec
|
||
|
||
## Goal
|
||
|
||
Package the Wayfinder-aware implementation workflow as three skills in this repository: `implement`, `test-driven-development`, and `review`.
|
||
The implementation should create source under `skills/implement/`, `skills/test-driven-development/`, and `skills/review/`.
|
||
The packaged workflow should carry forward the old Claude Code execution, TDD, and review behavior named in this spec while avoiding legacy `.claude/tasks` lifecycle mechanics.
|
||
|
||
## Source references
|
||
|
||
Use these old local skills as source material:
|
||
|
||
- `/home/alexion/wrk/dotfiles/modules/agents/claude-code/skills/implement/`
|
||
- `/home/alexion/wrk/dotfiles/modules/agents/claude-code/skills/test-driven-development/`
|
||
- `/home/alexion/wrk/dotfiles/modules/agents/claude-code/skills/review-uncommitted/`
|
||
|
||
Use `[[038-implement-skill-map]]` as the authoritative decision record.
|
||
|
||
## Deliverables
|
||
|
||
Create exactly these packaged skills:
|
||
|
||
- `skills/implement/SKILL.md`
|
||
- `skills/test-driven-development/SKILL.md`
|
||
- `skills/review/SKILL.md`
|
||
|
||
Create these reviewer prompt files under `skills/review/reviewers/` for independent review dimensions:
|
||
|
||
- `skills/review/reviewers/risk.md`
|
||
- `skills/review/reviewers/standards.md`
|
||
- `skills/review/reviewers/intent.md`
|
||
- `skills/review/reviewers/evidence.md`
|
||
- `skills/review/reviewers/documentation.md`
|
||
|
||
Do not package `codebase-design` as part of this workflow.
|
||
Fold only the necessary public-interface and seam guidance into `test-driven-development` and `review`.
|
||
|
||
## Scope exclusions
|
||
|
||
Do not add legacy `.claude/tasks` compatibility shims.
|
||
Old task files may be ordinary context if an agent chooses to read them, but this workflow must not preserve their lifecycle semantics.
|
||
|
||
Do not build a Pi extension for worktree orchestration, dirty-repo guards, lease cleanup, visible Herdr panes, or multi-agent execution.
|
||
Do not add dedicated Herdr, Treehouse, or future worktree-extension integration documentation.
|
||
The skills should remain compatible with external orchestration by assuming the current checkout is the assigned execution checkout.
|
||
|
||
Do not integrate external validation gates such as `no-mistakes` into `implement`.
|
||
The no-mistakes research informs local review and TDD design, not a direct runtime dependency.
|
||
|
||
## `implement` skill contract
|
||
|
||
`implement` is a workflow executor for scoped implementation work.
|
||
It must not police what work the agent is allowed to implement.
|
||
It defines execution protocol.
|
||
|
||
When selected work has a Wayfinder task artifact, `implement` must follow Wayfinder claim, blocker, result, resolution, and Frontier protocol.
|
||
When selected work has no Wayfinder artifact, `implement` proceeds with normal implementation, review, commit, and handoff protocol.
|
||
In both cases, the implementation artifact is the working code.
|
||
|
||
`implement` is worktree-compatible, not a worktree orchestrator.
|
||
It assumes the current session already has the right checkout.
|
||
It should report the checkout and branch it used.
|
||
It should not create, lease, clean up, or switch worktrees.
|
||
|
||
Before changing code, `implement` checks dirty state.
|
||
It stops on unrelated or ambiguous uncommitted changes.
|
||
It continues only when dirty state is clearly already part of the selected work.
|
||
It never auto-stashes.
|
||
|
||
When starting new work, `implement` uses or creates a non-default branch from an updated default branch when needed.
|
||
If an appropriate non-default branch is already prepared, it continues there.
|
||
If it must create a branch, branch naming follows repository, user, or orchestrator convention.
|
||
If no convention is discoverable, it chooses a clear short descriptive branch name and reports it.
|
||
|
||
`implement` preserves the old blocker reachability guardrail.
|
||
It checks declared blockers where they exist.
|
||
It stops when prerequisite implementation work is not reachable from the current base and reports the likely unmerged prerequisite.
|
||
It does not automatically branch from, merge, or cherry-pick sibling task work.
|
||
Explicit integration-branch or wide-refactor exceptions follow the plan that names the exception.
|
||
|
||
`implement` uses `test-driven-development` as a strong default when behavior can usefully be specified and tested before implementation.
|
||
It may skip TDD when the work is mechanical, documentation-only, exploratory, or when test-first would not add value.
|
||
Skipping TDD should be agent judgment, not omission.
|
||
|
||
`implement` must run `review` before final handoff or Wayfinder task resolution.
|
||
Review timing is agent judgment.
|
||
Review may happen before commit, after commit, or both.
|
||
The review must cover the final intended handoff state, or the agent must explain why it does not.
|
||
|
||
Blocking review findings should be fixed unless explicitly accepted by the agent or user.
|
||
When code changes are made in response to review, `implement` reruns review on the relevant change set.
|
||
Accepted blocking findings are recorded in the final handoff or Wayfinder closeout when they affect a future reader's decision to trust or continue the work.
|
||
Non-blocking findings are handled by agent judgment.
|
||
|
||
`implement` should record only durable closeout facts.
|
||
For Wayfinder task closeout, `status` is the lifecycle authority.
|
||
Acceptance criteria checkboxes may be updated as useful detail with `[x]` for satisfied and `[-]` for deliberately dropped criteria when practical.
|
||
Checkbox state is not the source of truth.
|
||
Implementation Notes should stay concise and capture only what future agents or humans need: deviations, dropped or changed scope, verification, branch, commit, PR, or why the ticket remains unresolved.
|
||
Partial or ambiguous outcomes are left to agent judgment, but lifecycle recording must be honest.
|
||
|
||
## `test-driven-development` skill contract
|
||
|
||
`test-driven-development` is process discipline, not a report-producing artifact.
|
||
Its result is working behavior plus useful tests.
|
||
|
||
Tests are always written by someone other than the agent doing implementation.
|
||
The normal contract requires an independent test writer.
|
||
If the runtime truly cannot spawn an independent worker, the implementation agent must state that limitation before writing tests and preserve role separation in-process as much as possible.
|
||
The implementation agent must not silently skip this requirement.
|
||
|
||
Use role names by function.
|
||
The implementation agent changes production code and drives the loop.
|
||
The test writer writes and edits tests.
|
||
|
||
The implementation agent gives the test writer one behavior, public seam and context, an independent expected-value source, and failure output.
|
||
The implementation agent must not edit test-writer-authored tests.
|
||
Mechanical test defects go back to the test writer.
|
||
Semantic disagreement pauses for user or intent clarification.
|
||
|
||
The implementation agent may keep a private behavior backlog, but that backlog is isolated from the test writer.
|
||
The test writer receives one behavior at a time, writes one failing test, and then the implementation agent makes the minimal production change to pass.
|
||
Repeat one behavior at a time.
|
||
|
||
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.
|
||
|
||
Include minimal public-interface and seam guidance.
|
||
Tests should target 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.
|
||
Avoid testing internals just to make RED easy.
|
||
|
||
Preserve the old local refactoring behavior.
|
||
Never refactor while RED.
|
||
After tests pass, the implementation agent may refactor production code.
|
||
The test writer updates tests only for deliberate public seam changes or test defects.
|
||
|
||
## `review` skill contract
|
||
|
||
The review skill is named `review`.
|
||
It is standalone and context-driven.
|
||
It does not know about implementation lifecycle, Wayfinder claiming, task resolution, PR creation, or ticket closeout.
|
||
|
||
`review` selects the relevant change set from current context.
|
||
The selected change set may be uncommitted changes, branch changes, a PR, explicit files, or another clear source.
|
||
It states what change set it selected before reviewing.
|
||
It asks or stops only when scope ambiguity would make the review untrustworthy.
|
||
|
||
`review` produces a structured review report, not a gate artifact and not a pass/fail verdict.
|
||
It remains report-only and must not edit code.
|
||
It may run targeted checks when useful.
|
||
|
||
The report should cover Risk, Standards, Intent, Evidence, and Documentation when those dimensions apply.
|
||
Do not overprescribe mode selection or intent-source taxonomy.
|
||
The skill should leave room for agent judgment when the selected change set and intent source are clear enough to review.
|
||
|
||
Use independent dimension reviewers when the runtime supports subagents or isolated workers.
|
||
Fallback to a single-agent review when independent workers are unavailable, but preserve separate report sections.
|
||
|
||
Reviewer prompt files live under `skills/review/reviewers/`.
|
||
Each reviewer file should be self-contained about its purpose, boundary, rubric, and output expectations.
|
||
The top-level `SKILL.md` orchestrates input capture and aggregation without overdefining each reviewer’s judgment.
|
||
|
||
Findings should use a flexible shared base schema with dimension-specific additions where useful.
|
||
The base shape includes severity, blocking yes/no, location, finding, why it matters, and suggested fix.
|
||
Reviewers classify findings as blocking or non-blocking.
|
||
The caller or user decides final disposition.
|
||
|
||
The Evidence reviewer should incorporate the anti-slop lessons from no-mistakes.
|
||
It should reject source-grep pseudo-tests as proof of behavior.
|
||
It should prefer evidence that demonstrates observable behavior, state, output, side effects, failure modes, or semantic meaning.
|
||
|
||
## Verification
|
||
|
||
After implementing the skills, run the repository verification expected for packaged skills.
|
||
At minimum run:
|
||
|
||
```bash
|
||
nix flake check "path:$PWD"
|
||
```
|
||
|
||
Use `nix flake check "path:$PWD"` rather than plain `nix flake check` when new skill files are untracked, because plain flake evaluation uses the Git snapshot.
|
||
|
||
## Acceptance criteria
|
||
|
||
- [ ] `skills/implement/SKILL.md` exists and implements the contract above.
|
||
- [ ] `skills/test-driven-development/SKILL.md` exists and implements the contract above.
|
||
- [ ] `skills/review/SKILL.md` exists and implements the contract above.
|
||
- [ ] `skills/review/reviewers/risk.md`, `skills/review/reviewers/standards.md`, `skills/review/reviewers/intent.md`, `skills/review/reviewers/evidence.md`, and `skills/review/reviewers/documentation.md` exist.
|
||
- [ ] The workflow does not add packaged `codebase-design`.
|
||
- [ ] The workflow does not add legacy `.claude/tasks` lifecycle compatibility.
|
||
- [ ] The workflow does not build or document a dedicated worktree orchestration integration.
|
||
- [ ] Repository verification passes with `nix flake check "path:$PWD"`.
|