---
name: quality-code-review
description: "Use when code has been written — orchestrates the review chain (safety → craft → reachability → gotcha-hunter) with risk-based escalation. Triggers on 'review my code', 'check this PR', 'is this ready', 'audit this diff'. Optional Codex adversarial cross-check fires when consent is on."
---

# Code Review

## Overview

Code review is not one pass — it is a chain of focused reviewers, each with its own context window. Safety, craft, reachability, and prior-art recall are different bug classes; conflating them dilutes every reviewer. This skill dispatches the chain in sequence, gathers findings, and produces a unified report.

**Core principle:** find what breaks first, then improve what works, then prove what runs, then check what we already learned.

**Announce at start:** "I'm using the quality-code-review skill to review this code."

## When to Use

- After completing a TDD cycle (per-task review).
- Before creating a PR (final review).
- When requested for ad-hoc code review.
- During `/feature`, `/bugfix`, `/refactor` commands at the review phase.

## The Review Chain

Each link in the chain is a separate subagent dispatch. Subagents do not share context — main session aggregates their reports.

```
[risk-classify] → [code-reviewer]   → [craft-reviewer]   → [support-runtime-reachability] → [gotcha-hunter]   → [Codex adversarial] → [aggregate + decide]
                  Pass 1: safety      Pass 2: craft        per-slice wiring gate              prior-art recall    optional cross-check
```

### Step 0: Codex consent

Run the Codex consent flow from `protocols/codex.md` before dispatching. The choice gates Pass 2.5 (adversarial cross-check), not the rest of the chain.

- **Takeover:** Skip Pass 1 + Pass 2; dispatch Codex `adversarial-review` for the entire diff. Continue to reachability after.
- **Verify:** Run the chain normally; Codex adversarial-reviews Claude's findings at Pass 2.5.
- **Skip / unavailable:** Run the chain normally; no adversarial step.

### Step 1: Risk classification

Classify before dispatching anything — risk level decides which links of the chain run.

| Risk | Trigger | Chain |
|---|---|---|
| **Low** | Style, docs, config, non-functional | code-reviewer (light) + gotcha-hunter |
| **Medium** | Business logic, API endpoints, data processing, UI with state | full chain |
| **High** | Auth/authz, payments, PII, encryption, user-input rendering, query construction from user input, SSRF surfaces, new external integrations | full chain + escalate to `quality-security-audit` |
| **Critical** | Infrastructure, deploy config, DB migrations, CI/CD | full chain + security audit + human approval before merge |

`gotcha-hunter` runs at every risk level — recall is cheap (the agent only reads markdown), and a "config" change that matches a known recurring failure pattern is exactly what the catalog exists to surface. Skip only `craft-reviewer` and `support-runtime-reachability` at low risk.

See **Risk Classification Guide** below for the full mapping.

### Step 2: Pass 1 — code-reviewer (safety)

Dispatch `code-reviewer` subagent. It owns: SQL safety, race conditions, auth boundaries, secret exposure, injection vulnerabilities, data-loss risks, unhandled async errors. Returns `[CRITICAL]` findings and a SAFETY REVIEW SUMMARY.

If `code-reviewer` returns `BLOCK`, halt the chain. Surface findings; require fixes before re-running.

### Step 3: Pass 2 — craft-reviewer

Dispatch `craft-reviewer` subagent. It owns: library idiom adherence (Context7-verified), codebase pattern conformance, stub detection. Returns severity-tagged findings and a CRAFT REVIEW SUMMARY.

`craft-reviewer` does not block the chain on its own — its findings inform the final decision.

### Step 4: Per-slice runtime reachability gate

Invoke the `support-runtime-reachability` skill. It walks the slice diff, finds exports, and verifies each has a production caller (or escape-hatch annotation). Writes the per-slice `runtime-reach` gate result to the manifest.

If the gate fails, halt the chain. Orphan exports must be wired, annotated, or removed before review continues.

### Step 5: Gotcha hunter — prior-art recall

Dispatch the `gotcha-hunter` subagent with the diff scope. It scans `aiwiki/gotchas/` (project) and `~/.claude/gotchas/` (global), matches entries against the diff, and returns a ranked relevance list. Each surfaced gotcha cites the existing prevention.

`gotcha-hunter` does not block the chain. Use its findings to:
- check whether earlier passes missed a known recurring failure mode,
- include the cited gotcha files in the final review report so the developer reads the prior lesson, not just the verdict.

If the agent reports `promotion-pending` entries, treat that as a separate warning — those are draft rules awaiting session-start review (see `support-gotcha` Step 6); the chain continues but the user should resolve them before the next slice closes.

### Step 6: Pass 2.5 — Codex adversarial cross-check (Verify mode only)

