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.
Overview
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 as a read-only or autofix review step inside larger workflows
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 |
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.
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. |
Autofix mode rules
- Skip all user questions. Never pause for approval or clarification once scope has been established.
- Apply only
safe_auto -> review-fixerfindings. Leavegated_auto,manual,human, andreleasework unresolved. - Write a run artifact under
.context/compound-engineering/ce-code-review/<run-id>/summarizing findings, applied fixes, residual actionable work, and advisory outputs. Orchestrators read this artifact to route residualdownstream-resolverfindings; 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-resolverfinding with severity, file:line, title, and autofix_class. Include the run-artifact path. Callers read this summary directly without parsing the artifact. When no residuals exist, stateResidual 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
.context/compound-engineering/ce-code-review/<run-id>/, do not file tickets, and do not commit, push, or create a PR. - Safe for parallel read-only verification.
mode:report-onlyis 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-onlymust run in an isolated checkout/worktree or stop instead of runninggh 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 (
AskUserQuestionin Claude Code,request_user_inputin Codex,ask_userin Gemini,ask_userin Pi (requires thepi-ask-userextension)) 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), emitReview 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-fixerfindings in a single pass. No bounded re-review rounds. Leavegated_auto,manual,human, andreleasework 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 the per-agent artifact files on disk (see Detail enrichment in Stage 6).
- Write a run artifact under
.context/compound-engineering/ce-code-review/<run-id>/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:headlessmust run in an isolated checkout/worktree or stop instead of runninggh pr checkout/git checkout. When stopping, emitReview 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 (appliessafe_autofixes). 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,
AskUserQuestionis 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), callToolSearchwith queryselect:AskUserQuestionto 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, Gemini, and Pi this preload step does not apply. - The numbered-list fallback only applies when the harness genuinely lacks a blocking question tool —
ToolSearchreturns no match, the tool call explicitly fails, or the runtime mode does not expose it (e.g., Codex edit modes whererequest_user_inputis unavailable). A pending schema load is not a fallback trigger; callToolSearchfirst 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_autotogated_autoormanual, but never the other way without stronger evidence. - Only
safe_auto -> review-fixerenters the in-skill fixer queue automatically. requires_verification: truemeans a fix is not complete without targeted tests, a focused re-review, or operational validation.
Reviewers
18 reviewer personas in layered conditionals, plus CE-specific agents. See the persona catalog included below for the full catalog.
Always-on (every review):
| Agent | Focus |
|---|---|
ce-correctness-reviewer | Logic errors, edge cases, state bugs, error propagation |
ce-testing-reviewer | Coverage gaps, weak assertions, brittle tests |
ce-maintainability-reviewer | Coupling, complexity, naming, dead code, abstraction debt |
ce-project-standards-reviewer | CLAUDE.md and AGENTS.md compliance -- frontmatter, references, naming, portability |
ce-agent-native-reviewer | Verify new features are agent-accessible |
ce-learnings-researcher | Search docs/solutions/ for past issues related to this PR |
Cross-cutting conditional (selected per diff):
| Agent | Select when diff touches... |
|---|---|
ce-security-reviewer | Auth, public endpoints, user input, permissions |
ce-performance-reviewer | DB queries, data transforms, caching, async |
ce-api-contract-reviewer | Routes, serializers, type signatures, versioning |
ce-data-migrations-reviewer | Migrations, schema changes, backfills |
ce-reliability-reviewer | Error handling, retries, timeouts, background jobs |
ce-adversarial-reviewer | Diff >=50 changed non-test/non-generated/non-lockfile lines, or auth, payments, data mutations, external APIs |
ce-cli-readiness-reviewer | CLI command definitions, argument parsing, CLI framework usage, command handler implementations |
ce-previous-comments-reviewer | Reviewing a PR that has existing review comments or threads |
Stack-specific conditional (selected per diff):
| Agent | Select when diff touches... |
|---|---|
ce-dhh-rails-reviewer | Rails architecture, service objects, session/auth choices, or Hotwire-vs-SPA boundaries |
ce-kieran-rails-reviewer | Rails application code where conventions, naming, and maintainability are in play |
ce-kieran-python-reviewer | Python modules, endpoints, scripts, or services |
ce-kieran-typescript-reviewer | TypeScript components, services, hooks, utilities, or shared types |
ce-julik-frontend-races-reviewer | Stimulus/Turbo controllers, DOM events, timers, animations, or async UI flows |
ce-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 |
CE conditional (migration-specific):
| Agent | Select when diff includes migration files |
|---|---|
ce-schema-drift-detector | Cross-references schema.rb against included migrations |
ce-deployment-verification-agent | Produces deployment checklist with SQL verification queries |
Review Scope
Every review spawns all 4 always-on personas plus the 2 CE always-on agents, then adds whichever cross-cutting and stack-specific conditionals fit the diff. The model naturally right-sizes: a small config change triggers 0 conditionals = 6 reviewers. A Rails auth feature might trigger security + reliability + kieran-rails + dhh-rails = 10 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/*-- requirements documents created by ce-brainstormdocs/plans/*.md-- plan files created by 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.
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-on