mirror of
https://github.com/obra/superpowers.git
synced 2026-07-17 15:34:00 +08:00
Compare commits
17 Commits
skill-detr
...
sdd-fix-lo
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
1f97eda0fc | ||
|
|
236077326b | ||
|
|
bdbe81c403 | ||
|
|
42f6a5772f | ||
|
|
da61283d43 | ||
|
|
4562d18dcf | ||
|
|
14603727c8 | ||
|
|
019e79cc46 | ||
|
|
d74653cf74 | ||
|
|
3550dd05cd | ||
|
|
8489d22016 | ||
|
|
22d65cf8f0 | ||
|
|
9d941bec3b | ||
|
|
9da6fec633 | ||
|
|
43d87baeed | ||
|
|
c81f29fc6b | ||
|
|
5e046b3db2 |
1649
docs/superpowers/plans/2026-07-15-sdd-fix-loop-redesign.md
Normal file
1649
docs/superpowers/plans/2026-07-15-sdd-fix-loop-redesign.md
Normal file
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,196 @@
|
|||||||
|
# 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.
|
||||||
@@ -77,6 +77,7 @@ digraph brainstorming {
|
|||||||
- Propose 2-3 different approaches with trade-offs
|
- Propose 2-3 different approaches with trade-offs
|
||||||
- Present options conversationally with your recommendation and reasoning
|
- Present options conversationally with your recommendation and reasoning
|
||||||
- Lead with your recommended option and explain why
|
- Lead with your recommended option and explain why
|
||||||
|
- YAGNI ruthlessly - remove unnecessary features from every approach and design
|
||||||
|
|
||||||
**Presenting the design:**
|
**Presenting the design:**
|
||||||
|
|
||||||
@@ -130,15 +131,6 @@ Wait for the user's response. If they request changes, make them and re-run the
|
|||||||
- Invoke the writing-plans skill to create a detailed implementation plan
|
- Invoke the writing-plans skill to create a detailed implementation plan
|
||||||
- Do NOT invoke any other skill. writing-plans is the next step.
|
- Do NOT invoke any other skill. writing-plans is the next step.
|
||||||
|
|
||||||
## Key Principles
|
|
||||||
|
|
||||||
- **One question at a time** - Don't overwhelm with multiple questions
|
|
||||||
- **Multiple choice preferred** - Easier to answer than open-ended when possible
|
|
||||||
- **YAGNI ruthlessly** - Remove unnecessary features from all designs
|
|
||||||
- **Explore alternatives** - Always propose 2-3 approaches before settling
|
|
||||||
- **Incremental validation** - Present design, get approval before moving on
|
|
||||||
- **Be flexible** - Go back and clarify when something doesn't make sense
|
|
||||||
|
|
||||||
## Visual Companion
|
## Visual Companion
|
||||||
|
|
||||||
A browser-based companion for showing mockups, diagrams, and visual options during brainstorming. Available as a tool — not a mode. Accepting the companion means it's available for questions that benefit from visual treatment; it does NOT mean every question goes through the browser.
|
A browser-based companion for showing mockups, diagrams, and visual options during brainstorming. Available as a tool — not a mode. Accepting the companion means it's available for questions that benefit from visual treatment; it does NOT mean every question goes through the browser.
|
||||||
|
|||||||
@@ -158,15 +158,6 @@ Agent 3 → Fix tool-approval-race-conditions.test.ts
|
|||||||
|
|
||||||
**Integration:** All fixes independent, no conflicts, full suite green
|
**Integration:** All fixes independent, no conflicts, full suite green
|
||||||
|
|
||||||
**Time saved:** 3 problems solved in parallel vs sequentially
|
|
||||||
|
|
||||||
## Key Benefits
|
|
||||||
|
|
||||||
1. **Parallelization** - Multiple investigations happen simultaneously
|
|
||||||
2. **Focus** - Each agent has narrow scope, less context to track
|
|
||||||
3. **Independence** - Agents don't interfere with each other
|
|
||||||
4. **Speed** - 3 problems solved in time of 1
|
|
||||||
|
|
||||||
## Verification
|
## Verification
|
||||||
|
|
||||||
After agents return:
|
After agents return:
|
||||||
@@ -174,12 +165,3 @@ After agents return:
|
|||||||
2. **Check for conflicts** - Did agents edit same code?
|
2. **Check for conflicts** - Did agents edit same code?
|
||||||
3. **Run full suite** - Verify all fixes work together
|
3. **Run full suite** - Verify all fixes work together
|
||||||
4. **Spot check** - Agents can make systematic errors
|
4. **Spot check** - Agents can make systematic errors
|
||||||
|
|
||||||
## Real-World Impact
|
|
||||||
|
|
||||||
From debugging session (2025-10-03):
|
|
||||||
- 6 failures across 3 files
|
|
||||||
- 3 agents dispatched in parallel
|
|
||||||
- All investigations completed concurrently
|
|
||||||
- All fixes integrated successfully
|
|
||||||
- Zero conflicts between agent changes
|
|
||||||
|
|||||||
@@ -11,7 +11,7 @@ Load plan, review critically, execute all tasks, report when complete.
|
|||||||
|
|
||||||
**Announce at start:** "I'm using the executing-plans skill to implement this plan."
|
**Announce at start:** "I'm using the executing-plans skill to implement this plan."
|
||||||
|
|
||||||
**Note:** Tell your human partner that Superpowers works much better with access to subagents. The quality of its work will be significantly higher if run on a platform with subagent support (Claude Code, Codex CLI, Codex App, Copilot CLI, and Gemini CLI all qualify; see the per-platform tool refs in `../using-superpowers/references/`). If subagents are available, use superpowers:subagent-driven-development instead of this skill.
|
**Note:** Tell your human partner that Superpowers works much better with access to subagents (Claude Code, Codex CLI, Codex App, Copilot CLI, and Gemini CLI all qualify; see the per-platform tool refs in `../using-superpowers/references/`). If subagents are available, use superpowers:subagent-driven-development instead of this skill.
|
||||||
|
|
||||||
## The Process
|
## The Process
|
||||||
|
|
||||||
|
|||||||
@@ -203,11 +203,3 @@ You understand 1,2,3,6. Unclear on 4,5.
|
|||||||
## GitHub Thread Replies
|
## GitHub Thread Replies
|
||||||
|
|
||||||
When replying to inline review comments on GitHub, reply in the comment thread (`gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies`), not as a top-level PR comment.
|
When replying to inline review comments on GitHub, reply in the comment thread (`gh api repos/{owner}/{repo}/pulls/{pr}/comments/{id}/replies`), not as a top-level PR comment.
|
||||||
|
|
||||||
## The Bottom Line
|
|
||||||
|
|
||||||
**External feedback = suggestions to evaluate, not orders to follow.**
|
|
||||||
|
|
||||||
Verify. Question. Then implement.
|
|
||||||
|
|
||||||
No performative agreement. Technical rigor always.
|
|
||||||
|
|||||||
@@ -5,7 +5,7 @@ description: Use when completing tasks, implementing major features, or before m
|
|||||||
|
|
||||||
# Requesting Code Review
|
# Requesting Code Review
|
||||||
|
|
||||||
Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history. This keeps the reviewer focused on the work product, not your thought process, and preserves your own context for continued work.
|
Dispatch a code reviewer subagent to catch issues before they cascade. The reviewer gets precisely crafted context for evaluation — never your session's history.
|
||||||
|
|
||||||
**Core principle:** Review early, review often.
|
**Core principle:** Review early, review often.
|
||||||
|
|
||||||
@@ -72,20 +72,12 @@ You: [Fix progress indicators]
|
|||||||
[Continue to Task 3]
|
[Continue to Task 3]
|
||||||
```
|
```
|
||||||
|
|
||||||
## Integration with Workflows
|
## Common Rationalizations
|
||||||
|
|
||||||
**Subagent-Driven Development:**
|
| Excuse | Reality |
|
||||||
- Review after EACH task
|
|--------|---------|
|
||||||
- Catch issues before they compound
|
| "I'll just review the diff myself instead of dispatching a reviewer" | You're the coordinator — reviewing the diff inline burns the context window you need to keep driving the work. Dispatch a reviewer subagent: the diff and the evaluation live in its context, and only the findings come back to you. |
|
||||||
- Fix before moving to next task
|
| "The reviewer needs my whole session history to understand the change" | Hand it precisely crafted context, never your session's history. That keeps the reviewer on the work product, not your thought process. |
|
||||||
|
|
||||||
**Executing Plans:**
|
|
||||||
- Review after each task or at natural checkpoints
|
|
||||||
- Get feedback, apply, continue
|
|
||||||
|
|
||||||
**Ad-Hoc Development:**
|
|
||||||
- Review before merge
|
|
||||||
- Review when stuck
|
|
||||||
|
|
||||||
## Red Flags
|
## Red Flags
|
||||||
|
|
||||||
|
|||||||
@@ -51,41 +51,85 @@ digraph process {
|
|||||||
subgraph cluster_per_task {
|
subgraph cluster_per_task {
|
||||||
label="Per Task";
|
label="Per Task";
|
||||||
"Dispatch implementer subagent (./implementer-prompt.md)" [shape=box];
|
"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];
|
"Answer questions, provide context" [shape=box];
|
||||||
"Implementer subagent implements, tests, commits, self-reviews" [shape=box];
|
"Implementer implements, tests, commits, self-reviews" [shape=box];
|
||||||
"Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [shape=box];
|
"Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)" [shape=box];
|
||||||
"Task reviewer reports spec ✅ and quality approved?" [shape=diamond];
|
"Spec ✅ and quality approved?" [shape=diamond];
|
||||||
"Dispatch fix subagent for Critical/Important findings" [shape=box];
|
"Finding conflicts with plan text?" [shape=diamond];
|
||||||
"Mark task complete in todo list and progress ledger" [shape=box];
|
"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];
|
"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];
|
"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)";
|
"Setup: worktree, ledger check, read plan, pre-flight review" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
||||||
"Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer subagent asks questions?";
|
"Dispatch implementer subagent (./implementer-prompt.md)" -> "Implementer asks questions?";
|
||||||
"Implementer subagent asks questions?" -> "Answer questions, provide context" [label="yes"];
|
"Implementer asks questions?" -> "Answer questions, provide context" [label="yes"];
|
||||||
"Answer questions, provide context" -> "Dispatch implementer subagent (./implementer-prompt.md)";
|
"Answer questions, provide context" -> "Implementer implements, tests, commits, self-reviews";
|
||||||
"Implementer subagent asks questions?" -> "Implementer subagent implements, tests, commits, self-reviews" [label="no"];
|
"Implementer asks questions?" -> "Implementer 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)";
|
"Implementer implements, tests, commits, self-reviews" -> "Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)";
|
||||||
"Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" -> "Task reviewer reports spec ✅ and quality approved?";
|
"Generate review package, dispatch task reviewer (./task-reviewer-prompt.md)" -> "Spec ✅ and quality approved?";
|
||||||
"Task reviewer reports spec ✅ and quality approved?" -> "Dispatch fix subagent for Critical/Important findings" [label="no"];
|
"Spec ✅ and quality approved?" -> "Append completion to ledger, mark todo complete" [label="yes"];
|
||||||
"Dispatch fix subagent for Critical/Important findings" -> "Write diff file, dispatch task reviewer subagent (./task-reviewer-prompt.md)" [label="re-review"];
|
"Spec ✅ and quality approved?" -> "Finding conflicts with plan text?" [label="no"];
|
||||||
"Task reviewer reports spec ✅ and quality approved?" -> "Mark task complete in todo list and progress ledger" [label="yes"];
|
"Finding conflicts with plan text?" -> "Ask human partner which governs" [label="yes"];
|
||||||
"Mark task complete in todo list and progress ledger" -> "More tasks remain?";
|
"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 implementer subagent (./implementer-prompt.md)" [label="yes"];
|
||||||
"More tasks remain?" -> "Dispatch final code reviewer subagent (../requesting-code-review/code-reviewer.md)" [label="no"];
|
"More tasks remain?" -> "Dispatch final code reviewer (../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";
|
"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
|
Ensure the work happens in an isolated workspace: use
|
||||||
superpowers:using-git-worktrees to create one or verify the existing one.
|
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:
|
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
|
**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
|
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
|
**Always specify the model explicitly when dispatching a subagent.** An
|
||||||
omitted model inherits your session's model — often the most capable and
|
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
|
- Touches multiple files with integration concerns → standard model
|
||||||
- Requires design judgment or broad codebase understanding → most capable 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:
|
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.
|
**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
|
### 3. Review the task
|
||||||
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
|
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
|
- Do not add open-ended directives like "check all uses" or "run race tests
|
||||||
if useful" without a concrete, task-specific reason
|
if useful" without a concrete, task-specific reason
|
||||||
- Do not ask a reviewer to re-run tests the implementer already ran on the
|
- 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
|
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
|
as a defect," "at most Minor," or "the plan chose" — stop: you are
|
||||||
pre-judging, usually to spare yourself a review loop.
|
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
|
The task reviewer may report "⚠️ Cannot verify from diff" items — requirements
|
||||||
Constraints section or the spec: exact values, exact formats, and the
|
that live in unchanged code or span tasks. These do not block the rest of the
|
||||||
stated relationships between components ("same layout as X", "matches
|
review, but you must resolve each one yourself before marking the task
|
||||||
Y"). The reviewer's template already carries the process rules (YAGNI,
|
complete: you hold the plan and cross-task context the reviewer
|
||||||
test hygiene, review method) — the constraints block is for what THIS
|
lacks. If you confirm an item is a real gap, treat it as a failed spec
|
||||||
project's spec demands.
|
review — it enters the fix loop with the other findings.
|
||||||
- Hand the reviewer its diff as a file: run this skill's
|
|
||||||
`scripts/review-package BASE HEAD` and pass the reviewer the file path
|
Template: [task-reviewer-prompt.md](task-reviewer-prompt.md)
|
||||||
it prints (or, without bash: `git log --oneline`, `git diff --stat`,
|
|
||||||
and `git diff -U10` for the range, redirected to one uniquely named
|
### 4. The fix loop
|
||||||
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
|
The loop triggers when the review reports spec ❌, any Critical or Important
|
||||||
call. Use the BASE you recorded before dispatching the implementer —
|
finding, or a ⚠️ item you confirmed as a real gap.
|
||||||
never `HEAD~1`, which silently truncates multi-commit tasks.
|
|
||||||
- A dispatch prompt describes one task, not the session's history. Do not
|
Before the loop starts, two routes leave it immediately:
|
||||||
paste accumulated prior-task summaries ("state after Tasks 1-3") into
|
|
||||||
later dispatches — a real session's dispatch hit 42k chars of which 99%
|
- Record Minor findings in the progress ledger as you go
|
||||||
was pasted history. A fresh subagent needs its task, the interfaces it
|
(`Task <N>: minor (deferred): <one-liner>`), and point the final
|
||||||
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
|
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
|
- 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
|
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.
|
contradiction: present the finding and the plan text, ask which governs.
|
||||||
Do not dismiss the finding because the plan mandates it, and do not
|
Do not dismiss the finding because the plan mandates it, and do not
|
||||||
dispatch a fix that contradicts the plan without asking.
|
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
|
**Rounds 1-3 — resume the original implementer.** Send it the open findings
|
||||||
prints back — stays resident in your context for the rest of the session
|
verbatim. Its context is intact: it knows the task, the code, and its own
|
||||||
and is re-read on every later turn. Hand artifacts over as files:
|
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
|
**Rounds 4-5 — dispatch a fresh implementer on a more capable model** (per
|
||||||
`scripts/task-brief PLAN_FILE N` — it extracts the task's full text to a
|
Model Selection), with the brief path, the report-file path, the open
|
||||||
uniquely named file and prints the path. Compose the dispatch so the
|
findings, and this framing: "A prior implementer attempted this task
|
||||||
brief stays the single source of requirements. Your dispatch should
|
[N] times; you own it now. Read the report file for what was tried." A loop
|
||||||
contain: (1) one line on where this task fits in the project; (2) the
|
that survives three resumes usually means the implementer cannot see its
|
||||||
brief path, introduced as "read this first — it is your requirements,
|
own problem — fresh eyes and a capability bump in one move.
|
||||||
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.
|
|
||||||
|
|
||||||
## 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,
|
**The re-review is scoped.** Run `scripts/review-package FIX_BASE HEAD`
|
||||||
controllers that lost their place have re-dispatched entire completed task
|
where FIX_BASE is the head the previous review saw, and dispatch
|
||||||
sequences — the single most expensive failure observed. Track progress in
|
[re-review-prompt.md](re-review-prompt.md) with the findings list, the
|
||||||
a ledger file, not only in todos.
|
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:
|
**After each round,** append to the ledger:
|
||||||
`cat "$(git rev-parse --show-toplevel)/.superpowers/sdd/progress.md"`. Tasks listed there
|
`Task <N>: fix round <R>/5 (<X> addressed, <Y> open — <finding one-liners>; commits <a7>..<b7>)`
|
||||||
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`.
|
|
||||||
|
|
||||||
## 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
|
**The breaker.** When round 5's re-review still leaves findings open, stop
|
||||||
- [task-reviewer-prompt.md](task-reviewer-prompt.md) - Dispatch task reviewer subagent (spec compliance + code quality)
|
dispatching. Adjudicate each open finding yourself — you hold the plan and
|
||||||
- Final whole-branch review: use superpowers:requesting-code-review's [code-reviewer.md](../requesting-code-review/code-reviewer.md)
|
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
|
## Example Workflow
|
||||||
|
|
||||||
```
|
```
|
||||||
You: I'm using Subagent-Driven Development to execute this plan.
|
You: I'm using Subagent-Driven Development to execute this plan.
|
||||||
|
|
||||||
[Read plan file once: docs/superpowers/plans/feature-plan.md]
|
[Setup: worktree verified, no ledger found, read plan, created todos]
|
||||||
[Create todos for all tasks]
|
|
||||||
|
|
||||||
Task 1: Hook installation script
|
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/)"
|
You: "User level (~/.config/superpowers/hooks/)"
|
||||||
|
|
||||||
Implementer: "Got it. Implementing now..."
|
Implementer: [Later]
|
||||||
[Later] Implementer:
|
|
||||||
- Implemented install-hook command
|
- Implemented install-hook command
|
||||||
- Added tests, 5/5 passing
|
- Added tests, 5/5 passing
|
||||||
- Self-review: Found I missed --force flag, added it
|
- Self-review: Found I missed --force flag, added it
|
||||||
@@ -299,109 +445,39 @@ Implementer: "Got it. Implementing now..."
|
|||||||
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
Task reviewer: Spec ✅ - all requirements met, nothing extra.
|
||||||
Strengths: Good test coverage, clean. Issues: None. Task quality: Approved.
|
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
|
Task 2: Recovery modes
|
||||||
|
|
||||||
[Run task-brief for Task 2; dispatch implementer with brief + report paths + context]
|
[Run task-brief for Task 2; dispatch implementer with brief + report paths + context]
|
||||||
|
|
||||||
Implementer: [No questions, proceeds]
|
Implementer: [No questions]
|
||||||
Implementer:
|
|
||||||
- Added verify/repair modes
|
- Added verify/repair modes
|
||||||
- 8/8 tests passing
|
- 8/8 tests passing
|
||||||
- Self-review: All good
|
|
||||||
- Committed
|
- Committed
|
||||||
|
|
||||||
[Run review-package, dispatch task reviewer with the printed path]
|
[Run review-package, dispatch task reviewer with the printed path]
|
||||||
Task reviewer: Spec ❌:
|
Task reviewer: Spec ❌:
|
||||||
- Missing: Progress reporting (spec says "report every 100 items")
|
- Missing: Progress reporting (spec says "report every 100 items")
|
||||||
- Extra: Added --json flag (not requested)
|
|
||||||
Issues (Important): Magic number (100)
|
Issues (Important): Magic number (100)
|
||||||
|
|
||||||
[Dispatch fix subagent with all findings]
|
[Fix round 1: resume the implementer with both findings]
|
||||||
Fixer: Removed --json flag, added progress reporting, extracted PROGRESS_INTERVAL constant
|
Implementer: Added progress reporting, extracted PROGRESS_INTERVAL constant.
|
||||||
|
Re-ran test/recovery.test.js — 10/10 passing. Fix report appended.
|
||||||
|
|
||||||
[Task reviewer reviews again]
|
[Run review-package FIX_BASE HEAD; dispatch scoped re-review]
|
||||||
Task reviewer: Spec ✅. Task quality: Approved.
|
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]
|
[After all tasks]
|
||||||
[Dispatch final code-reviewer]
|
[Run review-package MERGE_BASE HEAD; dispatch final code-reviewer, most capable model]
|
||||||
Final reviewer: All requirements met, ready to merge
|
Final reviewer: All requirements met. Deferred minors triaged: none block merge.
|
||||||
|
|
||||||
Done!
|
Done! Using superpowers:finishing-a-development-branch.
|
||||||
```
|
```
|
||||||
|
|
||||||
## Advantages
|
|
||||||
|
|
||||||
**vs. Manual execution:**
|
|
||||||
- Subagents follow TDD naturally
|
|
||||||
- Fresh context per task (no confusion)
|
|
||||||
- Parallel-safe (subagents don't interfere)
|
|
||||||
- Subagent can ask questions (before AND during work)
|
|
||||||
|
|
||||||
**vs. Executing Plans:**
|
|
||||||
- Same session (no handoff)
|
|
||||||
- Continuous progress (no waiting)
|
|
||||||
- Review checkpoints automatic
|
|
||||||
|
|
||||||
**Efficiency gains:**
|
|
||||||
- Controller curates exactly what context is needed; bulk artifacts move
|
|
||||||
as files, not pasted text
|
|
||||||
- Subagent gets complete information upfront
|
|
||||||
- Questions surfaced before work begins (not after)
|
|
||||||
|
|
||||||
**Quality gates:**
|
|
||||||
- Self-review catches issues before handoff
|
|
||||||
- Task review carries two verdicts: spec compliance and code quality
|
|
||||||
- Review loops ensure fixes actually work
|
|
||||||
- Spec compliance prevents over/under-building
|
|
||||||
- Code quality ensures implementation is well-built
|
|
||||||
|
|
||||||
**Cost:**
|
|
||||||
- More subagent invocations (implementer + reviewer per task)
|
|
||||||
- Controller does more prep work (extracting all tasks upfront)
|
|
||||||
- Review loops add iterations
|
|
||||||
- But catches issues early (cheaper than debugging later)
|
|
||||||
|
|
||||||
## 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,9 +106,12 @@ Subagent (general-purpose):
|
|||||||
|
|
||||||
## After Review Findings
|
## After Review Findings
|
||||||
|
|
||||||
If a reviewer finds issues and you fix them, re-run the tests that cover
|
If the task review finds issues, you will be resumed with the findings.
|
||||||
the amended code and append the results to your report file. Reviewers
|
Fix them, re-run the tests that cover the amended code, and append a fix
|
||||||
will not re-run tests for you — your report is the test evidence.
|
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.
|
||||||
|
|
||||||
## Report Format
|
## Report Format
|
||||||
|
|
||||||
|
|||||||
106
skills/subagent-driven-development/re-review-prompt.md
Normal file
106
skills/subagent-driven-development/re-review-prompt.md
Normal file
@@ -0,0 +1,106 @@
|
|||||||
|
# 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,6 +183,3 @@ Subagent (general-purpose):
|
|||||||
|
|
||||||
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
**Reviewer returns:** Spec Compliance verdict (✅/❌/⚠️), Strengths, Issues
|
||||||
(Critical/Important/Minor), Task quality verdict
|
(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,8 +7,6 @@ description: Use when encountering any bug, test failure, or unexpected behavior
|
|||||||
|
|
||||||
## Overview
|
## Overview
|
||||||
|
|
||||||
Random fixes waste time and create new bugs. Quick patches mask underlying issues.
|
|
||||||
|
|
||||||
**Core principle:** ALWAYS find root cause before attempting fixes. Symptom fixes are failure.
|
**Core principle:** ALWAYS find root cause before attempting fixes. Symptom fixes are failure.
|
||||||
|
|
||||||
**Violating the letter of this process is violating the spirit of debugging.**
|
**Violating the letter of this process is violating the spirit of debugging.**
|
||||||
@@ -283,11 +281,3 @@ These techniques are part of systematic debugging and available in this director
|
|||||||
- **`root-cause-tracing.md`** - Trace bugs backward through call stack to find original trigger
|
- **`root-cause-tracing.md`** - Trace bugs backward through call stack to find original trigger
|
||||||
- **`defense-in-depth.md`** - Add validation at multiple layers after finding root cause
|
- **`defense-in-depth.md`** - Add validation at multiple layers after finding root cause
|
||||||
- **`condition-based-waiting.md`** - Replace arbitrary timeouts with condition polling
|
- **`condition-based-waiting.md`** - Replace arbitrary timeouts with condition polling
|
||||||
|
|
||||||
## Real-World Impact
|
|
||||||
|
|
||||||
From debugging sessions:
|
|
||||||
- Systematic approach: 15-30 minutes to fix
|
|
||||||
- Random fixes approach: 2-3 hours of thrashing
|
|
||||||
- First-time fix rate: 95% vs 40%
|
|
||||||
- New bugs introduced: Near zero vs common
|
|
||||||
|
|||||||
@@ -209,69 +209,19 @@ When writing or changing any test, read [writing-good-tests.md](writing-good-tes
|
|||||||
- Keep test-only code in test utilities, out of production classes
|
- Keep test-only code in test utilities, out of production classes
|
||||||
- Understand a dependency's side effects before mocking it
|
- Understand a dependency's side effects before mocking it
|
||||||
|
|
||||||
## Why Order Matters
|
|
||||||
|
|
||||||
**"I'll write tests after to verify it works"**
|
|
||||||
|
|
||||||
Tests written after code pass immediately. Passing immediately proves nothing:
|
|
||||||
- Might test wrong thing
|
|
||||||
- Might test implementation, not behavior
|
|
||||||
- Might miss edge cases you forgot
|
|
||||||
- You never saw it catch the bug
|
|
||||||
|
|
||||||
Test-first forces you to see the test fail, proving it actually tests something.
|
|
||||||
|
|
||||||
**"I already manually tested all the edge cases"**
|
|
||||||
|
|
||||||
Manual testing is ad-hoc. You think you tested everything but:
|
|
||||||
- No record of what you tested
|
|
||||||
- Can't re-run when code changes
|
|
||||||
- Easy to forget cases under pressure
|
|
||||||
- "It worked when I tried it" ≠ comprehensive
|
|
||||||
|
|
||||||
Automated tests are systematic. They run the same way every time.
|
|
||||||
|
|
||||||
**"Deleting X hours of work is wasteful"**
|
|
||||||
|
|
||||||
Sunk cost fallacy. The time is already gone. Your choice now:
|
|
||||||
- Delete and rewrite with TDD (X more hours, high confidence)
|
|
||||||
- Keep it and add tests after (30 min, low confidence, likely bugs)
|
|
||||||
|
|
||||||
The "waste" is keeping code you can't trust. Working code without real tests is technical debt.
|
|
||||||
|
|
||||||
**"TDD is dogmatic, being pragmatic means adapting"**
|
|
||||||
|
|
||||||
TDD IS pragmatic:
|
|
||||||
- Finds bugs before commit (faster than debugging after)
|
|
||||||
- Prevents regressions (tests catch breaks immediately)
|
|
||||||
- Documents behavior (tests show how to use code)
|
|
||||||
- Enables refactoring (change freely, tests catch breaks)
|
|
||||||
|
|
||||||
"Pragmatic" shortcuts = debugging in production = slower.
|
|
||||||
|
|
||||||
**"Tests after achieve the same goals - it's spirit not ritual"**
|
|
||||||
|
|
||||||
No. Tests-after answer "What does this do?" Tests-first answer "What should this do?"
|
|
||||||
|
|
||||||
Tests-after are biased by your implementation. You test what you built, not what's required. You verify remembered edge cases, not discovered ones.
|
|
||||||
|
|
||||||
Tests-first force edge case discovery before implementing. Tests-after verify you remembered everything (you didn't).
|
|
||||||
|
|
||||||
30 minutes of tests after ≠ TDD. You get coverage, lose proof tests work.
|
|
||||||
|
|
||||||
## Common Rationalizations
|
## Common Rationalizations
|
||||||
|
|
||||||
| Excuse | Reality |
|
| Excuse | Reality |
|
||||||
|--------|---------|
|
|--------|---------|
|
||||||
| "Too simple to test" | Simple code breaks. Test takes 30 seconds. |
|
| "Too simple to test" | Simple code breaks. Test takes 30 seconds. |
|
||||||
| "I'll test after" | Tests passing immediately prove nothing. |
|
| "I'll test after" | Tests written after pass immediately — which proves nothing. They may test the wrong thing, test the implementation instead of the behavior, or miss the edge case you forgot. You never watched it fail, so you never proved it can catch the bug. Test-first forces that failure. |
|
||||||
| "Tests after achieve same goals" | Tests-after = "what does this do?" Tests-first = "what should this do?" |
|
| "Tests after achieve same goals (spirit not ritual)" | Tests-after answer "what does this do?"; tests-first answer "what should this do?" Tests written after are biased by the code you already wrote — you verify the cases you remembered, not the ones you'd have discovered. Coverage without proof the tests work. |
|
||||||
| "Already manually tested" | Ad-hoc ≠ systematic. No record, can't re-run. |
|
| "Already manually tested" | Manual testing is ad-hoc: no record of what you covered, no way to re-run it when the code changes, easy to forget cases under pressure. "Worked when I tried it" ≠ comprehensive. Automated tests run the same way every time. |
|
||||||
| "Deleting X hours is wasteful" | Sunk cost fallacy. Keeping unverified code is technical debt. |
|
| "Deleting X hours is wasteful" | Sunk cost fallacy — that time is already spent either way. The real choice: rewrite with TDD (high confidence) vs. keep it and bolt tests on after (low confidence, likely bugs). Keeping code you can't trust is the waste. |
|
||||||
| "Keep as reference, write tests first" | You'll adapt it. That's testing after. Delete means delete. |
|
| "Keep as reference, write tests first" | You'll adapt it. That's testing after. Delete means delete. |
|
||||||
| "Need to explore first" | Fine. Throw away exploration, start with TDD. |
|
| "Need to explore first" | Fine. Throw away exploration, start with TDD. |
|
||||||
| "Test hard = design unclear" | Listen to test. Hard to test = hard to use. |
|
| "Test hard = design unclear" | Listen to test. Hard to test = hard to use. |
|
||||||
| "TDD will slow me down" | TDD faster than debugging. Pragmatic = test-first. |
|
| "TDD will slow me down" | TDD IS the pragmatic path: catches bugs before commit, prevents regressions, lets you refactor without fear. "Pragmatic" shortcuts mean debugging in production — slower, not faster. |
|
||||||
| "Manual test faster" | Manual doesn't prove edge cases. You'll re-test every change. |
|
| "Manual test faster" | Manual doesn't prove edge cases. You'll re-test every change. |
|
||||||
| "Existing code has no tests" | You're improving it. Add tests for existing code. |
|
| "Existing code has no tests" | You're improving it. Add tests for existing code. |
|
||||||
|
|
||||||
|
|||||||
@@ -156,47 +156,12 @@ Ready to implement <feature-name>
|
|||||||
| Tests fail during baseline | Report failures + ask |
|
| Tests fail during baseline | Report failures + ask |
|
||||||
| No package.json/Cargo.toml | Skip dependency install |
|
| No package.json/Cargo.toml | Skip dependency install |
|
||||||
|
|
||||||
## Common Mistakes
|
## Common Rationalizations
|
||||||
|
|
||||||
### Fighting the harness
|
| Excuse | Reality |
|
||||||
|
|--------|---------|
|
||||||
- **Problem:** Using `git worktree add` when the platform already provides isolation
|
| "I'm obviously not in a worktree — no need to check" | Run Step 0. Harness-created isolation and submodules both fool eyeballing; the detection commands settle it. |
|
||||||
- **Fix:** Step 0 detects existing isolation. Step 1a defers to native tools.
|
| "`git worktree add` is quicker than hunting for a native tool" | A native tool (e.g. `EnterWorktree`) owns placement, branching, and cleanup. Bypassing it is the #1 mistake — it creates phantom state your harness can't see or manage. |
|
||||||
|
| "The worktree directory is surely ignored already" | Run `git check-ignore`. An unignored worktree directory commits the whole tree into the repo. |
|
||||||
### Skipping detection
|
| "Any directory name works" | Explicit instructions beat an existing project-local directory, which beats the `.worktrees/` default. |
|
||||||
|
| "The workspace is fresh — baseline tests can wait" | A dirty baseline makes every later failure ambiguous. Run the tests now; proceeding past failures is your human partner's call. |
|
||||||
- **Problem:** Creating a nested worktree inside an existing one
|
|
||||||
- **Fix:** Always run Step 0 before creating anything
|
|
||||||
|
|
||||||
### Skipping ignore verification
|
|
||||||
|
|
||||||
- **Problem:** Worktree contents get tracked, pollute git status
|
|
||||||
- **Fix:** Always use `git check-ignore` before creating project-local worktree
|
|
||||||
|
|
||||||
### Assuming directory location
|
|
||||||
|
|
||||||
- **Problem:** Creates inconsistency, violates project conventions
|
|
||||||
- **Fix:** Follow priority: explicit instructions > existing project-local directory > default
|
|
||||||
|
|
||||||
### Proceeding with failing tests
|
|
||||||
|
|
||||||
- **Problem:** Can't distinguish new bugs from pre-existing issues
|
|
||||||
- **Fix:** Report failures, get explicit permission to proceed
|
|
||||||
|
|
||||||
## Red Flags
|
|
||||||
|
|
||||||
**Never:**
|
|
||||||
- Create a worktree when Step 0 detects existing isolation
|
|
||||||
- Use `git worktree add` when you have a native worktree tool (e.g., `EnterWorktree`). This is the #1 mistake — if you have it, use it.
|
|
||||||
- Skip Step 1a by jumping straight to Step 1b's git commands
|
|
||||||
- Create worktree without verifying it's ignored (project-local)
|
|
||||||
- Skip baseline test verification
|
|
||||||
- Proceed with failing tests without asking
|
|
||||||
|
|
||||||
**Always:**
|
|
||||||
- Run Step 0 detection first
|
|
||||||
- Prefer native tools over git fallback
|
|
||||||
- Follow directory priority: explicit instructions > existing project-local directory > default
|
|
||||||
- Verify directory is ignored for project-local
|
|
||||||
- Auto-detect and run project setup
|
|
||||||
- Verify clean test baseline
|
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ Add to your Codex config (`~/.codex/config.toml`):
|
|||||||
multi_agent = true
|
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, you should always close implementer and reviewer subagents when they have finished all their work.
|
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.
|
||||||
|
|
||||||
## Environment Detection
|
## Environment Detection
|
||||||
|
|
||||||
|
|||||||
@@ -7,8 +7,6 @@ description: Use when about to claim work is complete, fixed, or passing, before
|
|||||||
|
|
||||||
## Overview
|
## Overview
|
||||||
|
|
||||||
Claiming work is complete without verification is dishonesty, not efficiency.
|
|
||||||
|
|
||||||
**Core principle:** Evidence before claims, always.
|
**Core principle:** Evidence before claims, always.
|
||||||
|
|
||||||
**Violating the letter of this rule is violating the spirit of this rule.**
|
**Violating the letter of this rule is violating the spirit of this rule.**
|
||||||
@@ -105,15 +103,6 @@ Skip any step = lying, not verifying
|
|||||||
❌ Trust agent report
|
❌ Trust agent report
|
||||||
```
|
```
|
||||||
|
|
||||||
## Why This Matters
|
|
||||||
|
|
||||||
From 24 failure memories:
|
|
||||||
- your human partner said "I don't believe you" - trust broken
|
|
||||||
- Undefined functions shipped - would crash
|
|
||||||
- Missing requirements shipped - incomplete features
|
|
||||||
- Time wasted on false completion → redirect → rework
|
|
||||||
- Violates: "Honesty is a core value. If you lie, you'll be replaced."
|
|
||||||
|
|
||||||
## When To Apply
|
## When To Apply
|
||||||
|
|
||||||
**ALWAYS before:**
|
**ALWAYS before:**
|
||||||
@@ -129,11 +118,3 @@ From 24 failure memories:
|
|||||||
- Paraphrases and synonyms
|
- Paraphrases and synonyms
|
||||||
- Implications of success
|
- Implications of success
|
||||||
- ANY communication suggesting completion/correctness
|
- ANY communication suggesting completion/correctness
|
||||||
|
|
||||||
## The Bottom Line
|
|
||||||
|
|
||||||
**No shortcuts for verification.**
|
|
||||||
|
|
||||||
Run the command. Read the output. THEN claim the result.
|
|
||||||
|
|
||||||
This is non-negotiable.
|
|
||||||
|
|||||||
@@ -135,12 +135,6 @@ Every step must contain the actual content an engineer needs. These are **plan f
|
|||||||
- Steps that describe what to do without showing how (code blocks required for code steps)
|
- Steps that describe what to do without showing how (code blocks required for code steps)
|
||||||
- References to types, functions, or methods not defined in any task
|
- References to types, functions, or methods not defined in any task
|
||||||
|
|
||||||
## Remember
|
|
||||||
- Exact file paths always
|
|
||||||
- Complete code in every step — if a step changes code, show the code
|
|
||||||
- Exact commands with expected output
|
|
||||||
- DRY, YAGNI, TDD, frequent commits
|
|
||||||
|
|
||||||
## Self-Review
|
## Self-Review
|
||||||
|
|
||||||
After writing the complete plan, look at the spec with fresh eyes and check the plan against it. This is a checklist you run yourself — not a subagent dispatch.
|
After writing the complete plan, look at the spec with fresh eyes and check the plan against it. This is a checklist you run yourself — not a subagent dispatch.
|
||||||
|
|||||||
@@ -677,13 +677,3 @@ How future agents find your skill:
|
|||||||
6. **Loads example** (only when implementing)
|
6. **Loads example** (only when implementing)
|
||||||
|
|
||||||
**Optimize for this flow** - put searchable terms early and often.
|
**Optimize for this flow** - put searchable terms early and often.
|
||||||
|
|
||||||
## The Bottom Line
|
|
||||||
|
|
||||||
**Creating skills IS TDD for process documentation.**
|
|
||||||
|
|
||||||
Same Iron Law: No skill without failing test first.
|
|
||||||
Same cycle: RED (baseline) → GREEN (write skill) → REFACTOR (close loopholes).
|
|
||||||
Same benefits: Better quality, fewer surprises, bulletproof results.
|
|
||||||
|
|
||||||
If you follow TDD for code, follow it for skills. It's the same discipline applied to documentation.
|
|
||||||
|
|||||||
Reference in New Issue
Block a user