mirror of
https://github.com/obra/superpowers.git
synced 2026-07-23 10:44:01 +08:00
Compare commits
11 Commits
codex-spin
...
exp/loop-e
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
d1f1ae1f1e | ||
|
|
4334ff508a | ||
|
|
ce6bdd4f4a | ||
|
|
374ea4f146 | ||
|
|
2e11d4c79a | ||
|
|
76c656a8b6 | ||
|
|
3f28d9c943 | ||
|
|
ef834a2949 | ||
|
|
f0ef65c126 | ||
|
|
70c9a9a26a | ||
|
|
aaa0a8f0f4 |
@@ -59,7 +59,7 @@ digraph process {
|
||||
"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 fix review (./fix-review-prompt.md)" [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];
|
||||
@@ -72,7 +72,7 @@ digraph process {
|
||||
"Setup: worktree, ledger check, read plan, pre-flight review" [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 fix review, adjudicate residuals" [shape=box];
|
||||
"Final findings? ONE fix dispatch, one scoped re-review, adjudicate residuals" [shape=box];
|
||||
"Final review clean: delete this plan's workspace" [shape=box];
|
||||
"Use superpowers:finishing-a-development-branch" [shape=box style=filled fillcolor=lightgreen];
|
||||
|
||||
@@ -88,8 +88,8 @@ digraph process {
|
||||
"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 fix review (./fix-review-prompt.md)";
|
||||
"Dispatch scoped fix review (./fix-review-prompt.md)" -> "All findings addressed?";
|
||||
"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"];
|
||||
@@ -101,8 +101,8 @@ digraph process {
|
||||
"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 (../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 fix review, adjudicate residuals";
|
||||
"Final findings? ONE fix dispatch, one scoped fix review, adjudicate residuals" -> "Final review clean: delete this plan's workspace";
|
||||
"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" -> "Final review clean: delete this plan's workspace";
|
||||
"Final review clean: delete this plan's workspace" -> "Use superpowers:finishing-a-development-branch";
|
||||
}
|
||||
```
|
||||
@@ -133,6 +133,15 @@ a ledger file, not only in todos.
|
||||
plan's progress: leave it in place and start your own, fresh.
|
||||
- Create the ledger with its identity as the first line:
|
||||
`# SDD ledger — plan: <plan file path>`.
|
||||
- During that same Setup read, copy the plan's Global Constraints section
|
||||
verbatim into `<workspace>/constraints.md`. Every dispatch's binding
|
||||
constraint values paste from that file — the plan itself stays closed
|
||||
after Setup, even across compaction.
|
||||
- The workspace is never committed to the project repo. Do not `git add`
|
||||
anything under it, and never write a commit whose purpose is to record,
|
||||
correct, or tidy a workspace artifact — reports and ledgers are session
|
||||
records, not deliverables. If the repo lacks a `.gitignore` entry for
|
||||
`.superpowers/`, leave the directory untracked rather than committing it.
|
||||
- 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.
|
||||
@@ -140,7 +149,11 @@ a ledger file, not only in todos.
|
||||
that happens, recover from `git log`.
|
||||
|
||||
Read the plan once, note its context and Global Constraints, and create a
|
||||
todo per task.
|
||||
todo per task. That is the plan's one full read for the whole session:
|
||||
after Setup, the ledger and `scripts/task-brief` extracts are your working
|
||||
memory — re-reading the plan or spec late in the run (to "double-check"
|
||||
completion, to rebuild the final-review dispatch) re-buys context you
|
||||
already paid for and is forbidden.
|
||||
|
||||
Before dispatching Task 1, scan the plan once for conflicts:
|
||||
|
||||
@@ -158,12 +171,6 @@ conflicts that only emerge from implementation.
|
||||
|
||||
Use the least powerful model that can handle each role to conserve cost and increase speed.
|
||||
|
||||
When your platform's reference file (using-superpowers → Platform
|
||||
Adaptation) defines a dispatch role table, that table IS this section's
|
||||
mapping for your harness. Follow it over the tier language below — including
|
||||
for the final review and fix-loop escalation — and follow the
|
||||
`dispatch:` hint lines the task-brief and review-package scripts print.
|
||||
|
||||
**Mechanical implementation tasks** (isolated functions, clear specs, 1-2 files): use a fast, cheap model. Most implementation tasks are mechanical when the plan is well-specified.
|
||||
|
||||
**Integration and judgment tasks** (multi-file coordination, pattern matching, debugging): use a standard model.
|
||||
@@ -174,7 +181,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 fix reviews of
|
||||
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
|
||||
@@ -201,7 +208,10 @@ that implementer. Single-file mechanical fixes also take the cheapest tier.
|
||||
|
||||
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.
|
||||
and is re-read on every later turn. Hand artifacts over as files. The same
|
||||
tax applies to your own words: checkpoint in one short line, keep
|
||||
bookkeeping in the ledger file, and never paste back into the conversation
|
||||
what a file already holds.
|
||||
|
||||
### 1. Dispatch the implementer
|
||||
|
||||
@@ -217,9 +227,15 @@ and fix-round diffs need it.
|
||||
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.
|
||||
(5) the report-file path and report contract; (6) the plan's binding
|
||||
constraint values — any Global Constraint that mandates an exact
|
||||
mechanical form (commit-message rules, naming rules, fixed literals) —
|
||||
pasted verbatim. Task-specific exact values (numbers, magic strings,
|
||||
signatures, test cases) appear only in the brief; binding constraint
|
||||
values are the one exception — they ride in EVERY dispatch, because a
|
||||
subagent that must recall a constraint from memory will reconstruct it
|
||||
from its own priors instead. 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
|
||||
@@ -279,6 +295,10 @@ needed.
|
||||
- **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.
|
||||
- **Persist the verdict:** when the review returns, write its full text to
|
||||
`<workspace>/task-<N>-review.md` before acting on it. The file is the
|
||||
gate: completion checks for the artifact, not for your memory of a
|
||||
verdict.
|
||||
- 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
|
||||
@@ -323,34 +343,37 @@ Before the loop starts, two routes leave it immediately:
|
||||
Do not dismiss the finding because the plan mandates it, and do not
|
||||
dispatch a fix that contradicts the plan without asking.
|
||||
Everything else enters the loop. A fix round is one fix dispatch plus one
|
||||
scoped fix review — a review of the fix diff, not a fresh review. Five
|
||||
rounds maximum per task:
|
||||
scoped re-review. Five rounds maximum per task:
|
||||
|
||||
**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.
|
||||
the findings, and the binding constraint values verbatim — the report file
|
||||
is the persistent memory either way.
|
||||
|
||||
**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.
|
||||
findings, the binding constraint values verbatim, 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.
|
||||
|
||||
**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 fix 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.
|
||||
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.
|
||||
Append the re-review's returned text to `<workspace>/task-<N>-review.md` as
|
||||
well — the artifact accumulates every verdict, and the file's last entry is
|
||||
the one completion relies on.
|
||||
|
||||
**The fix review is scoped.** Run `scripts/review-package --role fix-review PLAN_FILE FIX_BASE HEAD`
|
||||
**The re-review is scoped.** Run `scripts/review-package PLAN_FILE FIX_BASE HEAD`
|
||||
where FIX_BASE is the head the previous review saw, and dispatch
|
||||
[fix-review-prompt.md](fix-review-prompt.md) with the findings list, the
|
||||
brief, the report file, and the printed diff path. The fix reviewer verdicts
|
||||
[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
|
||||
@@ -362,7 +385,7 @@ minors — they never extend the loop.
|
||||
Never fix findings yourself in the controller session — your context stays
|
||||
clean for coordination, and controller fixes skip review.
|
||||
|
||||
**The breaker.** When round 5's fix review still leaves findings open, stop
|
||||
**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:
|
||||
|
||||
@@ -390,15 +413,25 @@ 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
|
||||
- `Task <N>: complete (commits <base7>..<head7>, deviation parked: <rule>)`
|
||||
when a landed commit violates a mechanical constraint that nothing
|
||||
downstream builds on. Record the ruling in the ledger. A green build with
|
||||
a parked, recorded deviation is complete — do not fail the task, and do
|
||||
not rewrite landed history to chase cosmetics. A load-bearing violation
|
||||
is different: that is a BLOCKED, not a deviation. This valve is not the
|
||||
breaker: it needs no exhausted fix rounds — a mechanical deviation
|
||||
discovered at completion parks here directly, with its ruling in the ledger.
|
||||
|
||||
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.
|
||||
Then mark the todo complete and move on — but only once
|
||||
`<workspace>/task-<N>-review.md` exists; a completion line without its review
|
||||
artifact is invalid, whatever you remember about the review. 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 --role final-review PLAN_FILE MERGE_BASE HEAD` (MERGE_BASE = the commit the
|
||||
`scripts/review-package PLAN_FILE 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
|
||||
@@ -406,29 +439,28 @@ 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.
|
||||
fixed before merge. Build that dispatch from the ledger alone — the
|
||||
completion lines, parked rulings, and deferred minors are the whole-run
|
||||
summary; do not re-read the plan, the spec, or per-task reports to
|
||||
reconstruct what the ledger already states. Write the returned review to
|
||||
`<workspace>/final-review.md` before dispatching any fix wave — the merge
|
||||
decision cites the artifact, not a recollection.
|
||||
|
||||
If the final whole-branch review returns findings, dispatch ONE fix subagent
|
||||
with the complete findings list — not one fixer per finding.
|
||||
with the complete findings list and the binding constraint values
|
||||
verbatim — 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 fix review of the fix wave
|
||||
(`scripts/review-package --role fix-review PLAN_FILE FIX_BASE HEAD` over the fix range,
|
||||
[fix-review-prompt.md](fix-review-prompt.md)).
|
||||
Then run exactly one scoped re-review of the fix wave
|
||||
(`scripts/review-package PLAN_FILE FIX_BASE HEAD` 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 —
|
||||
rulings, or stop on load-bearing ones. The same valve applies to
|
||||
mechanical-constraint misses discovered at the end: non-load-bearing means
|
||||
parked with a ruling, not a failed branch. There is no second fix wave —
|
||||
residual load-bearing findings surface to your human partner when
|
||||
finishing-a-development-branch presents the options.
|
||||
|
||||
The wave closing is policy, not a verdict. A sufficiently strong reviewer
|
||||
finds real defects indefinitely, so "review until one comes back clean"
|
||||
never terminates — the completed wave is the exit, not a clean report.
|
||||
New Critical/Important breakage in the final fix diff joins the residuals
|
||||
for adjudication; it does not start a second wave. And review procedures
|
||||
your human partner sets up for one review — competing reviewers, scoring,
|
||||
extra seats — apply to that review only. Never adopt them as standing
|
||||
procedure for reviews they didn't ask about.
|
||||
|
||||
## Finish
|
||||
|
||||
When the final whole-branch review is clean and its fixes are merged,
|
||||
@@ -445,13 +477,11 @@ Use superpowers:finishing-a-development-branch.
|
||||
| "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 fix reviews verify fixes; they cannot wander. New findings on untouched code go to the ledger, not the loop. |
|
||||
| "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 fix review" | Unreviewed fixes are how regressions land. Every round ends with a scoped fix review. |
|
||||
| "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. |
|
||||
| "This new finding is real — one more wave" | Real findings are infinite under a strong reviewer. The completed wave is the exit; adjudicate and route. |
|
||||
| "They liked competing reviewers earlier, I'll run them again" | One-off review procedures apply to the review they were given for. Re-adopting them unasked is scope creep in review clothing. |
|
||||
|
||||
## Example Workflow
|
||||
|
||||
@@ -501,8 +531,8 @@ Task reviewer: Spec ❌:
|
||||
Implementer: Added progress reporting, extracted PROGRESS_INTERVAL constant.
|
||||
Re-ran test/recovery.test.js — 10/10 passing. Fix report appended.
|
||||
|
||||
[Run review-package --role fix-review PLAN_FILE FIX_BASE HEAD; dispatch scoped fix review]
|
||||
Fix reviewer: Missing progress reporting — ADDRESSED (src/recovery.js:41).
|
||||
[Run review-package PLAN_FILE 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.
|
||||
|
||||
@@ -512,7 +542,7 @@ Fix reviewer: Missing progress reporting — ADDRESSED (src/recovery.js:41).
|
||||
...
|
||||
|
||||
[After all tasks]
|
||||
[Run review-package --role final-review PLAN_FILE MERGE_BASE HEAD; dispatch final code-reviewer, most capable model]
|
||||
[Run review-package PLAN_FILE MERGE_BASE HEAD; dispatch final code-reviewer, most capable model]
|
||||
Final reviewer: All requirements met. Deferred minors triaged: none block merge.
|
||||
|
||||
[Delete this plan's workspace — the record now lives in git]
|
||||
|
||||
@@ -15,6 +15,12 @@ Subagent (general-purpose):
|
||||
Read your task brief first: [BRIEF_FILE]
|
||||
It contains the full task text from the plan.
|
||||
|
||||
## Binding Constraint Values
|
||||
|
||||
[CONSTRAINT_VALUES — the plan's mechanical constraints, pasted verbatim
|
||||
by the dispatcher. If a rule here mandates an exact form (commit-message
|
||||
text, naming, fixed literals), reproduce it exactly — never from memory.]
|
||||
|
||||
## Context
|
||||
|
||||
[Scene-setting: where this fits, dependencies, architectural context]
|
||||
@@ -36,8 +42,12 @@ Subagent (general-purpose):
|
||||
2. Write tests (following TDD if task says to)
|
||||
3. Verify implementation works
|
||||
4. Commit your work
|
||||
5. Self-review (see below)
|
||||
6. Report back
|
||||
5. Immediately after each commit, verify it against the Binding Constraint
|
||||
Values above (commit-message rules, naming rules, fixed literals) while
|
||||
history is still local. A miss is cheap now — amend or forward-fix at
|
||||
once — and expensive after your work is delivered.
|
||||
6. Self-review (see below)
|
||||
7. Report back
|
||||
|
||||
Work from: [directory]
|
||||
|
||||
@@ -96,6 +106,10 @@ Subagent (general-purpose):
|
||||
- Did I only build what was requested?
|
||||
- Did I follow existing patterns in the codebase?
|
||||
|
||||
**Constraints:**
|
||||
- Does every commit message satisfy the Binding Constraint Values exactly?
|
||||
- Did I reproduce mandated literals from the constraint text, not from memory?
|
||||
|
||||
**Testing:**
|
||||
- Do tests actually verify behavior (not just mock behavior)?
|
||||
- Did I follow TDD if required?
|
||||
@@ -104,27 +118,26 @@ Subagent (general-purpose):
|
||||
|
||||
If you find issues during self-review, fix them now before reporting.
|
||||
|
||||
If a constraint violation is already in a landed commit you cannot safely
|
||||
amend, forward-fix it in a new commit when possible; when it is not, report
|
||||
the deviation explicitly with Status DONE_WITH_CONCERNS — never report the
|
||||
task incomplete solely for a cosmetic miss on an otherwise green build.
|
||||
Whether the deviation parks or blocks is the dispatching controller's call.
|
||||
|
||||
## 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. If your fix report claims a
|
||||
full-suite pass, that claim needs a fresh run after your last edit —
|
||||
a suite run from before the findings arrived no longer counts. Then
|
||||
reply with the same short status contract as your first report.
|
||||
you — your report is the test evidence. Then reply with the same short
|
||||
status contract as your first report.
|
||||
|
||||
## Report Format
|
||||
|
||||
Write your full report to [REPORT_FILE]:
|
||||
- What you implemented (or what you attempted, if blocked)
|
||||
- What you tested, and for every gate you claim — focused tests, full
|
||||
suite, lint, build — the exact command and the tail of its fresh
|
||||
output. Fresh means run after your final edit: if you edited anything
|
||||
since your last full-suite run, that run is stale — rerun it or
|
||||
report the suite as unverified. A gate claim without pasted fresh
|
||||
output is itself a defect for the reviewer to flag.
|
||||
- What you tested and test results
|
||||
- **TDD Evidence** (if TDD was required for this task):
|
||||
- RED: command run, relevant failing output before implementation, and why the failure was expected
|
||||
- GREEN: command run and relevant passing output after implementation
|
||||
@@ -143,7 +156,8 @@ Subagent (general-purpose):
|
||||
If BLOCKED or NEEDS_CONTEXT, put the specifics in the final message
|
||||
itself — the controller acts on it directly.
|
||||
|
||||
Use DONE_WITH_CONCERNS if you completed the work but have doubts about correctness.
|
||||
Use DONE_WITH_CONCERNS if you completed the work but have doubts about correctness, or
|
||||
when you are reporting a known deviation you could not safely fix.
|
||||
Use BLOCKED if you cannot complete the task. Use NEEDS_CONTEXT if you need
|
||||
information that wasn't provided. Never silently produce work you're unsure about.
|
||||
```
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
# Scoped Fix Review Prompt Template
|
||||
# Scoped Re-Review Prompt Template
|
||||
|
||||
Use this template when dispatching a fix review after a fix round. The
|
||||
fix reviewer verifies the findings were addressed and checks the fix diff for
|
||||
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
|
||||
@@ -9,11 +9,11 @@ that the fix itself broke nothing.
|
||||
|
||||
```
|
||||
Subagent (general-purpose):
|
||||
description: "Fix review Task N round R"
|
||||
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 reviewing one task's fix round. A previous review produced
|
||||
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.
|
||||
|
||||
@@ -47,7 +47,7 @@ Subagent (general-purpose):
|
||||
|
||||
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
|
||||
review code the fix did not touch: if you notice an issue entirely
|
||||
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.
|
||||
@@ -93,14 +93,14 @@ Subagent (general-purpose):
|
||||
|
||||
**Placeholders:**
|
||||
- `[MODEL]` — REQUIRED: reviewer model per SKILL.md Model Selection; scoped
|
||||
fix reviews of small fix diffs take a cheap-to-mid tier
|
||||
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 --role fix-review PLAN_FILE FIX_BASE HEAD` printed
|
||||
- `[DIFF_FILE]` — the path `scripts/review-package PLAN_FILE FIX_BASE HEAD` printed
|
||||
|
||||
**Fix reviewer returns:** per-finding verdicts (ADDRESSED / NOT ADDRESSED),
|
||||
**Re-reviewer returns:** per-finding verdicts (ADDRESSED / NOT ADDRESSED),
|
||||
new breakage in the fix diff, out-of-scope observations, and a round verdict.
|
||||
@@ -4,31 +4,13 @@
|
||||
# call. Using the recorded per-task BASE (not HEAD~1) keeps multi-commit
|
||||
# tasks intact.
|
||||
#
|
||||
# Usage: review-package [--role task-review|fix-review|final-review] PLAN_FILE BASE HEAD [OUTFILE]
|
||||
# Usage: review-package PLAN_FILE BASE HEAD [OUTFILE]
|
||||
# Default OUTFILE: <repo-root>/.superpowers/sdd/<plan-basename>/review-<base7>..<head7>.diff
|
||||
# (named per range, so a fix review after fixes gets a distinct fresh file).
|
||||
#
|
||||
# The trailing dispatch hint rides this output because the controller reads it
|
||||
# immediately before spawning the reviewer; skill text loaded at session start
|
||||
# does not survive context compaction, but this line is reprinted every round.
|
||||
# (named per range, so a re-review after fixes gets a distinct fresh file).
|
||||
set -euo pipefail
|
||||
|
||||
script_dir=$(cd "$(dirname "$0")" && pwd)
|
||||
|
||||
role=task-review
|
||||
if [ "${1:-}" = "--role" ]; then
|
||||
[ $# -ge 2 ] || { echo "usage: review-package [--role task-review|fix-review|final-review] PLAN_FILE BASE HEAD [OUTFILE]" >&2; exit 2; }
|
||||
role=$2
|
||||
shift 2
|
||||
fi
|
||||
|
||||
case "$role" in
|
||||
task-review|fix-review|final-review) hint_key=$role ;;
|
||||
*) echo "bad --role: ${role} (task-review|fix-review|final-review)" >&2; exit 2 ;;
|
||||
esac
|
||||
|
||||
if [ $# -lt 3 ] || [ $# -gt 4 ]; then
|
||||
echo "usage: review-package [--role task-review|fix-review|final-review] PLAN_FILE BASE HEAD [OUTFILE]" >&2
|
||||
echo "usage: review-package PLAN_FILE BASE HEAD [OUTFILE]" >&2
|
||||
exit 2
|
||||
fi
|
||||
|
||||
@@ -43,7 +25,7 @@ git rev-parse --verify --quiet "$head" >/dev/null || { echo "bad HEAD: $head" >&
|
||||
if [ $# -eq 4 ]; then
|
||||
out=$4
|
||||
else
|
||||
dir=$("$script_dir/sdd-workspace" "$plan")
|
||||
dir=$("$(cd "$(dirname "$0")" && pwd)/sdd-workspace" "$plan")
|
||||
out="$dir/review-$(git rev-parse --short "$base")..$(git rev-parse --short "$head").diff"
|
||||
fi
|
||||
|
||||
@@ -62,16 +44,3 @@ fi
|
||||
|
||||
commits=$(git rev-list --count "${base}..${head}")
|
||||
echo "wrote ${out}: ${commits} commit(s), $(wc -c < "$out" | tr -d ' ') bytes"
|
||||
|
||||
# Platform dispatch hints ride this output because the controller reads it
|
||||
# immediately before spawning; the lines themselves are owned by the platform
|
||||
# reference layer (using-superpowers/references/*-dispatch.hints), not this
|
||||
# script. Claude Code's dispatch templates carry model selection already, so
|
||||
# the relay is suppressed there and on any harness without a hints file.
|
||||
hints_file="$script_dir/../../using-superpowers/references/codex-dispatch.hints"
|
||||
if [ -z "${CLAUDECODE:-}" ] && [ -f "$hints_file" ]; then
|
||||
hint_line=$(grep "^${hint_key}:" "$hints_file" | head -1 | cut -d: -f2- | sed 's/^ *//') || true
|
||||
if [ -n "$hint_line" ]; then
|
||||
echo "$hint_line"
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -18,12 +18,10 @@ plan=$1
|
||||
n=$2
|
||||
[ -f "$plan" ] || { echo "no such plan file: $plan" >&2; exit 2; }
|
||||
|
||||
script_dir=$(cd "$(dirname "$0")" && pwd)
|
||||
|
||||
if [ $# -eq 3 ]; then
|
||||
out=$3
|
||||
else
|
||||
dir=$("$script_dir/sdd-workspace" "$plan")
|
||||
dir=$("$(cd "$(dirname "$0")" && pwd)/sdd-workspace" "$plan")
|
||||
out="$dir/task-${n}-brief.md"
|
||||
fi
|
||||
|
||||
@@ -41,16 +39,3 @@ if [ ! -s "$out" ]; then
|
||||
fi
|
||||
|
||||
echo "wrote ${out}: $(wc -l < "$out" | tr -d ' ') lines"
|
||||
|
||||
# Platform dispatch hints ride this output because the controller reads it
|
||||
# immediately before spawning; the lines themselves are owned by the platform
|
||||
# reference layer (using-superpowers/references/*-dispatch.hints), not this
|
||||
# script. Claude Code's dispatch templates carry model selection already, so
|
||||
# the relay is suppressed there and on any harness without a hints file.
|
||||
hints_file="$script_dir/../../using-superpowers/references/codex-dispatch.hints"
|
||||
if [ -z "${CLAUDECODE:-}" ] && [ -f "$hints_file" ]; then
|
||||
hint_line=$(grep "^implementer:" "$hints_file" | head -1 | cut -d: -f2- | sed 's/^ *//') || true
|
||||
if [ -n "$hint_line" ]; then
|
||||
echo "$hint_line"
|
||||
fi
|
||||
fi
|
||||
|
||||
@@ -1,11 +0,0 @@
|
||||
# Per-role dispatch lines for Codex spawn_agent, relayed by the
|
||||
# subagent-driven-development task-brief and review-package scripts at the
|
||||
# moment of dispatch (skill text loaded at session start does not survive
|
||||
# context compaction; these lines reprint every round).
|
||||
# Model names track Codex's spawn_agent allowlist (currently gpt-5.6-sol
|
||||
# and gpt-5.6-terra) — update this file when the allowlist changes.
|
||||
# Format: <role>: <line printed verbatim>
|
||||
implementer: dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high
|
||||
task-review: dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high
|
||||
fix-review: dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=medium
|
||||
final-review: dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high
|
||||
@@ -9,31 +9,6 @@ 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.
|
||||
|
||||
## SDD dispatch on Codex
|
||||
|
||||
Every SDD `spawn_agent` call sets `fork_turns: "none"` — the default
|
||||
`"all"` forks your whole transcript into the child and refuses model
|
||||
and effort overrides.
|
||||
|
||||
If your `spawn_agent` schema has `model` and `reasoning_effort`
|
||||
parameters (Codex 0.145+), set both on every dispatch: task-brief and
|
||||
review-package print a `dispatch:` hint line with the exact values —
|
||||
copy it onto the call verbatim, every time, even late in a long
|
||||
session. Those hints are the Model Selection mapping on Codex:
|
||||
reviewer tier never exceeds implementer tier, no fix round gets an
|
||||
effort bump, and rounds 4-5's "more capable model" means a fresh
|
||||
implementer at the same tier — needing more is a BLOCKED escalation
|
||||
to your human partner. Inherited frontier-tier subagents are a
|
||||
measured cause of runs spinning out for hours. (Values live in
|
||||
`codex-dispatch.hints` beside this file; they track the spawn_agent
|
||||
model allowlist.)
|
||||
|
||||
Without those parameters (Codex 0.144 and earlier), children inherit
|
||||
your model and effort with no override — role files in
|
||||
`~/.codex/agents/` do not attach to spawns either. Tell your human
|
||||
partner before starting a plan of more than a few tasks, and offer a
|
||||
lower-effort session instead.
|
||||
|
||||
## Environment Detection
|
||||
|
||||
Skills that create worktrees or finish branches should detect their
|
||||
|
||||
@@ -71,7 +71,11 @@ independently testable deliverable.
|
||||
[The spec's project-wide requirements — version floors, dependency limits,
|
||||
naming and copy rules, platform requirements — one line each, with exact
|
||||
values copied verbatim from the spec. Every task's requirements implicitly
|
||||
include this section.]
|
||||
include this section. Three things may never enter it: cosmetic absolutes
|
||||
on every commit (a fixed trailer or byline the work does not need), your
|
||||
own identity or model name promoted into a rule, and environment
|
||||
constraints (versions, platforms, paths) you have not verified against the
|
||||
environment the plan will execute in.]
|
||||
|
||||
---
|
||||
```
|
||||
@@ -125,6 +129,8 @@ git commit -m "feat: add specific feature"
|
||||
```
|
||||
````
|
||||
|
||||
Commit messages describe the change. Never mandate session boilerplate — trailers, bylines, model names — as a per-commit rule; what your session stamps on its commits is not a requirement of the work.
|
||||
|
||||
## No Placeholders
|
||||
|
||||
Every step must contain the actual content an engineer needs. These are **plan failures** — never write them:
|
||||
@@ -145,6 +151,8 @@ After writing the complete plan, look at the spec with fresh eyes and check the
|
||||
|
||||
**3. Type consistency:** Do the types, method signatures, and property names you used in later tasks match what you defined in earlier tasks? A function called `clearLayers()` in Task 3 but `clearFullLayers()` in Task 7 is a bug.
|
||||
|
||||
**4. Constraint hygiene:** Does any Global Constraint mandate a per-commit cosmetic absolute, name the authoring model or session, or assert an environment fact (version floor, platform, path) you did not verify? Cut or verify it.
|
||||
|
||||
If you find issues, fix them inline. No need to re-review — just fix and move on. If you find a spec requirement with no task, add the task.
|
||||
|
||||
## Execution Handoff
|
||||
|
||||
@@ -165,63 +165,6 @@ PLAN
|
||||
echo " got: $rp_explicit"
|
||||
fi
|
||||
|
||||
# --- platform dispatch hints ride the script output (suppressed on CC) ---
|
||||
local brief_hint
|
||||
brief_hint="$(cd "$repo" && env -u CLAUDECODE "$SDD_SCRIPTS/task-brief" plan-a.md 1)"
|
||||
if [[ "$brief_hint" == *"dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high"* ]]; then
|
||||
pass "task-brief relays the implementer dispatch hint off Claude Code"
|
||||
else
|
||||
fail "task-brief relays the implementer dispatch hint off Claude Code"
|
||||
echo " got: $brief_hint"
|
||||
fi
|
||||
|
||||
local rp_hint
|
||||
rp_hint="$(cd "$repo" && env -u CLAUDECODE "$SDD_SCRIPTS/review-package" plan-a.md HEAD~1 HEAD)"
|
||||
if [[ "$rp_hint" == *"dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high"* ]]; then
|
||||
pass "review-package relays the default-role hint off Claude Code"
|
||||
else
|
||||
fail "review-package relays the default-role hint off Claude Code"
|
||||
echo " got: $rp_hint"
|
||||
fi
|
||||
|
||||
local rp_fixreview
|
||||
rp_fixreview="$(cd "$repo" && env -u CLAUDECODE "$SDD_SCRIPTS/review-package" --role fix-review plan-a.md HEAD~1 HEAD)"
|
||||
if [[ "$rp_fixreview" == *"dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=medium"* ]]; then
|
||||
pass "review-package --role fix-review relays the medium-effort hint"
|
||||
else
|
||||
fail "review-package --role fix-review relays the medium-effort hint"
|
||||
echo " got: $rp_fixreview"
|
||||
fi
|
||||
|
||||
local rp_final
|
||||
rp_final="$(cd "$repo" && env -u CLAUDECODE "$SDD_SCRIPTS/review-package" --role final-review plan-a.md HEAD~1 HEAD)"
|
||||
if [[ "$rp_final" == *"dispatch (spawn_agent): fork_turns=none model=gpt-5.6-terra reasoning_effort=high"* ]]; then
|
||||
pass "review-package --role final-review relays the high-effort hint"
|
||||
else
|
||||
fail "review-package --role final-review relays the high-effort hint"
|
||||
echo " got: $rp_final"
|
||||
fi
|
||||
|
||||
local brief_cc rp_cc
|
||||
brief_cc="$(cd "$repo" && CLAUDECODE=1 "$SDD_SCRIPTS/task-brief" plan-a.md 1)"
|
||||
rp_cc="$(cd "$repo" && CLAUDECODE=1 "$SDD_SCRIPTS/review-package" plan-a.md HEAD~1 HEAD)"
|
||||
if [[ "$brief_cc" != *"dispatch (spawn_agent)"* && "$rp_cc" != *"dispatch (spawn_agent)"* ]]; then
|
||||
pass "dispatch hints are suppressed under Claude Code (CLAUDECODE set)"
|
||||
else
|
||||
fail "dispatch hints are suppressed under Claude Code (CLAUDECODE set)"
|
||||
echo " brief: $brief_cc"
|
||||
echo " rp: $rp_cc"
|
||||
fi
|
||||
|
||||
rc=0
|
||||
(cd "$repo" && "$SDD_SCRIPTS/review-package" --role bogus plan-a.md HEAD~1 HEAD >/dev/null 2>&1) || rc=$?
|
||||
if [[ "$rc" -eq 2 ]]; then
|
||||
pass "review-package rejects an unknown --role with exit 2"
|
||||
else
|
||||
fail "review-package rejects an unknown --role with exit 2"
|
||||
echo " exit: $rc"
|
||||
fi
|
||||
|
||||
# --- Worktree isolation: a linked worktree resolves its own workspace ---
|
||||
local wt="$TEST_ROOT/wt"
|
||||
( cd "$repo" && git worktree add -q "$wt" -b wt-feature )
|
||||
|
||||
Reference in New Issue
Block a user