| name | ce-code-review |
| description | Structured code review using tiered persona agents, confidence-gated findings, and a merge/dedup pipeline. Use when reviewing code changes before creating a PR. |
| argument-hint | [blank to review current branch, or provide PR link] |
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-fixer findings. Leave gated_auto, manual, human, and release work unresolved.
- Write a run artifact under
/tmp/compound-engineering/ce-code-review/<run-id>/ 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 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, 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
/tmp/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-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, request_user_input in Codex, ask_user in Gemini, ask_user in Pi (requires the pi-ask-user extension)) 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 the per-agent artifact files on disk (see Detail enrichment in Stage 6).
- Write a run artifact under
/tmp/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: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, Gemini, and Pi 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 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-brainstorm
docs/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-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 (ce-work, lfg, slfg) should prefer this to avoid the detection overhead. 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.