بنقرة واحدة
review
Use when a plan / design doc / RFC is ready for review or plan-review findings have landed.
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
القائمة
Use when a plan / design doc / RFC is ready for review or plan-review findings have landed.
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
استنادا إلى تصنيف SOC المهني
| name | review |
| description | Use when a plan / design doc / RFC is ready for review or plan-review findings have landed. |
| version | 1.0.0 |
| spec_backlink | archive/specs/2026-05/2026-05-06-review-super-skill.md |
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.
Condition: a plan / design doc exists; no reviewer has been invoked yet on this iteration.
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:
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)
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).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)
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.
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. |
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.
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.
coordinator:review-integrator BEFORE dispatching Reviewer 2.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.
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 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.
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. 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:
Rotational arch audit — scores systems, audits top-priority, packages spinoff candidates. Never edits code; updates Last-targeted-audit clock.
Use for new feature requests, vague/ambiguous requirements, or multi-subsystem decomposition before plan mode.
Systematic codebase bug hunt — find and fix all AI-fixable bugs in-session, defer blocked ones to backlog
Night-shift code health review — queries completion entries for today's surfaces, dispatches reviewer, applies findings, updates health tracking.
Run a Codex code review as a second-opinion gate. Returns structured result with findings or graceful skip. Used by /bug-sweep --codex-verify and /workweek-complete Step 7.4.
Cleans up branch sprawl. Triggers: consolidate branches, clean up branches, stale branches, merge all branches.