Code Review

Run tiered review with strict output gates.

Input Schema

{
  "scope": "optional working-tree|path|commit|pr; infer pr for bare number, #number, or PR URL",
  "target": "optional path, commit ref, PR number, PR URL, or current branch PR",
  "done_when": "blocking issues are identified with gate decision"
}

Scope And Routing

• working-tree: review unstaged/staged local changes.
• path: review one file/directory diff.
• commit: review a git diff revision spec, such as COMMIT^!, BASE..HEAD, or BASE...HEAD.
• pr: review an open pull request: collect GitHub PR metadata/review evidence, fetch target branch, update local checkout with gh pr checkout, inspect local files; target may be PR number, URL, or current-branch PR.

Input shorthand:

• Canonical in-session: $code-review 123 or $code-review #123 => scope=pr, target=123.
• Natural-language aliases: code-review 123, code-review #123, and code-review PR 123 => scope=pr, target=123.
• code-review <github-pr-url> => scope=pr, target=<github-pr-url>.
• Bare number = GitHub PR number; do not ask for scope=pr.

Never write to remote. PR scope may update local checkout to PR head; otherwise read-only except the run-directory artifacts defined below. Never pass --force to git or gh; if forced checkout seems needed to align local branch and PR head, stop, explain overwrite risk, and ask before retrying. To fix findings, switch to code-remediate after creating review artifact.

Workflow (Exact Commands)

01: Create run directory

Run create_run.py --skill code-review per ../../shared/helper-cli-contract.md and retain its printed timestamped path literally. A local review keeps that path for its complete lifecycle. A PR review begins there because current-branch input may not identify a PR before collection.

02: T0 mechanical scope gate

For local scopes, inspect python PLUGIN_ROOT/shared/collect_diff.py --help; collect normalized scope, optional target, and the literal <run-directory> path.

For PR scope, inspect python PLUGIN_ROOT/shared/collect_pr.py --help; collect the exact target into the literal <run-directory> path with checkout enabled.

After successful authoritative pr.json collection, run create_run.py --skill code-review --promote-pr-run <run-directory> and capture its single printed final path. The promotion derives the authoritative PR number from pr.json, allocates .reports/codex/code-review/pr-<number>/run-<NNN>/, and moves the complete run without overwriting another run. Use the printed promoted path literally for every later helper, artifact, specialist context, result, and final handoff. Never reconstruct the numbered path or keep writing to the temporary path.

If collection fails before authoritative PR identity exists, keep the timestamped run as an unavailable diagnostic. It is not an assessed PR review and must not be promoted. Existing flat timestamped runs remain discoverable historical artifacts; do not migrate them.

In runtimes with network sandboxing, execute the complete collector command with approved external network access from its first attempt under ../../shared/native-skill-contract.md. Before requesting it, state:

• Action and purpose: collect current PR evidence.
• External capability: read-only GitHub access plus the documented local checkout.
• Credential behavior: gh is an opaque local credential broker.
• Filesystem and worktree effects: write collection artifacts and may update the local checkout.
• Retry policy and safe denial outcome: one classified recovery only, otherwise the review is unavailable.
• For Codex exec, set sandbox_permissions="require_escalated" on the collector with a narrow read-only GitHub justification; never request a broad python approval prefix. Apply the other shared runtime and denial boundaries. A direct approval for gh pr view does not cover gh spawned by the collector: the outer collector command owns its nested GitHub CLI, HTTPS fallback, checkout, and Git fetch traffic. The PR request authorizes asking, never bypassing runtime approval.
• If an agent-caused unapproved attempt returns github-network before any user approval request or denial, rerun that same complete collector command once through the runtime's external-network approval mechanism before producing a terminal unavailable result. This recovery exists only for that pre-denial sandbox mistake; after the user denies approval, the current turn stops and the retry is forbidden. Only after that approved collector attempt fails, external-network approval is unavailable, or the user denies it may the terminal collection-failure gate apply; never repeat more than one approved recovery attempt.

PR evidence has two tiers.

