Review

coordinator:review

<!-- Purpose: Decision-tree router for plan-review workflows. Covers outgoing (pre-flight + dispatch) and incoming (triage + integrate) directions. Does NOT cover code / diff / PR review — that is coordinator:review-code (TBD). -->

Trigger: EM has a plan / design doc / RFC ready for review (outgoing), OR a plan-review's findings have landed and need processing (incoming).

When NOT to use: Code / diff / PR review → coordinator:review-code (TBD). Frozen weekly diff at /workweek-complete Step 7 → coordinator:parallel-code-review. Mid-drafting (plan not finished) → keep writing. Stuck pattern → see docs/wiki/stuck-detection.md. Pure mechanical citation check with no Opus → run docs-checker directly.

---

Branch A — Outgoing

Condition: a plan / design doc exists; no reviewer has been invoked yet on this iteration.

A.1 — Pre-flight workers (run BEFORE the Opus reviewer)

Both checks fire independently. A plan can be non-trivial AND cite C++/UE APIs — in that case both workers run.

Delta-scope on re-runs. The Pre-Dispatch Verification rule "Re-run mechanical pre-flights after material plan amendments" (coordinator CLAUDE.md § Pre-Dispatch Verification) is a re-run trigger, NOT a re-run-everything trigger. When re-running A.1 checks on an amended plan (not a first pass), scope each checker to the delta:

• prior-art-checker — re-run only if the amendment changed a claim, added a new subsystem, or introduced a new external reference. A pure wording/typo amendment does not re-trigger it.
• docs-checker — re-run only over the newly cited or changed external APIs, not the full citation set already verified in the prior pass.
• plan-coverage-checker — re-run when the oracle or slate table changed (rows added/removed/edited). A prior sidecar exists; the checker renames it (Phase 0) and the EM diffs the new sidecar against it to see what the amendment moved.

Name the delta in the re-dispatch brief ("amendment touched §X and the slate table; scope your check to those"). A full re-run is correct only when the amendment was structural enough that the prior pass's coverage no longer holds.

Check 1 — Triviality (prior-art-checker)

• Plan covers non-trivial work? (design docs, RFCs, architectural plans; anything beyond a single-file fix) → Before dispatching: run coordinator-doc-new --type prior-art-check --plan <stem> (where <stem> is the plan filename without extension — e.g. for docs/plans/2026-06-29-my-plan.md, <stem> is 2026-06-29-my-plan). This scaffolds docs/plans/<stem>.prior-art-check.md. Then dispatch prior-art-checker with the plan path; pass the scaffolded path in the dispatch brief and instruct the agent to fill the pre-scaffolded sidecar at docs/plans/<stem>.prior-art-check.md. Read sidecar at docs/plans/<stem>.prior-art-check.md. Act on buckets: Conflicts → surface to PM with wiki quote before continuing; Compatible-but-relevant → fold reference into plan's "Considered alternatives"; Silent → no action. See CLAUDE.md § Adding a Convention to the Coordinator System (Prior-art-checker tripwire).
• Plan is genuinely trivial? (one-line doc fix, typo, link repoint, no design content) → Skip prior-art-checker.

Check 2 — Cited external APIs (docs-checker) (runs independently of Check 1)

API surface cited in plan	docs-checker?
C++ or Unreal Engine APIs	Mandatory — run docs-checker regardless of EM judgment.
Other external library APIs	EM judgment — run if cost is justified; skip silently if not.
Pure prose / in-repo-only references / no cited external APIs	Skip docs-checker.

When dispatching docs-checker (Mandatory or EM-judgment rows above): before invoking the agent, run coordinator-doc-new --type docs-check --plan <stem> (where <stem> is the plan filename without extension). This scaffolds docs/plans/<stem>.docs-check.md. Include the scaffolded path in the dispatch brief and instruct the agent to fill the pre-scaffolded sidecar at docs/plans/<stem>.docs-check.md.

See docs/wiki/docs-checker-pre-review.md for full rows and sidecar consumption pattern.

Check 2b — Acceptance-criteria shape (offer, not block) (runs independently; offer-shaped)

