Review

Code Review Command

Perform an adaptive code review that auto-routes between QUICK and DEEP based on the diff's size and risk profile, and biases toward depth — only clearly trivial changes get the QUICK pass.

• QUICK — Inline checklist pass done by this agent. Categories: correctness, security, performance, maintainability, testing, architecture, conventions. Output is grouped CRITICAL / WARNING / INFO with a PASS / PASS-WITH-WARNINGS / FAIL verdict. Fast, lightweight.
• DEEP — Full multi-agent pipeline: 4 core Explore agents + conditional specialists (testing, performance, migration) + optional Codex CLI cross-model pass + MCP fallback + Round-2 verification of Critical/High findings. Comprehensive, confidence-scored.

This skill is read-only

Never git add, git commit, git push, gh pr create, gh pr merge, or gh pr comment from this skill — in any mode, for any reason, including when a fix looks obviously correct or the user seems to want it landed. Reviewing and landing are separate jobs; the user runs /ccmagic:push when they want the work committed. The git and gh grants in the frontmatter are deliberately narrowed to read-only subcommands so this cannot happen by accident. If you find yourself reaching for a write command, produce the finding instead.

The one permitted write is --fix (Step 7a), and it edits the working tree only — the user reviews the diff and commits it themselves. Without --fix, do not use Edit on source files at all.

Parallel execution: Launch independent agents simultaneously. Claude Code determines when this is safe.

Step 0: Parse Arguments

Parse $ARGUMENTS to determine review mode and options:

Argument	Mode	Description
(empty)	branch	Diff current branch against main (default)
branch	branch	Explicit branch diff
full	full	Full codebase review — partition into modules
<number>	pr	Review PR by number (e.g., 42)
--quick	(override)	Force QUICK inline checklist pass.
--deep	(override)	Force DEEP multi-agent pipeline.
--fix	(modifier)	Apply mechanical fixes to the working tree after reporting (Step 7a). Off by default — without it the review makes no code changes at all. Never commits or pushes, with or without the flag.
--threshold N	(modifier)	Confidence threshold for DEEP, default 80. Findings below this are dropped.
--no-codex	(modifier)	Skip Codex CLI review in DEEP mode even if installed
--all-specialists	(modifier)	Force all conditional specialists in DEEP, bypass adaptive gating

If on main with no changes and no argument:

"No branch changes detected. Use /ccmagic:review full for a full codebase review, or switch to a feature branch."

Step 0.5: Auto-route between QUICK and DEEP

Apply these rules in order — first match wins:

1. User override — --quick → QUICK. --deep → DEEP. full mode → always DEEP.
2. Auto-route — Default to DEEP. Drop to QUICK only when ALL of the following are true:
   • ≤ 2 files changed
   • ≤ 50 lines changed (additions + deletions combined, per git diff --shortstat)
   • No changed file path matches the risk patterns below
   • The diff adds no new types (no new class , interface , type , struct , enum , trait )
   • The diff changes no error-handling control flow (no new/modified try, catch, except, rescue, finally, panic, recover, Result<, Either)

Risk path patterns (any match → DEEP, regardless of size):

auth, login, session, token, secret, credential, password, key, oauth,
crypto, hash, sign, jwt, permission, role, acl, authz,
migration, schema, sql, query, exec, eval,
payment, billing, charge, refund, stripe,
security, sanitiz, escap, csrf, xss, injection,
deploy, infra, terraform, helm, dockerfile, ci/, .github/workflows

Match case-insensitively against the full path.

Announce the routing decision

Print one line before any review work begins — this is non-negotiable, the user needs to know which review they got:

Routing → DEEP — reason: 4 files, 120 lines, touches auth/login.ts

or

Routing → QUICK — reason: 1 file, 12 lines, no risk patterns, no new types

If QUICK was selected, skip ahead to the QUICK execution section. If DEEP, continue with Step 1.

QUICK execution

If Step 0.5 selected QUICK, walk this checklist line by line against the diff. Don't skip categories. Load full file context when needed.

Correctness

• Logic errors, edge cases (empty inputs, boundaries, unexpected types), null/undefined handling, off-by-one in loops/slices.

Security

• Input validation, SQL injection (string concatenation in queries), XSS (unescaped output), CSRF on state-changing ops, authorization checks, sensitive data in logs/responses/source.