• Core evidence: gh pr view metadata including contributor description/body, authoritative base-repository identity, refreshed target ancestry, an exact local PR head, and a diff derived with local git diff <base>...<head> after SHA verification.
• Supplemental evidence: GraphQL review-thread resolution state and derived diff statistics.

Collector and source boundary:

• The collector delegates remote GitHub state reads to github_read.py, which uses gh as an opaque local credential broker: it never invokes gh auth, reads token/keychain state, or writes CLI failure output to artifacts.
• That read-only boundary permits audited view commands, REST GET, and GraphQL query operations; public HTTPS fallback cannot establish private PR evidence.
• A classified core command failure is recorded in command-failure.json when diagnostics exist.

Checkout and source requirements:

• For an open PR, use fork-aware gh pr checkout <number> unless the current HEAD already exactly equals PR metadata; historical collection fetches GitHub's refs/pull/<number>/head, verifies its exact SHA, and checks it out detached.
• Inspect source only in the local checkout recorded by <run-directory>/local-checkout.json; diff.patch must record diff_source=verified-local-checkout provenance there.
• Never reconstruct changed source from curl, raw.githubusercontent.com, or head-files/ snapshots.
• If checkout or local-diff verification fails, fail instead of reviewing remote raw files.
• Do not retry with --force unless user explicitly confirms after receiving force reason and overwrite risk.

When gh pr view metadata fails, public unauthenticated HTTPS fallback is eligible only when all of these hold:

• The failure is github-network, github-auth, github-rate-limit, or command-timeout.
• The checkout target is trusted: a canonical PR URL must match a configured GitHub remote; a numeric target requires exactly one distinct configured GitHub repository identity.

Ambiguous or unsafe targets, permission failures, not-found failures, and unclassified failures remain fail-closed.

Fallback behavior:

• The fallback normalizes limited PR metadata, then uses the verified refs/pull/<number>/head ref for a detached checkout and derives the local diff; it never establishes private PR evidence.
• online-review-summary.json must list unavailable fallback evidence as sorted IDs.
• Raw GitHub CLI stderr is never persisted; terminal diagnostics may include a safe failure_reason enum alongside non-secret classification metadata.

Classify diff; write <run-directory>/scope.txt:

• TRIVIAL: no public API/config/security/ML behavior touched, <3 files, <50 changed lines.
• LOCAL: one subsystem or 3-7 files; local context explains behavior.
• BROAD: 8+ files, cross-subsystem change, dependency/config change, or unclear ownership.
• HIGH_RISK: public API, release, security, auth, credentials, deserialization, data pipeline, ML tensor math, CI/CD, or migration behavior.

For scope=pr, merge-oriented code review is limited to an OPEN PR. collect_pr.py can also collect historical evidence for a merged or closed PR, including its diff, online discussions, refreshed current target state, and exact checked-out PR head; that raw collector evidence is useful for diagnosis but must not receive a merge recommendation or feed code-remediate. For an open review, core evidence includes pr.json, pr-routing.json, remote-selection.json, target-branch.json, local-checkout.json, and locally derived diff.patch; online evidence includes comments, reviews, review-threads.json, unresolved-review-threads.json, and online-review-summary.json. Selected remote must match the base repository from the PR URL. The freshly fetched target must equal or descend from the PR-recorded base, proven by expected_base_is_ancestor=true; target advancement is integration context, never a PR finding or merge blocker. Genuine divergence fails collection. The local checkout head must exactly match open-PR metadata. Historical target-branch.json may record divergence. pr-routing.json and local-checkout.json must include force_policy proving no automatic forced checkout. Treat unresolved online threads/comments as candidate findings until triaged valid, duplicate, stale, out-of-scope, or already fixed. If GraphQL review-thread collection fails or is incomplete, continue source review with empty normalized thread arrays, review-threads-error.txt, review_threads_status=unavailable, explicit partial-online-triage notes, and confidence gap PR review-thread resolution status was unavailable; online review triage may be incomplete. Never convert that supplemental integration gap into a PR finding or merge blocker by itself.