<!-- spec-backlink: archive/specs/2026-05-24-acceptance-oracle-with-teeth.md §2.5 — review-skill offer -->

When the reviewed plan's ## Acceptance Criteria section is in old prose-checkbox form (lines like - [ ] prose description) rather than the bindable-table form (ID | Criterion | Test | Binding-Class | Status), NOTICE this and offer the template — do NOT block.

Offer: "This plan's acceptance criteria are in prose-checkbox form. If this plan warrants review, it warrants verifiable exit criteria — the bindable-table form makes acceptance tests mechanically enforceable at merge time. Template and two-altitude flow: docs/wiki/writing-plans.md § Acceptance Oracle. Pre-review Test cells stay pending realization; they're realized as named failing tests after review. No action needed now — this is just a heads-up before the reviewer reads the criteria as a design lens."

The reviewer's substantive design-lens job is unchanged — evaluating whether the criteria are testable-shaped, complete, and correctly scoped is always the reviewer's primary role here. This is a mechanical shape nudge, not a blocker. If the plan already uses the bindable-table form, skip this check silently.

Check 3 — Plan internal completeness (plan-coverage-checker) (runs independently of Checks 1 and 2)

Plan shape	plan-coverage-checker?
Plan contains an audit/findings/issues table (any size)	Run.
Plan is greenfield design with no found-facts oracle	Skip — agent emits SCOPE-MISMATCH.
Plan is single-file mechanical fix	Skip.
Plan is doc redesign / wiki rewrite	Skip.

See docs/wiki/plan-coverage-checker.md for trigger rationale and lens details. Skip is silent — no flag, no justification.

A.2 — Reviewer selection and dispatch

Routing table assembly: Read the base routing table from coordinator/routing.md, scan all enabled plugins for root-level routing.md fragments, merge into a composite routing table. Match the artifact's signals against the composite table to identify Reviewer 1 (domain specialist) and Reviewer 2 (generalist, if needed).

Composite routing table (reference — assembled at dispatch time from fragment discovery):

Signal	Reviewer 1 (Domain)	Reviewer 2 (Generalist)	Effort
Game dev / Unreal / example-sim-repo	the Game Dev Reviewer	the Staff Engineer	Medium → Medium
Architectural change, new subsystem	the Staff Engineer	(backstop: the Director of Engineering)	High
Cross-team / cross-repo seam (consumer ↔ producer, plugin ↔ host)	the Director of Engineering (standalone — DoE altitude)	(none)	High
Generic-substrate / consumer-leak risk on producer-side surface	the Director of Engineering (standalone — DoE altitude)	(none)	High
Front-end, CSS, UI components	the Front-End Reviewer	(backstop: the UX Reviewer)	Medium
Front-end + architecture	the Front-End Reviewer	the Staff Engineer	Medium → High
ML/AI pipeline, model serving, RAG	the Data Science Reviewer	the Staff Engineer	High → High
UX flow, user-facing feature	the UX Reviewer	(backstop: the Staff Engineer)	Low → Medium
Cross-cutting (many files, new pattern)	the Staff Engineer	(backstop: the Director of Engineering)	High
Major example-sim-repo feature / new game mode	the Game Dev Reviewer	the Staff Engineer	High → High
Other / unmatched	the Staff Engineer	(none)	Medium

Effort is PM-gated — it is not an EM dial. The Effort column above is PM-facing reference, NOT a parameter the EM sets, changes, or surfaces. Dispatch each persona at its natural Opus altitude; do not put an effort level (High, Medium, Low) in the dispatch prompt, and do not narrate one to the PM as the chosen level, unless the PM has explicitly named it. If the PM has not named an effort level, omit it entirely. Reading an effort off this table and applying it — even verbatim from the column — is the overreach this rule prevents (2026-06-17 PM correction). The PM owns the effort dial; the EM owns reviewer selection and sequencing.

the Director of Engineering standalone vs. The Director of Engineering backstop. When the signal matches a cross-team or consumer-leak row above, dispatch the Director of Engineering directly as the primary reviewer — describe the standalone / DoE-altitude posture in the brief (do NOT pass a mode argument; that is the harness tool param and will error). Do NOT run the Staff Engineer first. Standalone the Director of Engineering is a peer of the Staff Engineer in technical rigor with the additional cross-team authority the Staff Engineer's EM altitude would hedge on. The "(backstop: the Director of Engineering)" entries above are the chained-after-the Staff Engineer usage for High-effort architectural reviews; that posture is still in play but does not exhaust the Director of Engineering's role.

