Browse Source

Merge pull request #2059 from obra/fix/t1-sdd-no-worker-reviewers

fix(sdd): dispatched subagents never dispatch subagents
Drew Ritter 2 months ago
parent
commit
8acf8e5f24

+ 9 - 0
skills/requesting-code-review/code-reviewer.md

@@ -34,6 +34,15 @@ Subagent (general-purpose):
 
 
     Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
     Your review is read-only on this checkout. Do not mutate the working tree, the index, HEAD, or branch state in any way. Use tools like `git show`, `git diff`, and `git log` to inspect history. If you need a working copy of a different revision, check it out into a separate temporary directory (e.g. `git worktree add /tmp/review-[SHA] [SHA]`) — never move HEAD on this checkout.
 
 
+    ## You Do Not Dispatch Subagents
+
+    Do all of this review yourself. Never spawn a subagent to review part
+    of the diff, and never spawn another reviewer for a second opinion.
+    This process already provides every review seat the work gets; a
+    reviewer you spawn duplicates one of them at full cost, and its
+    verdict counts for nothing. If the diff feels too large for one
+    pass, review it in passes yourself and say so in your report.
+
     ## What to Check
     ## What to Check
 
 
     **Plan alignment:**
     **Plan alignment:**

+ 7 - 0
skills/subagent-driven-development/SKILL.md

@@ -234,6 +234,12 @@ and fix-round diffs need it.
   later dispatches — a real session's dispatch hit 42k chars of which 99%
   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
   was pasted history. A fresh subagent needs its task, the interfaces it
   touches, and the global constraints. Nothing else.
   touches, and the global constraints. Nothing else.
+- The dispatch carries the no-subagents contract (it is in the
+  implementer template): the implementer never dispatches subagents —
+  not helpers, and never a reviewer. Review arrives from you, after the
+  report. In real sessions, every reviewer a worker spawned duplicated
+  the task review the controller dispatched anyway — a full extra
+  review seat per task.
 - If an earlier task parked a finding in the area this task touches, carry
 - If an earlier task parked a finding in the area this task touches, carry
   a pointer to that ledger entry in the dispatch.
   a pointer to that ledger entry in the dispatch.
 - Record the implementer's agent identity from the dispatch result —
 - Record the implementer's agent identity from the dispatch result —
@@ -445,6 +451,7 @@ Use superpowers:finishing-a-development-branch.
 | "The fix was small, skip the re-review" | Unreviewed fixes are how regressions land. Every round ends with a scoped re-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. |
 | "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. |
 | "Ledger bookkeeping is overhead" | The ledger is what survives compaction. Controllers without one have re-dispatched entire completed task sequences. |
+| "The implementer spawned its own reviewer — free extra assurance" | It's a duplicate seat reviewing the same diff; the task review is the gate. A worker-spawned reviewer is a defect to flag, not rigor. |
 
 
 ## Example Workflow
 ## Example Workflow
 
 

+ 12 - 0
skills/subagent-driven-development/implementer-prompt.md

@@ -47,6 +47,18 @@ Subagent (general-purpose):
     While iterating, run the focused test for what you're changing; run the
     While iterating, run the focused test for what you're changing; run the
     full suite once before committing, not after every edit.
     full suite once before committing, not after every edit.
 
 
+    ## You Do Not Dispatch Subagents
+
+    Do all of this task's work yourself. Never spawn a subagent to
+    implement part of the task, and above all never spawn a reviewer to
+    check your work. Self-review (below) means reading your own diff.
+    Review is the controller's job: after you report, it dispatches a
+    fresh reviewer against your diff. A reviewer you spawn duplicates
+    that review at full cost, and its approval counts for nothing in
+    the process. If you catch yourself thinking "an independent review
+    would strengthen my report" — that review is already scheduled.
+    Report instead.
+
     ## Code Organization
     ## Code Organization
 
 
     You reason best about code you can hold in context at once, and your edits are more
     You reason best about code you can hold in context at once, and your edits are more

+ 9 - 0
skills/subagent-driven-development/re-review-prompt.md

@@ -43,6 +43,15 @@ Subagent (general-purpose):
     Your review is read-only on this checkout. Do not mutate the working
     Your review is read-only on this checkout. Do not mutate the working
     tree, the index, HEAD, or branch state in any way.
     tree, the index, HEAD, or branch state in any way.
 
 
+    ## You Do Not Dispatch Subagents
+
+    Do all of this review yourself. Never spawn a subagent to review part
+    of the diff, and never spawn another reviewer for a second opinion.
+    This process already provides every review seat the work gets; a
+    reviewer you spawn duplicates one of them at full cost, and its
+    verdict counts for nothing. If the diff feels too large for one
+    pass, review it in passes yourself and say so in your report.
+
     ## Scope
     ## Scope
 
 
     Your scope is the findings list and the fix diff. Verdict every finding.
     Your scope is the findings list and the fix diff. Verdict every finding.

+ 9 - 0
skills/subagent-driven-development/task-reviewer-prompt.md

@@ -52,6 +52,15 @@ Subagent (general-purpose):
     Your review is read-only on this checkout. Do not mutate the working
     Your review is read-only on this checkout. Do not mutate the working
     tree, the index, HEAD, or branch state in any way.
     tree, the index, HEAD, or branch state in any way.
 
 
+    ## You Do Not Dispatch Subagents
+
+    Do all of this review yourself. Never spawn a subagent to review part
+    of the diff, and never spawn another reviewer for a second opinion.
+    This process already provides every review seat the work gets; a
+    reviewer you spawn duplicates one of them at full cost, and its
+    verdict counts for nothing. If the diff feels too large for one
+    pass, review it in passes yourself and say so in your report.
+
     ## Do Not Trust the Report
     ## Do Not Trust the Report
 
 
     Treat the implementer's report as unverified claims about the code. It
     Treat the implementer's report as unverified claims about the code. It