Code Review

Reviews code changes using dynamically selected reviewer personas. Spawns parallel sub-agents that return structured JSON, then merges and deduplicates findings into a single report.

When to Use

• Before creating a PR
• After completing a task during iterative implementation
• When feedback is needed on any code changes
• Can be invoked standalone
• Can run inside larger workflows; use mode:agent when the caller needs JSON instead of markdown tables

Argument Parsing

Parse $ARGUMENTS for optional tokens. Strip each recognized token before interpreting the remainder as a PR number, GitHub URL, or branch name.

Token	Example	Effect
mode:agent	mode:agent	Report-only: return JSON instead of markdown tables and skip the Stage 5c apply (the caller applies). Does not change reviewer selection, merge logic, or scope rules (see Output format)
mode:headless	mode:headless	Deprecated alias for mode:agent
mode:report-only	mode:report-only	Deprecated — ignored. Former no-artifacts mode; default behavior is review-only without checkout
base:<sha-or-ref>	base:abc1234 or base:origin/main	Diff base on the current checkout (explicit; skips auto base detection)
plan:<path>	plan:docs/plans/2026-03-25-001-feat-foo-plan.md	Plan file for requirements verification (explicit). Supports markdown and HTML unified plans.
depth:full	depth:full	Force the full reviewer roster — skip the Stage 3c small-diff lite path so every always-on persona runs regardless of diff size. Use when a deep/thorough review is explicitly requested (the one escalation signal Stage 3c cannot infer from the diff). Does not change conditional selection, merge, or scope.
depth:auto	depth:auto	Default — self-right-size via Stage 3c (lite roster for trivial, low-risk, code-only diffs; full roster otherwise).
grouping:auto	grouping:auto	Default — build thematic triage groups when findings span distinct concerns (Stage 5 step 9b)
grouping:off	grouping:off	Suppress triage groups: no Triage Groups section, empty triage_groups in JSON
grouping:always	grouping:always	Always build triage groups, even for small reviews

Grouping is presentation, not a mode. The grouping: tokens change how the finding set is organized for triage — never reviewer selection, merge logic, scope rules, or the Stage 5c apply decision.

Mode alias: mode:headless normalizes to mode:agent. mode:agent + mode:headless is not a conflict.

Conflicting arguments: Stop without dispatching reviewers when:

• Multiple incompatible scope selectors appear together (e.g. base: and a PR number/branch target — base: means "review the current checkout against this base")
• Multiple distinct mode: tokens other than the mode:agent/mode:headless alias pair
• Multiple distinct grouping: tokens (e.g. grouping:off and grouping:always)

Deprecated mode:autofix is not a conflict — ignore the token and proceed with the normal flow (see below).

Emit a one-line failure reason. In mode:agent, return JSON: {"status":"failed","reason":"..."}.

Operating principles

Same pipeline for default and mode:agent:

