mirror of
https://github.com/obra/superpowers.git
synced 2026-07-17 23:44:04 +08:00
Compare commits
3 Commits
sdd-fix-lo
...
dev
| Author | SHA1 | Date | |
|---|---|---|---|
|
fb7b07088e | ||
|
|
7a81eb7177 | ||
|
|
2b1c06a849 |
@@ -784,10 +784,10 @@ Use this as the live index; when in doubt, read the files, not this table.
|
||||
|
||||
| Harness | Entry point | Bootstrap mechanism | Tool mapping | Tests | Distribution |
|
||||
|---|---|---|---|---|---|
|
||||
| Claude Code | `.claude-plugin/plugin.json` + `hooks/hooks.json` | shell hook → `hooks/session-start` (`hookSpecificOutput.additionalContext`) | native `Skill` tool; `references/claude-code-tools.md` | `tests/hooks/` | marketplace |
|
||||
| Claude Code | `.claude-plugin/plugin.json` + `hooks/hooks.json` | shell hook → `hooks/session-start` (`hookSpecificOutput.additionalContext`) | native `Skill` tool; no adapter file needed | `tests/hooks/` | marketplace |
|
||||
| Codex | `.codex-plugin/plugin.json` (declares empty `hooks`) | native skill discovery (no session-start hook) | `references/codex-tools.md` | `tests/codex/`, `tests/codex-plugin-sync/` | fork sync (`scripts/sync-to-codex-plugin.sh`) |
|
||||
| Cursor | `.cursor-plugin/plugin.json` + `hooks/hooks-cursor.json` | shell hook → `hooks/session-start` (`additional_context`) | `references/claude-code-tools.md` | `tests/hooks/` | hand-authored |
|
||||
| Copilot CLI | (shares Claude Code hook path; `COPILOT_CLI` env) | shell hook → `hooks/session-start` (`additionalContext`) | `references/copilot-tools.md` | `tests/hooks/` | — |
|
||||
| Cursor | `.cursor-plugin/plugin.json` + `hooks/hooks-cursor.json` | shell hook → `hooks/session-start` (`additional_context`) | none needed (Claude Code–compatible tool surface) | `tests/hooks/` | hand-authored |
|
||||
| Copilot CLI | (shares Claude Code hook path; `COPILOT_CLI` env) | shell hook → `hooks/session-start` (`additionalContext`) | none needed (Claude Code–compatible tool surface) | `tests/hooks/` | — |
|
||||
| Gemini CLI | `gemini-extension.json` + `GEMINI.md` | instructions file `@`-includes bootstrap + mapping | `references/gemini-tools.md` | — | `gemini extensions install` |
|
||||
| Kimi Code | `.kimi-plugin/plugin.json` | manifest `sessionStart.skill` loads `using-superpowers` | inline `skillInstructions` in manifest | `tests/kimi/` | marketplace or `/plugins install` GitHub URL |
|
||||
| OpenCode | `.opencode/plugins/superpowers.js` (declared via root `package.json` `main`) | in-process: `config` hook registers skills dir; `experimental.chat.messages.transform` injects user message | inline in `superpowers.js` | `tests/opencode/` | `opencode.json` plugin git URL |
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -1,196 +0,0 @@
|
||||
# SDD Fix-Loop Redesign — Design Spec
|
||||
|
||||
**Status:** Approved design (brainstormed with Jesse 2026-07-15); implementation
|
||||
plan to follow.
|
||||
**Objective:** make the subagent-driven-development skill's review-fix loop
|
||||
convergent and autonomous, and make the document readable, without rewriting
|
||||
its eval-tuned language.
|
||||
**Hard invariant:** existing eval-tuned sentences move; they do not get
|
||||
reworded. New machinery ships with drill evidence.
|
||||
|
||||
## Problems
|
||||
|
||||
Four, all observed in real sessions:
|
||||
|
||||
1. **Pathological review loops.** The loop is literally "Repeat until
|
||||
approved" — no round cap. Each re-review is a fresh full review of the
|
||||
whole diff, so a nondeterministic frontier reviewer surfaces new findings
|
||||
every round instead of verifying fixes. Result: implement, review, fix,
|
||||
review, review, fix, review, fix — with no circuit breaker. The
|
||||
strict-cost spec (2026-06-10) independently measured review-loop count as
|
||||
the biggest run-to-run cost variance.
|
||||
2. **Contradictory fix policy.** The process diagram and "Constructing
|
||||
Reviewer Prompts" dispatch dedicated fix subagents; Red Flags says
|
||||
"Implementer (same subagent) fixes them"; implementer-prompt.md's "After
|
||||
Review Findings" section assumes the implementer will be re-engaged. Three
|
||||
answers to "who fixes?" in one skill.
|
||||
3. **Accreted structure.** Thirteen top-level sections; guidance for one
|
||||
activity is scattered across four of them. "Constructing Reviewer Prompts"
|
||||
is a grab-bag holding reviewer guidance, fix policy, final-review policy,
|
||||
and plan-conflict adjudication.
|
||||
4. **Red Flags format.** Seven sibling skills use the `| Excuse | Reality |`
|
||||
rationalization table; SDD carries a 17-bullet "Never" list plus three
|
||||
"If X" mini-blocks.
|
||||
|
||||
## Design Decisions
|
||||
|
||||
| # | Decision | Rationale |
|
||||
|---|----------|-----------|
|
||||
| 1 | The original implementer fixes its own review findings — resume it in place. | It already holds the task context; ownership beats a drive-by patcher. Fresh "fix subagents" rebuild context per finding and lack the task frame. |
|
||||
| 2 | Re-reviews are scoped to the findings. | Fresh full reviews each round are the churn engine. Scoped re-reviews make the loop structurally convergent; the final whole-branch review remains the broad safety net. |
|
||||
| 3 | Circuit breaker at five fix rounds: three resumes, then two fresh dispatches on a more capable model. | Jesse's call. A loop that survives three resumes usually means the implementer cannot see its own problem — the fresh capable dispatch de-anchors and capability-bumps in one move. |
|
||||
| 4 | At trip, the controller adjudicates and routes. No new human checkpoint — structural failures reach the existing BLOCKED stop. | SDD's point is autonomous execution. The controller holds the plan and cross-task context the reviewer lacks; the existing text already sanctions it ("adjudicate it in the review loop") without ever specifying the mechanism. |
|
||||
| 5 | Reorganize SKILL.md by lifecycle, preserving tuned sentences. | Fixes "hard to follow" at the root. Content moves to its point of use, matching the house direction (recent commits fold recap sections into points of use). |
|
||||
| 6 | Convert Red Flags to a `| Excuse | Reality |` rationalization table; relocate hard rules to their points of use. | Matches the other seven skills. Excuses get rebuttals; rules get enforced where the reader acts. |
|
||||
|
||||
## The Fix Loop
|
||||
|
||||
Trigger: a task review returns spec ❌ or any Critical/Important finding.
|
||||
|
||||
**Rounds 1–3 — resume the original implementer.** Send the findings verbatim
|
||||
(Critical/Important plus spec gaps). The implementer fixes, re-runs the
|
||||
covering tests, appends the fix report to its existing report file, and
|
||||
returns the short contract. On a harness without agent resume, a "resume" is
|
||||
a fresh dispatch carrying the brief, the report file, and the findings — the
|
||||
report file is the persistent memory either way.
|
||||
|
||||
**Rounds 4–5 — fresh implementer, more capable model.** Full task context:
|
||||
brief, report file, open findings, and the framing "a prior implementer
|
||||
attempted this N times; you own the task now."
|
||||
|
||||
**Every round's re-review is scoped.** The re-reviewer receives the brief,
|
||||
the updated report, the original findings list, and a fix-scoped diff package
|
||||
(`review-package FIX_BASE HEAD`, where FIX_BASE is the head the reviewer
|
||||
last reviewed; the script already takes arbitrary ranges).
|
||||
It verdicts each finding addressed / not addressed and flags new breakage in
|
||||
the fix diff only. Novel findings on code the fix did not touch are reported
|
||||
as non-blocking; the controller ledgers them for the final review.
|
||||
|
||||
**Fix-report completeness gate (existing rule, kept):** before dispatching a
|
||||
re-review, confirm the fix report names the covering tests, the command run,
|
||||
and the output.
|
||||
|
||||
**No early exit.** The controller never adjudicates before the cap — an early
|
||||
exit reopens the "pre-judge findings to spare yourself a review loop" hole
|
||||
the current content deliberately closed. One exception, unchanged from
|
||||
today: a finding that conflicts with what the plan's text mandates goes to
|
||||
the human immediately (plan authority, not loop churn).
|
||||
|
||||
**Minor findings** never enter the loop: ledger them as they arrive (existing
|
||||
rule, kept).
|
||||
|
||||
### Adjudication at Trip
|
||||
|
||||
After round five fails, the controller stops dispatching and judges each open
|
||||
finding against the brief, the plan, and cross-task context:
|
||||
|
||||
- **Contested or wrong** → ledger with a one-line adjudication ("controller:
|
||||
reviewer wrong because X"), continue. The final review sees both sides.
|
||||
- **Real, not load-bearing** → ledger as known-open, continue. Later
|
||||
dispatches touching that area carry a pointer to the entry.
|
||||
- **Real and load-bearing** (later tasks build on it, or it reveals a plan
|
||||
defect) → the existing BLOCKED stop. Park-and-continue defers a structural
|
||||
failure to the most expensive point and lets dependents build on it, so
|
||||
structural failures stop the run — through the stop condition that already
|
||||
exists, not a new checkpoint.
|
||||
|
||||
Every adjudication is a ledger entry. Silent discards stay forbidden.
|
||||
|
||||
## Document Restructure
|
||||
|
||||
New skeleton, in execution order:
|
||||
|
||||
1. Intro — why subagents, core principle, narration, continuous execution
|
||||
2. When to Use — unchanged, including the decision graph
|
||||
3. The Process — diagram updated for the new loop
|
||||
4. Setup — worktree, ledger check/resume, pre-flight plan review, todos
|
||||
5. Model Selection — stays one cross-cutting section; every dispatch
|
||||
consults it, so folding it into points of use would repeat it five times
|
||||
6. The Task Loop — five numbered steps:
|
||||
1. Dispatch the implementer (task-brief script, five-part dispatch
|
||||
composition, model line required)
|
||||
2. Handle the report (DONE / DONE_WITH_CONCERNS / NEEDS_CONTEXT / BLOCKED)
|
||||
3. Review the task (review-package script, reviewer dispatch composition,
|
||||
constraints lens, no pre-judging, ⚠️ handling)
|
||||
4. Fix loop (the machinery above)
|
||||
5. Complete the task (ledger append, todo update)
|
||||
7. Final Review — package, model pin, one fix wave, one scoped re-review,
|
||||
adjudication
|
||||
8. Finish — finishing-a-development-branch
|
||||
9. Common Rationalizations — the table
|
||||
10. Example Workflow — updated to show a resume-based fix round and the
|
||||
breaker not tripping
|
||||
|
||||
"Constructing Reviewer Prompts," "File Handoffs," and "Durable Progress"
|
||||
dissolve into the steps where each rule applies. Every eval-tuned sentence
|
||||
lands in exactly one new location; a move map in the implementation plan
|
||||
tracks source → destination so review can verify nothing was dropped or
|
||||
reworded.
|
||||
|
||||
## Rationalization Table
|
||||
|
||||
Excuse-shaped Never items convert to rows; new rows cover the loop
|
||||
pathology. Draft rows (final wording at implementation):
|
||||
|
||||
| Excuse | Reality |
|
||||
|--------|---------|
|
||||
| "Close enough on spec compliance" | Reviewer found gaps = not done. |
|
||||
| "I'll fix it myself, dispatching is overhead" | Controller fixes pollute your context and skip review. Resume the implementer. |
|
||||
| "One more round will converge" | Past the cap, rounds don't converge. Adjudicate. |
|
||||
| "The reviewer will just find something new anyway" | Scoped re-reviews check fixes, not taste. New findings on untouched code go to the ledger, not the loop. |
|
||||
| "This finding is obviously wrong, I'll drop it" | You adjudicate only at the cap, and every adjudication is a ledger entry. Silent discards are forbidden. |
|
||||
| "The fix was small, skip the re-review" | Unreviewed fixes are how regressions land. |
|
||||
|
||||
Hard rules that are not excuses (never parallel implementers, never dispatch
|
||||
a reviewer without a diff file, model line required, never re-dispatch
|
||||
ledger-complete tasks) move to their points of use.
|
||||
|
||||
## Prompt Templates
|
||||
|
||||
- **implementer-prompt.md** — "After Review Findings" rewritten for resume
|
||||
semantics: you will be resumed with findings; fix, re-run covering tests,
|
||||
append to your report file, return the short contract.
|
||||
- **task-reviewer-prompt.md** — initial review only; the trailing re-review
|
||||
sentence moves out.
|
||||
- **re-review-prompt.md (new)** — the scoped re-review contract: inputs are
|
||||
brief, updated report, original findings, fix-scoped diff package; output
|
||||
is a per-finding verdict (addressed / not addressed), new breakage in the
|
||||
fix diff, and non-blocking observations outside it. A separate template
|
||||
because it is a different contract — overloading the full-review template
|
||||
produced the current ambiguity.
|
||||
- **Takeover dispatch (rounds 4–5)** — composed from implementer-prompt.md
|
||||
plus SKILL.md guidance (brief, report path, open findings, takeover
|
||||
framing); no new template file.
|
||||
|
||||
## Final Review Loop
|
||||
|
||||
Unchanged: merge-base package, most capable model, ONE fixer with the
|
||||
complete findings list. New: exactly one scoped re-review of the fix wave,
|
||||
then controller adjudication. Residual load-bearing findings surface at
|
||||
finishing-a-development-branch, where the human already is. The end of the
|
||||
branch gets a bounded loop too.
|
||||
|
||||
## Evals
|
||||
|
||||
Three new drill scenarios in `evals/`:
|
||||
|
||||
1. **Resume, don't re-dispatch:** a task review returns findings; the
|
||||
controller must resume the same implementer rather than dispatch a fix
|
||||
subagent.
|
||||
2. **Breaker trips:** a seeded never-satisfied reviewer; the controller must
|
||||
stop dispatching after the fifth round fails, adjudicate, ledger, and
|
||||
continue — not loop.
|
||||
3. **Structural finding stops:** a load-bearing finding (later tasks depend
|
||||
on it); the controller must stop via BLOCKED rather than park.
|
||||
|
||||
Plus before/after runs of the existing SDD scenarios to catch regressions
|
||||
from the reorganization.
|
||||
|
||||
## Non-Goals
|
||||
|
||||
- Ledger session-scoping — PR #1943 owns it. This work touches the same
|
||||
sections, so the implementation plan notes the collision risk.
|
||||
- Script changes — task-brief and review-package already do what the new
|
||||
loop needs.
|
||||
- Changes to executing-plans or requesting-code-review beyond the final-
|
||||
review pointer continuing to resolve.
|
||||
@@ -51,85 +51,41 @@ digraph process {
|
||||
subgraph cluster_per_task {
|
||||
label="Per Task";
|
||||
"Dispatch implementer subagent (./implementer-prompt.md)" [shape=box];
|
||||
"Implementer asks questions?" [shape=diamond];
|
||||
"Implementer subagent asks questions?" [shape=diamond];
|
||||
"Answer questions, provide context" [shape=box];
|
||||
"Implementer implements, tests, commits, self-reviews" [shape=box];
|
||||
"Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)" [shape=box];
|
||||
"Spec ✅ and quality approved?" [shape=diamond];
|
||||
"Finding conflicts with plan text?" [shape=diamond];
|
||||
"Ask human partner which governs" [shape=box];
|
||||
"Fix round R of 5: R≤3 resume implementer; R≥4 fresh implementer, more capable model" [shape=box];
|
||||
"Dispatch scoped re-review (./re-review-prompt.md)" [shape=box];
|
||||
"All findings addressed?" [shape=diamond];
|
||||
"R = 5?" [shape=diamond];
|
||||
"Adjudicate each open finding" [shape=box];
|
||||
"Any load-bearing finding?" [shape=diamond];
|
||||
"STOP: report BLOCKED to human partner" [shape=box];
|
||||
"Park findings in ledger with rulings" [shape=box];
|
||||
"Append completion to ledger, mark todo complete" [shape=box];
|
||||
"Implementer subagent implements, tests, commits, self-reviews" [shape=box];
|
||||
"Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [shape=box];
|
||||
"Task reviewer reports spec ✅ and quality approved?" [shape=diamond];
|
||||
"Dispatch fix subagent for Critical/Important findings" [shape=box];
|
||||
"Mark task complete in todo list and progress ledger" [shape=box];
|
||||
}
|
||||
|
||||
"Setup: worktree, ledger check, read plan, pre-flight review" [shape=box];
|
||||
"Read plan, note context and global constraints, create todos" [shape=box];
|
||||
"More tasks remain?" [shape=diamond];
|
||||
"Dispatch final code reviewer (../requesting-code-review/code-reviewer.md)" [shape=box];
|
||||
"Final findings? ONE fix dispatch, one scoped re-review, adjudicate residuals" [shape=box];
|
||||
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [shape=box];
|
||||
"Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
|
||||
|
||||
"Setup: worktree, ledger check, read plan, pre-flight review" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||
"Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer asks questions?";
|
||||
"Implementer asks questions?" -> "Answer questions, provide context" [label="yes"];
|
||||
"Answer questions, provide context" -> "Implementer implements, tests, commits, self-reviews";
|
||||
"Implementer asks questions?" -> "Implementer implements, tests, commits, self-reviews" [label="no"];
|
||||
"Implementer implements, tests, commits, self-reviews" -> "Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)";
|
||||
"Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)" -> "Spec ✅ and quality approved?";
|
||||
"Spec ✅ and quality approved?" -> "Append completion to ledger, mark todo complete" [label="yes"];
|
||||
"Spec ✅ and quality approved?" -> "Finding conflicts with plan text?" [label="no"];
|
||||
"Finding conflicts with plan text?" -> "Ask human partner which governs" [label="yes"];
|
||||
"Ask human partner which governs" -> "Fix round R of 5: R≤3 resume implementer; R≥4 fresh implementer, more capable model";
|
||||
"Finding conflicts with plan text?" -> "Fix round R of 5: R≤3 resume implementer; R≥4 fresh implementer, more capable model" [label="no"];
|
||||
"Fix round R of 5: R≤3 resume implementer; R≥4 fresh implementer, more capable model" -> "Dispatch scoped re-review (./re-review-prompt.md)";
|
||||
"Dispatch scoped re-review (./re-review-prompt.md)" -> "All findings addressed?";
|
||||
"All findings addressed?" -> "Append completion to ledger, mark todo complete" [label="yes"];
|
||||
"All findings addressed?" -> "R = 5?" [label="no"];
|
||||
"R = 5?" -> "Fix round R of 5: R≤3 resume implementer; R≥4 fresh implementer, more capable model" [label="no - next round"];
|
||||
"R = 5?" -> "Adjudicate each open finding" [label="yes - breaker trips"];
|
||||
"Adjudicate each open finding" -> "Any load-bearing finding?";
|
||||
"Any load-bearing finding?" -> "STOP: report BLOCKED to human partner" [label="yes"];
|
||||
"Any load-bearing finding?" -> "Park findings in ledger with rulings" [label="no"];
|
||||
"Park findings in ledger with rulings" -> "Append completion to ledger, mark todo complete";
|
||||
"Append completion to ledger, mark todo complete" -> "More tasks remain?";
|
||||
"Read plan, note context and global constraints, create todos" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||
"Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer subagent asks questions?";
|
||||
"Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"];
|
||||
"Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||
"Implementer subagent asks questions?" -> "Implementer subagent implements, tests, commits, self-reviews" [label="no"];
|
||||
"Implementer subagent implements, tests, commits, self-reviews" -> "Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)";
|
||||
"Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" -> "Task reviewer reports spec ✅ and quality approved?";
|
||||
"Task reviewer reports spec ✅ and quality approved?" -> "Dispatch fix subagent for Critical/Important findings" [label="no"];
|
||||
"Dispatch fix subagent for Critical/Important findings" -> "Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [label="re-review"];
|
||||
"Task reviewer reports spec ✅ and quality approved?" -> "Mark task complete in todo list and progress ledger" [label="yes"];
|
||||
"Mark task complete in todo list and progress ledger" -> "More tasks remain?";
|
||||
"More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"];
|
||||
"More tasks remain?" -> "Dispatch final code reviewer (../requesting-code-review/code-reviewer.md)" [label="no"];
|
||||
"Dispatch final code reviewer (../requesting-code-review/code-reviewer.md)" -> "Final findings? ONE fix dispatch, one scoped re-review, adjudicate residuals";
|
||||
"Final findings? ONE fix dispatch, one scoped re-review, adjudicate residuals" -> "Use superpowers:finishing-a-development-branch";
|
||||
"More tasks remain?" -> "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [label="no"];
|
||||
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" -> "Use superpowers:finishing-a-development-branch";
|
||||
}
|
||||
```
|
||||
|
||||
## Setup
|
||||
## Pre-Flight Plan Review
|
||||
|
||||
Ensure the work happens in an isolated workspace: use
|
||||
superpowers:using-git-worktrees to create one or verify the existing one.
|
||||
Never start implementation on a main/master branch without your human
|
||||
partner's explicit consent.
|
||||
|
||||
Conversation memory does not survive compaction. In real sessions,
|
||||
controllers that lost their place have re-dispatched entire completed task
|
||||
sequences — the single most expensive failure observed. Track progress in
|
||||
a ledger file, not only in todos.
|
||||
|
||||
- At skill start, check for a ledger:
|
||||
`cat "$(git rev-parse --show-toplevel)/.superpowers/sdd/progress.md"`. Tasks with
|
||||
a `Task <N>: complete` line are DONE — do not re-dispatch them; resume at
|
||||
the first task without one. A task whose last line is a fix round is
|
||||
mid-loop: resume the loop at the next round.
|
||||
- The ledger is your recovery map: the commits it names exist in git even
|
||||
when your context no longer remembers creating them. After compaction,
|
||||
trust the ledger and `git log` over your own recollection.
|
||||
- `git clean -fdx` will destroy the ledger (it's git-ignored scratch); if
|
||||
that happens, recover from `git log`.
|
||||
|
||||
Read the plan once, note its context and Global Constraints, and create a
|
||||
todo per task.
|
||||
|
||||
Before dispatching Task 1, scan the plan once for conflicts:
|
||||
|
||||
@@ -157,11 +113,7 @@ capable available model, not the session default.
|
||||
|
||||
**Review tasks**: choose the model with the same judgment, scaled to the
|
||||
diff's size, complexity, and risk. A small mechanical diff does not need the
|
||||
most capable model; a subtle concurrency change does. Scoped re-reviews of
|
||||
small fix diffs take a cheap-to-mid tier.
|
||||
|
||||
**Fix-loop escalation (rounds 4-5)**: use a model at least one tier above
|
||||
the implementer that got stuck.
|
||||
most capable model; a subtle concurrency change does.
|
||||
|
||||
**Always specify the model explicitly when dispatching a subagent.** An
|
||||
omitted model inherits your session's model — often the most capable and
|
||||
@@ -180,47 +132,7 @@ that implementer. Single-file mechanical fixes also take the cheapest tier.
|
||||
- Touches multiple files with integration concerns → standard model
|
||||
- Requires design judgment or broad codebase understanding → most capable model
|
||||
|
||||
## The Task Loop
|
||||
|
||||
Everything you paste into a dispatch prompt — and everything a subagent
|
||||
prints back — stays resident in your context for the rest of the session
|
||||
and is re-read on every later turn. Hand artifacts over as files.
|
||||
|
||||
### 1. Dispatch the implementer
|
||||
|
||||
Record BASE (`git rev-parse HEAD`) before dispatching — the review package
|
||||
and fix-round diffs need it.
|
||||
|
||||
- **Task brief:** before dispatching an implementer, run this skill's
|
||||
`scripts/task-brief PLAN_FILE N` — it extracts the task's full text to a
|
||||
uniquely named file and prints the path. Compose the dispatch so the
|
||||
brief stays the single source of
|
||||
requirements. Your dispatch should contain: (1) one line on where this
|
||||
task fits in the project; (2) the brief path, introduced as "read this
|
||||
first — it is your requirements, with the exact values to use verbatim";
|
||||
(3) interfaces and decisions from earlier tasks that the brief cannot
|
||||
know; (4) your resolution of any ambiguity you noticed in the brief;
|
||||
(5) the report-file path and report contract. Exact values (numbers,
|
||||
magic strings, signatures, test cases) appear only in the brief. Never
|
||||
make a subagent read the whole plan file.
|
||||
- **Report file:** name the implementer's report file after the brief
|
||||
(brief `…/task-N-brief.md` → report `…/task-N-report.md`) and put it in
|
||||
the dispatch prompt. The implementer writes the full report there and
|
||||
returns only status, commits, a one-line test summary, and concerns.
|
||||
- A dispatch prompt describes one task, not the session's history. Do not
|
||||
paste accumulated prior-task summaries ("state after Tasks 1-3") into
|
||||
later dispatches — a real session's dispatch hit 42k chars of which 99%
|
||||
was pasted history. A fresh subagent needs its task, the interfaces it
|
||||
touches, and the global constraints. Nothing else.
|
||||
- If an earlier task parked a finding in the area this task touches, carry
|
||||
a pointer to that ledger entry in the dispatch.
|
||||
- Record the implementer's agent identity from the dispatch result —
|
||||
fix-loop rounds 1-3 resume this agent.
|
||||
- Never dispatch multiple implementation subagents in parallel (conflicts).
|
||||
|
||||
Template: [implementer-prompt.md](implementer-prompt.md)
|
||||
|
||||
### 2. Handle the report
|
||||
## Handling Implementer Status
|
||||
|
||||
Implementer subagents report one of four statuses. Handle each appropriately:
|
||||
|
||||
@@ -238,37 +150,20 @@ Implementer subagents report one of four statuses. Handle each appropriately:
|
||||
|
||||
**Never** ignore an escalation or force the same model to retry without changes. If the implementer said it's stuck, something needs to change.
|
||||
|
||||
If the implementer asks questions — before starting or mid-task — answer
|
||||
clearly and completely, provide additional context if needed, and don't
|
||||
rush it into implementation.
|
||||
## Handling Reviewer ⚠️ Items
|
||||
|
||||
### 3. Review the task
|
||||
The task reviewer may report "⚠️ Cannot verify from diff" items — requirements
|
||||
that live in unchanged code or span tasks. These do not block the rest of the
|
||||
review, but you must resolve each one yourself before marking the task
|
||||
complete: you hold the plan and cross-task context the reviewer
|
||||
lacks. If you confirm an item is a real gap, treat it as a failed spec
|
||||
review — send it back to the implementer and re-review.
|
||||
|
||||
## Constructing Reviewer Prompts
|
||||
|
||||
Per-task reviews are task-scoped gates. The broad review happens once, at the
|
||||
final whole-branch review. Never skip the task review, and never accept a
|
||||
report missing either verdict — spec compliance AND task quality are both
|
||||
required. Implementer self-review never replaces the task review; both are
|
||||
needed.
|
||||
final whole-branch review. When you fill a reviewer template:
|
||||
|
||||
- Hand the reviewer its diff as a file: run this skill's
|
||||
`scripts/review-package BASE HEAD` and pass the reviewer the file path
|
||||
it prints (or, without bash: `git log --oneline`, `git diff --stat`,
|
||||
and `git diff -U10` for the range, redirected to one uniquely named
|
||||
file). The output never enters your own context, and the reviewer sees
|
||||
the commit list, stat summary, and full diff with context in one Read
|
||||
call. Use the BASE you recorded before dispatching the implementer —
|
||||
never `HEAD~1`, which silently truncates multi-commit tasks. Never
|
||||
dispatch a task reviewer without a diff file.
|
||||
- **Reviewer inputs:** the task reviewer gets three paths — the same brief
|
||||
file, the report file, and the review package — plus the global
|
||||
constraints that bind the task.
|
||||
- The global-constraints block you hand the reviewer is its attention
|
||||
lens. Copy the binding requirements verbatim from the plan's Global
|
||||
Constraints section or the spec: exact values, exact formats, and the
|
||||
stated relationships between components ("same layout as X", "matches
|
||||
Y"). The reviewer's template already carries the process rules (YAGNI,
|
||||
test hygiene, review method) — the constraints block is for what THIS
|
||||
project's spec demands.
|
||||
- Do not add open-ended directives like "check all uses" or "run race tests
|
||||
if useful" without a concrete, task-specific reason
|
||||
- Do not ask a reviewer to re-run tests the implementer already ran on the
|
||||
@@ -279,153 +174,111 @@ needed.
|
||||
loop. If the prompt you are writing contains "do not flag," "don't treat X
|
||||
as a defect," "at most Minor," or "the plan chose" — stop: you are
|
||||
pre-judging, usually to spare yourself a review loop.
|
||||
|
||||
The task reviewer may report "⚠️ Cannot verify from diff" items — requirements
|
||||
that live in unchanged code or span tasks. These do not block the rest of the
|
||||
review, but you must resolve each one yourself before marking the task
|
||||
complete: you hold the plan and cross-task context the reviewer
|
||||
lacks. If you confirm an item is a real gap, treat it as a failed spec
|
||||
review — it enters the fix loop with the other findings.
|
||||
|
||||
Template: [task-reviewer-prompt.md](task-reviewer-prompt.md)
|
||||
|
||||
### 4. The fix loop
|
||||
|
||||
The loop triggers when the review reports spec ❌, any Critical or Important
|
||||
finding, or a ⚠️ item you confirmed as a real gap.
|
||||
|
||||
Before the loop starts, two routes leave it immediately:
|
||||
|
||||
- Record Minor findings in the progress ledger as you go
|
||||
(`Task <N>: minor (deferred): <one-liner>`), and point the final
|
||||
- The global-constraints block you hand the reviewer is its attention
|
||||
lens. Copy the binding requirements verbatim from the plan's Global
|
||||
Constraints section or the spec: exact values, exact formats, and the
|
||||
stated relationships between components ("same layout as X", "matches
|
||||
Y"). The reviewer's template already carries the process rules (YAGNI,
|
||||
test hygiene, review method) — the constraints block is for what THIS
|
||||
project's spec demands.
|
||||
- Hand the reviewer its diff as a file: run this skill's
|
||||
`scripts/review-package BASE HEAD` and pass the reviewer the file path
|
||||
it prints (or, without bash: `git log --oneline`, `git diff --stat`,
|
||||
and `git diff -U10` for the range, redirected to one uniquely named
|
||||
file). The output never enters your own context, and the reviewer sees
|
||||
the commit list, stat summary, and full diff with context in one Read
|
||||
call. Use the BASE you recorded before dispatching the implementer —
|
||||
never `HEAD~1`, which silently truncates multi-commit tasks.
|
||||
- A dispatch prompt describes one task, not the session's history. Do not
|
||||
paste accumulated prior-task summaries ("state after Tasks 1-3") into
|
||||
later dispatches — a real session's dispatch hit 42k chars of which 99%
|
||||
was pasted history. A fresh subagent needs its task, the interfaces it
|
||||
touches, and the global constraints. Nothing else.
|
||||
- Dispatch fix subagents for Critical and Important findings. Record Minor
|
||||
findings in the progress ledger as you go, and point the final
|
||||
whole-branch review at that list so it can triage which must be fixed
|
||||
before merge. A roll-up nobody reads is a silent discard. Minor findings
|
||||
never enter the loop.
|
||||
before merge. A roll-up nobody reads is a silent discard.
|
||||
- A finding labeled plan-mandated — or any finding that conflicts with
|
||||
what the plan's text requires — is the human's decision, like any plan
|
||||
contradiction: present the finding and the plan text, ask which governs.
|
||||
Do not dismiss the finding because the plan mandates it, and do not
|
||||
dispatch a fix that contradicts the plan without asking.
|
||||
- The final whole-branch review gets a package too: run
|
||||
`scripts/review-package MERGE_BASE HEAD` (MERGE_BASE = the commit the
|
||||
branch started from, e.g. `git merge-base main HEAD`) and include the
|
||||
printed path in the final review dispatch, so the final reviewer reads
|
||||
one file instead of re-deriving the branch diff with git commands.
|
||||
- Every fix dispatch carries the implementer contract: the fix subagent
|
||||
re-runs the tests covering its change and reports the results. Name the
|
||||
covering test files in the dispatch — a one-line fix does not need the
|
||||
whole suite. Before re-dispatching the reviewer, confirm the fix report
|
||||
contains the covering tests, the command run, and the output; dispatch
|
||||
the re-review once all three are present.
|
||||
- If the final whole-branch review returns findings, dispatch ONE fix
|
||||
subagent with the complete findings list — not one fixer per finding.
|
||||
Per-finding fixers each rebuild context and re-run suites; a real
|
||||
session's final-review fix wave cost more than all its tasks combined.
|
||||
|
||||
Everything else enters the loop. A fix round is one fix dispatch plus one
|
||||
scoped re-review. Five rounds maximum per task:
|
||||
## File Handoffs
|
||||
|
||||
**Rounds 1-3 — resume the original implementer.** Send it the open findings
|
||||
verbatim. Its context is intact: it knows the task, the code, and its own
|
||||
choices. If your harness cannot send another message to a live subagent,
|
||||
dispatch a fresh implementer carrying the brief path, the report-file path,
|
||||
and the findings — the report file is the persistent memory either way.
|
||||
Everything you paste into a dispatch prompt — and everything a subagent
|
||||
prints back — stays resident in your context for the rest of the session
|
||||
and is re-read on every later turn. Hand artifacts over as files:
|
||||
|
||||
**Rounds 4-5 — dispatch a fresh implementer on a more capable model** (per
|
||||
Model Selection), with the brief path, the report-file path, the open
|
||||
findings, and this framing: "A prior implementer attempted this task
|
||||
[N] times; you own it now. Read the report file for what was tried." A loop
|
||||
that survives three resumes usually means the implementer cannot see its
|
||||
own problem — fresh eyes and a capability bump in one move.
|
||||
- **Task brief:** before dispatching an implementer, run this skill's
|
||||
`scripts/task-brief PLAN_FILE N` — it extracts the task's full text to a
|
||||
uniquely named file and prints the path. Compose the dispatch so the
|
||||
brief stays the single source of requirements. Your dispatch should
|
||||
contain: (1) one line on where this task fits in the project; (2) the
|
||||
brief path, introduced as "read this first — it is your requirements,
|
||||
with the exact values to use verbatim"; (3) interfaces and decisions
|
||||
from earlier tasks that the brief cannot know; (4) your resolution of
|
||||
any ambiguity you noticed in the brief; (5) the report-file path and
|
||||
report contract. Exact values (numbers, magic strings, signatures, test
|
||||
cases) appear only in the brief.
|
||||
- **Report file:** name the implementer's report file after the brief
|
||||
(brief `…/task-N-brief.md` → report `…/task-N-report.md`) and put it in
|
||||
the dispatch prompt. The implementer writes the full report there and
|
||||
returns only status, commits, a one-line test summary, and concerns.
|
||||
- **Reviewer inputs:** the task reviewer gets three paths — the same brief
|
||||
file, the report file, and the review package — plus the global
|
||||
constraints that bind the task.
|
||||
- Fix dispatches append their fix report (with test results) to the same
|
||||
report file and return a short summary; re-reviews read the updated file.
|
||||
|
||||
**Every round, either way:** the implementer fixes, re-runs the tests
|
||||
covering the amended code, appends its fix report to the same report file,
|
||||
and returns the short contract. Before re-dispatching the reviewer, confirm
|
||||
the fix report contains the covering tests, the command run, and the
|
||||
output; dispatch the re-review once all three are present. Name the
|
||||
covering test files in the fix message — a one-line fix does not need the
|
||||
whole suite.
|
||||
## Durable Progress
|
||||
|
||||
**The re-review is scoped.** Run `scripts/review-package FIX_BASE HEAD`
|
||||
where FIX_BASE is the head the previous review saw, and dispatch
|
||||
[re-review-prompt.md](re-review-prompt.md) with the findings list, the
|
||||
brief, the report file, and the printed diff path. The re-reviewer verdicts
|
||||
each finding ADDRESSED or NOT ADDRESSED and flags new breakage in the fix
|
||||
diff only. New Critical/Important breakage in the fix diff joins the open
|
||||
findings list. Out-of-scope observations go to the ledger as deferred
|
||||
minors — they never extend the loop.
|
||||
Conversation memory does not survive compaction. In real sessions,
|
||||
controllers that lost their place have re-dispatched entire completed task
|
||||
sequences — the single most expensive failure observed. Track progress in
|
||||
a ledger file, not only in todos.
|
||||
|
||||
**After each round,** append to the ledger:
|
||||
`Task <N>: fix round <R>/5 (<X> addressed, <Y> open — <finding one-liners>; commits <a7>..<b7>)`
|
||||
- At skill start, check for a ledger:
|
||||
`cat "$(git rev-parse --show-toplevel)/.superpowers/sdd/progress.md"`. Tasks listed there
|
||||
as complete are DONE — do not re-dispatch them; resume at the first task
|
||||
not marked complete.
|
||||
- When a task's review comes back clean, append one line to the ledger in
|
||||
the same message as your other bookkeeping:
|
||||
`Task N: complete (commits <base7>..<head7>, review clean)`.
|
||||
- The ledger is your recovery map: the commits it names exist in git even
|
||||
when your context no longer remembers creating them. After compaction,
|
||||
trust the ledger and `git log` over your own recollection.
|
||||
- `git clean -fdx` will destroy the ledger (it's git-ignored scratch); if
|
||||
that happens, recover from `git log`.
|
||||
|
||||
Never fix findings yourself in the controller session — your context stays
|
||||
clean for coordination, and controller fixes skip review.
|
||||
## Prompt Templates
|
||||
|
||||
**The breaker.** When round 5's re-review still leaves findings open, stop
|
||||
dispatching. Adjudicate each open finding yourself — you hold the plan and
|
||||
the cross-task context the reviewer lacks:
|
||||
|
||||
- **The reviewer is wrong, or the point is contestable:** park it —
|
||||
`Task <N>: parked — <finding> — ruling: <why the code stands>`. The final
|
||||
review sees both sides.
|
||||
- **Real, but nothing downstream builds on it:** park it the same way, with
|
||||
a ruling that says it's real and deferred.
|
||||
- **Real and load-bearing** — a later task builds on it, or it reveals a
|
||||
plan defect: STOP. Append `Task <N>: BLOCKED — <reason>` and report to
|
||||
your human partner with the finding, the plan text it collides with, and
|
||||
the fix history. Parking a structural failure lets every dependent task
|
||||
build on it and hands the final review a problem it cannot fix either.
|
||||
|
||||
Adjudicate only at the cap. Adjudicating earlier to end a loop is
|
||||
pre-judging with a different name. Every adjudication is a ledger entry —
|
||||
a silent discard is forbidden.
|
||||
|
||||
### 5. Complete the task
|
||||
|
||||
When the review comes back clean — or every open finding is parked with a
|
||||
ruling at the cap — append the completion line to the ledger in the same
|
||||
message as your other bookkeeping:
|
||||
|
||||
- `Task <N>: complete (commits <base7>..<head7>, review clean)`
|
||||
- `Task <N>: complete (commits <base7>..<head7>, <K> parked)` after a
|
||||
tripped breaker
|
||||
|
||||
Then mark the todo complete and move on. Never move to the next task while
|
||||
the review has open Critical/Important issues that are neither fixed nor
|
||||
parked-with-ruling at the cap.
|
||||
|
||||
## Final Review
|
||||
|
||||
The final whole-branch review gets a package too: run
|
||||
`scripts/review-package MERGE_BASE HEAD` (MERGE_BASE = the commit the
|
||||
branch started from, e.g. `git merge-base main HEAD`) and include the
|
||||
printed path in the final review dispatch, so the final reviewer reads
|
||||
one file instead of re-deriving the branch diff with git commands. Dispatch
|
||||
on the most capable available model (see Model Selection), using
|
||||
superpowers:requesting-code-review's
|
||||
[code-reviewer.md](../requesting-code-review/code-reviewer.md). Point it at
|
||||
the ledger's deferred-minor and parked lines so it can triage which must be
|
||||
fixed before merge.
|
||||
|
||||
If the final whole-branch review returns findings, dispatch ONE fix subagent
|
||||
with the complete findings list — not one fixer per finding.
|
||||
Per-finding fixers each rebuild context and re-run suites; a real
|
||||
session's final-review fix wave cost more than all its tasks combined.
|
||||
Then run exactly one scoped re-review of the fix wave
|
||||
(`scripts/review-package` over the fix range, [re-review-prompt.md](re-review-prompt.md)).
|
||||
Adjudicate any residual findings as in the task loop's breaker: park with
|
||||
rulings, or stop on load-bearing ones. There is no second fix wave —
|
||||
residual load-bearing findings surface to your human partner when
|
||||
finishing-a-development-branch presents the options.
|
||||
|
||||
## Finish
|
||||
|
||||
Use superpowers:finishing-a-development-branch.
|
||||
|
||||
## Common Rationalizations
|
||||
|
||||
| Excuse | Reality |
|
||||
|--------|---------|
|
||||
| "Close enough on spec compliance" | Reviewer found spec gaps = not done. Fix or hit the cap and adjudicate — those are the only exits. |
|
||||
| "I'll fix it myself, dispatching is overhead" | Controller fixes pollute your context and skip review. Resume the implementer. |
|
||||
| "One more round will converge" | Past the cap, rounds don't converge — the failure is structural. Adjudicate and route. |
|
||||
| "The reviewer will just find something new anyway" | Scoped re-reviews verify fixes; they cannot wander. New findings on untouched code go to the ledger, not the loop. |
|
||||
| "This finding is obviously wrong, I'll drop it" | You adjudicate only at the cap, and every ruling is a ledger entry. Silent discards are forbidden. |
|
||||
| "The fix was small, skip the re-review" | Unreviewed fixes are how regressions land. Every round ends with a scoped re-review. |
|
||||
| "Reviews slow the loop down" | The loop without reviews is just unverified churn. Reviews are the loop's brakes and steering. |
|
||||
| "Ledger bookkeeping is overhead" | The ledger is what survives compaction. Controllers without one have re-dispatched entire completed task sequences. |
|
||||
- [implementer-prompt.md](implementer-prompt.md) - Dispatch implementer subagent
|
||||
- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality)
|
||||
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
|
||||
|
||||
## Example Workflow
|
||||
|
||||
```
|
||||
You: I'm using Subagent-Driven Development to execute this plan.
|
||||
|
||||
[Setup: worktree verified, no ledger found, read plan, created todos]
|
||||
[Read plan file once: docs/superpowers/plans/feature-plan.md]
|
||||
[Create todos for all tasks]
|
||||
|
||||
Task 1: Hook installation script
|
||||
|
||||
@@ -435,7 +288,8 @@ Implementer: "Before I begin - should the hook be installed at user or system le
|
||||
|
||||
You: "User level (~/.config/superpowers/hooks/)"
|
||||
|
||||
Implementer: [Later]
|
||||
Implementer: "Got it. Implementing now..."
|
||||
[Later] Implementer:
|
||||
- Implemented install-hook command
|
||||
- Added tests, 5/5 passing
|
||||
- Self-review: Found I missed --force flag, added it
|
||||
@@ -445,39 +299,77 @@ Implementer: [Later]
|
||||
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
||||
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
||||
|
||||
[Ledger: Task 1: complete (commits a1b2c3d..d4e5f6a, review clean)]
|
||||
[Mark Task 1 complete]
|
||||
|
||||
Task 2: Recovery modes
|
||||
|
||||
[Run task-brief for Task 2; dispatch implementer with brief + report paths + context]
|
||||
|
||||
Implementer: [No questions]
|
||||
Implementer: [No questions, proceeds]
|
||||
Implementer:
|
||||
- Added verify/repair modes
|
||||
- 8/8 tests passing
|
||||
- Self-review: All good
|
||||
- Committed
|
||||
|
||||
[Run review-package, dispatch task reviewer with the printed path]
|
||||
Task reviewer: Spec ❌:
|
||||
- Missing: Progress reporting (spec says "report every 100 items")
|
||||
- Extra: Added --json flag (not requested)
|
||||
Issues (Important): Magic number (100)
|
||||
|
||||
[Fix round 1: resume the implementer with both findings]
|
||||
Implementer: Added progress reporting, extracted PROGRESS_INTERVAL constant.
|
||||
Re-ran test/recovery.test.js — 10/10 passing. Fix report appended.
|
||||
[Dispatch fix subagent with all findings]
|
||||
Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant
|
||||
|
||||
[Run review-package FIX_BASE HEAD; dispatch scoped re-review]
|
||||
Re-reviewer: Missing progress reporting — ADDRESSED (src/recovery.js:41).
|
||||
Magic number — ADDRESSED (src/recovery.js:7). New breakage: none.
|
||||
Verdict: all findings addressed.
|
||||
[Task reviewer reviews again]
|
||||
Task reviewer: Spec ✅. Task quality: Approved.
|
||||
|
||||
[Ledger: Task 2: fix round 1/5 (2 addressed, 0 open; commits d4e5f6a..b7c8d9e)]
|
||||
[Ledger: Task 2: complete (commits d4e5f6a..b7c8d9e, review clean)]
|
||||
[Mark Task 2 complete]
|
||||
|
||||
...
|
||||
|
||||
[After all tasks]
|
||||
[Run review-package MERGE_BASE HEAD; dispatch final code-reviewer, most capable model]
|
||||
Final reviewer: All requirements met. Deferred minors triaged: none block merge.
|
||||
[Dispatch final code-reviewer]
|
||||
Final reviewer: All requirements met, ready to merge
|
||||
|
||||
Done! Using superpowers:finishing-a-development-branch.
|
||||
Done!
|
||||
```
|
||||
|
||||
## Red Flags
|
||||
|
||||
**Never:**
|
||||
- Start implementation on main/master branch without explicit user consent
|
||||
- Skip task review, or accept a report missing either verdict (spec compliance AND task quality are both required)
|
||||
- Proceed with unfixed issues
|
||||
- Dispatch multiple implementation subagents in parallel (conflicts)
|
||||
- Make a subagent read the whole plan file (hand it its task brief —
|
||||
`scripts/task-brief` — instead)
|
||||
- Skip scene-setting context (subagent needs to understand where task fits)
|
||||
- Ignore subagent questions (answer before letting them proceed)
|
||||
- Accept "close enough" on spec compliance (reviewer found spec issues = not done)
|
||||
- Skip review loops (reviewer found issues = implementer fixes = review again)
|
||||
- Let implementer self-review replace actual review (both are needed)
|
||||
- Tell a reviewer what not to flag, or pre-rate a finding's severity in the
|
||||
dispatch prompt ("treat it as Minor at most") — the plan's example code is
|
||||
a starting point, not evidence that its weaknesses were chosen
|
||||
- Dispatch a task reviewer without a diff file — generate it first
|
||||
(`scripts/review-package BASE HEAD`) and name the printed path in the
|
||||
prompt
|
||||
- Move to next task while the review has open Critical/Important issues
|
||||
- Re-dispatch a task the progress ledger already marks complete — check
|
||||
the ledger (and `git log`) after any compaction or resume
|
||||
|
||||
**If subagent asks questions:**
|
||||
- Answer clearly and completely
|
||||
- Provide additional context if needed
|
||||
- Don't rush them into implementation
|
||||
|
||||
**If reviewer finds issues:**
|
||||
- Implementer (same subagent) fixes them
|
||||
- Reviewer reviews again
|
||||
- Repeat until approved
|
||||
- Don't skip the re-review
|
||||
|
||||
**If subagent fails task:**
|
||||
- Dispatch fix subagent with specific instructions
|
||||
- Don't try to fix manually (context pollution)
|
||||
|
||||
@@ -106,12 +106,9 @@ Subagent (general-purpose):
|
||||
|
||||
## After Review Findings
|
||||
|
||||
If the task review finds issues, you will be resumed with the findings.
|
||||
Fix them, re-run the tests that cover the amended code, and append a fix
|
||||
report to your report file: what you changed, the covering tests you
|
||||
ran, the command, and the output. Reviewers will not re-run tests for
|
||||
you — your report is the test evidence. Then reply with the same short
|
||||
status contract as your first report.
|
||||
If a reviewer finds issues and you fix them, re-run the tests that cover
|
||||
the amended code and append the results to your report file. Reviewers
|
||||
will not re-run tests for you — your report is the test evidence.
|
||||
|
||||
## Report Format
|
||||
|
||||
|
||||
@@ -1,106 +0,0 @@
|
||||
# Scoped Re-Review Prompt Template
|
||||
|
||||
Use this template when dispatching a re-review after a fix round. The
|
||||
re-reviewer verifies the findings were addressed and checks the fix diff for
|
||||
new breakage. It is not a fresh review — the full review already happened.
|
||||
|
||||
**Purpose:** Verify each finding from the previous review was addressed, and
|
||||
that the fix itself broke nothing.
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
description: "Re-review Task N fix round R"
|
||||
model: [MODEL — REQUIRED: choose per SKILL.md Model Selection; an omitted
|
||||
model silently inherits the session's most expensive one]
|
||||
prompt: |
|
||||
You are re-reviewing one task's fix round. A previous review produced
|
||||
findings; an implementer has attempted to fix them. Your job is to
|
||||
verdict each finding and inspect the fix diff — nothing else.
|
||||
|
||||
## The Task
|
||||
|
||||
Read the task brief: [BRIEF_FILE]
|
||||
|
||||
## The Findings Under Verification
|
||||
|
||||
[FINDINGS]
|
||||
|
||||
## The Fix
|
||||
|
||||
Read the implementer's report (fix reports are appended at the end):
|
||||
[REPORT_FILE]
|
||||
|
||||
**Fix base:** [FIX_BASE_SHA] (the head the previous review saw)
|
||||
**Head:** [HEAD_SHA]
|
||||
**Diff file:** [DIFF_FILE]
|
||||
|
||||
Read the diff file once — it contains the fix commits, a stat summary,
|
||||
and the fix diff with surrounding context. Do not re-run git commands.
|
||||
If the diff file is missing, fetch the diff yourself:
|
||||
`git diff --stat [FIX_BASE_SHA]..[HEAD_SHA]` and
|
||||
`git diff [FIX_BASE_SHA]..[HEAD_SHA]`.
|
||||
|
||||
Your review is read-only on this checkout. Do not mutate the working
|
||||
tree, the index, HEAD, or branch state in any way.
|
||||
|
||||
## Scope
|
||||
|
||||
Your scope is the findings list and the fix diff. Verdict every finding.
|
||||
Inspect the fix diff for new problems the fix itself introduced. Do NOT
|
||||
re-review code the fix did not touch: if you notice an issue entirely
|
||||
outside the fix diff, report it under Out-of-Scope Observations — it
|
||||
does not block this task and does not extend the loop. A broad
|
||||
whole-branch review happens after all tasks are complete.
|
||||
|
||||
## Tests
|
||||
|
||||
The implementer re-ran the tests covering the amended code and appended
|
||||
the results to the report file. Treat the report as unverified claims:
|
||||
confirm the fix report names the covering tests and shows their output,
|
||||
and verify the claims against the diff. Do not re-run the suite to
|
||||
confirm their report. Run a test only when reading the code raises a
|
||||
specific doubt that no existing run answers — and then a focused test,
|
||||
never a package-wide suite.
|
||||
|
||||
## Output Format
|
||||
|
||||
Your final message is the report itself: begin directly with the first
|
||||
finding's verdict. Every line is a verdict, a finding with file:line,
|
||||
or a check you ran — no preamble, no process narration.
|
||||
|
||||
### Finding Verdicts
|
||||
|
||||
For each finding in The Findings Under Verification, in order:
|
||||
- **[finding one-liner]** — ADDRESSED | NOT ADDRESSED, with file:line
|
||||
evidence. "Attempted" is not addressed: the specific defect must no
|
||||
longer exist.
|
||||
|
||||
### New Breakage in the Fix Diff
|
||||
|
||||
Anything the fix itself broke or introduced, with severity
|
||||
(Critical/Important/Minor) and file:line. "None" if clean.
|
||||
|
||||
### Out-of-Scope Observations
|
||||
|
||||
Issues you noticed entirely outside the fix diff. Non-blocking; the
|
||||
controller ledgers these for the final review. "None" if none.
|
||||
|
||||
### Verdict
|
||||
|
||||
**Fix round:** [All findings addressed, no new Critical/Important
|
||||
breakage | Findings remain open] — list the open ones.
|
||||
```
|
||||
|
||||
**Placeholders:**
|
||||
- `[MODEL]` — REQUIRED: reviewer model per SKILL.md Model Selection; scoped
|
||||
re-reviews of small fix diffs take a cheap-to-mid tier
|
||||
- `[BRIEF_FILE]` — the task brief file (same file the implementer worked from)
|
||||
- `[FINDINGS]` — the Critical/Important findings and spec gaps from the
|
||||
previous review, copied verbatim, one per bullet
|
||||
- `[REPORT_FILE]` — the implementer's report file (fix reports appended)
|
||||
- `[FIX_BASE_SHA]` — the head the previous review saw
|
||||
- `[HEAD_SHA]` — current commit
|
||||
- `[DIFF_FILE]` — the path `scripts/review-package FIX_BASE HEAD` printed
|
||||
|
||||
**Re-reviewer returns:** per-finding verdicts (ADDRESSED / NOT ADDRESSED),
|
||||
new breakage in the fix diff, out-of-scope observations, and a round verdict.
|
||||
@@ -183,3 +183,6 @@ Subagent (general-purpose):
|
||||
|
||||
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
||||
(Critical/Important/Minor), Task quality verdict
|
||||
|
||||
A fix dispatch can address spec gaps and quality findings together;
|
||||
re-review after fixes covers both verdicts.
|
||||
|
||||
@@ -7,7 +7,7 @@ Add to your Codex config (`~/.codex/config.toml`):
|
||||
multi_agent = true
|
||||
```
|
||||
|
||||
This enables `spawn_agent`, `wait_agent`, and `close_agent` for skills like `dispatching-parallel-agents` and `subagent-driven-development`. When using subagent-driven-development, close reviewer subagents when their review returns. Keep each implementer subagent open until its task's review passes — the fix loop resumes the implementer — then close it. If your harness cannot send another message to a spawned agent, dispatch each fix round as a fresh implementer carrying the brief, the report file, and the findings.
|
||||
This enables `spawn_agent`, `wait_agent`, and `close_agent` for skills like `dispatching-parallel-agents` and `subagent-driven-development`. When using subagent-driven-development, you should always close implementer and reviewer subagents when they have finished all their work.
|
||||
|
||||
## Environment Detection
|
||||
|
||||
|
||||
@@ -9,7 +9,7 @@ description: Use when creating new skills, editing existing skills, or verifying
|
||||
|
||||
**Writing skills IS Test-Driven Development applied to process documentation.**
|
||||
|
||||
**Personal skills live in your runtime's skills directory** — see [claude-code-tools.md](../using-superpowers/references/claude-code-tools.md), [codex-tools.md](../using-superpowers/references/codex-tools.md), [copilot-tools.md](../using-superpowers/references/copilot-tools.md), or [gemini-tools.md](../using-superpowers/references/gemini-tools.md) for the path on your runtime. Codex, Copilot CLI, and Gemini CLI all also recognize `~/.agents/skills/` as a cross-runtime alias.
|
||||
**Personal skills live in your runtime's skills directory** (`~/.claude/skills/` on Claude Code) — see [codex-tools.md](../using-superpowers/references/codex-tools.md) or [gemini-tools.md](../using-superpowers/references/gemini-tools.md) for the path on those runtimes. Codex, Copilot CLI, and Gemini CLI all also recognize `~/.agents/skills/` as a cross-runtime alias.
|
||||
|
||||
You write test cases (pressure scenarios with subagents), watch them fail (baseline behavior), write the skill (documentation), watch tests pass (agents comply), and refactor (close loopholes).
|
||||
|
||||
|
||||
@@ -2,8 +2,9 @@
|
||||
# Validate the Antigravity (agy) integration. agy installs the existing plugin
|
||||
# directly (`agy plugin install <repo-url>`): it loads the bundled skills and
|
||||
# runs the SessionStart hook for bootstrap, so there is no agy-specific scaffold
|
||||
# to test. What IS agy-specific is the tool mapping — agy has no `Skill` tool and
|
||||
# loads skills by reading SKILL.md with view_file — and SKILL.md pointing at it.
|
||||
# to test. What IS agy-specific is the tool mapping — subagent dispatch via
|
||||
# invoke_subagent (self/research types) and task tracking via a task artifact —
|
||||
# and SKILL.md pointing at it.
|
||||
#
|
||||
# Mirrors tests/pi/test-pi-extension.mjs's "tools reference documents
|
||||
# harness-specific mappings" check. CI-safe: does not require `agy` installed.
|
||||
@@ -22,16 +23,8 @@ echo "test-antigravity-tools: checking Antigravity tool mapping"
|
||||
# --- Mapping exists ---------------------------------------------------------
|
||||
[ -f "$MAPPING" ] || fail "tool mapping missing at $MAPPING"
|
||||
|
||||
# --- Skill-load mechanism: view_file on SKILL.md (IsSkillFile), no Skill tool -
|
||||
grep -qiE "view_file" "$MAPPING" \
|
||||
|| fail "mapping does not document view_file as the file/skill-read tool"
|
||||
grep -qiE "SKILL\.md" "$MAPPING" \
|
||||
|| fail "mapping does not document reading SKILL.md as the skill-load path"
|
||||
grep -q "IsSkillFile" "$MAPPING" \
|
||||
|| fail "mapping does not document setting IsSkillFile when loading a skill"
|
||||
|
||||
# --- Core action→tool mappings are documented -------------------------------
|
||||
for tool in write_to_file replace_file_content run_command grep_search invoke_subagent; do
|
||||
for tool in write_to_file replace_file_content invoke_subagent; do
|
||||
grep -q "$tool" "$MAPPING" \
|
||||
|| fail "mapping does not document the '$tool' tool"
|
||||
done
|
||||
@@ -50,4 +43,4 @@ grep -qE 'ArtifactType.*task|task. artifact' "$MAPPING" \
|
||||
grep -q "antigravity-tools.md" "$SKILL" \
|
||||
|| fail "SKILL.md Platform Adaptation does not reference antigravity-tools.md"
|
||||
|
||||
echo "PASS: Antigravity tool mapping valid (view_file skill-load, agy tools, SKILL.md link)"
|
||||
echo "PASS: Antigravity tool mapping valid (subagent dispatch, task artifact, SKILL.md link)"
|
||||
|
||||
@@ -122,7 +122,16 @@ test('pi tools reference documents pi-specific mappings', async () => {
|
||||
assert.equal(existsSync(piToolsPath), true, 'pi-tools.md should exist');
|
||||
const text = await readFile(piToolsPath, 'utf8');
|
||||
|
||||
for (const expected of ['Skill', 'Task', 'TodoWrite', 'read', 'write', 'edit', 'bash']) {
|
||||
assert.match(text, new RegExp(expected));
|
||||
}
|
||||
// Assert against the mapping-table rows only. The surrounding prose mentions
|
||||
// these same tokens, so matching the whole file would still pass if the table
|
||||
// were deleted — the exact regression this test exists to catch.
|
||||
const rows = text.split('\n').filter((line) => line.startsWith('|'));
|
||||
assert.ok(
|
||||
rows.some((row) => /subagent/i.test(row)),
|
||||
'mapping table documents subagent dispatch',
|
||||
);
|
||||
assert.ok(
|
||||
rows.some((row) => /todo|task/i.test(row)),
|
||||
'mapping table documents task tracking',
|
||||
);
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user