If the user selected **Verify** at Step 0, dispatch Codex `adversarial-review` to cross-check what Claude's chain found. Codex is looking for: gaps Claude missed, dead code, cross-module inconsistencies, security concerns the chain didn't surface.

```bash
node "${CLAUDE_PLUGIN_ROOT}/scripts/codex-companion.mjs" adversarial-review --scope branch \
  "Review the diff for gaps, dead code, cross-module inconsistencies, and security concerns the prior chain missed. Focus on what is WRONG or MISSING, not style."
```

Surface Codex's findings alongside Claude's; flag disagreements for the user to resolve.

### Step 7: Plan alignment

Verify the implementation aligns with the plan:

1. Read the task from `.forge/work/{type}/{name}/tasks.md`.
2. Compare implementation against the task description.
3. Check API contracts at `.forge/work/{type}/{name}/architecture/api-contract.md`.
4. Check DB schema at `.forge/work/{type}/{name}/architecture/db-schema.md`.
5. Verify decision log consistency at `aiwiki/decisions/`.

Flag deviations. Intentional deviations go in the decision log.

### Step 8: Aggregate and decide

Combine findings from all subagents into a unified report (see **Review Report Format** below). The main session decides — never blindly accept a subagent verdict.

## Subagent Dispatch

| Subagent | Owns | When |
|---|---|---|
| `code-reviewer` | Pass 1 safety | Always (unless Codex Takeover) |
| `craft-reviewer` | Pass 2 craft + idioms + stubs | Medium / High / Critical risk |
| `support-runtime-reachability` (skill) | Slice wiring gate | Medium / High / Critical risk; per-slice gate before final approval |
| `gotcha-hunter` | Prior-art recall (project + global gotcha catalogs) | Always — runs after the chain's other passes (after the wiring gate when present, otherwise immediately after `code-reviewer`); surfaces relevant gotchas + any promotion-pending warnings |
| Codex `adversarial-review` | Cross-check (gaps/dead-code/inconsistencies) | If user selected Verify or Takeover |

Each subagent receives: changed files, test results, architecture artifacts, decision log. Each runs in its own context window so the main session stays lean.

## Risk Classification Guide

| Change Area | Default Risk | Escalate If |
|---|---|---|
| README, comments | Low | Never |
| CSS, styling | Low | Involves user input (XSS) |
| Config files | Low | Secrets, deploy targets |
| Business logic | Medium | Handles money or PII |
| API endpoints | Medium | Auth-required endpoints |
| Database queries | Medium | Schema changes, migrations |
| Auth/authz | High | Always |
| Payment/billing | High | Always |
| User data handling | High | PII, GDPR-relevant |
| Encryption/hashing | High | Always |
| User-input rendering | High | Output to HTML/templates without encoding |
| Query construction | High | User input in SQL/NoSQL/LDAP/command strings |
| Server-side requests | High | User-controlled URL or hostname |
| External integrations | High | New third-party service, auth token, or webhook |
| Infrastructure/deploy | Critical | Always |
| CI/CD pipeline | Critical | Always |
| Database migrations | Critical | Production database |

When the diff touches a High row, escalate to `quality-security-audit` after the chain. When the diff touches a Critical row, require human approval before merge.

## Auto-Fix vs Ask

| Issue Type | Action |
|---|---|
| Formatting, whitespace | Auto-fix silently |
| Obvious naming improvements | Auto-fix, mention in report |
| Missing error handling | Auto-fix if pattern is clear from the codebase |
| Architecture decisions | Ask user (batch all questions into one) |
| Tradeoffs (performance vs readability) | Ask user with recommendation |
| Convention conflicts | Check decision log first; ask if no precedent |

Batch all "ask user" items into a single question. Do not ask one at a time.

## Review Report Format