Performance

• N+1 queries, unnecessary loops, missing indexes for new query patterns, memory leaks (unclosed resources, growing collections), caching opportunities.

Maintainability

• Clear names, focused functions (~30 lines), reasonable cyclomatic complexity, DRY violations, dead code.

Testing

• Coverage for new/changed code, edge-case and error-path coverage, appropriate mock use.

Architecture

• SOLID, dependency direction, module boundaries, coupling.

Conventions

• Read CLAUDE.md, .claude/CLAUDE.md, and context/conventions.md if present. Check the diff against any explicit documented rules. No conventions loaded → skip this category.

Output

Group findings by severity:

• CRITICAL — must fix before merge (security, data loss, incorrect behavior).
• WARNING — should fix (perf issues, maintainability, missing tests for important paths).
• INFO — consider improving (style, minor naming, optional refactors).

For each finding include: severity, category, file/line(s), description, suggested fix.

Final verdict line:

• PASS — no critical or warning findings.
• PASS WITH WARNINGS — warnings only, merge at reviewer's discretion.
• FAIL — one or more critical findings; address before merge.

After producing the report, stop. Do not continue into DEEP mode steps.

If --fix was passed, apply mechanical fixes under the Step 7a rules (working tree only, clean-tree precondition, no commit) and then stop. Without --fix, QUICK ends at the report — change nothing.

---

DEEP execution

If Step 0.5 selected DEEP, continue with the steps below. This is the comprehensive pipeline.

Step 1: Load Project Conventions

Read these files if they exist (silent skip if missing):

1. CLAUDE.md in project root
2. .claude/CLAUDE.md
3. context/conventions.md

Collect their content into a {PROJECT_CONVENTIONS} string. This is injected into every agent prompt via ${CLAUDE_SKILL_DIR}/agent-instructions.md. If none found, agents use language-generic best practices only.

Step 2: Gather Changes and Prioritize Files

Branch mode

git diff --name-only main...HEAD
git diff --stat main...HEAD
git log --oneline main...HEAD

Full mode

1. Detect build configs: package.json, go.mod, Cargo.toml, pyproject.toml, pom.xml, build.gradle
2. Partition source files into modules by top-level directory structure
3. Exclude: node_modules/, vendor/, dist/, build/, .git/, generated files
4. Cap at 6 modules. If more, group smaller modules together.

PR mode

gh pr diff {N} --name-only
gh pr view {N} --json title,body,baseRefName
gh pr diff {N}

File Prioritization (branch and PR modes)

Sort changed files into tiers for agent attention:

Tier 1 — Always review individually:

• Files matching: auth, security, crypto, secret, token, password, permission, session, middleware
• API boundary files (routes, controllers, handlers, endpoints)
• New files (not just modifications)
• Data access layers (models, repositories, DAOs, migrations)

Tier 2 — Review if capacity:

• Error handling modules
• Configuration files
• Service/business logic files

Tier 3 — Summary pass only:

• Test files (check for correctness but deprioritize)
• Documentation files
• CSS/style files
• Small modifications (<10 lines) to well-established files

For diffs with >50 files: agents focus on Tier 1 and 2. Tier 3 files get a brief summary pass only.

Step 2.5: Scope Drift Detection

Cross-reference the diff against stated intent to detect scope creep and missing requirements.

Gather intent sources (in priority order)

1. Plan file: Check ~/.claude/plans/ for recent .md files referencing the current branch or repo. Read the most recent match.
2. PR description: gh pr view --json body -q .body 2>/dev/null
3. Commit messages: git log --oneline main...HEAD

If no intent source exists, skip this step silently.

Analyze

From the intent source, extract the stated goal — what this branch is supposed to accomplish.

Compare against git diff --name-only main...HEAD:

Scope creep — files changed that are unrelated to the stated intent:

• Changes to modules not mentioned in the plan
• New features or refactors not in scope
• "While I was in there..." changes

Missing requirements — intent items with no evidence in the diff:

• Plan items not addressed
• Requirements mentioned but not implemented

Output (informational, does not block)

Scope Check: [CLEAN | DRIFT DETECTED | REQUIREMENTS MISSING]
Intent: <1-line summary of what was requested>
Source: <plan file | PR description | commit messages>
Delivered: <1-line summary of what the diff actually does>
[If drift: list each out-of-scope change]
[If missing: list each unaddressed requirement]