If --reviewers "name1,name2" was provided, skip auto-detection. Use the explicit list — first name is Reviewer 1, second (if any) is Reviewer 2. Report: "PM-directed review: [name1] then [name2]."

Matching review tier to plan complexity:

Match tier to complexity, not importance. Routing every "important" plan to a staff session burns budget without finding more bugs. The heuristic: would a second reviewer likely contradict the first, or just add diminishing-return notes? If contradiction is unlikely, one reviewer is enough.

Situation	Correct tier
Single-domain plan (new feature, doc redesign, refactor)	One Opus-persona reviewer (auto-detects domain from routing table above)
Single-domain refactor where a domain reviewer already covered the load-bearing concerns	One reviewer (the domain persona). Do NOT chain a generalist (the Staff Engineer) backstop by default — empirically the second pass yields P2 framings, not architectural redirects (2026-05-18: the Staff Engineer on a UE-only plan after the Game Dev Reviewer = 3 P2s, 0 redirects). This default applies ONLY when the domain reviewer's findings demonstrably engaged the architectural layer (abstraction boundaries, cross-system seams, the load-bearing design choice) — NOT merely that a domain pass ran. A domain pass that returned only surface findings does NOT license skipping the generalist; in that case a generalist backstop is still warranted. Generalist backstop is explicit opt-in: --reviewers "<domain>,the Staff Engineer".
Cross-domain plan (e.g., UE + data pipeline, front-end + arch)	Two sequential Opus-persona reviewers: --reviewers "<domain>,the Staff Engineer"
Contested architectural choice with ≥2 valid approaches AND PM authorized	/staff-session review-mode
"This is important, I want it done right"	One Opus-persona reviewer (auto-detects domain)
"the Staff Engineer feels heavy for this; route to code-reviewer instead"	Not a valid row. code-reviewer is the Sonnet diff reviewer, not a plan reviewer. The fork is named Opus persona OR skip review (implement and let code-reviewer catch issues on the diff at /workstream-complete). Sonnet-on-plan-body is not on the menu. See skills/plan/SKILL.md § Exit ¶ Reviewer altitude is binary.

• Plan is genuinely trivial? (one-line doc fix, typo, link repoint) → No review needed; commit and proceed.
• PM has explicitly waived review on a non-trivial plan? ("ship it", "skip review", "straight to execution") → Exit; this skill does not run. Log the waiver in the plan frontmatter (review: skipped per PM direction YYYY-MM-DD).

See CLAUDE.md § Challenging the PM — /staff-session is PM-gated; ask first.

Pipeline phases (docs-checker, (prior-art-checker ∥ plan-coverage-checker), external-pattern-checker, integrator, backstop, report) live in docs/wiki/reviewer-pipeline.md. Walk those phases inline — they are not optional. Walk Phase 2.5 → 2.7 → (2.7b ∥ 2.7d) → 2.7c → 2.8, then dispatch, then Phase 3.5 → 3.7 → 4 → 5.

<!-- Review: code-reviewer — listed phases as a flat sequence, obscuring that prior-art-checker and plan-coverage-checker run in parallel; updated to (prior-art-checker ∥ plan-coverage-checker) to match the runtime shape. -->

Persist the persona verdict — snippet-append, no EM pre-scaffold. Personas are dual-use (advisory or sidecar-review); self-persist is NOT baked into their prompts. When a persona review feeds an integrator, append the contents of snippets/findings-self-persist-sentinel.md verbatim to the reviewer's dispatch brief. The persona then scaffolds its own sidecar in state/review-trail/findings/ and returns a pointer line of the form DONE: <sidecar-path> | verdict: <OK|WARN|BLOCKED> | findings: <N>. EM reads that returned path and passes it to the integrator. There is no EM pre-scaffold (coordinator-doc-new --type review is not called), no injected docs/plans/*.review.md path, and no cs_write_review_claim. Multi-reviewer chain: each reviewer self-persists to its own sidecar in state/review-trail/findings/; the integrator is dispatched once per reviewer pointing at the reviewer's returned path. Spec backlink: cross-repo/inbox/2026-07-01-reviewer-selfpersist-confinement-redirect.md.