If files.txt and untracked.txt are empty with no explicit target, fail before gates. If scope=pr and pr-error.txt exists, fail with captured reason and do not begin T1/T2 source review.

Terminal review-unavailable output gate: A core T0 PR collection failure is a process failure, not a review result.

• State PR Review Availability: unavailable; Source findings: not assessed; and Merge decision: not made.
• Use plain diagnostic prose with exactly a process diagnostic, recovery action, and evidence path.
• Do not emit a Markdown table: neither PR Evidence Collection Recovery nor Review Findings and Merge Blocks applies before source assessment.
• Do not emit needs-more-work, minor-changes, reject, not-aligned, or any other merge recommendation.
• Retain current-attempt metadata, checkout state, or partial diff artifacts for diagnosis, but label them unassessed and never turn them into findings.
• Name the classified failure and <run-directory>/pr-error.txt, then stop.
• Still write a canonical result.json with status=fail, zero findings, review_status=unavailable, and collection_failure={"code": "<pr-error.txt text>", "artifact": "pr-error.txt"}; the review-specific validator rejects a review decision, source findings, specialist artifacts, any table, or assessed-review sections.

For retryable github-network, github-rate-limit, or command-timeout, explain that no review occurred and ask the user to retry the unchanged collector later; rate-limit diagnostics deliberately retain no server interval.

• If checkout-state.json exists, say the local checkout command may have changed the worktree, tell the user to inspect that local state before retrying, and never claim no checkout was produced.
• For github-auth or a permission failure, stop and explain that the local gh configuration/account access needs repair; tell the user to run gh auth status and, if needed, gh auth login privately outside the agent workflow, verify repository access, and never paste tokens, keychain data, or credential output into chat.
• For missing-command:gh, tell the user to install or repair gh locally before retrying.
• For github-not-found, ask for the canonical PR URL and repository identity.
• For definitive unsafe-gh-command, invalid protocol/JSON, missing required PR identity, or an unclassified deterministic collector error, stop at the unavailable result, explain the classified code and artifact, and suggest filing a Codex Rig bug with the plugin version, command label, failure code, and sanitized artifacts.
• Never retry a deterministic target, permission, safety-guard, or plugin-contract failure automatically.

Terminal close gate (PR only): After successful T0 collection for an OPEN PR and before structural context or T1/T2, screen the PR goal, description, minimal verified diff evidence, authoritative project policy/history, and linked upstream evidence for one conclusive proposal-level close reason. This is a disposition decision, not a source review. If evidence is inconclusive, continue to T1/T2; never close from suspicion, reviewer preference, contributor identity, AI authorship/style, or a merely related change.

Use exactly one close code:

Code	Conclusive evidence	Insufficient alone
FALSE_GOAL	The stated goal contradicts a citable invariant, specification, domain fact, or verified current behavior.	Implementation disagreement, stale wording, or an unverified claim.
BREAKING_CONDUCT	Direct evidence that the contribution is intentionally malicious or adversarial by design, such as a backdoor, exfiltration, or supply-chain attack.	An accidental security bug, poor code, suspicion, or inferred intent.
WRONG_SCOPE	A documented roadmap, maintainer decision, ADR, or contribution boundary directly excludes the proposed goal.	Size, mixed files, or an undocumented preference.
WRONG_PROVENANCE	A documented license or rights requirement and objective evidence of an incompatible or unresolvable provenance conflict.	Fork ownership, code similarity, unknown provenance, or a missing CLA/DCO signature that the project permits the contributor to fix.
DUPLICATE	A verified merged change or resolved upstream issue already supplies the same still-applicable outcome.	A similar title, overlapping files, related open work, or the same issue area.
UNADDRESSED_REVERT	The PR semantically reintroduces a reverted change and does not address the documented reason for that revert.	File overlap, patch similarity, or a revert title alone.
SPAM	Objective irrelevant, promotional, repeated-submission, or non-substantive evidence shows no bona fide project change.	A small change, missing tests, low quality, or AI-generated content by itself.
ARCHITECTURE_VIOLATION	The proposal directly contradicts a documented current architectural principle.	Style preference, abstraction concern, or reasoning that requires detailed source review.