This output appears before the main review findings. It gives context for interpreting the review — a finding in out-of-scope code is a stronger signal to revert than fix.

---

Step 3: Launch Parallel Review Agents (Round 1)

Load ${CLAUDE_SKILL_DIR}/agent-instructions.md for detailed agent prompts. Load ${CLAUDE_SKILL_DIR}/finding-schema.md for the output format agents must use.

Branch/PR mode — 4 parallel Explore agents:

1. Correctness Agent — Logic errors, null access, race conditions, broken error propagation. Must construct concrete triggering scenarios. Ignores style, formatting, performance.

2. Security Agent — Injection with exploit path, auth bypasses, data exposure, insecure defaults. Must describe attacker's path with preconditions. Ignores internal code without user input.

3. Convention Compliance Agent — Checks changes against {PROJECT_CONVENTIONS} from Step 1. Only flags violations of explicit documented rules. Reports zero findings if no conventions were loaded.

4. Architecture Agent — Breaking API changes, circular deps, coupling violations, missing error handling at boundaries, integration gaps. Identifies impact scope and affected dependents.

Each agent receives:

• The file list (prioritized per Step 2)
• The actual diff or file contents
• The {PROJECT_CONVENTIONS} string
• Instructions to use the finding schema exactly
• The Universal reporting rules (all agents) section from agent-instructions.md, prepended verbatim

Conditional Specialist Agents

After the 4 core agents are launched, detect scope signals from the diff and dispatch additional specialist agents in parallel. These are additive — they run alongside the core agents.

Scope detection:

# Detect what the diff touches
CHANGED=$(git diff --name-only main...HEAD)
HAS_TESTS=$(echo "$CHANGED" | grep -E '(test|spec|__tests__)' | head -1)
HAS_SOURCE=$(echo "$CHANGED" | grep -vE '(test|spec|__tests__|\.test\.|\.spec\.|_test\.)' | grep -E '\.(ts|js|py|rb|go|rs|java|kt|swift|php)$' | head -1)
HAS_MIGRATIONS=$(echo "$CHANGED" | grep -iE '(migrat|\.up\.sql|\.down\.sql|alembic|db/migrate)' | head -1)
HAS_BACKEND=$(echo "$CHANGED" | grep -iE '(model|service|controller|handler|endpoint|api|route|repo)' | head -1)
HAS_FRONTEND=$(echo "$CHANGED" | grep -iE '(component|page|hook|view|template|\.tsx|\.jsx|\.vue|\.svelte)' | head -1)
DIFF_LINES=$(git diff --stat main...HEAD | tail -1 | grep -oE '[0-9]+ insertion' | grep -oE '[0-9]+')

Dispatch rules:

Specialist	Condition	Agent prompt
Testing	HAS_SOURCE is non-empty (source code changed)	Testing Agent from agent-instructions.md
Performance	HAS_BACKEND or HAS_FRONTEND is non-empty	Performance Agent from agent-instructions.md
Data Migration	HAS_MIGRATIONS is non-empty	Data Migration Agent from agent-instructions.md

Launch all matching specialists in a single message alongside the core agents (up to 7 total agents in parallel).

Adaptive gating (skip specialists that consistently produce zero findings):

If this project has a review history file at context/review-stats.json, read it before dispatching. The file tracks per-specialist hit rates:

{"testing": {"dispatched": 8, "findings": 12}, "performance": {"dispatched": 8, "findings": 0}, "migration": {"dispatched": 3, "findings": 2}}

• If a specialist has 0 findings across 10+ dispatches, skip it. Print: [specialist] auto-gated (0 findings in N reviews)
• Exception: Security is never gated — it's an insurance policy
• Override: --all-specialists flag forces all specialists regardless of gating

After the review completes (Step 7), update context/review-stats.json with the dispatch and finding counts from this review.

---

Full mode — module-based agents:

1. Launch 1 Explore agent per module (capped at 6) covering all 4 concern areas within that module. Use the "Module Agent" prompt from agent-instructions.md.
2. Once the module agents have reported — or their deadline has passed, per the fan-in protocol in Step 3.9 — launch a Cross-Module Agent to check inter-module concerns. Do not block indefinitely on a module agent that never reports; proceed with the modules you have and mark the missing one unavailable — did not report.