A.3 — Sequencing (HARD RULE for plan reviews)

• Default → sequential. Integrate Reviewer 1's findings via coordinator:review-integrator BEFORE dispatching Reviewer 2.
• The merge-gate parallel-review carve-out does NOT apply to plan reviews — plans are never parallelized.
• See CLAUDE.md § Review Sequencing.

---

Branch B — Incoming

Condition: a reviewer has returned output on a plan. EM is deciding what to do with each finding.

Forbidden triage outcomes (never valid): defer-to-later, capture-for-backlog, time-estimate-as-rationale. If any of these would be the disposition, surface to PM — the PM decides whether to defer, not the EM. See CLAUDE.md § Core Principles ('Implement and iterate over deliberate and defer').

Walk each finding against the triage table below — it lands in exactly one row:

• Tradeoff-free correctness fix? (factual error, broken citation, wrong API name in stub, missing cross-reference, internally inconsistent rule) → Dispatch coordinator:review-integrator with mode: "auto" and findings. EM spot-checks the diff. Never hand-author the fix yourself, however small it looks. The integrator is not a cheaper typist — it is a fresh agent that independently re-checks each finding against current disk before applying it. That second check catches findings that were wrong, stale, or mis-scoped (a concurrent executor moved the schema; the rename collides with a constraint that landed after the review) — exactly what self-authoring discards by applying the reviewer's claim at face value. "One line / obvious" is the rationalization this rule defeats, not an exception to it: the cheaper the integrator pass looks, the easier it is to skip the check that occasionally catches the obvious-looking finding that was actually stale. See CLAUDE.md § Review Sequencing (after-review integrator rule) and § Reviewer findings — apply, don't ratify; review-integration-doctrine.md § Review Sequencing.

• Plan-shape tradeoff? (architectural direction, scope question, sequencing call) → Surface to PM with finding + reasoning. Wait for direction. Plan reviews skew heavily toward this row — most plan findings are about what to build, not how it's coded.

  • (i) YAGNI / scope-trim argument from reviewer? → Always escalation, never auto-trim. Even framed as tradeoff-free, YAGNI is a product decision. Surface to PM. See CLAUDE.md § Challenging the PM.
  • (ii) Refactor-over-patch signal? → Refactor is the default when AI is the implementer. Surface to PM with refactor proposal. See CLAUDE.md § Core Principles ('Do the right thing, not the easy thing').
  • (iii) Build-vs-defer call? → Always PM. Never EM-unilateral. See CLAUDE.md § Challenging the PM ¶ Ask the PM when.

• Multiple findings collectively suggest the plan needs a structural refactor (not just patches)? → Do NOT integrate piecemeal. Surface to PM with a refactor proposal — the aggregate signal is the finding. See CLAUDE.md § Core Principles ('Refactor over patch') and § Convergence as Confidence.

