mirror of
https://github.com/obra/superpowers.git
synced 2026-06-11 21:29:07 +08:00
refine(skills): staff-review round — fix spec-access contradiction, qualify constant bumps
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 <noreply@anthropic.com>
This commit is contained in:
@@ -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
|
**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.
|
**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
|
## Spec Context
|
||||||
|
|
||||||
If the plan's header cites a spec (`**Spec:** <path>`), 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:** <path>`), 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
|
## Model Selection
|
||||||
|
|
||||||
@@ -248,7 +248,7 @@ Done!
|
|||||||
|
|
||||||
**Never:**
|
**Never:**
|
||||||
- Start implementation on main/master branch without explicit user consent
|
- 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
|
- Proceed with unfixed issues
|
||||||
- Dispatch multiple implementation subagents in parallel (conflicts)
|
- Dispatch multiple implementation subagents in parallel (conflicts)
|
||||||
- Make subagent read plan file (provide full text instead)
|
- Make subagent read plan file (provide full text instead)
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ Subagent (general-purpose):
|
|||||||
|
|
||||||
[FULL TEXT of task from plan - paste it here, don't make subagent read file]
|
[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
|
## Context
|
||||||
|
|
||||||
|
|||||||
@@ -12,7 +12,7 @@ Subagent (general-purpose):
|
|||||||
|
|
||||||
## What Was Requested
|
## 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
|
## What Implementer Claims They Built
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user