Step 3.5: Codex CLI Review (optional, parallel)

Load ${CLAUDE_SKILL_DIR}/codex-pass.md and follow it. It covers availability detection, the run-scoped workspace, the bounded codex exec invocation, and how to classify the result.

In short: if Codex and a timeout binary are both available, launch the adversarial pass in the background alongside the Step 3 agents, bound it with timeout --kill-after=30 300, record its exit status into the output file, and classify by that status rather than by keyword. Codex is additive and never blocking — every failure mode continues the review with Explore agent findings only.

---

Step 3.9: Collect agent results (fan-in)

Load ${CLAUDE_SKILL_DIR}/fan-in-protocol.md and follow it at every fan-out in this skill — Step 3 (both modes), Step 3.5, and Step 5c.

In short: stamp a deadline when you dispatch a batch, not when you start to worry. When it expires, produce the report with whatever arrived and mark the rest unavailable — did not report. The deadline is absolute, never conditional on a majority having reported. While waiting, never poll with no-op commands, never SendMessage a completed agent, and never invent a missing agent's findings.

---

Step 4: Expert Analysis (MCP Fallback)

Priority order — use first available:

1. mcp__pal__codereview with model o3 — pass the diff and project conventions
2. mcp__pal__codereview with model gemini-2.5-pro
3. If no MCP tools available: skip this step

Merge MCP findings into the pool using the same finding schema. MCP findings enter the triage process like any other finding.

Step 5: Triage and Validate (Round 2)

Load ${CLAUDE_SKILL_DIR}/triage-instructions.md for the full triage process.

5a. Deduplicate

Same file + overlapping line range + same issue type → merge. Keep highest confidence, most specific detail.

5b. Apply confidence threshold

• Drop findings below threshold (default 80, or --threshold N)
• Exception: Critical findings with confidence 60+ survive

5c. Verify Critical and High findings

For each Critical/High finding that survived, launch a parallel verification Explore agent (capped at 4 concurrent):

• Reads actual code in context (not just the diff)
• Checks if issue is mitigated elsewhere (middleware, callers, error handlers)
• Attempts to construct concrete triggering scenario
• Returns verdict: CONFIRMED / MITIGATED / FALSE_POSITIVE

This is a fan-out like any other — apply the Step 3.9 collection protocol here too, with its own deadline stamped at dispatch. A verification agent that never reports must not hold the report hostage.

5d. Process verdicts

• CONFIRMED → keep with "[Verified]" tag
• MITIGATED → downgrade severity by one level, note the mitigation
• FALSE_POSITIVE → move to Dismissed Findings section
• No verdict (agent did not report before the deadline) → keep the finding at its original severity, tagged [Unverified — verification agent did not report]. Never silently drop a Critical/High finding because its verifier went missing, and never promote an unverified finding to [Verified].

5e. Handle MCP vs Explore disagreements

If MCP and Explore agents disagree on a finding → flag for user decision in Step 8.

Step 6: Generate Report

# Code Review Report

## Summary
- **Scope**: branch changes | full codebase | PR #X
- **Branch**: [current branch or PR base]
- **Files Analyzed**: N total (M in Tier 1/2)
- **Agents**: 4 core + N specialists (testing, performance, migration — list which ran) [+ Codex CLI]
- **Coverage**: list any dimension that did not report as `unavailable — did not report`, and Codex as `findings | completed — 0 findings | timed out | unparseable | unavailable | auth failed | failed (exit N)`. If everything ran, say `full`. Never leave a dimension unlisted — an omission reads as "came back clean".
- **Confidence Threshold**: [threshold used]
- **Convention Sources**: [files loaded, or "none found"]
- **Findings**: X Critical, Y High, Z Medium, W Low, V Convention

## Scope Check
[Output from Step 2.5 — CLEAN / DRIFT DETECTED / REQUIREMENTS MISSING]

## Critical Issues ({count}) — Verified
For each:
> **[severity] [confidence]%** `file:line`
> **Issue**: one-line summary
> **Detail**: explanation with triggering scenario
> **Verification**: CONFIRMED — [evidence]
> **Suggestion**: minimal fix

## High Priority Issues ({count}) — Verified
[same format]

## Medium Priority Issues ({count})
[same format, without verification section]