• Premise / hypothesis question? (reviewer challenges the plan's framing or motivating hypothesis) → Read the cited prior art (wiki, lessons, archived spec). Confirm or revise premise. See docs/wiki/reviewer-premise-challenge.md.

• Multiple reviewers converged on the same issue from different entry points? → High-confidence; apply via integrator without per-finding verification. See CLAUDE.md § Convergence as Confidence.

• Worker Dispatch Recommendations block present in reviewer output? → Dispatch each named worker. For plan reviews: doc-link-checker (most relevant); dep-cve-auditor (if plan introduces a dependency). Feed worker output back into EM context. test-evidence-parser and security-audit-worker do NOT fire for plan reviews — they require runtime artifacts. If a reviewer names them on a plan, treat as miscalibration and surface to PM. See docs/wiki/reviewer-routed-workers.md and CLAUDE.md § Reviewer-Routed Workers.

• Default / unmatched? → Apply via integrator. Default is to integrate, not to ratify. See docs/wiki/receiving-code-review.md (triage tables, push-back patterns, performative-agreement guard) and CLAUDE.md § Reviewer findings — apply, don't ratify.

---

Prior-Art Mutability and Reviewer Elevation

Prior-art mutability as first-class deliverable. When the PM authorizes prior-art mutation as an explicit deliverable of the review (i.e., the plan is intended to update settled doctrine, not simply comply with it), the DoE-elevated reviewer (typically the Director of Engineering) MAY override the prior-art-checker sidecar's update-plan / update-prior-art recommendation with an explicit cross-reference to the PM authorization in their findings. Default mode is still "plan adapts to prior art"; prior-art mutation is opt-in and requires PM sign-off stated in the dispatch brief.

Reviewer elevation must be stated verbatim in the dispatch brief. Elevating a reviewer past their charter (e.g., the Director of Engineering from ambition-backstop to DoE-with-mutation-authority) without verbatim brief language reverts at integrator-apply — the reviewer's prompt-defined charter is the default boundary. If the PM authorizes elevation, the EM must include the exact authorization phrase (e.g., "PM-authorized to override prior-art-checker on this run") in the dispatch brief to the reviewer; otherwise the integrator will treat the override as out-of-charter and escalate as ASK.

---

Cross-reference exit

After Branch B completes for a multi-reviewer review and Reviewer 1 is integrated, return to A.2 to dispatch Reviewer 2. This skill is re-entrant — each pass walks one direction.

When all reviewer integration is complete, surface to the PM and WAIT: "Plan reviewed and integrated. Proceed to execute?" Review approval is not execution authorization — the EM does not begin dispatching on its own, and the write-bar is the PM's words, not their sentiment: the stamp below is written ONLY when the PM's message NAMES execution ("execute", "proceed to execute", "run it", "ship it", "go ahead and execute"). Plan-approval words alone ("looks good", "lgtm", "approved", "nice") authorize the PLAN, NOT its execution, and MUST NOT trigger the stamp. On execution-naming approval, the default forward verb is (a) stamp the plan frontmatter execution_authorized_by: PM + execution_authorized_at: <date> + execution_authorized_sha: <hash> (the plan-body hash — recipe in docs/wiki/plan-execute-session-split.md, computed over the FINAL plan body after all integration) + execution_authorized_note: "<verbatim PM utterance>", then (b) /handoff (execution handoff) so a fresh session runs /execute-plan. Same-session /execute-plan is the token-economics carve-out only — narrow, named-reason, never the default. The two bypasses of this checkpoint: (a) the PM has explicitly post-review authorized execution under the token-economics carve-out, or (b) the session is in /autonomous mode (the autonomous skill writes /tmp/autonomous-run-${SESSION_ID}; the checkpoint is sentinel-keyed in consumers, and /autonomous bypasses to same-session /execute-plan without the handoff). See docs/wiki/plan-execute-session-split.md for the full rule, rationale, and exceptions. <!-- Review: code-reviewer — path-form consistency: bare `docs/wiki/...` matches this file's own convention and sibling skills (Finding 2) --> Only after authorization does the EM own the dispatch-gate graph (below) — for same-session execution under the carve-out or /autonomous; the default execution-handoff path hands the dispatch-gate graph to the fresh session that picks up via /pickup → /execute-plan.

After authorization, the EM (same session under the carve-out//autonomous, or the fresh execution session under the default) owns the dispatch-gate graph before the first executor dispatch:

<!-- Review: code-reviewer — prior opening "When all reviewer integration is complete and the plan is ready for execution, the EM owns..." was a second independent trigger clause that could read as bypassing the authorization checkpoint above; rewritten to make ordering unconditional. --> enumerate touched files per task, mark file-overlap / output-consumption / contract-change gates (and only those — narrative causality is not a gate), size per-executor scope to ~5-10 min (15 min hard ceiling), and author parallel-wave prompts with explicit peer-scope prohibition. Procedure: `coordinator:execute-plan` Phase 1.5. Taxonomy: `docs/wiki/dispatching-parallel-agents.md` § Dispatch-Gate Taxonomy and § Peer-Scope Prohibition in Parallel-Wave Prompts.