mirror of
https://github.com/obra/superpowers
synced 2026-07-17 08:04:30 +00:00
feat(sdd): lifecycle restructure with resume-based fix loop, five-round breaker, and rationalization table
This commit is contained in:
@@ -51,41 +51,85 @@ digraph process {
|
||||
subgraph cluster_per_task {
|
||||
label="Per Task";
|
||||
"Dispatch implementer subagent (./implementer-prompt.md)" [shape=box];
|
||||
"Implementer subagent asks questions?" [shape=diamond];
|
||||
"Implementer asks questions?" [shape=diamond];
|
||||
"Answer questions, provide context" [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];
|
||||
"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];
|
||||
}
|
||||
|
||||
"Read plan, note context and global constraints, create todos" [shape=box];
|
||||
"Setup: worktree, ledger check, read plan, pre-flight review" [shape=box];
|
||||
"More tasks remain?" [shape=diamond];
|
||||
"Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [shape=box];
|
||||
"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];
|
||||
"Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
|
||||
|
||||
"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?";
|
||||
"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?";
|
||||
"More tasks remain?" -> "Dispatch implementer subagent (./implementer-prompt.md)" [label="yes"];
|
||||
"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";
|
||||
"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";
|
||||
}
|
||||
```
|
||||
|
||||
## Pre-Flight Plan Review
|
||||
## Setup
|
||||
|
||||
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:
|
||||
|
||||
@@ -113,7 +157,11 @@ 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.
|
||||
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.
|
||||
|
||||
**Always specify the model explicitly when dispatching a subagent.** An
|
||||
omitted model inherits your session's model — often the most capable and
|
||||
@@ -132,7 +180,47 @@ 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
|
||||
|
||||
## Handling Implementer Status
|
||||
## 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
|
||||
|
||||
Implementer subagents report one of four statuses. Handle each appropriately:
|
||||
|
||||
@@ -150,20 +238,37 @@ 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.
|
||||
|
||||
## Handling Reviewer ⚠️ Items
|
||||
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.
|
||||
|
||||
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
|
||||
### 3. Review the task
|
||||
|
||||
Per-task reviews are task-scoped gates. The broad review happens once, at the
|
||||
final whole-branch review. When you fill a reviewer template:
|
||||
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.
|
||||
|
||||
- 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
|
||||
@@ -174,111 +279,153 @@ final whole-branch review. When you fill a reviewer template:
|
||||
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 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
|
||||
|
||||
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
|
||||
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.
|
||||
before merge. A roll-up nobody reads is a silent discard. Minor findings
|
||||
never enter the loop.
|
||||
- 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.
|
||||
|
||||
## File Handoffs
|
||||
Everything else enters the loop. A fix round is one fix dispatch plus one
|
||||
scoped re-review. Five rounds maximum per task:
|
||||
|
||||
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 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.
|
||||
|
||||
- **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.
|
||||
**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.
|
||||
|
||||
## Durable Progress
|
||||
**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.
|
||||
|
||||
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.
|
||||
**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.
|
||||
|
||||
- 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`.
|
||||
**After each round,** append to the ledger:
|
||||
`Task <N>: fix round <R>/5 (<X> addressed, <Y> open — <finding one-liners>; commits <a7>..<b7>)`
|
||||
|
||||
## Prompt Templates
|
||||
Never fix findings yourself in the controller session — your context stays
|
||||
clean for coordination, and controller fixes skip review.
|
||||
|
||||
- [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)
|
||||
**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. |
|
||||
|
||||
## Example Workflow
|
||||
|
||||
```
|
||||
You: I'm using Subagent-Driven Development to execute this plan.
|
||||
|
||||
[Read plan file once: docs/superpowers/plans/feature-plan.md]
|
||||
[Create todos for all tasks]
|
||||
[Setup: worktree verified, no ledger found, read plan, created todos]
|
||||
|
||||
Task 1: Hook installation script
|
||||
|
||||
@@ -288,8 +435,7 @@ Implementer: "Before I begin - should the hook be installed at user or system le
|
||||
|
||||
You: "User level (~/.config/superpowers/hooks/)"
|
||||
|
||||
Implementer: "Got it. Implementing now..."
|
||||
[Later] Implementer:
|
||||
Implementer: [Later]
|
||||
- Implemented install-hook command
|
||||
- Added tests, 5/5 passing
|
||||
- Self-review: Found I missed --force flag, added it
|
||||
@@ -299,77 +445,39 @@ Implementer: "Got it. Implementing now..."
|
||||
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
||||
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
||||
|
||||
[Mark Task 1 complete]
|
||||
[Ledger: Task 1: complete (commits a1b2c3d..d4e5f6a, review clean)]
|
||||
|
||||
Task 2: Recovery modes
|
||||
|
||||
[Run task-brief for Task 2; dispatch implementer with brief + report paths + context]
|
||||
|
||||
Implementer: [No questions, proceeds]
|
||||
Implementer:
|
||||
Implementer: [No questions]
|
||||
- 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)
|
||||
|
||||
[Dispatch fix subagent with all findings]
|
||||
Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant
|
||||
[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.
|
||||
|
||||
[Task reviewer reviews again]
|
||||
Task reviewer: Spec ✅. Task quality: Approved.
|
||||
[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.
|
||||
|
||||
[Mark Task 2 complete]
|
||||
[Ledger: Task 2: fix round 1/5 (2 addressed, 0 open; commits d4e5f6a..b7c8d9e)]
|
||||
[Ledger: Task 2: complete (commits d4e5f6a..b7c8d9e, review clean)]
|
||||
|
||||
...
|
||||
|
||||
[After all tasks]
|
||||
[Dispatch final code-reviewer]
|
||||
Final reviewer: All requirements met, ready to merge
|
||||
[Run review-package MERGE_BASE HEAD; dispatch final code-reviewer, most capable model]
|
||||
Final reviewer: All requirements met. Deferred minors triaged: none block merge.
|
||||
|
||||
Done!
|
||||
Done! Using superpowers:finishing-a-development-branch.
|
||||
```
|
||||
|
||||
## 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)
|
||||
|
||||
Reference in New Issue
Block a user