From 60f617489cc4f78434f7577840d8af92ccb67ec3 Mon Sep 17 00:00:00 2001 From: Drew Ritter Date: Wed, 10 Jun 2026 18:26:11 -0700 Subject: [PATCH] =?UTF-8?q?refine(skills):=20staff-review=20round=20?= =?UTF-8?q?=E2=80=94=20fix=20spec-access=20contradiction,=20qualify=20cons?= =?UTF-8?q?tant=20bumps?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Staff-review findings (4-reviewer panel): - CONTRADICTION FIX: Spec Context said "Subagents never read the spec file themselves" while spec-reviewer-prompt grants exactly that access. Now: implementers never read it; the spec reviewer may, at the cited path. - "a constant bump" was an unqualified trivial example — a one-line BCRYPT_ROUNDS or session-TTL change is a security-posture change; now qualified "with no security or behavioral consequences" (matching brainstorming's config-change qualifier). The diff-property definition adds "nothing security-relevant". - Proportionality rewritten 146→~115 words (house style; one statement of the multi-task containment instead of two). - Red Flags Never-line trimmed 33→14 words (pointer to Proportionality instead of third in-file restatement). - Prompt-template rationale tails cut (the controller just read Spec Context; subagents need the pasted text, not the policy rationale). Co-Authored-By: Claude Fable 5 --- skills/subagent-driven-development/SKILL.md | 6 +++--- skills/subagent-driven-development/implementer-prompt.md | 2 +- skills/subagent-driven-development/spec-reviewer-prompt.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/skills/subagent-driven-development/SKILL.md b/skills/subagent-driven-development/SKILL.md index 962d8369..587d6a62 100644 --- a/skills/subagent-driven-development/SKILL.md +++ b/skills/subagent-driven-development/SKILL.md @@ -11,7 +11,7 @@ Execute plan by dispatching fresh subagent per task, with two-stage review after **Core principle:** Fresh subagent per task + two-stage review (spec then quality) = high quality, fast iteration -**Proportionality:** Review fanout scales with the change. When the entire plan is one trivial, fully-specified mechanical change — a log statement, a constant bump, a typo fix — implement it directly (or with a single implementer subagent), verify it (run the relevant tests or command and confirm the output — superpowers:verification-before-completion applies), commit, and skip the review subagents and the final reviewer: three review dispatches for a one-line diff cost more than the change itself. Trivial is a property of the diff, not of the plan's description of itself: the diff changes no logic, control flow, or observable behavior beyond the literally stated value. When in doubt whether a change is trivial, it is not — run the full pipeline. Within a multi-task plan, run the full pipeline for every task regardless of size; this exception applies only when the whole plan is one trivial change. +**Proportionality:** Review fanout scales with the change. When the entire plan is one trivial, fully-specified mechanical change — a log statement, a typo fix, a constant bump with no security or behavioral consequences — implement it directly (or with a single implementer subagent), verify per superpowers:verification-before-completion (run the relevant command, confirm output), commit, and skip all review subagents, including the final reviewer: three review dispatches cost more than a one-line diff. Trivial is a property of the diff — it changes no logic, no control flow, and nothing security-relevant — not of the plan's self-description. Any doubt means not trivial: run the full pipeline. Within a multi-task plan, never skip reviews, regardless of task size. **Continuous execution:** Do not pause to check in with your human partner between tasks. Execute all tasks from the plan without stopping. The only reasons to stop are: BLOCKED status you cannot resolve, ambiguity that genuinely prevents progress, or all tasks complete. "Should I continue?" prompts and progress summaries waste their time — they asked you to execute the plan, so execute it. @@ -95,7 +95,7 @@ digraph process { ## Spec Context -If the plan's header cites a spec (`**Spec:** `), read it once during plan extraction. Plans reference requirements rather than restating them — when a task cites a spec section, paste that section's text into the implementer and spec-reviewer prompts along with the task text. Subagents never read the spec file themselves. +If the plan's header cites a spec (`**Spec:** `), read it once during plan extraction. Plans reference requirements rather than restating them — when a task cites a spec section, paste that section's text into the implementer and spec-reviewer prompts along with the task text. Implementer subagents never read the spec file themselves; the spec reviewer may additionally read it at the cited path (its prompt says so). ## Model Selection @@ -248,7 +248,7 @@ Done! **Never:** - Start implementation on main/master branch without explicit user consent -- Skip reviews — the only exception is a plan whose ENTIRE content is one trivial, fully-specified mechanical change (see Proportionality); within a multi-task plan, never skip reviews regardless of task size +- Skip reviews — sole exception: a plan that is entirely one trivial change (see Proportionality) - Proceed with unfixed issues - Dispatch multiple implementation subagents in parallel (conflicts) - Make subagent read plan file (provide full text instead) diff --git a/skills/subagent-driven-development/implementer-prompt.md b/skills/subagent-driven-development/implementer-prompt.md index 083c542d..ec9005f6 100644 --- a/skills/subagent-driven-development/implementer-prompt.md +++ b/skills/subagent-driven-development/implementer-prompt.md @@ -12,7 +12,7 @@ Subagent (general-purpose): [FULL TEXT of task from plan - paste it here, don't make subagent read file] - [If the task cites spec sections, paste the cited sections' text here too — the plan references requirements rather than restating them] + [If the task cites spec sections, paste the cited sections' text here too] ## Context diff --git a/skills/subagent-driven-development/spec-reviewer-prompt.md b/skills/subagent-driven-development/spec-reviewer-prompt.md index 5f74ed28..793dd96f 100644 --- a/skills/subagent-driven-development/spec-reviewer-prompt.md +++ b/skills/subagent-driven-development/spec-reviewer-prompt.md @@ -12,7 +12,7 @@ Subagent (general-purpose): ## What Was Requested - [FULL TEXT of task requirements, including the text of any spec sections the task cites — the plan references requirements rather than restating them] + [FULL TEXT of task requirements, including the text of any spec sections the task cites] ## What Implementer Claims They Built