• Apply locally; never push. Never push, open PRs, or file tickets in any mode — push is the outward step the user owns. In default (interactive) mode the review applies safe, verified fixes and commits them when the pre-review tree was clean (Stage 5c owns the full rule). In mode:agent it never mutates the tree — it reports and the caller applies.
• No blocking prompts. Never use AskUserQuestion, request_user_input, ask_user, or other blocking question tools. Infer intent, plan, and scope from explicit tokens, git state, PR metadata, and conversation. Note uncertainty in Coverage or the verdict — do not stop to ask.
• Explicit mutations only. Never run gh pr checkout, git checkout, git switch, or similar branch-switch commands. Passing a PR number, URL, or branch name selects review scope, not permission to mutate the working tree. To review local uncommitted work on a feature branch, check out that branch yourself (or stay on it) and pass base: or no target.
• Smart defaults. Untracked files: review tracked changes only and list excluded paths in Coverage. Plan: use plan: when passed; otherwise discover conservatively from PR body or branch keywords. Weak advisory P2/P3 from testing/maintainability alone: demote to testing_gaps / residual_risks per Stage 5.
• Report outcomes, not machinery. What you show the user is about the review: what's being examined (the PR/branch), which coverage is included and the one-line reason for each conditional lens, the independent cross-model pass and which model runs it, and the findings. Keep the skill's internals out of user-facing text — model-tier assignments, raw scope-mode codenames (local-aligned/pr-remote), staging the diff to disk, loading persona files, parallel-dispatch bookkeeping, and step-by-step narration of your own setup. Name what the user would recognize (a PR number, a reviewer's concern, a peer model), not the plumbing. This governs what you surface and suppress; it does not script the wording — use your own voice.

Output format

Invocation	Deliverable
Default	Markdown report (pipe-delimited finding tables) + Actionable Findings summary
mode:agent	One JSON object (see ### JSON output format below) + the same /tmp/.../ce-code-review/<run-id>/ artifacts

mode:agent is report-only: it skips the Stage 5c apply (the caller applies) and serializes findings as JSON instead of markdown. It does not change reviewer selection, merge logic, or scope rules — the JSON is the deterministic contract for programmatic and cross-harness callers (Codex, Gemini, etc.). The default markdown is the human view; keep it ASCII-safe (pipe tables, -> not middot ·, no box-drawing) so it degrades gracefully across terminals.

Quick Review Short-Circuit

If $ARGUMENTS indicates the user wants a quick, fast, or light code review — and mode:agent is not active — do not dispatch the multi-agent flow.

Announce the chosen path before any other work (Quick review vs Multi-agent review). Skip this announcement when mode:agent is active.

Sequence:

1. Run the harness's built-in code review. Forward any review target after stripping tokens. Then stop — do not dispatch the multi-agent pipeline.
2. Exemption: If no built-in review exists, continue into the full multi-agent review.
3. mode:agent bypasses this short-circuit — always run the full multi-agent review and return JSON.

Deprecated: mode:autofix is no longer supported — there is no apply mode. If passed, ignore the token and proceed with the normal flow (default applies safe fixes via Stage 5c; mode:agent reports and the caller applies).

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. autofix_class and owner are signal describing follow-up shape for callers — not apply permission or an apply gate. The apply decision is judgment (Stage 5c), not a function of autofix_class: default mode applies; in mode:agent this skill does not mutate the checkout — the caller applies. See references/action-class-rubric.md for persona guidance.

autofix_class	Default owner	Meaning
gated_auto	downstream-resolver or human	Concrete suggested_fix proposed; caller applies after judgment
manual	downstream-resolver or human	Actionable work needing design input or handoff
advisory	human or release	Report-only — learnings, rollout notes, 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 gated_auto to manual, but never widen without stronger evidence.
• Reject safe_auto and review-fixer if present — drop the finding or remap to gated_auto / downstream-resolver during synthesis.
• requires_verification: true means any caller-applied fix needs targeted tests or follow-up validation.

Reviewers

14 reviewer personas in layered conditionals, plus CE local prompt assets. Quick roster with one-line triggers below; the persona catalog included at the bottom has the full per-persona selection criteria and spawn gates. Each selected reviewer is a generic subagent seeded with a local prompt file from references/personas/; do not dispatch standalone agents by type/name.

Always-on (full review): local prompt assets correctness-reviewer, testing-reviewer, maintainability-reviewer, project-standards-reviewer, plus CE local prompt assets agent-native-reviewer and learnings-researcher. (Stage 3c may reduce this set to a lite roster for trivial, low-risk diffs.)

Cross-cutting conditional (per diff):

• security-reviewer — auth, public endpoints, user input, permissions
• performance-reviewer — DB queries, data transforms, caching, async
• api-contract-reviewer — routes, serializers, type signatures, versioning
• data-migration-reviewer — migration files / schema dumps / backfills (see spawn gate in Stage 3)
• reliability-reviewer — error handling, retries, timeouts, background jobs
• adversarial-reviewer — >=50 changed code lines, or auth / payments / data mutations / external APIs. When selected, a cross-model adversarial pass (Stage 4) additionally runs the same brief through a different model family via a peer CLI — additive, non-blocking
• previous-comments-reviewer — PR with existing review comments (PR-only, comment-gated)

Stack-specific conditional (per diff): julik-frontend-races-reviewer (Stimulus/Turbo, DOM events, async UI) and swift-ios-reviewer (Swift/SwiftUI/UIKit, entitlements, Core Data, .pbxproj).

CE conditional (migration-specific): local prompt asset deployment-verification-agent — deployment checklist + rollback when the migration gate applies and the change is risky.

Review Scope

A full review spawns generic subagents for all 4 always-on personas plus the 2 CE always-on local prompt assets, then adds whichever cross-cutting and stack-specific conditionals fit the diff (Stage 3c can collapse this to a lite roster for trivial, low-risk diffs). The model naturally right-sizes: a small config change triggers 0 conditionals = 6 reviewers. A Rails auth feature might trigger security + reliability + adversarial = 9 reviewers.

Protected Artifacts

The following paths are compound-engineering pipeline artifacts and must never be flagged for deletion, removal, or gitignore by any reviewer:

• docs/brainstorms/* -- legacy requirements documents created by older ce-brainstorm versions
• docs/plans/*.{md,html} -- unified plan artifacts created by ce-brainstorm or ce-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.

Plan Requirements Completeness

When a plan is provided via plan:<path> or discovered from PR/branch context, classify readiness before checking completeness:

• Unified artifact: metadata includes artifact_contract: ce-unified-plan/v1.
  • artifact_readiness: requirements-only can inform product intent, but it must not trigger implementation-unit completeness findings. Report that the artifact was not implementation-ready if the diff appears to implement it.
  • artifact_readiness: implementation-ready is eligible for full requirements and U-ID completeness checks.
  • Invalid progress-like readiness values (active, in_progress, completed, done) are contract errors.
• Legacy plan: use the existing completeness checks.

Extract requirements from these shapes, in order:

1. Unified Product Contract -> ### Requirements
2. Legacy top-level ## Requirements
3. Legacy ## Requirements Trace

For unified implementation-ready plans, also extract U-IDs from ## Implementation Units and compare against PR body/branch context when available. Do not require every Product Contract R-ID to map one-to-one to a single U-ID; verify that implemented U-IDs cite the relevant R/F/AE/KTD IDs and that no claimed U-ID is missing from the plan.

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. Callers reviewing the current checkout should pass explicit base: when auto-detection is unnecessary. 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."

If a PR number or GitHub URL is provided as an argument:

Do not check out the PR branch. Scope comes from GitHub read APIs plus optional local alignment when HEAD already matches the PR head branch.

Skip-condition pre-check. Before scope detection, run a PR-state probe:

gh pr view <number-or-url> --json state,title,body,files

Apply skip rules in order:

• state is CLOSED or MERGED -> stop with reason PR is closed/merged; not reviewing.
• Trivial-PR judgment: spawn a lightweight sub-agent on the platform's cheapest capable model when a known override exists; otherwise omit the model override and inherit. Give it the PR title, body, and changed file paths. The agent's task: "Is this an automated or trivial PR that does not warrant a code review? Consider: dependency lock-file or manifest-only bumps, automated release commits, chore version increments with no substantive code changes. When in doubt, answer no — false negatives (skipped reviews that should have run) are more costly than false positives (unnecessary reviews)." If the judgment returns yes: stop with reason 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, stop without dispatching reviewers. Default mode: emit the reason as plain text. mode:agent: emit JSON only — {"status":"skipped","reason":"<same message>"} — so programmatic callers can parse the outcome. Standalone, base:, and branch-remote paths are unaffected. Draft PRs are reviewed normally.

If no skip rule fires, fetch PR metadata without checkout:

gh pr view <number-or-url> --json title,body,baseRefName,headRefName,headRefOid,isCrossRepository,url,files,reviews,comments --jq '{title, body, baseRefName, headRefName, headRefOid, isCrossRepository, url, files: [.files[].path], hasPriorComments: ((.reviews | map(select(.state != "APPROVED" or .body != "")) | length) > 0 or (.comments | length) > 0)}'

Set BASE: to pr:<number-or-url> (logical marker — not a git SHA). Set UNTRACKED: from git ls-files --others --exclude-standard on the current checkout (usually empty during PR-remote review).

PR scope mode. Classify as local-aligned only when all of these hold; otherwise use pr-remote. A matching branch name alone is not enough — a fork PR or a stale local branch can share a name with the PR head while pointing at unrelated code, and trusting the name would diff and inspect the wrong tree.

1. git rev-parse --abbrev-ref HEAD equals headRefName.
2. The PR is not cross-repository (isCrossRepository is false).
3. The PR head commit is contained in the local checkout: git merge-base --is-ancestor <headRefOid> HEAD exits 0. This confirms the working tree actually carries the PR head (allowing unpushed local fixes layered on top) rather than an unrelated same-named branch.

• local-aligned — all three checks pass. Local Read/Grep/git blame against workspace files are valid for PR changed paths.
• pr-remote — any check fails. The working tree is not the PR head; workspace file contents for changed paths may be stale or unrelated.

Diff by scope mode (do not mix remote and local diffs — contradictory hunks cause false positives):

• local-aligned: Resolve <resolved-base-ref> from baseRefName (fetch if needed). Compute BASE=$(git merge-base HEAD <resolved-base-ref>), then set FILES: from git diff --name-only $BASE and DIFF: from git diff -U10 $BASE (includes committed, staged, and unstaged changes on the PR branch). Do not call gh pr diff or append remote hunks — when unpushed fixes exist, the local tree is canonical. Note in Coverage: scope: local-aligned (PR; local tree diff).
• pr-remote: Set FILES: from the PR files array. Set DIFF: from gh pr diff <number-or-url> --color=never. If gh pr diff fails, stop with an actionable error — do not fall back to checkout.

When pr-remote, before Stage 4:

1. Best-effort fetch PR head without checkout: git fetch --no-tags origin <headRefName>:refs/review/pr-<number>-head (substitute PR number from metadata).
2. When fetch succeeds, set PR_HEAD_REF=refs/review/pr-<number>-head for reviewers and validators. When fetch fails, omit PR_HEAD_REF and note in Coverage — reviewers must rely on diff hunks only.
3. Best-effort fetch the PR base without checkout: git fetch --no-tags origin <baseRefName>. When it succeeds, resolve a concrete ref with git rev-parse FETCH_HEAD and set PR_BASE_REF to that SHA — a real git base ref reviewers and validators use for file-level git diffs (e.g. data-migration-reviewer runs git diff <PR_BASE_REF> -- db/schema.rb/structure.sql). The pr:<number-or-url> logical marker in BASE: stays the scope marker; PR_BASE_REF is the diffable base. When the fetch fails, omit PR_BASE_REF and note in Coverage — schema-drift and other git-diff checks fall back to diff hunks only and must not assume main.
4. Include <pr-scope-mode>pr-remote</pr-scope-mode> and, when set, <pr-head-ref>...</pr-head-ref> and <pr-base-ref>...</pr-base-ref> in the Stage 4 review context bundle.

Reviewers and Stage 5b validators in pr-remote mode must not Read/Grep workspace paths for files in FILES:. Inspect via git show <PR_HEAD_REF>:<path> when PR_HEAD_REF is set, otherwise use only the provided diff hunks. local-aligned uses normal workspace inspection.

If a branch name is provided as an argument:

Substitute the provided branch name as <branch>. Do not check out <branch>.

If git rev-parse --abbrev-ref HEAD equals <branch>, use the standalone (current branch) path below — same tree, explicit branch name; do not use remote-only diff.

Otherwise diff the remote/local ref without checkout:

1. Try gh pr view <branch> --json baseRefName,url,headRefName — if a PR exists, prefer the PR number/URL path above (same remote diff rules).
2. Else resolve <branch> as origin/<branch> or <branch> after git fetch --no-tags origin <branch> when needed.
3. Resolve default base branch (same logic as standalone). Compute BASE=$(git merge-base <base-ref> <branch-ref>) and git diff -U10 $BASE <branch-ref>.
4. If <branch-ref> cannot be resolved locally, stop: "Cannot diff branch <branch> without checkout. Check out that branch, pass its open PR URL/number, or review the current branch with base:."

On success for remote branch diff, set branch-remote scope. The working tree is not <branch>. Include <pr-scope-mode>branch-remote</pr-scope-mode> and <branch-head-ref><branch-ref></branch-head-ref> in the Stage 4 review context bundle. Reviewers and Stage 5b validators must not Read/Grep workspace paths for files in FILES:. Inspect via git show <branch-ref>:<path> or diff hunks only.

Produce:

echo "BASE:$BASE" && echo "FILES:" && git diff --name-only $BASE <branch-ref> && echo "DIFF:" && git diff -U10 $BASE <branch-ref> && echo "UNTRACKED:" && git ls-files --others --exclude-standard

If no argument (standalone on current branch):

Apply the same base-detection logic as branch mode above, using the current branch (i.e., gh pr view --json baseRefName,url with no argument defaults to the current branch).

If no base can be resolved, stop. Do not fall back to git diff HEAD — a standalone review without the base 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 UNTRACKED:. Untracked paths are out of scope unless staged. When non-empty, list excluded files in Coverage and continue on tracked changes only — never stop or prompt.

Stage 1b: Compute scope signals (cheap, deterministic)

Derive deterministic signals from the resolved diff once, so reviewer selection (Stage 3) and the small-diff fast path (Stage 3c) do not each re-reason over the whole diff. These signals only ever shrink the roster via Stage 3c, and that gate fails closed (Stage 3c) — so any failure here (unresolved base, count failure, an uncounted file type) must surface as UNKNOWN/non-zero UNCOUNTED_FILES, never as a silent 0 that reads as "trivial."

Set DIFF_A/DIFF_B to the two endpoints to diff, by Stage 1 scope mode:

• local-aligned / standalone / base: — DIFF_A="$BASE" (a real SHA/ref), DIFF_B empty (diffs base vs working tree).
• pr-remote / branch-remote — DIFF_A=<PR_BASE_REF>, DIFF_B=<PR_HEAD_REF> (or <branch-head-ref>) — the fetched refs from Stage 1. Do not model-count from hunks (it drifts per host/model). If either ref was not fetched, skip the block and emit EXEC_LINES:UNKNOWN + UNCOUNTED_FILES:1 so Stage 3c forces the full roster.