```markdown
## Code Review Report

**Scope:** [files reviewed]
**Risk:** [Low / Medium / High / Critical]
**Reviewers:** code-reviewer, craft-reviewer, support-runtime-reachability, gotcha-hunter[, Codex]

### Pass 1 — Safety (code-reviewer)
- SQL safety: PASS
- Race conditions: PASS
- Auth boundaries: PASS
- Secret exposure: PASS
- Injection vulnerabilities: PASS
- Data-loss risks: 1 finding (auto-fixed)

### Pass 2 — Craft (craft-reviewer)
- Idiom adherence: PASS (axios v1.7 verified)
- Pattern conformance: 1 finding (existing pattern at src/handlers/users.ts:22)
- Stub detection: PASS

### Slice gate — runtime-reach (support-runtime-reachability)
- 4 exports checked, all wired ✓

### Prior-art recall (gotcha-hunter)
- Project gotchas relevant: 0
- Global gotchas relevant: 1 — Stub logger silently drops events (~/.claude/gotchas/{YYYY-MM-DD}-{slug}.md)
- Promotion-pending: 0

### Pass 2.5 — Codex adversarial (if Verify)
- 1 finding: dead import in src/lib/utils.ts

### Plan Alignment
- Matches task description ✓
- API contract compliant ✓
- Decision log consistent ✓

### Findings
| # | Reviewer | Severity | Description | Status |
|---|---|---|---|---|
| 1 | code-reviewer | Critical | Missing transaction in createOrder | Auto-fixed |
| 2 | craft-reviewer | Important | Throws bare Error for validation | Awaiting user |
| 3 | gotcha-hunter | Advisory | Logger stub pattern matches a global gotcha — confirm via console-tee | Cited |
| 4 | Codex | Important | Dead import `formatDate` in utils.ts | Auto-fixed |

### Decision
**APPROVED** / **APPROVED WITH CONCERNS** / **CHANGES REQUESTED** / **BLOCKED**
```

## Decision Outcomes

| Decision | Meaning | Action |
|---|---|---|
| APPROVED | No issues, or all issues auto-fixed | Proceed |
| APPROVED WITH CONCERNS | Non-blocking issues documented | Proceed; log concerns |
| CHANGES REQUESTED | Issues must be fixed | Return to `build-tdd` |
| BLOCKED | Critical safety, runtime-reach failure, or architecture problem | Escalate to user |

## Common Mistakes

| Mistake | Fix |
|---|---|
| Reviewing without running tests first | Tests must pass before review starts |
| Conflating safety, craft, and reachability into one pass | Dispatch the four subagents separately; each owns its concern |
| Skipping `craft-reviewer` because Pass 1 was clean | Pass 2 catches different bugs; safety alone isn't enough |
| Treating runtime-reach as optional | The wiring gate is per-slice; orphan exports block the slice |
| Skipping `gotcha-hunter` because nothing felt familiar | The catalog exists because we already forgot — the agent is recall, not intuition |
| Asking questions one at a time | Batch all "ask user" items into a single message |
| Auto-fixing architecture decisions | Architecture changes require user approval |

## Red Flags

**Never:**
- Skip the safety pass.
- Auto-fix security or architecture decisions.
- Proceed with critical safety findings unresolved.
- Pass the runtime-reach gate by adding a no-op caller.

**Always:**
- Run safety first; halt the chain on a `BLOCK`.
- Classify risk before dispatching.
- Aggregate findings from all reviewers — don't hide one to make the report look clean.
- Verify plan alignment after the chain runs.

## I/O Contract

| Field | Value |
|---|---|
| **Requires** | Implementation code + passing tests. Architecture artifacts (`.forge/work/{type}/{name}/architecture/`) and decision log (`aiwiki/decisions/`) are read when present (they enrich cross-reference checks) but are not mandatory — trivial-scope work (e.g. `/bugfix --size trivial`) may run without them. |
| **Produces** | Unified review report (in conversation or `.forge/work/{type}/{name}/review-report.md`), auto-fixes applied to code |
| **Feeds into** | `quality-test-execution` (final review), next TDD task (per-task review), `build-pr-workflow` (PR gate) |
| **Updates manifest** | `slice_graph.slices.<id>.gates.{code-review, runtime-reach}` per-slice; `phases.quality.code-review-final.gate-passed: true` at manifest level |

## Graphify Context

**Protocol:** `protocols/graphify.md` | **Guard:** Run the status check from the protocol before Step 2.

Graph context reveals cross-module blast radius — surface to the subagents in their dispatch prompts:

- **Communities** identify subsystems the diff touches; cross-community changes are higher risk.
- **God nodes** get extra scrutiny.
- **EXTRACTED edges** trace dependency paths.

CLI queries (if graph exists):
- `graphify query "what depends on [changed file]" --budget 1000 --graph graphify-out/graph.json`
- `graphify explain "[changed module]" --graph graphify-out/graph.json`

## Integration

**Called by:**
- `/feature` (per-task review + final review)
- `/bugfix` (review before PR)
- `/refactor` (review before PR)
- On-demand

**Dispatches:**
- `code-reviewer` (Pass 1 safety)
- `craft-reviewer` (Pass 2 craft)
- `support-runtime-reachability` skill (per-slice gate)
- `gotcha-hunter` (prior-art recall)
- Codex `adversarial-review` (optional cross-check)

**Pairs with:**
- `build-tdd` — feeds the diff to review
- `quality-security-audit` — escalation for High/Critical risk
- `quality-test-execution` — runs after final review approves
- `build-pr-workflow` — approved review gates PR creation