| name | spec-code-review |
| description | Review code diffs, PRs, or branch implementation changes using tiered persona reviewers, confidence-gated findings, and merge/dedup synthesis. Do not use for requirements/plan/task-pack document review, implementation execution, planning unresolved work, or commit/push/PR creation. |
| argument-hint | [blank to review current branch, or provide PR link] |
Code Review
Reviews code changes using dynamically selected reviewer personas. When the host exposes a reviewer dispatch primitive, spawns parallel sub-agents by default that return structured JSON, then merges and deduplicates findings into a single report. When dispatch is unavailable, explicitly disabled, or unsafe, falls back to a single-agent report-only review instead of bypassing host boundaries.
Workflow Contract Summary
When To Use
Use before creating a PR, after completing implementation work, or whenever a scoped code diff needs structured review with confidence-gated findings and optional safe fixes.
When Not To Use
Do not use for requirements/plan-only document review, planning unresolved work, creating commits/pushes/PRs, filing tracker tickets without an explicit routing decision, or treating optional external-tool startup failure as a reviewer failure.
Inputs
Current branch diff, PR URL/number, branch target, or explicit base:<sha-or-ref>; optional plan:<path> and mode token (mode:autofix, mode:report-only, or mode:headless); repository instructions, plan/task/work artifacts, package/test context, and optional external-tool evidence as advisory review context.
Outputs
A merged findings report with severity, confidence, evidence, autofix_class, owner routing, residual status, Diff Boundary Review, Graph-Assisted Impact Review Coverage, first-class test_gaps, and Coverage; structured headless/autofix output when a mode token requests it; safe_auto edits only when the selected mode allows mutation.
Artifacts
Session-scoped review artifacts live under the OS temp directory and are named in Artifact: lines or structured returns. Durable repo-local evidence exists only when the workflow explicitly routes it, such as accepted residual docs or PR text.
Failure Modes
Conflicting mode flags, missing headless diff scope, unsafe shared-checkout switching, unavailable/unsafe dispatch, degraded optional external-tool evidence, or zero reviewer results. Fall back to single-agent report-only when safe, or emit the documented failure envelope for headless/programmatic callers.
Workflow
Resolve scope and mode, run runtime/readiness preflight, select scale-aware reviewers, dispatch or fall back, synthesize/deduplicate findings, apply only allowed safe_auto fixes, then present or return the mode-specific handoff.
Downstream Consumers
spec-work shipping review, PR preparation, tracker-defer handoff, human reviewers, and spec-compound when accepted findings become reusable knowledge.
Scenario Capability
Follows docs/contracts/workflows/scenario-capability-matrix.md with high-risk overrides because this workflow can make user-visible findings and, in autofix modes, mutate source.
Overrides: high-risk
foreign-residual-workspace -> blocked-action-required: stop before autofix or PR-ready review claims until spec-first clean --workspace-orphans preview and spec-first init refresh local artifacts, or the user explicitly accepts degraded evidence.
- optional external-tool evidence unavailable ->
fallback-only: use bounded direct diff/source/test evidence and record the limitation in Coverage; do not claim blast radius, related tests, or changed-symbol coverage that was not confirmed from direct evidence.
non-git-build-workspace coverage gaps -> partial: review covered git roots normally, but treat uncovered build modules as direct-read/test candidates before raising cross-module findings.
Examples As Context
When editing or reviewing this workflow prompt, or when running fresh-source eval for review posture drift, read skills/spec-code-review/evals/examples.json as examples-as-context. These examples are not a deterministic router, state machine, semantic readiness gate, or substitute for LLM judgment during ordinary code reviews.
Context Orientation Anchor
Orient review from the diff scope, current user request, plan/task/work artifacts when present, already-loaded host/project instructions, package manifests and command registries, nearby implementation files, nearby tests, and test results. Treat AGENTS.md, CLAUDE.md, and project role docs as host instruction sources that are normally already loaded by the current session, not automatic re-read targets for every review run. Read those source instruction files only when docs/contracts/context-governance.md's Host Instruction Reuse Policy allows it; Stage 3b is the narrow project-standards persona exception and discovers paths before leaf reviewers read relevant sections. Use bounded direct diff/source reads, rg, ast-grep when useful, package/test facts, logs, and user-provided artifacts as review evidence. External tools may prioritize inspection, but they do not define scope authority or replace reviewer judgment. Findings still need direct source, diff, test, contract, log, or artifact confirmation.
Domain Language And Decision Ledger
When review findings depend on domain terminology, project-specific concepts, or ADR-like decisions, consume existing context before asking questions or raising gaps that repo/docs can answer: already-loaded project standards and host instructions, docs/contracts/, existing brainstorms/plans/solutions, and any repo-local glossary or ADR-like artifacts that actually exist. Team standards under docs/standards/** are governed by docs/contracts/team-standards.md; project-standards findings may enforce only trust=confirmed,lifecycle_state=active, scope-matched rules and must cite both the standard rule ID/section and the diff/source violation. observed, suggested, imported, conflict, confirmed-draft, and docs/standards/candidates/** are not hard findings. Read AGENTS.md / CLAUDE.md source only under the Host Instruction Reuse Policy or the Stage 3b project-standards persona exception, not as a default domain-context step. Do not require a fixed CONTEXT.md, docs/adr/, or glossary directory. If those artifacts are absent, record the limitation in Coverage as advisory context rather than blocking the review.
For major review decisions or residuals, carry a lightweight decision note: question, recommended_answer, source_tag, chosen_answer, consequence, and deferred_reason when unresolved. Use source tags such as confirmed, advisory, session-local, stale, or user. Recommend an ADR-like artifact only when the decision is hard to reverse, would be surprising without context, and reflects a real tradeoff; do not create the artifact from review unless an explicit workflow route chooses that work.
Feedback Loop Review Boundary
When reviewing behavior-bearing changes, check whether the work established and reran a feedback loop appropriate to the change: failing or characterization tests, CLI invocation, HTTP/browser script, trace replay, throwaway harness, property/fuzz loop, or another focused command. Findings should name the missing observable risk, not demand TDD ritual by default.
For docs-only and config-only changes, docs contract checks, schema/help/render checks, generated catalog diff checks, or diff-shape review can be sufficient verification. Do not flag "no test-first loop" when the change has no behavior-bearing code and another observable check proves the intended effect.
Anti-Rationalization Red Flags
| 红旗念头 | 停下来做什么 |
|---|
| 「看着没问题,跳过对抗复核」 | 跑该跑的对抗、证伪或 confidence-gate 视角,再给出 verdict。 |
| 「这条 finding 大概成立」 | 回到 source、diff、test、log 或 artifact 证据核对,确认后再定级;advisory 不能当 confirmed。 |
| 「口头说一下结论就行」 | 产出结构化 finding、Coverage 与 residual risk,让下游能处理而不是靠会话记忆。 |
这是注意力提醒,不是 gate,也不替代 LLM 判断;最终是否停下、如何处理仍由你按当前证据决定。
Runtime Context Exclusion
遵循 docs/contracts/context-governance.md:普通 Code Review context 默认排除 .spec-first/audits/**、.spec-first/governance/** 和 generated mirrors(.claude/**、.codex/**、.agents/skills/**、.cursor/skills/**、.cursor/spec-first/**、.cursor/mcp.json、.kiro/skills/**、.kiro/agents/**、.kiro/spec-first/**、.kiro/settings/**、.qoder/commands/spec-*.md、.qoder/commands/spec/**、.qoder/skills/**、.qoder/agents/**、.qoder/spec-first/**、.qoder/settings.local.json)。除非 diff 或用户请求明确指向 setup/update/runtime drift/audit/governance evidence,否则不要把这些路径放进 reviewer prompt、broad repo search 或 review context bundle;被排除时,在 Coverage 中报告 path 或 reason,而不是静默扫描。Cursor-native .cursor/rules/** / .cursor/agents/**、Kiro-native .kiro/specs/** 与 Qoder-native .qoder/rules/** 只有显式点名时作为 advisory input。
Cache-Friendly Context Layout
保持本 SKILL.md 作为 stable instruction prefix:workflow contract、hard boundaries、reviewer routing 和 reference index。把 volatile data 放入 dynamic suffix:当前 user request、diff summary、tool summary、temporary evidence、artifact-summary.v1,以及来自 docs/contracts/context-bundle.md 的 context-bundle.v1。Stage 5 synthesis 先消费 reviewer JSON;reviewer 返回 schema 以 skills/spec-code-review/references/findings-schema.json 为准,使用 P0-P3 severity 与 0/25/50/75/100 confidence anchors。docs/contracts/workflows/review-finding.md 的 review-finding.v1 仅用于 downstream / compact mapped summary,不替代 code-review reviewer JSON schema。
Maintain a run-local context ledger for this workflow: paths read, reason, phase, and compact summary. Reuse loaded summaries within the same workflow run. Re-read only when exact wording is needed, the file changed, prior evidence is insufficient, or the user explicitly asks.
Summary-First Handoff
When consuming plan, work, task-pack, debug, or compound artifacts, read an artifact-summary.v1 summary and precise artifact path first. Open the full artifact only when full_artifact_read_triggers apply: the summary is missing requirement/task/finding/evidence detail needed for review, exact prose or line references are required for an actionable finding, or 互依赖任务 need concrete implementation details rather than only upstream conclusions. If no usable summary exists, record summary_missing and inspect the smallest explicit source path needed. If full content is opened, record full_artifact_read_reason with the matched trigger.
When producing downstream review handoff, provide an artifact-summary.v1-style summary with verdict, actionable findings, residual status, evidence paths, reviewer artifact path, limitations, and recommended next action. If handing off a context-bundle.v1, keep context budget accounting in the existing related_paths, evidence_paths, excluded_context, budget, and budget_used fields; do not introduce a second included/omitted schema.
Direct Review Evidence Boundary
Code Review does not require external-tool readiness before reviewer dispatch. Use direct diff reads, source reads, rg, ast-grep, package/test facts, logs, and user-provided artifacts to build review context and confirm findings. If a claimed impact surface cannot be confirmed from bounded direct evidence, record it as residual risk or a test candidate instead of raising it as a confirmed finding.
When review runs from a parent workspace containing multiple independent Git repos, group changed files by Git repo and keep file references, suggested fixes, and risk assessments scoped to the repo that owns the file. For read-only review questions without a diff, ask for or infer only bounded candidate repos from the user request and direct file discovery. Autofix review must not edit a child repo unless that repo is explicit in the diff or target_repo scope.
Diff Boundary Review
Every review treats "did the diff stay inside the authorized work?" as a first-class review axis, not as incidental intent checking.
Compute and carry these run-level fields from Stage 2 through Stage 6:
scope_boundary: clean | concern | violation | unknown
authorized_scope_source: explicit-touch-set | declared-files-only | inferred-plan | diff-only | unknown
scope_boundary_evidence: compact source refs, plan refs, declared files, diff files, or limitation notes that justify the boundary verdict
Boundary verdict rules:
- Use
clean only when an explicit touch set, declared files, or plan requirements directly cover the changed files and behavior. A diff-only inference is never enough for clean.
- Use
unknown when no plan, task, PR intent, explicit touch set, or declared file list exists. Do not label thin intent as clean.
- Use
concern when the diff plausibly reaches outside stated intent but needs owner or plan confirmation before calling it unauthorized.
- Use
violation when direct diff/source/plan evidence shows an unauthorized file, behavior, source/runtime boundary change, generated runtime edit, or requirement omission.
- Implementer reports, PR prose, commit messages, or work closeouts are claim sources only. Verify them against diff/source/test/log/contract evidence before using them to lower boundary risk.
- When
CHANGELOG.md, release notes, review docs, or validation docs add repo-relative path references, verify referenced artifacts that are claimed as shipped are in FILES: / tracked diff, already tracked by git ls-files -- <path>, or explicitly declared excluded/not-shipping. If a newly referenced path exists only in UNTRACKED:, set scope_boundary: concern, derive finding_type: untracked_referenced_artifact alongside missing_verification, and keep it actionable until the artifact is staged/tracked or the reference is removed/deferred. Do not lower this to clean only because the file exists on disk.
Stage 5 synthesis derives finding_type for surfaced findings without changing references/findings-schema.json. Minimum derived labels are scope_creep, unauthorized_file_change, unverifiable_claim, and missing_verification. High-confidence scope-boundary findings must remain in the primary finding set; do not silently demote them into residual risks or testing gaps only. Schema promotion is deferred until report/headless consumers prove they need reviewer-return fields.
Capability-Class Evidence Boundary
Follows docs/contracts/project-graph-consumption.md. When setup/runtime facts expose optional capability-class candidates such as code-graph or project-graph, use them only as advisory review inputs through their native MCP or CLI surface. Confirm freshness through readiness_status, lifecycle display bits, and direct source/test/log evidence before relying on impact, affected-test, or ownership candidates; provider self-reported freshness is not a confirmed review fact. A stale graph still serves exploration-tier orientation when you annotate that it lags HEAD, but a review conclusion must be re-grounded regardless. When the capability is missing, when readiness facts are unavailable or self-reported as unknown/unverified, on call failure, or when disabled/unsafe, continue with bounded diff/source reads, rg, ast-grep, package/test facts, and logs. Record any used candidate in Coverage as provider_untrusted; never-block review on its availability, and keep setup-side lifecycle.fallback_used separate from consumption-side fallback notes.
Graph-Assisted Impact Review is a bounded advisory lens for impact-sensitive diffs. Trigger it for shared helpers, public APIs, workflow/contract/source-runtime changes, exported symbols, CLI/runtime generation, security/permission surfaces, cross-module changes, or owner-requested impact review. Classify graph use with stable Coverage fields:
graph_assist: used | fallback | not_applicable
graph_reason_code: candidate_results | provider_missing | readiness_unknown | stale | call_failed | disabled_or_unsafe | markdown_only_diff | no_candidates
provider_untrusted.summaries[]: query/tool summary, accepted candidates, rejected candidates, freshness/readiness notes, and limitations
expansion_budget: minimal-first budget used for candidate expansion, normally max_5_high_impact_symbols
Candidate fields, when present, are advisory only: changed_symbols, changed_entrypoints, changed_contracts, symbol_mapping_status, tests_for_query_result, missing_test_confirmation, impact_chain_candidates, blast_radius_candidates, affected_test_candidates, caller_callee_paths, review_priority_candidates, test_gaps, and limitations. These shrink the next direct read; they do not prove affected tests, ownership, severity, confidence, scope boundary, merge readiness, or a confirmed finding.
Use minimal-first expansion: start with at most 5 high-impact symbols or entrypoints. Expand callers, flows, source snippets, and affected-test candidates only for medium/high risk, public/contract/source-runtime/security/permission changes, explicit test gaps, or owner-requested impact review. Ordinary docs-only/prose-only diffs can use graph_assist: not_applicable with graph_reason_code: markdown_only_diff; source-runtime instruction prose such as skills/**, workflow contracts, templates, runtime projection source, or CLI/workflow harness docs remains impact-sensitive and should use fallback/readiness_unknown when provider readiness is unknown. graph_assist: used requires graph_reason_code: candidate_results plus at least one of changed_symbols, caller_callee_paths, or affected_test_candidates; no_candidates proves only that the provider path ran.
Progress Reporting Boundary
User-visible progress updates are operational evidence, not a reasoning scratchpad. During long reviews, keep updates short and grounded in concrete facts or actions: scope resolution, file counts, selected reviewers, fallback mode, validation status, or the next inspection step. Do not expose private deliberation, tentative inner monologue, or first-person reasoning such as "I'm thinking", "I need to consider", or "I think". If a point is uncertain, state the verified limitation and the next check instead of narrating speculation. Use the repository language policy from the active CLAUDE.md / AGENTS.md spec-first:lang block for new prose unless the user requested otherwise.
Argument Parsing
Parse $ARGUMENTS for the following optional tokens. Strip each recognized token before interpreting the remainder as the PR number, GitHub URL, or branch name.
| Token | Example | Effect |
|---|
mode:autofix | mode:autofix | Select autofix mode (see Mode Detection below) |
mode:report-only | mode:report-only | Select report-only mode |
mode:headless | mode:headless | Select headless mode for programmatic callers (see Mode Detection below) |
base:<sha-or-ref> | base:abc1234 or base:origin/main | Skip scope detection — use this as the diff base directly. For manual re-review of the same branch, pass the last reviewed HEAD SHA to review only newer changes. |
plan:<path> | plan:docs/plans/2026-03-25-001-feat-foo-plan.md | Load this plan for requirements verification |
All tokens are optional. Each one present means one less thing to infer. When absent, fall back to existing behavior for that stage.
Conflicting mode flags: If multiple mode tokens appear in arguments, stop and do not dispatch agents. If mode:headless is one of the conflicting tokens, emit the headless error envelope: Review failed (headless mode). Reason: conflicting mode flags — <mode_a> and <mode_b> cannot be combined. Otherwise emit the generic form: Review failed. Reason: conflicting mode flags — <mode_a> and <mode_b> cannot be combined.
Quick Review Short-Circuit
If $ARGUMENTS indicates the user wants a quick, fast, or light code review, prefer the current harness's built-in lightweight review surface when one exists. Announce the chosen path before other work: Quick review or Multi-agent review.
Programmatic callers (mode:autofix, mode:report-only, or mode:headless) bypass this shortcut and always run the full pipeline because callers depend on structured output and artifacts.
Sequence:
- Use a real built-in only. If the current harness exposes a built-in code review command or tool, run it and stop. Forward a PR number, GitHub URL, or branch target when the built-in accepts one; otherwise review the current checkout.
- No invented fallback. If the current harness has no built-in review command or tool, do not pretend a quick review happened. Continue into the full spec-code-review pipeline.
- Codex note. Codex currently has no universal slash-command review primitive in this source contract. Unless the active runtime explicitly provides one, quick intent falls through to the full pipeline.
Mode Detection
| Mode | When | Behavior |
|---|
| Interactive (default) | No mode token present | Review, apply safe_auto fixes automatically, present findings, ask for policy decisions on gated/manual findings, and optionally continue into fix/push/PR next steps |
| Autofix | mode:autofix in arguments | No user interaction. Review, apply only policy-allowed safe_auto fixes, re-review in bounded rounds, write a run artifact capturing residual downstream work |
| Report-only | mode:report-only in arguments | Strictly read-only. Review and report only, then stop with no edits, artifacts, commits, pushes, or PR actions |
| Headless | mode:headless in arguments | Programmatic mode for skill-to-skill invocation. Apply safe_auto fixes silently (single pass), return all other findings as structured text output, write run artifacts, and return "Review complete" signal. No interactive prompts. |
Run artifact boundary
<review-artifact-dir>/ is a session/orchestrator handoff, not repo-local durable truth. Resolve it under the current OS temp directory (os.tmpdir() / $TMPDIR / %TEMP%) and include the concrete path in every Artifact: line or structured return. Do not hardcode /tmp; Windows native runs commonly use %TEMP%. Use the artifact directory to coordinate reviewer JSON, detail enrichment, autofix residuals, and headless callers during the current run. Do not promise it will be committed or retained. Durable review evidence is created only when the workflow explicitly routes it: PR descriptions may include accepted Known Residuals, and the no-PR shipping path may create docs/residual-review-findings/<branch-or-head-sha>.md for accepted residuals. Do not copy full-detail reviewer JSON into repo-local docs by default.
Autofix mode rules
- Skip all user questions. Never pause for approval or clarification once scope has been established.
- Apply only
safe_auto -> review-fixer findings. Leave gated_auto, manual, human, and release work unresolved.
- Write a run artifact under
<review-artifact-dir>/ summarizing findings, applied fixes, residual actionable work, and advisory outputs. Orchestrators read this artifact to route residual downstream-resolver findings; the skill itself does not file tickets or prompt the user in autofix.
- Emit a compact Residual Actionable Work summary in the autofix return listing each residual
downstream-resolver finding with its stable #, severity, file:line, title, and autofix_class. Structure the summary as two separate contiguous sections: applied safe_auto fixes first, then residual non-auto findings. Within the residual section, reuse each finding's stable # from Stage 5 -- never renumber. Include the run-artifact path. Callers read this summary directly without parsing the artifact. When no residuals exist, state Residual actionable work: none. explicitly.
- Never commit, push, or create a PR from autofix mode. Parent workflows own those decisions.
Report-only mode rules
- Skip all user questions. Infer intent conservatively if the diff metadata is thin.
- Never edit files or externalize work. Do not write
<review-artifact-dir>/, do not file tickets, and do not commit, push, or create a PR.
- Safe for parallel read-only verification.
mode:report-only is the only mode that is safe to run concurrently with browser testing on the same checkout.
- Do not switch the shared checkout. If the caller passes an explicit PR or branch target,
mode:report-only must run in an isolated checkout/worktree or stop instead of running gh pr checkout / git checkout.
- Do not overlap mutating review with browser testing on the same checkout. If a future orchestrator wants fixes, run the mutating review phase after browser testing or in an isolated checkout/worktree.
Headless mode rules
- Skip all user questions. Never use the platform question tool (
AskUserQuestion in Claude Code or request_user_input in Codex) or other interactive prompts. Infer intent conservatively if the diff metadata is thin.
- Require a determinable diff scope. If headless mode cannot determine a diff scope (no branch, PR, or
base: ref determinable without user interaction), emit Review failed (headless mode). Reason: no diff scope detected. Re-invoke with a branch name, PR number, or base:<ref>. and stop without dispatching agents.
- Apply only
safe_auto -> review-fixer findings in a single pass. No bounded re-review rounds. Leave gated_auto, manual, human, and release work unresolved and return them in the structured output.
- Return all non-auto findings as structured text output. Use the headless output envelope format (see Stage 6 below) preserving severity, autofix_class, owner, requires_verification, confidence, pre_existing, and suggested_fix per finding. Enrich with detail-tier fields (why_it_matters, evidence[]) from reviewer returns first, using parent-owned artifact files only as an optional cache (see Detail enrichment in Stage 6).
- Write a run artifact under
<review-artifact-dir>/ summarizing findings, applied fixes, and advisory outputs. Include the artifact path in the structured output.
- Do not file tickets or externalize work. The caller receives structured findings and routes downstream work itself.
- Do not switch the shared checkout. If the caller passes an explicit PR or branch target,
mode:headless must run in an isolated checkout/worktree or stop instead of running gh pr checkout / git checkout. When stopping, emit Review failed (headless mode). Reason: cannot switch shared checkout. Re-invoke with base:<ref> to review the current checkout, or run from an isolated worktree.
- Not safe for concurrent use on a shared checkout. Unlike
mode:report-only, headless mutates files (applies safe_auto fixes). Callers must not run headless concurrently with other mutating operations on the same checkout.
- Never commit, push, or create a PR from headless mode. The caller owns those decisions.
- End with "Review complete" as the terminal signal so callers can detect completion. If all reviewers fail or time out, emit
Code review degraded (headless mode). Reason: 0 of N reviewers returned results. followed by "Review complete".
Interactive mode rules
- Pre-load the platform question tool before any question fires. In Claude Code,
AskUserQuestion is a deferred tool — its schema is not available at session start. At the start of Interactive-mode work (before Stage 2 intent-ambiguity questions, the After-Review routing question, walk-through per-finding questions, bulk-preview Proceed/Cancel, and tracker-defer failure sub-questions), call ToolSearch with query select:AskUserQuestion to load the schema. Load it once, eagerly, at the top of the Interactive flow — do not wait for the first question site and do not decide it on a per-site basis. On Codex this preload step does not apply.
- The numbered-list fallback only applies when the harness genuinely lacks a blocking question tool —
ToolSearch returns no match, the tool call explicitly fails, or the runtime mode does not expose it (e.g., Codex edit modes where request_user_input is unavailable). A pending schema load is not a fallback trigger; call ToolSearch first per the pre-load rule. Rendering a question as narrative text because the tool feels inconvenient, because the model is in report-formatting mode, or because the instruction was buried in a long skill is a bug. A question that calls for a user decision must either fire the tool or fall back loudly.
Severity Scale
All reviewers use P0-P3:
| Level | Meaning | Action |
|---|
| P0 | Critical breakage, exploitable vulnerability, data loss/corruption | Must fix before merge |
| P1 | High-impact defect likely hit in normal usage, breaking contract | Should fix |
| P2 | Moderate issue with meaningful downside (edge case, perf regression, maintainability trap) | Fix if straightforward |
| P3 | Low-impact, narrow scope, minor improvement | User's discretion |
Action Routing
Severity answers urgency. Routing answers who acts next and whether this skill may mutate the checkout.
autofix_class | Default owner | Meaning |
|---|
safe_auto | review-fixer | Local, deterministic fix suitable for the in-skill fixer when the current mode allows mutation |
gated_auto | downstream-resolver or human | Concrete fix exists, but it changes behavior, contracts, permissions, or another sensitive boundary that should not be auto-applied by default |
manual | downstream-resolver or human | Actionable work that should be handed off rather than fixed in-skill |
advisory | human or release | Report-only output such as learnings, rollout notes, or residual risk |
Routing rules:
- Synthesis owns the final route. Persona-provided routing metadata is input, not the last word.
- Choose the more conservative route on disagreement. A merged finding may move from
safe_auto to gated_auto or manual, but never the other way without stronger evidence.
- Only
safe_auto -> review-fixer enters the in-skill fixer queue automatically.
requires_verification: true means a fix is not complete without targeted tests, a focused re-review, or operational validation.
Reviewers
18 reviewer personas in layered conditionals, plus Spec-First-specific agents. See the persona catalog included below for the full catalog.
CLI readiness boundary: Keep spec-cli-readiness-reviewer as the conditional reviewer for CLI-facing diffs. This project is itself a CLI/workflow harness, so changes to src/cli/, command definitions, argument parsing, runtime generation, or command handler behavior need autonomous-agent usability review. spec-cli-agent-readiness-reviewer is a separate manual/deep-dive agent for CLI source, plans, or specs; it is not a replacement for the structured JSON persona.
Default core reviewers (full or sensitive reviews):
The scale-aware reviewer preflight in Stage 3 may replace this default core with a smaller minimum set for low-risk diffs. For medium, broad, sensitive, or unclear diffs, use the full default core below.
| Agent | Focus |
|---|
spec-correctness-reviewer | Logic errors, edge cases, state bugs, error propagation |
spec-testing-reviewer | Coverage gaps, weak assertions, brittle tests |
spec-maintainability-reviewer | Coupling, complexity, naming, dead code, abstraction debt |
spec-project-standards-reviewer | CLAUDE.md and AGENTS.md compliance -- frontmatter, references, naming, portability |
spec-agent-native-reviewer | Verify new features are agent-accessible |
spec-learnings-researcher | Search docs/solutions/ for past issues related to this PR |
Cross-cutting conditional (selected per diff):
| Agent | Select when diff touches... |
|---|
spec-security-reviewer | Auth, public endpoints, user input, permissions |
spec-performance-reviewer | DB queries, data transforms, caching, async |
spec-api-contract-reviewer | Routes, serializers, type signatures, versioning |
spec-data-migrations-reviewer | Migration files, schema dumps (db/schema.rb, structure.sql), backfill scripts, or data transformations -- not model/query-only changes without migration artifacts |
spec-reliability-reviewer | Error handling, retries, timeouts, background jobs |
spec-adversarial-reviewer | Diff >=50 changed non-test/non-generated/non-lockfile lines, or auth, payments, data mutations, external APIs |
spec-cli-readiness-reviewer | CLI command definitions, argument parsing, CLI framework usage, command handler implementations |
spec-previous-comments-reviewer | Reviewing a PR that has existing review comments or threads |
Stack-specific conditional (selected per diff):
| Agent | Select when diff touches... |
|---|
spec-dhh-rails-reviewer | Rails architecture, service objects, session/auth choices, or Hotwire-vs-SPA boundaries |
spec-kieran-rails-reviewer | Rails application code where conventions, naming, and maintainability are in play |
spec-kieran-python-reviewer | Python modules, endpoints, scripts, or services |
spec-kieran-typescript-reviewer | TypeScript components, services, hooks, utilities, or shared types |
spec-julik-frontend-races-reviewer | Stimulus/Turbo controllers, DOM events, timers, animations, or async UI flows |
spec-swift-ios-reviewer | Swift files, SwiftUI views, UIKit controllers, entitlements, privacy manifests, Core Data models, SPM manifests, storyboards/XIBs, or semantic build-setting/target/signing changes in .pbxproj |
Spec-First conditional (migration-specific):
| Agent | Select when diff includes migration artifacts |
|---|
spec-schema-drift-detector | Cross-references schema.rb against included migrations |
spec-deployment-verification-agent | Produces deployment checklist with SQL verification queries for risky migration artifacts |
Review Scope
When dispatch is available, every full multi-persona review first runs the Stage 3 scale-aware reviewer preflight, then spawns the selected core reviewers plus whichever cross-cutting and stack-specific conditionals fit the diff. Low-risk tiny diffs can use a minimum set of 2-3 reviewers. Medium, broad, sensitive, or unclear diffs use the full default core of 4 persona reviewers plus the 2 Spec-First agents. A Rails auth feature might trigger security + reliability + kieran-rails + dhh-rails on top of the full default core.
If dispatch is unavailable, explicitly disabled by the user, or unsafe for the selected review mode, run the single-agent report-only fallback described in Stage 4. Do not silently skip review and do not work around the boundary by invoking hidden helpers or external CLIs as pseudo-agents.
Protected Artifacts
The following paths are spec-first pipeline artifacts and must never be flagged for deletion, removal, or gitignore by any reviewer:
docs/brainstorms/* -- requirements documents created by spec-brainstorm
docs/plans/*.md -- plan files created by spec-plan (decision artifacts; execution progress is derived from git, not stored in plan bodies)
docs/solutions/*.md -- solution documents created during the pipeline
If a reviewer flags any file in these directories for cleanup or removal, discard that finding during synthesis.
How to Run
Stage 1: Determine scope
Compute the diff range, file list, and diff. Minimize permission prompts by combining into as few commands as possible.
If base: argument is provided (fast path):
The caller already knows the diff base. Skip all base-branch detection, remote resolution, and merge-base computation. Use the provided value directly:
BASE_ARG="{base_arg}"
BASE=$(git merge-base HEAD "$BASE_ARG" 2>/dev/null) || BASE="$BASE_ARG"
Then produce the same output as the other paths:
echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard
This path works with any ref — a SHA, origin/main, a branch name. Automated callers (spec-work, best-judgment, sbest-judgment) should prefer this to avoid the detection overhead. Interactive users can also use it for manual incremental re-review: record the HEAD SHA from the last completed review, then rerun review with base:<that-sha> so only changes since that point are in scope. This only narrows the diff range; reviewers still analyze the new range independently and no cross-run finding deduplication is implied. Do not combine base: with a PR number or branch target. If both are present, stop with an error: "Cannot use base: with a PR number or branch target — base: implies the current checkout is already the correct branch. Pass base: alone, or pass the target alone and let scope detection resolve the base." This avoids scope/intent mismatches where the diff base comes from one source but the code and metadata come from another.
If a PR number or GitHub URL is provided as an argument:
If mode:report-only or mode:headless is active, do not run gh pr checkout <number-or-url> on the shared checkout. For mode:report-only, tell the caller: "mode:report-only cannot switch the shared checkout to review a PR target. Run it from an isolated worktree/checkout for that PR, or run report-only with no target argument on the already checked out branch." For mode:headless, emit Review failed (headless mode). Reason: cannot switch shared checkout. Re-invoke with base:<ref> to review the current checkout, or run from an isolated worktree. Stop here unless the review is already running in an isolated checkout.
Skip-condition pre-check. Before checkout or scope detection, run a PR-state probe to decide whether the review should proceed:
gh pr view <number-or-url> --json state,title,body,files
Apply skip rules in order:
state is CLOSED or MERGED -> stop with message PR is closed/merged; not reviewing.
- Trivial-PR judgment: make a conservative inline orchestrator judgment from the PR title, body, and changed file paths. Do not call
Agent, Task, spawn_agent, or an equivalent dispatch primitive in this Stage 1 skip pre-check; reviewer dispatch authorization is not established until the Stage 4 dispatch gate. Consider dependency lock-file or manifest-only bumps, automated release commits, and chore version increments with no substantive code changes. When in doubt, answer no — skipped reviews that should have run are more costly than unnecessary reviews. If the judgment returns yes: stop with message PR appears to be a trivial automated PR; not reviewing. Run without a PR argument to review the current branch, or pass base:<ref> if review is intended.
When any skip rule fires, emit the message and stop without dispatching reviewers, switching the checkout, or running scope detection. Standalone branch mode and base: mode are unaffected -- they always run the full review. Draft PRs are reviewed normally -- draft status is not a skip condition; early feedback on in-progress work is valuable.
If no skip rule fires, proceed to the checkout logic below.
First, verify the worktree is clean before switching branches:
git status --porcelain
If the output is non-empty, inform the user: "You have uncommitted changes on the current branch. Stash or commit them before reviewing a PR, or use standalone mode (no argument) to review the current branch as-is." Do not proceed with checkout until the worktree is clean.
Then check out the PR branch so persona agents can read the actual code (not the current checkout):
gh pr checkout <number-or-url>
Then fetch PR metadata. Capture the base branch name and the PR base repository identity, not just the branch name. Project reviews and comments to a hasPriorComments boolean via --jq -- counting only, not materializing review or comment bodies into the orchestrator's context. The reviews filter excludes approval-state submissions with empty bodies, so PRs with only approval clicks correctly fall through the gate. Stage 3 uses hasPriorComments to decide whether to spawn previous-comments:
gh pr view <number-or-url> --json title,body,baseRefName,headRefName,url,reviews,comments --jq '{title, body, baseRefName, headRefName, url, hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}'
Use the repository portion of the returned PR URL as <base-repo> (for example, sunrain520/spec-first from https://github.com/sunrain520/spec-first/pull/348).
Then compute a local diff against the PR's base branch so re-reviews also include local fix commits and uncommitted edits. Substitute the PR base branch from metadata (shown here as <base>) and the PR base repository identity derived from the PR URL (shown here as <base-repo>). Resolve the base ref from the PR's actual base repository, not by assuming origin points at that repo:
PR_BASE_REMOTE=$(git remote -v | awk 'index($2, "github.com:<base-repo>") || index($2, "github.com/<base-repo>") {print $1; exit}')
if [ -n "$PR_BASE_REMOTE" ]; then PR_BASE_REMOTE_REF="$PR_BASE_REMOTE/<base>"; else PR_BASE_REMOTE_REF=""; fi
PR_BASE_REF=$(git rev-parse --verify "$PR_BASE_REMOTE_REF" 2>/dev/null || git rev-parse --verify <base> 2>/dev/null || true)
if [ -z "$PR_BASE_REF" ]; then
if [ -n "$PR_BASE_REMOTE_REF" ]; then
git fetch --no-tags "$PR_BASE_REMOTE" <base>:refs/remotes/"$PR_BASE_REMOTE"/<base> 2>/dev/null || git fetch --no-tags "$PR_BASE_REMOTE" <base> 2>/dev/null || true
PR_BASE_REF=$(git rev-parse --verify "$PR_BASE_REMOTE_REF" 2>/dev/null || git rev-parse --verify <base> 2>/dev/null || true)
else
if git fetch --no-tags https://github.com/<base-repo>.git <base> 2>/dev/null; then
PR_BASE_REF=$(git rev-parse --verify FETCH_HEAD 2>/dev/null || true)
fi
if [ -z "$PR_BASE_REF" ]; then PR_BASE_REF=$(git rev-parse --verify <base> 2>/dev/null || true); fi
fi
fi
if [ -n "$PR_BASE_REF" ]; then BASE=$(git merge-base HEAD "$PR_BASE_REF" 2>/dev/null) || BASE=""; else BASE=""; fi
if [ -n "$BASE" ]; then echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard; else echo "ERROR: Unable to resolve PR base branch <base> locally. Fetch the base branch and rerun so the review scope stays aligned with the PR."; fi
Extract PR title/body, base branch, and PR URL from gh pr view, then extract the base marker, file list, diff content, and UNTRACKED: list from the local command. Do not use gh pr diff as the review scope after checkout -- it only reflects the remote PR state and will miss local fix commits until they are pushed. If the base ref still cannot be resolved from the PR's actual base repository after the fetch attempt, stop instead of falling back to git diff HEAD; a PR review without the PR base branch is incomplete.
If a branch name is provided as an argument:
Check out the named branch, then diff it against the base branch. Substitute the provided branch name (shown here as <branch>).
If mode:report-only or mode:headless is active, do not run git checkout <branch> on the shared checkout. For mode:report-only, tell the caller: "mode:report-only cannot switch the shared checkout to review another branch. Run it from an isolated worktree/checkout for <branch>, or run report-only on the current checkout with no target argument." For mode:headless, emit Review failed (headless mode). Reason: cannot switch shared checkout. Re-invoke with base:<ref> to review the current checkout, or run from an isolated worktree. Stop here unless the review is already running in an isolated checkout.
First, verify the worktree is clean before switching branches:
git status --porcelain
If the output is non-empty, inform the user: "You have uncommitted changes on the current branch. Stash or commit them before reviewing another branch, or provide a PR number instead." Do not proceed with checkout until the worktree is clean.
git checkout <branch>
Then detect the review base branch and compute the merge-base. Run the trusted skills/spec-code-review/scripts/resolve-base.sh helper, which handles fork-safe remote resolution with multi-fallback detection (PR metadata -> origin/HEAD -> gh repo view -> common branch names). Runtime adapters rewrite this source path to the loaded workflow skill directory during spec-first init; never run a repo-root scripts/resolve-base.sh from the project under review.
RESOLVE_OUT=$(bash skills/spec-code-review/scripts/resolve-base.sh) || { echo "ERROR: resolve-base.sh failed"; exit 1; }
if [ -z "$RESOLVE_OUT" ] || echo "$RESOLVE_OUT" | grep -q '^ERROR:'; then echo "${RESOLVE_OUT:-ERROR: resolve-base.sh produced no output}"; exit 1; fi
BASE=$(echo "$RESOLVE_OUT" | sed 's/^BASE://')
If the script outputs an error, stop instead of falling back to git diff HEAD; a branch review without the base branch would only show uncommitted changes and silently miss all committed work.
On success, produce the diff:
echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard
You may still fetch additional PR metadata with gh pr view for title, body, linked issues, and the same projected hasPriorComments boolean from PR mode. Do not fail if no PR exists -- leave hasPriorComments=false.
If no argument (standalone on current branch):
Detect the review base branch and compute the merge-base using the same trusted skills/spec-code-review/scripts/resolve-base.sh helper as branch mode:
RESOLVE_OUT=$(bash skills/spec-code-review/scripts/resolve-base.sh) || { echo "ERROR: resolve-base.sh failed"; exit 1; }
if [ -z "$RESOLVE_OUT" ] || echo "$RESOLVE_OUT" | grep -q '^ERROR:'; then echo "${RESOLVE_OUT:-ERROR: resolve-base.sh produced no output}"; exit 1; fi
BASE=$(echo "$RESOLVE_OUT" | sed 's/^BASE://')
If the script outputs an error, stop instead of falling back to git diff HEAD; a standalone review without the base branch would only show uncommitted changes and silently miss all committed work on the branch.
On success, produce the diff:
echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE && echo "DIFF:" && git diff -U10 $BASE && echo "UNTRACKED:" && git ls-files --others --exclude-standard
Using git diff $BASE (without ..HEAD) diffs the merge-base against the working tree, which includes committed, staged, and unstaged changes together.
Untracked file handling: Always inspect the UNTRACKED: list, even when FILES:/DIFF: are non-empty. Untracked files are outside review scope until staged. If the list is non-empty, tell the user which files are excluded. If any of them should be reviewed, stop and tell the user to git add them first and rerun. Only continue when the user is intentionally reviewing tracked changes only. In mode:headless or mode:autofix, do not stop to ask — proceed with tracked changes only and note the excluded untracked files in the Coverage section of the output.
Stage 2: Intent discovery
Understand what the change is trying to accomplish. The source of intent depends on which Stage 1 path was taken:
PR/URL mode: Use the PR title, body, and linked issues from gh pr view metadata. Supplement with commit messages from the PR if the body is sparse.
Branch mode: Run git log --oneline ${BASE}..<branch> using the resolved merge-base from Stage 1.
Standalone (current branch): Run:
echo "BRANCH:" && git rev-parse --abbrev-ref HEAD && echo "COMMITS:" && git log --oneline ${BASE}..HEAD
Combined with conversation context (plan section summary, PR description), write a 2-3 line intent summary:
Intent: Simplify tax calculation by replacing the multi-tier rate lookup
with a flat-rate computation. Must not regress edge cases in tax-exempt handling.
Pass this to every reviewer in their spawn prompt. Intent shapes how hard each reviewer looks, not which reviewers are selected.
When intent is ambiguous:
- Interactive mode: Ask one question using the platform's blocking question tool (
AskUserQuestion in Claude Code or request_user_input in Codex): "What is the primary goal of these changes?" Do not spawn reviewers until intent is established. Claude Code only: if AskUserQuestion has not yet been loaded this session (per the Interactive mode rules pre-load), call ToolSearch with query select:AskUserQuestion first before asking. Fall back to numbered options in chat only when the harness genuinely lacks a blocking tool or the call errors (e.g., Codex edit modes) — not because a schema load is required. Never silently skip the question.
- Autofix/report-only/headless modes: Infer intent conservatively from the branch name, diff, PR metadata, and caller context. Note the uncertainty in Coverage or Verdict reasoning instead of blocking.
Stage 2b: Plan discovery (requirements verification)
Locate the plan document so Stage 6 can verify requirements completeness. Check these sources in priority order — stop at the first hit:
plan: argument. If the caller passed a plan path, use it directly. Read the file to confirm it exists.
- PR body. If PR metadata was fetched in Stage 1, scan the body for paths matching
docs/plans/*.md. If exactly one match is found and the file exists, use it as plan_source: explicit. If multiple plan paths appear, treat as ambiguous — demote to plan_source: inferred for the most recent match that exists on disk, or skip if none exist or none clearly relate to the PR title/intent. Always verify the selected file exists before using it — stale or copied plan links in PR descriptions are common.
- Auto-discover. Extract 2-3 keywords from the branch name (e.g.,
feat/onboarding-skill -> onboarding, skill). Glob docs/plans/* and filter filenames containing those keywords. If exactly one match, use it. If multiple matches or the match looks ambiguous (e.g., generic keywords like review, fix, update that could hit many plans), skip auto-discovery — a wrong plan is worse than no plan. If zero matches, skip.
Confidence-first tagging: Record how the plan was found:
plan: argument -> plan_source: explicit (high confidence)
- Single unambiguous PR body match ->
plan_source: explicit (high confidence)
- Multiple/ambiguous PR body matches ->
plan_source: inferred (lower confidence)
- Auto-discover with single unambiguous match ->
plan_source: inferred (lower confidence)
If a plan is found, read its Requirements section — ## Requirements in current plans, ## Requirements Trace in legacy ones — and the R-IDs (R1, R2, etc.) listed there, plus Implementation Units under the ## Implementation Units section. Recognize both current heading-style units (### U1. [Name]) and legacy list-item units (- U1. **[Name]**); the reader must remain compatible with older plans while new plans use heading-style units. Store the extracted requirements list, implementation unit IDs/titles, and plan_source for Stage 6. Do not block the review if no plan is found — requirements verification is additive, not required.
Stage 2c: Boundary source discovery
Build the Diff Boundary Review input from the strongest available source:
- Explicit touch set or task pack declared files ->
authorized_scope_source: explicit-touch-set.
- PR/work/plan text with concrete declared files only ->
authorized_scope_source: declared-files-only.
- Plan requirements and implementation units that imply file or behavior scope ->
authorized_scope_source: inferred-plan.
- Diff and branch/commit intent only ->
authorized_scope_source: diff-only.
- No usable scope signal ->
authorized_scope_source: unknown.
Record scope_boundary_evidence as a compact list. It may include plan requirement IDs, implementation unit IDs, declared file paths, diff files, user-stated intent, and limitations. Do not record implementer claims as confirmed evidence unless they were checked against diff/source/test/log/contract. If the strongest source is diff-only or unknown, initialize scope_boundary: unknown unless Stage 5 later finds direct evidence for concern or violation.
Changelog/release-note path reference check: If CHANGELOG.md or a release-note/validation doc is in FILES:, inspect added lines for newly added repo-relative source/artifact path references (backtick paths, Markdown links, or plain path-like refs such as docs/..., skills/..., src/..., templates/..., agents/..., tests/...). Compare to FILES:, git ls-files -- <path>, and UNTRACKED:. A referenced path that exists only in UNTRACKED: is excluded from review and shipping scope; initialize or raise scope_boundary: concern, add untracked-referenced-artifact:<path> to scope_boundary_evidence, and derive finding_type: untracked_referenced_artifact. The derived label is workflow-local and does not require a findings-schema change. In mode:headless or mode:autofix, do not stage the artifact; report the boundary concern and continue only within the tracked review scope.
Stage 3: Select reviewers
Read the diff and file list from Stage 1. Start with the deterministic scale-aware reviewer preflight below, then decide which conditional reviewers fit the diff. Conditional selection is agent judgment, not keyword matching.
Scale-aware reviewer preflight
Compute and record these facts before choosing the reviewer team:
changed_file_count: tracked files in FILES:.
untracked_excluded_count: untracked files excluded from review scope.
non_test_non_generated_non_lock_line_count: changed executable/source lines excluding tests, generated files, vendored files, lockfiles, snapshots, and markdown/prose-only files.
docs_only: every tracked changed file is Markdown, docs prose, images, examples, or non-runtime documentation.
simple_config_only: changed files are package metadata, lint/test config, YAML/JSON/TOML config, or CI config with no executable source edits.
sensitive_diff: any changed file or diff hunk touches auth, permissions, secrets, payments, migrations, public APIs, src/cli/, bin/, templates/, skills/, agents/, runtime generation, release packaging, CI publish/release gates, database schema/data, or production deploy config.
prior_comments_present: Stage 1 hasPriorComments=true.
plan_explicit: Stage 2b found an explicit plan.
When the source checkout has the helper available, also run the deterministic resource lens before reviewer selection:
spec-first internal resource-governance-lens \
--target-repo <repo-root> \
--json
Record resource_lens_status, advisory dimensions, and reason_codes with the preflight facts. Resource lens facts are advisory: they may justify reviewer focus or closeout notes, but they do not block review, do not replace reviewer judgment, and do not treat generated runtime paths as evidence_ref values. If the helper is unavailable, keep the existing scale-aware preflight and record resource-governance-lens unavailable in Coverage.
When confirmed findings or resource advisories reveal a repeated governance miss, the synthesis may record a small advisory shadow hit only when the current mode permits writes. In mode:report-only or single-agent report-only fallback, do not run this command; render Rule Maturity Candidates in Stage 6 and note skipped recording in Coverage instead.
spec-first internal rule-maturity record --rule-id summary-generated-output-staged --workflow spec-code-review --evidence-ref <durable-review-artifact-or-finding-ref> --reason-code <reason-code> --json
Only record when there is durable evidence and one of these signals exists: a P1/P2 finding exposes a repeated governance gap, the same low-level issue appears at least twice in one review, or an advisory clearly violates a registered contract or plan non-goal. Use rule_id as lens-family + problem-class kebab-case, carry similar_existing_rule_ids when known, and continue with Coverage noting degraded posture if rule-maturity record is unavailable or rejected. Do not automatically adjudicate, promote, demote, or convert the observation into a code-review finding.
Progressive disclosure boundary: low-risk docs-only, simple config, and tiny executable diffs may use a minimum reviewer set; high-risk workflow, contract, release, source/runtime boundary, external-tool evidence, security, or cross-module changes must use the full default core plus applicable conditional reviewers. The goal is to avoid unbounded fan-out on small diffs without hiding risk.
Use the minimum reviewer set only when all of these are true:
changed_file_count <= 2.
untracked_excluded_count == 0.
sensitive_diff == false.
prior_comments_present == false.
plan_explicit == false.
- Either
docs_only == true, simple_config_only == true, or non_test_non_generated_non_lock_line_count <= 25.
Minimum sets:
| Diff class | Reviewers |
|---|
docs_only | spec-project-standards-reviewer, spec-maintainability-reviewer |
simple_config_only | spec-correctness-reviewer, spec-testing-reviewer, spec-project-standards-reviewer |
| tiny executable diff | spec-correctness-reviewer, spec-testing-reviewer, spec-maintainability-reviewer |
If any minimum-set condition is false, use the full default core: spec-correctness-reviewer, spec-testing-reviewer, spec-maintainability-reviewer, spec-project-standards-reviewer, spec-agent-native-reviewer, and spec-learnings-researcher. Always add applicable conditional reviewers after core selection. mode:headless and mode:report-only keep their structured output contracts while using the same scale-aware reviewer selection. mode:autofix may use the minimum set only for docs_only or simple_config_only; otherwise use the full default core because mutating review needs stronger coverage.
Record the preflight facts, selected core tier (minimum or full), and reason in Coverage. Also include resource lens advisory status when available. If the facts are missing, ambiguous, or contradicted by the diff, choose the full default core.
File-type awareness for conditional selection: Instruction-prose files (Markdown skill definitions, JSON schemas, config files) are product code but do not benefit from runtime-focused reviewers. The adversarial reviewer's techniques (race conditions, cascade failures, abuse cases) target executable code behavior. For diffs that only change instruction-prose files, skip adversarial unless the prose describes auth, payment, or data-mutation behavior. Count only executable code lines toward line-count thresholds.
previous-comments is PR-only AND comment-gated. Only select this persona when both conditions hold:
- Stage 1 gathered PR metadata (PR number or URL was provided as an argument, or
gh pr view returned metadata for the current branch).
hasPriorComments from Stage 1 is true.
Skip it for standalone branch reviews, PRs with no prior feedback yet, and approval-only reviews with empty bodies. There is nothing for the persona to verify in those cases, and an empty subagent run still costs the full startup overhead.
Stack-specific personas are additive. A Rails UI change may warrant kieran-rails plus julik-frontend-races; a TypeScript API diff may warrant kieran-typescript plus api-contract and reliability.
For Spec-First conditional agents, check if the diff includes files matching db/migrate/*.rb, db/schema.rb, structure.sql, or data backfill scripts. Do not trigger migration-only agents for model/query-only changes without migration artifacts.
Announce the team before spawning:
Review team:
- core tier: full -- sensitive CLI/runtime diff
- correctness (core)
- testing (core)
- maintainability (core)
- project-standards (core)
- spec-agent-native-reviewer (core)
- spec-learnings-researcher (core)
- security -- new endpoint in routes.rb accepts user-provided redirect URL
- kieran-rails -- controller and Turbo flow changed in app/controllers and app/views
- dhh-rails -- diff adds service objects around ordinary Rails CRUD
- data-migrations -- adds migration 20260303_add_index_to_orders
- spec-schema-drift-detector -- migration files present
This is progress reporting, not a blocking confirmation.
Direct evidence routing candidates
When the diff is broad or impact-sensitive, Stage 3 records direct evidence targets instead of external-tool calls:
- Route handler / public API diff -> inspect handler source, callers/consumers found by
rg, route definitions, tests, and response-shape contracts when present.
- Response shape / consumer access diff -> inspect the route response source, consumer property reads, fixtures, and tests before raising shape drift.
- Shared symbol / helper diff -> inspect direct imports/callers found by
rg or ast-grep, then sample representative tests.
- MCP/RPC tool definition diff -> inspect the tool definition, handler, descriptions, generated runtime expectations, and tests.
- Workspace multi-repo diff -> resolve direct evidence per child repo and keep findings scoped to the child repo that owns the file.
Direct evidence targets are review focus, not scope expansion. Do not raise a finding solely from a name match; every finding must be confirmed by diff/source/test/contract/log evidence before it enters the merged finding set.
Graph-assisted impact candidates
For impact-sensitive diffs, decide whether Graph-Assisted Impact Review applies. Reuse docs/contracts/project-graph-consumption.md; do not introduce a graph-specific evidence schema and do not read raw project-graph artifact JSON.
When a capability-class provider is ready and relevant, gather a minimal candidate set and record:
changed_symbols, changed_entrypoints, and changed_contracts when the diff can be mapped to source symbols or contract surfaces.
symbol_mapping_status: mapped | degraded | not_applicable, with a limitation when large hunks, Markdown/prose, generated files, or provider gaps prevent reliable symbol mapping.
caller_callee_paths, impact_chain_candidates, and blast_radius_candidates only within the minimal-first budget.
affected_test_candidates, tests_for_query_result, missing_test_confirmation, and test_gaps when candidate tests exist, when expected tests are missing, or when provider/read evidence cannot identify or confirm a test path.
review_priority_candidates ordered by impact, publicness, contract/source-runtime sensitivity, security/permission risk, and missing-test signal.
limitations explaining unsupported file types, stale/unknown readiness, no candidates, or bounded sampling.
Every graph-derived candidate is provider_untrusted until confirmed by bounded source/test/log/contract evidence. A reviewer may use candidates to choose which files to inspect next, but a finding that cites graph impact must cite the confirming source, diff, test, log, or contract evidence as the finding evidence.
Stage 3b: Discover project standards paths
Before spawning sub-agents, find the file paths (not contents) of all relevant standards files for the project-standards persona. This is the explicit leaf-reviewer exception to the Host Instruction Reuse Policy: the parent orchestrator discovers paths, and the project-standards reviewer reads only the relevant sections. Use the native file-search/glob tool to locate:
- Use the native file-search tool (e.g., Glob in Claude Code) to find all
**/CLAUDE.md and **/AGENTS.md in the repo.
- Filter to those whose directory is an ancestor of at least one changed file. A standards file governs all files below it (e.g.,
plugins/spec-first/AGENTS.md applies to everything under plugins/spec-first/).
- If
docs/contracts/team-standards.md exists, include that contract and docs/standards/index.md in the path list. The leaf reviewer uses the contract/index to select only trust=confirmed,lifecycle_state=active, scope-matched rule files from docs/standards/**; it must not receive or read the full standards corpus by default.
Pass the resulting path list to the project-standards persona inside a <standards-paths> block in its review context (see Stage 4). The persona reads the files itself, targeting only the sections and standards rule cards relevant to the changed file types. This keeps the orchestrator's work cheap (path discovery only), avoids bloating the subagent prompt with content the reviewer may not fully need, and prevents ordinary review orientation from re-reading root host instruction files or dumping docs/standards/**.
Stage 4: Spawn sub-agents
Runtime readiness preflight
Before creating a run ID or dispatching reviewers, run a read-only host/runtime readiness preflight for the current repo. This preflight is deterministic evidence prepared by spec-mcp-setup; it does not decide review quality or scope.
Use the current-host runtime path when the workflow is installed, and the source path when developing spec-first itself:
| Context | Preflight command |
|---|
| Codex runtime | bash .agents/skills/spec-mcp-setup/scripts/detect-tools.sh |
| Claude runtime | bash .claude/spec-first/workflows/spec-mcp-setup/scripts/detect-tools.sh |
| Source checkout | bash skills/spec-mcp-setup/scripts/detect-tools.sh |
If the target repo was selected with an explicit --repo / child scope earlier in the workflow, pass the same repo selector to the preflight. Do not use this preflight to select a repo; repo scope still comes from Stage 1 and the current review target.
Interpret the JSON facts narrowly:
host_config_status: ready | fallback-active | registry-args-drift | not-required means the host config is acceptable for dispatch. registry-args-drift is acceptable-but-degraded: record the tool id, result, reason_code, and next action posture in Coverage, but do not treat it as unsafe by itself.
host_config_status: action-required | precedence-blocked, missing required dependencies, or a non-ready required MCP project status means the current runtime is not safe for multi-persona dispatch.
- A required MCP startup/config failure is a runtime boundary issue, not a code-review finding. Record it once in Coverage with the tool id, status, and next action.
- Missing optional external-tool evidence does not by itself disable reviewer dispatch; it only limits the review claims and should be carried into Coverage when relevant.
After detect-tools.sh and before reviewer dispatch, consolidate direct handoff evidence once:
- If a
plan: argument or Stage 2b discovery found a plan, inspect its source refs, direct evidence notes, limitations, and repo scope.
- If the caller explicitly handed off a spec-work run artifact path /
run_id, or if the source-owned reader can read a recent artifact, read direct source/test/log evidence as best-effort supplement. Prefer spec-first internal spec-work-run-artifact read --target-repo <repo> and add --workspace-slug / --run-id when an exact selector is available; do not directly scan .spec-first/workflows/spec-work/** or implement "latest run.json" selection in this skill prose.
- Before consuming work artifact evidence, confirm it is bound to the current review scope: explicit path/run id came from this handoff, or artifact
plan_path / source_refs reasonably match the current plan:, review base, and changed files. If the reader returns not-found/not-readable, direct evidence is missing, schema/shape is unavailable, or scope mismatches, record the limitation and do not inject the artifact evidence into reviewer prompts.
- Carry the consolidated direct evidence posture to Stage 6 Coverage. Do not ask each persona reviewer to repeat the same setup preflight.
When a required MCP server is not host-config-ready before dispatch, do not spawn reviewer agents in Codex or Claude. This is a runtime boundary issue, and the fallback is mode-aware:
- Interactive/report-only: set
single_agent_report_only_fallback: true, treat the effective mode as report-only, and run the selected persona lenses inline with bounded direct repo reads. This avoids multiplying the same MCP startup failure across every leaf reviewer.
- Headless: emit
Review failed (headless mode). Reason: required MCP runtime not ready: <tool id> <status>/<reason_code>. Stop without dispatching reviewers, applying fixes, creating <review-artifact-dir>/, or claiming "Review complete".
- Autofix: emit
Review failed. Reason: required MCP runtime not ready: <tool id> <status>/<reason_code>. Mutating review requires safe reviewer/fixer dispatch capability. Stop without dispatching reviewers, applying fixes, or writing run artifacts.
If the preflight script is missing or cannot run, do not invent readiness facts; record runtime readiness preflight unavailable in Coverage and continue only if the host has not already reported MCP startup failure in the current session and the selected mode can safely continue without mutation. If the host has already reported MCP startup incomplete or equivalent startup failure, apply the same mode-aware fallback above.
Dispatch capability gate
Before creating a run ID or dispatching any reviewer, confirm the current host exposes a dispatch primitive and the selected reviewers are part of this documented code-review phase. Dispatch capability is part of the runtime boundary, not a reviewer-selection preference.
Reviewers are analysis agents, not implementation workers. Dispatch is bounded to the resolved diff scope, selected reviewer personas, advisory facts, and output schema. Do not create hidden implement/check agents from code review. Mutation is allowed only through documented safe_auto / selected Apply paths in the chosen mode; report-only fallback, unsafe runtime, or missing dispatch capability must not edit source, generated runtime mirrors, or workflow artifacts.
- In Codex, the current tool contract controls dispatch permission. A workflow entrypoint by itself is not enough to call
spawn_agent; require an explicit user request for subagents/parallel agents/delegated review or an explicit parent-orchestrator delegation whose visible parent request or handoff evidence carries that permission.
- In Claude, follow the current host's documented workflow-owned dispatch admission rules; if the host contract is unavailable or ambiguous, prefer the single-agent report-only fallback.
- If the user explicitly requested subagents, parallel agents, or delegated review and the host exposes a dispatch primitive, continue with normal multi-persona dispatch.
- If the active workflow or parent orchestrator explicitly delegated this code-review workflow and that visible delegation includes reviewer-dispatch permission, continue with normal multi-persona dispatch.
- If the user explicitly requests report-only/no-agents mode, the host lacks a dispatch primitive, or the current runtime cannot call it, do not call
Agent, Task, spawn_agent, or equivalent dispatch tools.
- Codex may expose reviewer dispatch through
spawn_agent, but use it only when both the host capability and the current permission boundary allow it. Do not downgrade solely because the host is Codex when the permission boundary is satisfied.
When dispatch is unavailable, explicitly disabled, or unsafe, set single_agent_report_only_fallback: true and run the rest of the review in read-only form:
- Treat the effective mode as report-only, even if no
mode:report-only token was provided.
- If the user requested
mode:autofix or mode:headless, stop and explain that mutating review requires reviewer/fixer dispatch capability or an isolated workflow that permits it; offer report-only as the safe fallback.
- Do not create
<review-artifact-dir>/ and do not write reviewer artifacts.
- The orchestrator applies the selected persona lenses itself, serially, using the same diff, plan, standards, and direct evidence.
- Skip Stage 5b validator dispatch and all fixer paths.
- In Coverage, state
single-agent report-only fallback: reviewer dispatch unavailable, explicitly disabled, or unsafe.
Model tiering
Three reviewers inherit the session model with no override: spec-correctness-reviewer, spec-security-reviewer, and spec-adversarial-reviewer. These perform the highest-stakes analysis — logic bugs, security vulnerabilities, adversarial failure scenarios — and should run at whatever capability level the user has configured. If the user is on Opus, these get Opus.
All other persona sub-agents and Spec-First agents use the platform's mid-tier model to reduce cost and latency when the host exposes a stable, configured model alias. See the Spawning subsection for the dispatch-time override; the imperative lives there so it is applied at the point of action. On other platforms, do not invent a model name from memory; use only a host-provided stable alias or omit the model parameter, and on other platforms use a host-provided cheap stable alias or omit the model parameter and let agents inherit the default -- a working review on the parent model is better than a broken dispatch from an unrecognized model name.
The orchestrator (this skill) also inherits the session model; it handles intent discovery, reviewer selection, finding merge/dedup, and synthesis -- tasks that benefit from the same reasoning capability the user configured.
Run ID
Generate a unique run identifier before dispatching any agents. This ID scopes parent/orchestrator-owned reviewer detail files and the post-review run artifact to the same directory.
const crypto = require('node:crypto');
const fs = require('node:fs');
const os = require('node:os');
const path = require('node:path');
const runId = `${new Date().toISOString().replace(/[-:.TZ]/g, '')}-${crypto.randomBytes(4).toString('hex')}`;
const reviewArtifactDir = path.join(os.tmpdir(), 'spec-first', 'spec-code-review', runId);
fs.mkdirSync(reviewArtifactDir, { recursive: true });
Pass {run_id} and {review_artifact_dir} to every persona sub-agent as correlation metadata only. Do not ask leaf reviewers to write files directly. Reviewer agents return full structured JSON to the orchestrator; after each return, the orchestrator may write that JSON to <review-artifact-dir>/{reviewer_name}.json in modes that permit run artifacts.
Report-only mode: Skip run-id generation and directory creation. Do not pass {run_id} to agents. Agents return full structured JSON to the parent with no file write, consistent with report-only's no-write contract.
Single-agent report-only fallback: also skip run-id generation and directory creation. There are no agent artifact files; Stage 6 must omit artifact-enriched detail that is unavailable and name the fallback in Coverage.
Spawning
Omit the mode parameter when dispatching sub-agents so the user's configured permission settings apply. Do not pass mode: "auto".
Codex spawn_agent parameter hygiene. Codex reviewer prompts are self-contained: pass the persona, diff-scope rules, output schema, PR metadata, intent, file list, diff, and standards paths in the message or items payload instead of relying on inherited thread context. Dispatch one reviewer per spawn_agent call; do not bundle multiple reviewer personas into one sub-agent prompt. For Codex reviewer personas, prefer the default sub-agent type and omit agent_type; these reviewers are specialized by the prompt, not by a generic explorer/worker role. If a specific runtime genuinely needs an agent_type, omit fork_context (or leave it false); do not combine fork_context: true with agent_type. If a Codex dispatch fails before the reviewer starts because of parameter incompatibility, correct the parameters once and retry through the bounded scheduler; record it as an orchestrator dispatch correction, not a reviewer failure. If a runtime requires fork_context: true for a particular dispatch, omit agent_type and still include the full self-contained review context.
Model override at dispatch time. Pass the platform's mid-tier model on every dispatch except spec-correctness-reviewer, spec-security-reviewer, and spec-adversarial-reviewer, which inherit the session model. In Claude Code, add model: "sonnet" to the Agent tool call. On other platforms, use only a host-provided stable alias or omit the override. Check this on every Agent / spawn_agent / equivalent call in the dispatch loop.
Bounded parallel dispatch. Respect the current harness's active-subagent limit. Queue selected reviewers, dispatch only as many as the harness accepts, and fill freed slots as reviewers complete. Treat active-agent/thread/concurrency-limit spawn errors as backpressure, not reviewer failure: leave the reviewer queued and retry after a slot frees. Record a reviewer as failed only after successful dispatch times out/fails, or when dispatch fails for a non-capacity reason.
Codex scheduling rules:
- Start with at most 4 active reviewer agents unless the runtime explicitly advertises a lower or higher safe cap. Do not launch every selected reviewer in one burst.
- A generic
Agent spawn failed with one or more active reviewers is presumed capacity/backpressure first, even if the error text does not name a limit. Wait for any active reviewer to complete, then retry the same queued reviewer once.
- A spawn failure that includes
MCP startup incomplete, MCP startup failed, or a required MCP server name is a runtime readiness failure. Stop launching new reviewers, record the degraded tool once in Coverage, collect any already-started reviewers that can complete, and apply remaining persona lenses inline through the single-agent report-only fallback.
- Parameter incompatibility remains separate from capacity: correct
spawn_agent parameters once per the Codex parameter hygiene rule, then retry through the same queue.
- Only mark a queued reviewer as failed after the bounded retry path rules out capacity/backpressure and runtime-readiness fallback, or after a successfully spawned reviewer times out/fails.
Spawn each selected persona reviewer using the subagent template included below. Each persona sub-agent receives:
- Their persona file content (identity, failure modes, calibration, suppress conditions)
- Shared diff-scope rules from the diff-scope reference included below
- The JSON output contract from the findings schema included below
- PR metadata: title, body, and URL when reviewing a PR (empty string otherwise). Passed in a
<pr-context> block so reviewers can verify code against stated intent
- Review context: intent summary, file list, diff
- Run ID and review artifact directory for modes that create run artifacts, plus reviewer name for correlation and parent-owned artifact filenames
- Boundary context from Stage 2c, wrapped in
<boundary-context>: scope_boundary, authorized_scope_source, scope_boundary_evidence, declared files/touch set when known, plan requirement or implementation-unit refs, and limitations.