| name | ce-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, and emit residual downstream work when needed |
| Report-only | mode:report-only in arguments | Strictly read-only. Review and report only, then stop with no edits, artifacts, todos, 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, skip todos, 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
.context/compound-engineering/ce-review/<run-id>/ summarizing findings, applied fixes, residual actionable work, and advisory outputs.
- Create durable todo files only for unresolved actionable findings whose final owner is
downstream-resolver. Load the todo-create skill for the canonical directory path and naming convention.
- 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-review/<run-id>/, do not create todo files, 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 (
ask_user in Copilot CLI) 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 from
references/review-process.md, preserving severity, autofix_class, owner, requires_verification, confidence, evidence[], and pre_existing per finding.
- Write a run artifact under
.context/compound-engineering/ce-review/<run-id>/ summarizing findings, applied fixes, and advisory outputs. Include the artifact path in the structured output.
- Do not create todo files. 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".
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
17 reviewer personas in layered conditionals, plus CE-specific agents. Load references/persona-catalog.md only when the full catalog is needed.
Runtime Agent Types
ce-review is this skill/orchestrator, not an Agent tool agent_type.
Never call the Agent tool with agent_type: ce-review. When this skill needs subagents, use runtime-valid reviewer agent types such as correctness-reviewer, testing-reviewer, maintainability-reviewer, project-standards-reviewer, security-reviewer, performance-reviewer, api-contract-reviewer, data-migrations-reviewer, reliability-reviewer, adversarial-reviewer, cli-readiness-reviewer, previous-comments-reviewer, dhh-rails-reviewer, kieran-rails-reviewer, kieran-python-reviewer, kieran-typescript-reviewer, julik-frontend-races-reviewer, schema-drift-detector, deployment-verification-agent, agent-native-reviewer, learnings-researcher, or repo-research-analyst.
If a desired persona does not exist as a runtime agent type, use code-review with the persona instructions in the task prompt rather than inventing an agent type. A broader valid reviewer is better than a failed dispatch.
Always-on (every review):
| Agent | Focus |
|---|
correctness-reviewer | Logic errors, edge cases, state bugs, error propagation |
testing-reviewer | Coverage gaps, weak assertions, brittle tests |
maintainability-reviewer | Coupling, complexity, naming, dead code, abstraction debt |
project-standards-reviewer | AGENTS.md compliance -- frontmatter, references, naming, portability |
agent-native-reviewer | Verify new features are agent-accessible |
learnings-researcher | Search docs/solutions/ for past issues related to this PR |
Cross-cutting conditional (selected per diff):
| Agent | Select when diff touches... |
|---|
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-migrations-reviewer | Migrations, schema changes, backfills |
reliability-reviewer | Error handling, retries, timeouts, background jobs |
adversarial-reviewer | Diff >=50 changed non-test/non-generated/non-lockfile lines, or auth, payments, data mutations, external APIs |
cli-readiness-reviewer | CLI command definitions, argument parsing, CLI framework usage, command handler implementations |
previous-comments-reviewer | Reviewing a PR that has existing review comments or threads |
Stack-specific conditional (selected per diff):
| Agent | Select when diff touches... |
|---|
dhh-rails-reviewer | Rails architecture, service objects, session/auth choices, or Hotwire-vs-SPA boundaries |
kieran-rails-reviewer | Rails application code where conventions, naming, and maintainability are in play |
kieran-python-reviewer | Python modules, endpoints, scripts, or services |
kieran-typescript-reviewer | TypeScript components, services, hooks, utilities, or shared types |
julik-frontend-races-reviewer | Stimulus/Turbo controllers, DOM events, timers, animations, or async UI flows |
CE conditional (migration-specific):
| Agent | Select when diff includes migration files |
|---|
schema-drift-detector | Cross-references schema changes against included migrations |
deployment-verification-agent | Produces deployment checklist with 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 kb-brainstormdocs/plans/*.md -- plan files created by kb-plan (living documents with progress checkboxes)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
Use the review process reference only when executing a review. It contains scope detection, intent discovery, plan discovery, reviewer selection, sub-agent dispatch, merge/dedup, synthesis, and headless output details.
Load references/review-process.md when:
- a review is actually being run;
- scope/base/PR handling is needed;
- reviewer selection or dispatch details are needed;
- findings need merging, deduplication, or headless formatting.
Default flow:
- Determine diff scope and base.
- Discover intent and relevant plan/requirements context.
- Select always-on and conditional reviewers.
- Spawn valid reviewer agents with
references/subagent-template.md and references/findings-schema.json.
- Merge, confidence-gate, deduplicate, and synthesize findings.
- Apply the quality gates below before reporting.
Quality Gates
Before delivering the review, verify:
- Every finding is actionable. Re-read each finding. If it says "consider", "might want to", or "could be improved" without a concrete fix, rewrite it with a specific action. Vague findings waste engineering time.
- No false positives from skimming. For each finding, verify the surrounding code was actually read. Check that the "bug" isn't handled elsewhere in the same function, that the "unused import" isn't used in a type annotation, that the "missing null check" isn't guarded by the caller.
- Severity is calibrated. A style nit is never P0. A SQL injection is never P3. Re-check every severity assignment.
- Line numbers are accurate. Verify each cited line number against the file content. A finding pointing to the wrong line is worse than no finding.
- Protected artifacts are respected. Discard any findings that recommend deleting or gitignoring files in
docs/brainstorms/, docs/plans/, or docs/solutions/.
- Findings don't duplicate linter output. Don't flag things the project's linter/formatter would catch (missing semicolons, wrong indentation). Focus on semantic issues.
Language-Aware Conditionals
This skill uses stack-specific reviewer agents when the diff clearly warrants them. Keep those agents opinionated. They are not generic language checkers; they add a distinct review lens on top of the always-on and cross-cutting personas.
Do not spawn them mechanically from file extensions alone. The trigger is meaningful changed behavior, architecture, or UI state in that stack.
After Review
Use the post-review flow reference only after findings have been synthesized.
Load references/post-review-flow.md when:
safe_auto findings need to be applied;- interactive, autofix, report-only, or headless post-review behavior matters;
- residual actionable work needs an artifact or todo handoff;
- the user wants to push fixes or create a PR after review.
Default rule: apply only safe_auto -> review-fixer automatically when the active mode allows mutation. Do not widen fixes by severity alone. Never commit, push, or create a PR from autofix/headless/report-only modes.
Fallback
If the platform doesn't support parallel sub-agents, run reviewers sequentially. Everything else (stages, output format, merge pipeline) stays the same.
Lazy References
Load these only when the named phase needs them:
references/persona-catalog.md - reviewer catalog and selection matrix.references/review-process.md - full review execution flow.references/post-review-flow.md - fixer, artifact, todo, push, and PR behavior after review.references/subagent-template.md - prompt contract for reviewer subagents.references/diff-scope.md - primary/secondary/pre-existing scope rules.references/findings-schema.json - structured finding contract.references/review-output-template.md - report formatting.