A close decision requires confidence >= 0.90, two distinct evidence sources, a recorded counterevidence/falsification check, and binding to the verified current PR head. Public-HTTPS fallback evidence cannot close because its confidence cap is 0.89. For WRONG_PROVENANCE, a missing required CLA/DCO signature remains a normal blocking item unless documented project policy makes the conflict terminal. For BREAKING_CONDUCT, an accidental security defect remains a normal blocking finding; only evidenced by-design harm reaches this gate.

On close, skip structural context, T1, T2, specialist routing, detailed findings, severity classification, and the normal recommendation step. Write review-notes.md with Review Decision: close, source findings not assessed, detailed review skipped, the exact close reason, summary, rationale, evidence, counterevidence checked, and GitHub mutation: not performed. Emit status=pass for the successfully completed workflow, zero findings, review_status=closed, and close_decision={"schema_version": 1, "code": "<CODE>", "advisory_only": true, "head_sha": "<verified PR head>", "summary": "<summary>", "rationale": "<rationale>", "evidence": [{"claim": "<observed fact>", "source": "<artifact, repository path, or authoritative URL>"}], "counterevidence_checked": ["<falsification check>"]}. Include at least two distinct evidence entries. Omit review_decision, recommendations, follow-up, review routing, specialist artifacts, and every Markdown table. Run the shared gates with detailed-review checks marked not applicable and the review gate validating the close artifact, then run both artifact validators. This result only advises the user to close; never close, comment on, merge, or otherwise mutate GitHub.

Structural context (optional): after the diff is collected, also probe codemap-py once for changed-symbol blast radius: python PLUGIN_ROOT/shared/codemap_adapter.py context --category review --out <run-directory>/codemap-context.json. Per ../../shared/codemap-contract.md, absence/incompatibility is non-fatal — continue with T1/T2 as scoped by scope.txt alone. Persist the diff-impact evidence once here; T2 specialist fan-out (step 04) includes <run-directory>/codemap-context.json in each triggered context pack, never a fresh per-specialist query.

03: T1 primary diff review

Review axes, in order:

• API and behavior regressions.
• Test coverage and edge-case gaps.
• Error handling and logging.
• Project coding principles: changed code follows the applicable AGENTS.md layers for simplicity, readability, reproducibility, short reusable units without low-value argument-remapping wrappers, guard clauses or early return/yield/continue, project docstring-style detection, concise purpose docstrings, and inline comments only for non-trivial implementation blocks.
• Security, data, ML, CI/CD, or release risks signaled by T0.
• Documentation or migration gaps caused by behavior/API changes.

Blocking defaults guide merge judgment; they are not automatic labels:

Category	Default	Nuance
CI red or failing check	blocking	Only a major or required-check failure. Note a single flaky-looking rerun blip without automatically blocking.
Missing test coverage for new or changed logic	blocking	Require coverage proportional to the changed contract and regression risk.
Accidental security bug	blocking	Evidenced by-design harm is terminal BREAKING_CONDUCT at the close gate.
Breaking API change without deprecation or migration path	blocking	Require the project-compatible transition before merge.
Missing docs for new or changed public behavior	blocking	Missing CHANGELOG entry alone is not blocking and may be completed through the release workflow.
Performance regression	contextual	Block an unexplained regression against recent releases; do not block when a correctness fix necessarily removes invalid prior speed.
Merge conflicts	not blocking	Conflict resolution belongs to code-remediate; review does not gate on the conflict alone.
Incomplete implementation	blocking	Includes TODOs in changed paths, missing expected error handling, or an unfinished public contract.
Missing CLA/DCO signature	blocking only when the project requires it	Verify a CLA/DCO bot check or explicit contribution policy first; without such a requirement it is not applicable.

04: T2 risk-routed specialist fan-out

Always:

