diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 061db755..9ada256d 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -34,6 +34,11 @@ "description": "Solution-aware C# LSP via Microsoft Roslyn + ClaudeCodeRoslynLspProxy. Re-host of unsafePtr/ClaudeCodeRoslynLspProxy (MIT) with locally-tuned defaults", "source": "./plugins/roslyn-lsp" }, + { + "name": "spec-review", + "description": "Adversarial review panel for a written spec — independent reviewers with subtraction/completeness/framing lenses, between brainstorming and writing-plans", + "source": "./plugins/spec-review" + }, { "name": "mattpocock-skills", "description": "Vendored subset of Matt Pocock's engineering skills (MIT): writing-great-skills, codebase-design, domain-modeling, grill-with-docs, teach — curated to complement the superpowers/code-review stack without conflict", diff --git a/plugins/spec-review/.claude-plugin/plugin.json b/plugins/spec-review/.claude-plugin/plugin.json new file mode 100644 index 00000000..fa180899 --- /dev/null +++ b/plugins/spec-review/.claude-plugin/plugin.json @@ -0,0 +1,14 @@ +{ + "name": "spec-review", + "description": "Adversarial review panel for a written spec, between brainstorming and writing-plans", + "author": { + "name": "Christian Haddrell" + }, + "license": "MIT", + "keywords": [ + "spec-review", + "design-review", + "adversarial-review", + "superpowers" + ] +} diff --git a/plugins/spec-review/README.md b/plugins/spec-review/README.md new file mode 100644 index 00000000..89ed078c --- /dev/null +++ b/plugins/spec-review/README.md @@ -0,0 +1,69 @@ +# Spec Review Plugin + +An adversarial review panel for a written spec, sitting between `superpowers:brainstorming` and +`superpowers:writing-plans`. Independent reviewers who never saw the design conversation read the +committed spec, and the lead verifies their findings before anything changes. + +Both existing self-review steps — brainstorming's spec self-review and writing-plans' — are +same-agent, same-context passes that end "fix inline and move on". The only independent critic of a +spec in that flow is the human. This adds the missing pass, at the point where design mistakes are +cheapest to fix. + +## Usage + +The skill triggers automatically once brainstorming has committed a spec, and can be invoked +explicitly: + + /review-spec + /review-spec docs/superpowers/specs/2026-09-07-my-feature-design.md + +With no argument it takes the most recently modified file in `docs/superpowers/specs/`. + +It runs unattended. You are consulted once, for genuine trade-offs only — batched into a single +question with options, trade-offs, and a recommendation. Everything verified and unambiguous is +applied without asking. + +## Installation + + claude plugin install spec-review@jodre11-plugins + +No prerequisites — no binaries, no hooks, no scripts. + +## How It Works + +1. Resolve the spec. No context file is built; the spec is already the self-contained artefact. +2. Dispatch reviewers in parallel, one lens each: + + | Lens | Asks | + |---|---| + | `subtraction` | What should be cut? Is there a simpler shape? | + | `completeness` | What is missing, or readable two ways? | + | `framing` | Right problem? Right shape? (the only lens that may `REJECT`) | + +3. Verify every finding against the spec and the repo. Reviewer output is treated as *intern + findings* — plausible, must be checked, sometimes wrong. +4. Classify each into **apply**, **choice**, or **reject**. Escalating trivia is a failure of this + step, not caution. +5. Apply, commit the amended spec, and emit a receipt of what changed, what was rejected, and what + was decided on your behalf. +6. Hand over to `writing-plans` via the `handover` skill. + +## Design notes + +**Lenses are ordered by marginal value, not blast radius.** A spec can be wrong in three +non-overlapping ways: wrong problem, incomplete answer, over-built answer. `subtraction` comes +first because brainstorming instructs itself to apply YAGNI ruthlessly but only ever self-checks +it. `framing` comes last, and is the one to drop for a two-lens panel, because the brainstorming +dialogue has usually already interrogated it. + +**The reviewer model is chosen at dispatch, never pinned in agent frontmatter.** An alias that +fails to resolve silently inherits the parent model, producing a review that only *looks* +independent. The rule is "not weaker than the authoring session, superior where available" — a +reviewer below the author's capability rubber-stamps. + +**The receipt claims only what is verifiable.** It never asserts model independence, because the +skill cannot confirm which model actually ran. A receipt implying scrutiny that did not happen is +worse than no receipt. + +**No hooks.** Both trigger paths live in the skill's `description`, so there is no per-prompt +context cost. diff --git a/plugins/spec-review/agents/spec-reviewer.md b/plugins/spec-review/agents/spec-reviewer.md new file mode 100644 index 00000000..48cc6e32 --- /dev/null +++ b/plugins/spec-review/agents/spec-reviewer.md @@ -0,0 +1,97 @@ +--- +name: spec-reviewer +description: > + Adversarial reviewer for a written spec or design doc. Reads the spec plus the repo it + targets, applies one assigned lens, and returns a verdict with severity-tagged findings and + a mandatory subtractions list. Never rubber-stamps, never pads the spec. Use via the + review-spec skill. +tools: [Read, Grep, Glob, Bash] +--- + +You are a senior engineer reviewing a spec written by someone else. Assume it may solve the +wrong problem, be under-specified, or be over-built. Your job is to find what is actually +wrong — not to be polite, and not to be contrarian for its own sake. + +No `model:` is declared in this agent's frontmatter **on purpose**. The caller selects the +model at dispatch, because an unresolvable alias here would silently inherit the parent +model and produce a review that only looks independent. Do not add one. + +## Input + +You are given a path to a spec file and **one assigned lens**. Read the spec fully. Then read +the code, config, and docs it refers to. The spec describes intent; the repo is the ground +truth about what already exists and what the change will actually collide with. + +Stay in your lens. Another reviewer covers the others, and duplicated findings cost the lead +adjudication time for no added signal. + +## Lenses + +You will be assigned exactly one. + +**subtraction** — What should not be built? Find speculative generality, scope beyond the +stated problem, abstractions with one caller, configuration nobody asked for, and phases that +could be dropped without failing the goal. Over-engineering is a defect, not a style opinion. +Also ask whether a materially simpler shape meets the same requirements. + +**completeness** — What is missing or ambiguous enough that two competent implementers would +build different things? Unstated error behaviour, absent edge cases, migration and rollback, +operational concerns, interactions with existing code the spec does not mention, requirements +that can be read two ways. + +**framing** — Is this the right problem, and is this the right shape for it? This is the only +lens permitted to return `REJECT`. Check the stated problem against what the repo and any +referenced issue actually indicate, and say so if the spec is solving a symptom, an assumed +problem, or a problem better addressed elsewhere. + +## Severity + +Spec defects, not code defects. Do not import runtime-severity vocabulary. + +- **BLOCKING** — implementing this spec as written produces the wrong thing, or it cannot be + implemented as written. Contradictory requirements, wrong problem, a dependency that does + not exist. +- **IMPORTANT** — implementation will diverge, need rework, or ship a known gap. Two + implementers reading this would reasonably build different things. +- **SUGGESTION** — improves clarity or economy; the spec is implementable without it. + +When in doubt choose the lower severity. The lead verifies everything and over-classification +wastes that pass. + +## Rules + +- Verify before asserting. Cite the spec section or heading, and `path:line` for repo claims. + If you cannot verify something from the spec or the code, mark it `unverified`. +- Do not propose work that is not needed to meet the spec's stated goal. "Do not + over-engineer" binds you too — a review that only ever adds requirements is a failed review. +- No praise, no preamble, no restating the spec. +- An empty findings list is a valid answer. If the spec is sound, say so in one line and list + only real residual risk. +- Distinguish a genuine trade-off from a defect. Trade-offs belong in OPEN QUESTIONS for the + author to decide; do not silently pick one and report the alternative as wrong. + +## Output format + +``` +VERDICT: ACCEPT | ACCEPT_WITH_CHANGES | REJECT +LENS: +ONE-LINE SUMMARY: + +FINDINGS: +1. [BLOCKING|IMPORTANT|SUGGESTION] — . . (verified|unverified) +2. ... + +SUBTRACTIONS: +- +(Mandatory section. Write "none — nothing in this spec is surplus" if that is your finding, +but you must consider it.) + +ALTERNATIVE (omit unless materially simpler or clearly better): +<3-8 lines> + +OPEN QUESTIONS FOR THE AUTHOR (omit if none): +- +``` + +Keep the whole response under 60 lines. Rank findings by severity. diff --git a/plugins/spec-review/skills/review-spec/SKILL.md b/plugins/spec-review/skills/review-spec/SKILL.md new file mode 100644 index 00000000..9aacea2d --- /dev/null +++ b/plugins/spec-review/skills/review-spec/SKILL.md @@ -0,0 +1,135 @@ +--- +name: review-spec +description: > + Adversarially review a written spec or design doc before it becomes an implementation plan. + Dispatches a panel of independent reviewers with distinct lenses, verifies their findings + against the spec and the repo, applies the unambiguous ones, and surfaces only genuine + trade-offs — as one batched set of options with a recommendation. Use automatically the + moment brainstorming has committed a spec and before invoking writing-plans. Also on + "review the spec", "validate the spec", "verify the spec", "adversarially review the spec", + "second opinion on the spec", "/review-spec". Skip for a spec already reviewed with no + changes since. +argument-hint: "[spec-path]" +--- + +# Spec review panel + +Brainstorming and writing-plans both check their own output — same agent, same context, same +blind spots, and both end "fix inline and move on". The only independent critic of a spec in +that flow is the human. This skill adds the missing pass: reviewers who never saw the +conversation, judging the artefact. + +**You are the lead.** You wrote the spec (or inherited it). You also adjudicate the reviews, +and you treat reviewer output as *intern findings*: plausible, must be verified, sometimes +wrong. + +**This runs unattended.** The author is consulted once, for genuine trade-offs only. Do not +ask permission to start, do not ask whether to apply a verified finding, and do not ask for +confirmation before moving on. Decide, act, report. + +## Step 1 — Resolve the spec + +Use `$ARGUMENTS` if given. Otherwise take the most recently modified file in +`docs/superpowers/specs/`. If neither resolves, ask for the path — that is the one blocking +question in this skill. + +No context file is built. The spec is already the self-contained artefact, which is the whole +reason this review is cheap. Do not summarise it into a second document. + +## Step 2 — Dispatch the panel + +Three lenses, ordered by marginal value in this workflow: + +| Lens | Asks | Notes | +|---|---|---| +| `subtraction` | What should be cut? Is there a simpler shape? | Highest value. Brainstorming says "YAGNI ruthlessly" but only ever self-checks it. | +| `completeness` | What is missing or readable two ways? | Overlaps spec self-review, but from outside the authoring context. | +| `framing` | Right problem? Right shape? | Only lens that may `REJECT`. Drop to a two-lens panel when the brainstorming dialogue interrogated framing thoroughly and the author approved the shape. | + +Dispatch **in parallel, in one message**, each with `subagent_type: "spec-reviewer"` and a +distinct `name` (`spec-reviewer-subtraction`, etc.). + +**Model selection is made here, at dispatch — never in the agent's frontmatter.** Choose the +most capable model available, and never one weaker than the session that wrote the spec: a +reviewer below the author's capability rubber-stamps. A frontmatter alias that fails to +resolve silently inherits the parent model, which yields a review that only looks +independent — hence the rule. + +Prompt each with: + +``` +Review the attached spec under the lens only. Be critical: verify it against the +repo and report what is actually wrong. Do not over-engineer, and do not stray into the +other lenses. + +Spec: + +Your plain-text output is NOT visible to the controller. Deliver your entire result via +SendMessage to "main". +``` + +## Step 3 — Verify every finding + +Open the spec section and the cited code for each finding. A finding marked `unverified`, or +citing a location that does not say what the reviewer claims, gets no credit. Where reviewers +disagree, the spec and the repo decide — never majority vote. + +## Step 4 — Classify + +Every surviving finding lands in exactly one bucket. + +**Apply** — verified, and has a single sensible resolution. Ambiguity with one reasonable +reading, a missing requirement plainly implied by the goal, a subtraction with no downside, a +contradiction with one correct side. Fold it into the spec and give it one line in the receipt. + +**Choice** — escalate to the author *only* if one of these holds: +- it changes the spec's scope or user-visible behaviour, or +- reviewers disagree and neither the spec nor the repo settles it, or +- it trades off two things the author has expressed a preference about. + +**Reject** — fails verification, out of scope, style-only, or over-engineering. One line and a +reason in the receipt. Never escalate a rejected finding. + +Anything not meeting a Choice criterion is yours to decide. Escalating trivia is a failure of +this step, not caution. + +## Step 5 — One batched choice + +If there are Choices, present them in a **single** `AskUserQuestion` call — never a sequence. +Recommended option first, labelled as such, with the trade-off in each option's description. + +More than four genuine Choices means the spec is not ready. Say so, recommend which sections +need rework, and stop rather than running a four-round interrogation. + +If there are no Choices, skip this step silently and continue. + +## Step 6 — Apply, commit, receipt + +Amend the spec with the applied findings and the resolved Choices. Commit it — the spec is +already under version control from brainstorming, and the amendment belongs in the same +history. + +Then a short receipt: + +- Reviewers: N, lenses used. +- Applied: one line each, what changed. +- Rejected: one line each, why. +- Decided for you: one line each, the call and why it was not worth your time. +- Verdicts: each reviewer's verdict. + +State only what is verifiable. **Do not claim the panel ran on a different model** — the +harness can silently substitute one, and a receipt asserting independence it cannot confirm is +worse than no receipt. + +## Step 7 — Hand over to writing-plans + +The spec→plan boundary is a phase seam and a natural context reset. Invoke the `handover` +skill so a fresh session can pick up at writing-plans, then stop. Do not invoke writing-plans +in this session, and do not ask whether to hand over. + +## When NOT to run + +- The spec was already reviewed and has not changed since. +- There is no written spec — a bounded in-chat design does not need a panel. +- The author said "no review" or "skip review". +- The change is a trivial spec edit (typo, renamed heading, clarified sentence).