skills: implement branches off main and opens a PR; TDD tests come from a sub-agent
implement: sync main and cut task-<NNNN>-<slug> before work, with stop-and-ask guards for a dirty tree, a diverged main, and an unmerged blocker. Replaces the no-commit rule with one commit, a push, and a PR carrying the review's Risk block plus any unaddressed findings. test-driven-development: every test is written by a general-purpose sub-agent that never reads the implementation; the main agent writes the code and never edits a test.
This commit is contained in:
@@ -1,10 +1,10 @@
|
||||
---
|
||||
name: implement
|
||||
description: Implement a task file produced by /to-tasks, review it, and close it out.
|
||||
description: Implement a task file produced by /to-tasks on its own branch, review it, close it out, and open a PR.
|
||||
disable-model-invocation: true
|
||||
---
|
||||
|
||||
Implement a task file end-to-end: build it, review it, and close it out.
|
||||
Implement a task file end-to-end: branch, build it, review it, close it out, and open a PR.
|
||||
|
||||
## Process
|
||||
|
||||
@@ -14,24 +14,59 @@ The user passes the path to a task file (`.claude/tasks/<NNNN>-slug.md`, as prod
|
||||
|
||||
If the task's frontmatter has a `blocked-by` field, read each referenced task file and check for any unresolved `- [ ]` acceptance criterion. If any blocker isn't fully resolved, warn the user which one and why, and confirm before proceeding — don't refuse outright.
|
||||
|
||||
### 2. Implement
|
||||
### 2. Sync `main` and branch off it
|
||||
|
||||
Switch to `main`, fast-forward it (`git pull --ff-only`), then create and switch to a branch named `task-<NNNN>-<slug>` — taken verbatim from the task file's basename, so `.claude/tasks/0003-issue-view-and-truncation.md` gives `task-0003-issue-view-and-truncation`.
|
||||
Use whatever git invocation the project itself uses; a repo may wrap it.
|
||||
|
||||
Stop and ask the user before going further if:
|
||||
|
||||
- **The working tree has uncommitted changes.** Never stash them automatically.
|
||||
- **`git pull --ff-only` fails.** Local `main` has diverged; report what diverged. Never `reset --hard`.
|
||||
- **The task's `blocked-by` work isn't reachable from `main`.** The blocker's PR is likely unmerged; name it.
|
||||
|
||||
If the task branch already exists, switch to it and carry on — don't recreate it, and don't rebase it onto the freshly pulled `main`.
|
||||
Always branch off `main`, never off a sibling task branch.
|
||||
|
||||
### 3. Implement
|
||||
|
||||
Build the work described in the task's "What to build" section, satisfying its acceptance criteria. Use `/test-driven-development` where possible, at the seams already agreed when the spec or task was written.
|
||||
|
||||
Run typechecking regularly, single test files regularly, and the full test suite once at the end.
|
||||
|
||||
### 3. Stage the changes
|
||||
### 4. Stage the changes
|
||||
|
||||
Stage (`git add`) each file you create or modify, specifically — not `git add -A` — so nothing untracked and unrelated gets swept in.
|
||||
|
||||
### 4. Review
|
||||
### 5. Review
|
||||
|
||||
Run `/review-uncommitted`, passing the task file itself as the spec source — it already links back to its parent spec via its `spec` frontmatter field, if any. Address anything it raises before moving on.
|
||||
|
||||
### 5. Close out the task file
|
||||
Keep its report — step 7 puts part of it in the PR.
|
||||
|
||||
### 6. Close out the task file
|
||||
|
||||
Mark every acceptance criterion `[x]` if satisfied or `[-]` if deliberately dropped, so none are left `[ ]`. Append a `## Implementation Notes` section explaining any deviations from the plan — dropped criteria (referencing which, and why), scope changes, decisions made mid-implementation, follow-ups worth flagging. Skip the section only if nothing deviated. Leave the `spec` and `blocked-by` frontmatter fields untouched — they're a permanent record, not a checklist to clear (see `to-tasks`'s `TASK-FORMAT.md`).
|
||||
|
||||
Stage the updated task file with the rest.
|
||||
|
||||
Do not commit — leave the commit itself for the user to make.
|
||||
### 7. Commit, push, and open a PR
|
||||
|
||||
Make **one** commit for the whole task, code and task file together.
|
||||
Match the repo's existing commit convention — read its recent history or its CLAUDE.md, don't assume one — and reference the task in the subject, e.g. `(task 0003)`.
|
||||
|
||||
Push the branch (`git push -u origin task-<NNNN>-<slug>`) and open a pull request against `main` with the repo's forge CLI: `tea` for Gitea, `gh` for GitHub.
|
||||
Never base the PR on a sibling task branch.
|
||||
Open it ready, not draft.
|
||||
|
||||
The PR body carries:
|
||||
|
||||
- The task file's path.
|
||||
- A short summary of what was built, and any deviations — the same ones just written into `## Implementation Notes`.
|
||||
- A `## Review` section: the `## Risk` block from step 5 verbatim (overall rating plus its six factor lines), then **only** the Standards and Spec findings left unaddressed, each with a one-line reason. Findings that were fixed are already in the diff; leave them out.
|
||||
|
||||
Don't ask for confirmation before pushing or opening the PR.
|
||||
If the repo has no remote, stop after the commit and report that no PR was opened.
|
||||
|
||||
Stay on the task branch when done.
|
||||
Report the branch name, the PR URL, and any unaddressed review findings.
|
||||
|
||||
@@ -28,7 +28,9 @@ This produces **crap tests**:
|
||||
- Tests become insensitive to real changes - they pass when behavior breaks, fail when behavior is fine
|
||||
- You outrun your headlights, committing to test structure before understanding the implementation
|
||||
|
||||
**Correct approach**: Vertical slices via tracer bullets. One test → one implementation → repeat. Each test responds to what you learned from the previous cycle. Because you just wrote the code, you know exactly what behavior matters and how to verify it.
|
||||
**Correct approach**: Vertical slices via tracer bullets. One test → one implementation → repeat. Each test responds to what you learned from the previous cycle.
|
||||
|
||||
The test-writer sub-agent (below) is handed **one behavior at a time** and never sees the behavior backlog, so it can't bulk-write the suite.
|
||||
|
||||
```
|
||||
WRONG (horizontal):
|
||||
@@ -42,6 +44,25 @@ RIGHT (vertical):
|
||||
...
|
||||
```
|
||||
|
||||
## Roles
|
||||
|
||||
Every test is written by a **test-writer sub-agent**. The main agent writes every line of implementation, and never writes or edits a test.
|
||||
|
||||
The sub-agent must not read the implementation source of the module under test — that is what keeps its tests from asserting _how_ instead of _what_. It works from the public interface alone.
|
||||
|
||||
Use one `general-purpose` sub-agent for the whole task: spawn it at the first RED, then continue it with `SendMessage` for each subsequent RED, so it keeps the test file and conventions it established. Cold-spawn a replacement only if its ID is lost.
|
||||
|
||||
### Test-writer sub-agent prompt — include:
|
||||
|
||||
- **One behavior**, quoted verbatim from the acceptance criterion or the agreed behavior list. Never the task file, never the rest of the list.
|
||||
- The **public interface** under test — signatures only.
|
||||
- The existing test file(s) for the module, and the project's test conventions (fixtures, helpers, runner invocation).
|
||||
- [tests.md](tests.md) and [mocking.md](mocking.md).
|
||||
- The **independent source of truth for the expected value** — the spec excerpt, worked example, or known-good literal. Without it the sub-agent recomputes the expected value the way the code would, and the test is tautological.
|
||||
- `.claude/CONTEXT.md` (if it exists) and any ADRs in the area, so test names and interface vocabulary match the project's domain language.
|
||||
- The test-side checklist from [Checklist Per Cycle](#checklist-per-cycle), pasted in full — the sub-agent has no other access to it.
|
||||
- The brief: "Write ONE test for this behavior. Do not read the implementation source of the module under test. Write it to the test file, run it, and confirm it fails with a genuine assertion failure — not an import, syntax, or collection error, which prove nothing. Report the test's name and the exact failure message you saw."
|
||||
|
||||
## Workflow
|
||||
|
||||
### 1. Planning
|
||||
@@ -63,13 +84,15 @@ Ask: "What should the public interface look like? Which behaviors are most impor
|
||||
|
||||
**You can't test everything.** Confirm with the user exactly which behaviors matter most. Focus testing effort on critical paths and complex logic, not every possible edge case.
|
||||
|
||||
Planning stays with the main agent on both paths — exploration, interface, and the order behaviors are tested in. The sub-agent receives behaviors one at a time; it never chooses what to test next.
|
||||
|
||||
### 2. Tracer Bullet
|
||||
|
||||
Write ONE test that confirms ONE thing about the system:
|
||||
ONE test that confirms ONE thing about the system:
|
||||
|
||||
```
|
||||
RED: Write test for first behavior → test fails
|
||||
GREEN: Write minimal code to pass → test passes
|
||||
RED: Spawn the test-writer sub-agent with the first behavior → it writes the test, runs it, reports a genuine failure
|
||||
GREEN: Main agent writes minimal code to pass → test passes
|
||||
```
|
||||
|
||||
This is your tracer bullet - proves the path works end-to-end.
|
||||
@@ -79,8 +102,8 @@ This is your tracer bullet - proves the path works end-to-end.
|
||||
For each remaining behavior:
|
||||
|
||||
```
|
||||
RED: Write next test → fails
|
||||
GREEN: Minimal code to pass → passes
|
||||
RED: SendMessage the same sub-agent the next behavior → it writes the test, runs it, reports a genuine failure
|
||||
GREEN: Main agent writes minimal code to pass → passes
|
||||
```
|
||||
|
||||
Rules:
|
||||
@@ -90,6 +113,13 @@ Rules:
|
||||
- Don't anticipate future tests
|
||||
- Keep tests focused on observable behavior
|
||||
|
||||
### When a test looks wrong
|
||||
|
||||
The main agent never edits a sub-agent-authored test — not to fix an import, not to "simplify" an assertion, not to reach GREEN.
|
||||
|
||||
- **Mechanical defect** — bad import path, a fixture or helper that doesn't exist, doesn't parse. Send the error output back to the sub-agent and let it fix its own test.
|
||||
- **Semantic disagreement** — you believe the expected value or the asserted behavior is wrong. Stop and ask the user. Do not resolve it yourself; this disagreement is the signal the sub-agent exists to surface, and half the time it's the code that's wrong.
|
||||
|
||||
### 4. Refactor
|
||||
|
||||
After all tests pass, look for [refactor candidates](refactoring.md):
|
||||
@@ -102,13 +132,22 @@ After all tests pass, look for [refactor candidates](refactoring.md):
|
||||
|
||||
**Never refactor while RED.** Get to GREEN first.
|
||||
|
||||
A test that breaks during refactor means the refactor broke behavior — fix the code. The one exception is a public interface change you made deliberately (a module deepened, a signature moved, as agreed in the plan): send the interface change to the sub-agent and let it update its own tests. There is no case where the main agent edits the test itself.
|
||||
|
||||
## Checklist Per Cycle
|
||||
|
||||
Test-writer sub-agent, per test — paste into its prompt:
|
||||
|
||||
```
|
||||
[ ] Test describes behavior, not implementation
|
||||
[ ] Test uses public interface only
|
||||
[ ] Test would survive internal refactor
|
||||
[ ] Expected values are independent literals, not recomputed from the code
|
||||
```
|
||||
|
||||
Main agent, per GREEN:
|
||||
|
||||
```
|
||||
[ ] Code is minimal for this test
|
||||
[ ] No speculative features added
|
||||
```
|
||||
|
||||
Reference in New Issue
Block a user