/review

name: review
description: "Reviews code for quality, security, correctness. Triggers: code review, quality review, security review, review PR, review branch."
user-invocable: true
effort: high
argument-hint: "[target: branch, pr, file path, or staged changes]"
agent: code-reviewer
context: fork
allowed-tools: Read, Grep, Glob, Bash

Code Review

$ARGUMENTS

Reviews code changes for quality and issues.

Changed files context

• Changes: !`git diff --stat main...HEAD 2>/dev/null || git diff --cached --stat 2>/dev/null || echo "no changes detected"`

Signal Collection (never stop at the first red)

Collect every failing signal up front, then review the diff in full **anyway**:

| Signal | How to read it | |--------|----------------| | Merge conflict with base | `gh pr view --json mergeable,mergeStateStatus` or `git merge-tree` | | Failing CI checks | `gh pr checks` or the platform equivalent | | Lint / typecheck failure | the project's own commands |

Each failing signal becomes a `blocker` finding. **None of them ends the run.**

A review that aborts on the first red signal spends the whole cycle repeating what the tracker already displayed, while the finding that would have told the author something new never gets written. One invocation produces the most complete picture of the change that it can.

Automated Diff Analysis

Before starting manual review, run the diff analyzer script to get a structured risk assessment:

python3 ${CLAUDE_SKILL_DIR}/scripts/diff-analyzer.py [base_branch]
# Default base branch: main
# Example: python3 ${CLAUDE_SKILL_DIR}/scripts/diff-analyzer.py develop

The script outputs JSON with:

• **files**: each changed file with additions, deletions, category (security/test/config/migration/infra/docs/logic), and risk level
• **risk_score**: overall assessment (high/medium/low)
• **hotspots**: top 5 files by additions
• **secrets_scan**: potential secret leaks detected in added lines
• **test_coverage_estimate**: whether test files accompany logic changes (good/partial/none)
• **parallel_review_recommended**: boolean flag

If the script reports `parallel_review_recommended: true`, use the Parallel Review (Agent Teams) mode below.

---

Parallel Review (Agent Teams)

For significant PRs or large changesets, create a parallel review team:

Create an agent team to review [target]:
- Teammate 1 (security-auditor): "Review for security vulnerabilities, auth issues,
  injection risks, secret leaks. Report with severity ratings." Use Opus.
- Teammate 2 (performance-optimizer): "Check for N+1 queries, memory leaks,
  unnecessary allocations, caching opportunities. Report with impact ratings." Use Opus.
- Teammate 3 (test-engineer): "Validate test coverage, edge cases, mock quality,
  missing assertions. Report coverage gaps." Use Opus.
Each reviewer should report findings independently. Do NOT modify files.

After all reviewers complete: 1. Synthesize findings into unified Code Review Report 2. Prioritize by severity (blocker > major > minor > nit) 3. Issue verdict per the verdict rule below — not by impression

> **When to use**: PRs with >5 files changed, cross-module changes, security-sensitive code. > **READ-ONLY**: No teammate should modify files during review.

---

Sequential Review (Default)

1. **Reads** changed files 2. **Analyzes** for issues 3. **Checks** best practices 4. **Reports** findings

Review Scope

| Target | What's Reviewed | |--------|-----------------| | (none) | Staged changes | | `branch` | Branch vs main | | `pr` | Pull request changes | | `file.ts` | Specific file |

Review Checklist

Code Quality

• [ ] Clear naming
• [ ] Proper error handling
• [ ] No code duplication
• [ ] Appropriate abstractions

Security (OWASP Top 10)

• [ ] A01: Proper auth/authorization on all endpoints
• [ ] A02: No weak crypto, HTTPS for external comms
• [ ] A03: Input validation, parameterized queries, output encoding (XSS)
• [ ] A04: Threat model assumptions documented for new features
• [ ] A05: No debug mode, default credentials, or verbose errors in prod config
• [ ] A06: Dependencies checked for known CVEs
• [ ] A07: No hardcoded secrets, session management correct
• [ ] A08: Integrity checks on deserialized data, CI/CD pipeline safety
• [ ] A09: Security-relevant events logged (without PII)
• [ ] A10: External URL handling validates scheme/host (SSRF prevention)
• [ ] Cross-scope replay: can an identifier from one tenant/user/org be replayed in another?
• [ ] Fails closed wherever the path affects security, money, or data retention
• [ ] Secrets at rest: a new column, setting or queued payload holding a token, password or key is encrypted (read back) or keyed-hashed (only compared), and a test reads the raw stored value; see `reference/secrets-at-rest.md` in `security-patterns`
• [ ] Commercial messages: a new message type is classified commercial or transactional in code; a commercial one re-checks current per-channel consent and carries an opt-out; see `reference/commercial-messages.md` in `security-patterns`

API / Contract Changes

• [ ] Backward compatibility preserved (no silent breaking changes)
• [ ] API versioning updated if contract changed
• [ ] Schema validation on request/response
• [ ] Client validation uses the authoritative input contract, including operation groups/defaults, finite bounds, nested paths and documented Unicode units; read `reference/input-validation.md` from the `security-patterns` skill located through the current client's installed catalog
• [ ] Shared backend/client fixtures and generation drift checks cover changed rules; unsupported/server-only checks are explicit, and local refusals do not masquerade as HTTP responses
• [ ] Error responses follow project convention
• [ ] Statuses and messages distinguish input/state refusals from infrastructure failures; original causes remain available in authorized diagnostics
• [ ] Error filtering preserves machine codes, field pa