## Convention Violations ({count})
For each:
> **[confidence]%** `file:line`
> **Rule**: [quoted convention from CLAUDE.md/conventions.md]
> **Violation**: what the code does differently
> **Suggestion**: how to comply

## Low Priority / Suggestions ({count})
[grouped by file]

## Dismissed Findings ({count})
For each:
> ~~`file:line` — [original issue]~~
> **Dismissed**: [reason — FALSE_POSITIVE or below threshold]

## Fixes Applied ({count}) — only when `--fix` was passed
[Working-tree edits made under Step 7a. Omit this section entirely when `--fix` was not passed. Never list commit hashes here — this skill does not commit.]
> `src/handlers/user.ts:42` FINDING-003 — added the missing null check on `session.user`
> `src/orders/list.ts:88` FINDING-007 — batched the per-order lookup into one query
>
> Uncommitted. Review with `git diff`, then commit with `/ccmagic:push` or your own commit.

## Positive Findings
[Well-implemented patterns, good practices observed]

## Specialist Report
[Which specialists ran, which were skipped (scope), which were gated (0 findings in N+ reviews)]

## Overall Assessment
[Quality rating, key risks, actionable recommendation]

Step 7: Follow-up

Default (no --fix): steps 7a and 7b do not run. The report is the deliverable. Go straight to 7c and 7d, make no code changes, and do not offer to apply the fixes — the user asks for --fix when they want them applied.

7a. Apply mechanical fixes — --fix only

Skip this step entirely unless --fix was passed.

Preconditions, both required:

• --fix present in $ARGUMENTS.
• Working tree clean — run git status --porcelain before touching any file. If it returns anything, apply nothing and say so: mixing your edits into the user's uncommitted work makes the two indistinguishable.

For findings marked fixable: true by triage (see triage-instructions.md Step 7):

• Apply each fix directly in source code with Edit
• Prefer minimal changes — one fix per finding
• Output per fix: [FIXED] file:line — issue → what was changed

Stop at the working tree. Do not git add, do not git commit, do not git push, do not open or update a PR. Close with:

N fixes applied to the working tree, uncommitted. Review with `git diff`, then commit when you're satisfied.

7b. Batch-ask about judgment calls — --fix only

Skip this step entirely unless --fix was passed. Without --fix, judgment-call findings are reported in Step 6 and tracked in 7c; do not open an AskUserQuestion about fixing them.

For findings marked fixable: false, present via a single AskUserQuestion:

I fixed N issues in the working tree. M need your input:

1. [HIGH] file:line — issue description
   Suggested fix: what to change
   → A) Fix  B) Skip

2. [MEDIUM] file:line — issue description
   Suggested fix: what to change
   → A) Fix  B) Skip

RECOMMENDATION: Fix #1 because [reason]. Skip #2 because [reason].

Apply fixes for items where the user chose "Fix." These also stay uncommitted.

If 0 fixable findings exist, skip 7a. If 0 judgment-call findings exist, skip 7b.

7c. Track remaining findings

Create TodoWrite entries for all remaining (unfixed) findings, grouped by severity then by file:

• Critical/High: one todo per finding
• Medium: group related findings per file into one todo
• Low/Convention: one summary todo per category

7d. Update review stats

If context/review-stats.json exists (or conditional specialists were dispatched), update per-specialist dispatch and finding counts for adaptive gating in future reviews.

Step 8: Handle Disputed Findings

For findings where:

• Verification was uncertain (no clear CONFIRMED/FALSE_POSITIVE)
• MCP and Explore agents disagree
• Convention interpretation is ambiguous

Present both sides and ask the user via AskUserQuestion:

Disputed: [issue summary at file:line] Agent A says: [assessment] Agent B says: [assessment] Options: Fix | Defer | Dismiss

Record the user's decision in the report.

Execution

When invoked, immediately begin the review process without asking for confirmation. Be thorough but concise. Every finding must meet the confidence threshold and follow the finding schema. Quality over quantity — 5 verified, actionable findings beat 30 unvalidated observations.

End state: a report. Without --fix, the repository is byte-for-byte unchanged when you finish. With --fix, the working tree carries the fixes and nothing is staged, committed, pushed, or turned into a PR. Committing and pushing belong to /ccmagic:push; merging belongs to /ccmagic:merge. A review that lands its own changes has destroyed the thing that makes it a review — an independent read of work someone else decided to keep.