• Write <run-directory>/review-routing.json with schema_version=1; declared risk tier; every exact boolean signal below; signal_evidence as an object containing every signal with a non-empty JSON list[str] value for each true/false decision; sorted triggered_roles; and trigger_reasons as an object containing only triggered roles with a non-empty JSON list[str] value. When and only when a Sol-pinned role is explicitly selected, add sol_selection with that exact role as its only key and an object containing only source=explicit-user-selection, non-empty parent_event_id, and lowercase 64-hex selection_sha256; the manifest must mirror this record exactly.
• For example, write "signal_evidence": {"bug_fix": ["PR body and changed test identify the corrected behavior."]} and "trigger_reasons": {"qa-specialist": ["Bug-fix and test-path evidence require QA."]}. Bare strings are invalid.
• Then run python PLUGIN_ROOT/skills/code-review/review_routing.py --out <run-directory> so the shipped deterministic producer replaces mechanical_risk_tier and mechanical_risk_evidence from files.txt, untracked.txt, and numstat.txt; never calculate or copy those fields manually.
• Keep the declared tier at or above mechanical file/line, binary-size, config/dependency, CI, migration, or security-path evidence.
• Set matching signals true for mechanically detected test, docs, data/tensor, CI, and security paths.
• Write <run-directory>/specialist-manifest.json, with empty passes when no role triggers. Never add untriggered manifest roles.

Required routing signals:

• QA risk: behavior_change, bug_fix, test_or_error_path, data_tensor_boundary.
• Challenge risk: high_candidate, unresolved_material_assumption, material_no_finding, explicit_adversarial.
• Conditional axes: axis_solution_architect, axis_security_auditor, axis_data_steward, axis_cicd_steward, axis_linting_expert, axis_doc_scribe, axis_oss_shepherd, axis_squeezer, axis_scientist, axis_web_explorer.

Routing rules:

• TRIVIAL: no automatic QA/challenger pass; conditional axes may trigger.
• LOCAL: QA only for QA-risk; challenger only for challenge-risk. File-count-only LOCAL triggers neither.
• BROAD and HIGH_RISK: always real QA and challenger passes.
• Non-Sol conditional role only when matching axis_<role> signal is true. solution-architect and security-auditor additionally require valid explicit-user-selection evidence; an axis signal alone fails routing and never selects Sol.

For every triggered pass:

• Create <run-directory>/specialists and one markdown output per triggered spawned/substituted pass.
• Apply ../../shared/specialist-orchestration.md.
• Before the pass, write narrow <run-directory>/specialists/<role>-context.md: objective, axis, relevant evidence, excluded noise, concrete questions, output contract, stop rule.
• Never give every specialist whole PR/repository.

Parent owns final severity, duplicate merge, conflict resolution, and decision.

For a spawned attempt:

• Hash completed context before spawn; task name review_<role_with_underscores>_<first_12_context_sha256>_a<attempt>.
• Record full agent path. This binds runtime child identity to role, context artifact, and attempt even when rollout schema leaves agent_role null.
• Runtime encrypts actual inter-agent payload: do not claim cryptographic proof plaintext exactly equals saved context; record residual limit in confidence metadata.

Compute SHA-256 for diff.patch and every context pack. Require exact first specialist line (replace placeholders):

<!-- codex-review-provenance role=<role> run=<review_run_id> input=<review_input_sha256> context=<context_sha256> attempt=<n> -->

Routed specialist axes:

• qa-specialist: tests, edges, regressions, tensor/data boundaries.
• challenger: adversarial assumptions, high findings, migration/API risks, material no-finding conclusions.
• Conditional roles: data-steward, cicd-steward, linting-expert, doc-scribe, oss-shepherd, squeezer, scientist, and web-explorer cover named domains. solution-architect and security-auditor are Sol-pinned and never triggered by a matching domain alone: use either only when the user expressly requests Sol or selects that role, then return its bounded read-only evidence artifact to the Terra parent/session for review acceptance.

Use runtime-provided subagents when independence materially helps and follow the portable route order in the shared orchestration policy.