/implementation-review

name: implementation-review
description: "Pre-commit quality gate. Invoke before every git commit, after /flagrare:staleness-audit. Seven checks, plan gaps, use-case coverage gaps, missing test scenarios, test philosophy violations (Kent Dodds Testing Trophy), SOLID violations, Clean Code violations, and security vulnerabilities. Each check is delegated to a parallel subagent. Surfaces findings before they land in history. Also invoke when the user says review this, am I done, did I miss anything, or check the quality."

Implementation Review

Run this before every commit, after `/flagrare:staleness-audit`. The goal: **what was planned is implemented, what is implemented is tested, and what is tested is correct**.

Each of the seven checks is run by a dedicated subagent. Spawn all seven in parallel, collect their reports, then synthesise into the output format below.

**REQUIRED BACKGROUND for Checks 2-4:** the test-related checks apply `/flagrare:testing-philosophy` (behavior over implementation, the Testing Trophy, the e2e necessity floor). Pass that skill's content into the Check 2, 3, and 4 subagent briefs so they judge against the same definition of "good test" the planning side uses.

**REQUIRED BACKGROUND for Check 7:** the security check applies `/flagrare:security-audit` (threat taxonomy, three-phase methodology, confidence gate, false-positive precedents, generic dependency audit). Pass that skill's content into the Check 7 subagent brief so it judges against the same security discipline a standalone `/flagrare:security-audit` run uses.

---

Step 1: Gather inputs (main agent)

Before spawning subagents, collect in the main agent:

• `git diff --staged`, full diff (the primary input for all subagents)
• `git diff --staged --name-only`, file list
• The **active plan**: look in this order:

1. Session context: did `/flagrare:atdd-plan` run earlier in this conversation? Use that output. 2. `~/.claude/plans/*.md`, the most recently modified file, if the directory exists. 3. The project's decision log (`docs/decisions/`, `docs/adr/`, RFCs, or equivalent), the foundational decision is always the mission/scope anchor. 4. README roadmap, checked/unchecked items define what is in scope.

• Full content of test files touched or related to the staged changes.
• Full content of non-test source files in the staged diff.

If no plan is findable, say so explicitly and skip Checks 1-2 in the subagent briefs.

---

Step 2: Dispatch seven subagents in parallel

Spawn all seven subagents simultaneously using `model: "sonnet"`. Each subagent receives the relevant slice of inputs (described in each brief below) and returns findings in the format `Check N · [name]: ✓ clean | ⚠ [finding] | ✗ [blocking finding]`.

Do not run checks sequentially in the main agent. Spawn → collect → synthesise.

**Rules every brief below inherits.** Paste these into each subagent's prompt ahead of its brief. They are adapted from the review prompts in alibaba/open-code-review; the teardown is at [`docs/research/2026-09-14-open-code-review-teardown.md`](../../../../docs/research/2026-09-14-open-code-review-teardown.md).

• **Report your coverage, in your check's unit.** Open your report with one line: what you were given, what you examined, what you skipped with a one-phrase reason each. Files for the code checks (3 through 7), plan items for Check 1, use cases for Check 2. A file whose hunks you did not read is skipped, not examined. Examining an implementation file does not cover its interface, its config, its type declarations, or its barrel export; the secondary file is where contract drift hides, and it is the one most often skipped.
• **Reading is unbounded, findings are not.** Read whatever you need to understand the change, including files outside the diff. The set of code you *evaluate* stays exactly as your brief says (Check 4 judges test files only, Check 5 non-test files only). File findings only against code inside the staged diff; deleted lines are reference context, not a target.
• **Write when the evidence is sufficient.** Once you can cite the construct and say what is wrong with it, write the finding and move on. Never call the same tool twice with the same arguments. There is no verification step behind you, so do not stop short of the evidence either.
• **Finding nothing is a result.** If a sweep turns up nothing real, report the check clean and finish. Do not keep probing for marginal findings, and do not manufacture one to prove you read the diff; your coverage line is that proof, and it is what makes a clean check credible. An invented finding costs more than a missed one, because it trains the reader to skim the whole report.
• **Every finding cites its construct.** File, line, and the code you are talking about. A finding without a citation is a hunch, and hunches do not leave the subagent.

---

Subagent brief: Check 1: Plan gap analysis

**Inputs:** full `git diff --staged`, the active plan document.

Compare what the active plan said would be built against what the staged diff actually implements.

For each item in the plan's "Implementation Phases" or task list:

• Is it in the staged diff? → ✓ implemented
• Is it partially there? → ⚠ partial, what is missing
• Is it absent entirely? → ✗ gap, was it intentionally deferred, or forgotten?

Ask: would a reader of the plan consider this commit "phase complete"? If the plan defined a gate ("Phase N is done when ATs #1-4 pass"), does the commit satisfy it?

Flag every gap. A deferred item is not a gap, but it must be explicitly deferred, not silently absent.

---

Subagent brief: Check 2: Use-case coverage

**Inputs:** full `git diff --staged`, the project's foundational scope document (ADR-0001, RFC-001, mission doc, or equivalent, whichever anchors what this project is for), README feature description.

For every **user-facing capability** in scope for this commit:

• Is there at least one test that exercises it throug