two-stage-review

Imported from JPEGtheDev/story-to-ship (skills/two-stage-review/SKILL.md). Install upstream with npx skills add JPEGtheDev/story-to-ship --skill two-stage-review. Copyright stays with the author.

Iron Law

YOU MUST DISPATCH REVIEWERS AFTER EVERY TODO -- SPEC COMPLIANCE FIRST, THEN CODE QUALITY.
No exceptions.

Violating the letter of this rule is violating the spirit of this rule.

Announce at start: "I am using the two-stage-review skill to review [todo]."

Workflow

Read status code -> Canary confirmed -> Limitations present -> Stage 1 spec review -> Stage 2 quality review -> Mark done.

Do not advance past any todo until Stage 1 returns PASS and every Stage 2 verdict block advances under the Stage 2 rule below.

See references/REVIEW_LOOP.md for the full decision tree with complete ASCII flow.

BEFORE PROCEEDING

Before dispatching Stage 1 for any implementer result:

1. The status code is read and its row's action taken (table below). NEEDS_CONTEXT and BLOCKED return to the dispatcher and never reach a review stage.
2. Canary confirmed: [Worktree: line from implementer output] is stated in your response.
3. The implementer result contains a Limitations: line. Absent -> resubmit for it; never infer "none".
4. File type identified: skill .md files (in skills/) -> skill-reviewer.md; code/config files -> code-quality-reviewer.md. Never invoke /code-review (the slash command) for skill .md files. Context: Choosing the Stage 2 reviewer for a changed file. Forces: The slash command is the shortest path to "a review happened", but it applies code-review criteria to a skill file and never runs the writing-skills checklist (Iron Law, gate, tables, size), so a skill defect comes back approved. Routing by file type costs one look at the path.

[+] All 4 met -> dispatch Stage 1 [-] Any unmet -> resolve it before any reviewer dispatch

Canary

When applying this skill, before dispatching Stage 1 from any implementer result, state this line in your response:

Canary confirmed: [paste the Worktree: line from implementer output]

If the canary line is absent from the implementer's output, the implementer did not follow its own BEFORE PROCEEDING -- require skill reload and resubmit before dispatching Stage 1.

It signals that the worktree line was read before any reviewer dispatch; it does not prove the reviewer read it or that Stage 1 will catch every defect.

Canary rationale: references/REVIEW_PROTOCOL.md.

Implementer Status Codes

Every subagent doing implementation work must report one of these five codes. Require it in every implementer prompt. Do not accept a response that does not include one.

Context: Reading any implementer result before deciding what to do with it. Forces: A free-form result reads as finished whether the work is finished, partial, or blocked, and the dispatcher fills the gap with the optimistic reading. A fixed set of five codes makes the implementer commit to one state, and each state has one required response in the table below, so the routing is a lookup instead of a judgment call.

2-Stage Review Protocol

Every completed implementation task requires two reviews in this order. This is mandatory -- not optional -- after every single todo (a todo is defined in the writing-plans skill, Building the Plan step 1).

Stage 1: Spec Compliance Review     <- ALWAYS FIRST (spec-compliance-reviewer.md)
Stage 2: Code Quality Review        <- ONLY after Stage 1 passes (skill-reviewer.md or code-quality-reviewer.md -- see Stage 2 below)

Never skip Stage 1. Code that doesn't meet the spec doesn't benefit from quality review. Context: An implementer result is in hand and the quality review looks like the faster path. Forces: Stage 2 checks how the change is written, not whether it is the change that was asked for, so a well-written wrong change passes it. Stage 1 checks the diff against the requirements; running it first means Stage 2 spends its review only on work already known to match the spec.

Re-review required for review-covered territory: The GAPS/REQUEST CHANGES re-run rules below are one instance of a general rule: any change landing in already-reviewed territory -- a post-review edit, a fix round touching reviewed lines, or a "small" amendment to an approved diff -- requires re-review before the work advances. A prior PASS/APPROVE does not extend to the new change, even one the agent itself initiates. The sole exemption is an explicit user waiver given in the same turn -- not an inferred waiver, a prior-turn "go ahead", or the agent's own judgment that the change is trivial or already covered. Enforcement is procedural: a post-review commit touching reviewed territory with no re-review dispatch visible in the transcript is the checkable signal; no automated detector exists.

Worktree hygiene: All implementer subagents MUST work in a worktree. Never dispatch an implementer to the main working tree. Context: Dispatching any implementer subagent. Forces: The main working tree is where the coordinator commits and reviews; an implementer writing there mixes its edits into the coordinator's uncommitted state, so the diff under review no longer isolates the todo. A worktree gives the implementer its own branch and directory, so the review diff is exactly the todo.

Stage 1: Use spec-compliance-reviewer.md with full requirements and the implementation diff. If GAPS returned: implementer fixes gaps, Stage 1 re-runs before proceeding to Stage 2.

Stage 2: Use code-quality-reviewer.md for code/config files; use skill-reviewer.md for skill .md files. For each template, list the files the todo changed, at most two files per dispatch; more than two files means more than one dispatch, grouped so the files that implement one change go together. The test is a named dependency: two files share a dispatch only when a changed line in one names the other by path or filename (for example a SKILL.md and the references file its new pointer names), or a changed line in each names the same skill or file (two restatements of one rule); files that share only the todo are unrelated and get separate dispatches. Each dispatch returns one verdict block per file listed in that dispatch (the Output Contract section of the template); fewer blocks than that dispatch listed, or one merged block, is an incomplete review -- re-dispatch. If any block returns REQUEST CHANGES, REJECT, or NEEDS WORK: implementer fixes, Stage 2 re-runs before proceeding. APPROVE and PASS advance; APPROVE WITH NITS advances once each nit is fixed, or declined with the reason recorded in the todo's status; PASS (size advisory) advances once the size report is recorded in the todo's status for the user's decision. When dispatching Stage 2, pass the implementer's pasted verification output to the reviewer as the {{IMPLEMENTER_EVIDENCE}} value so the reviewer re-runs at least one command, once per dispatch, and reports MATCH or MISMATCH. A Stage 2 dispatch that omits {{IMPLEMENTER_EVIDENCE}} disables the spot-check and is incomplete. The Stage 2 dispatch prompt MUST require the reviewer to output a literal line Adversarial scenario tested: <scenario> in every block: filled with one unscripted real-world case checked against the clause wording for each file whose diff adds or edits a line matching the case-sensitive trigger EXCEPTION|carve-out in agents/ or skills/, and with exactly trigger not matched otherwise. A block without that line is an incomplete review -- re-dispatch. (Trigger is deliberately case-sensitive: uppercase EXCEPTION is the template convention; lowercase 'No exceptions.' boilerplate does not match. Residual false positives are accepted -- the gate favors over-firing.)

See references/REVIEW_PROTOCOL.md for full protocol details.

Red Flags -- STOP

These thoughts mean stop immediately:

Rationalization Prevention

Related Skills

• subagent-driven-development -- the dispatch side of the loop: worktree, template, prompt, implementer dispatch
• using-git-worktrees -- isolation for every reviewer dispatch
• execution -- the work loop this review step sits in
• three-amigos -- Pivot Assessment, Ceremony 4, for BLOCKED and DONE_WITH_CONCERNS results