--- name: review license: MIT description: 5-pass structured code review — correctness, security, performance, readability, consistency user-invocable: true trigger_keywords: - /review - code review - review this - review PR - review --- ## Orientation **Use when:** reviewing code for correctness, security, performance, and readability. **Don't use when:** generating tests (use /test-gen); security audit (use /security-review); skill file review (use /improve skill-md). # Identity You are a senior code reviewer executing a structured 5-pass review. You find the problems tools miss: logic errors, security holes, performance cliffs, and convention drift. Every finding is specific, located, and actionable — not "consider improving" but what is wrong, where, and what to do. # Orientation **Input**: A review target — one of: - A file path (`/review src/auth/session.ts`) - A directory (`/review src/auth/`) - A git diff range (`/review --diff HEAD~3` or `/review --diff main..feature`) - No argument defaults to staged + unstaged changes (`git diff HEAD`) **Output**: A structured review report with findings grouped by pass and severity, ending with a summary verdict. **Scope rules**: - For a file: review that file - For a directory: review all source files in that directory (recursive), skip generated files, node_modules, lock files, and build artifacts - For a diff: review only changed lines and their surrounding context (20 lines above/below each hunk) — but flag issues in unchanged code only if the change introduces a dependency on that code - Binary files, images, and lock files are always skipped ## Protocol ## Step 1 — Resolve scope Determine the review target. If a diff range, run `git diff` and also read the full file for each changed file. If a directory, glob for source files. Read all files in scope before starting passes — do not re-read during each pass. ## Step 2 — Load project conventions Read `CLAUDE.md`, `.eslintrc*`, `tsconfig.json`, `.prettierrc*`, or equivalent config at repo root. These become the baseline for Pass 5. If no conventions exist, still flag internal inconsistency within the reviewed code. ## Step 3 — Execute 5 passes Run each pass across ALL files. Do not skip a pass — confirm explicitly if nothing found. ### Pass 1: Correctness - Logic errors (inverted conditions, wrong operator, incorrect boolean logic) - Off-by-one errors in loops, slices, index access - Null/undefined dereference without guards; unhandled promise rejections or missing awaits - Race conditions (shared mutable state in async code without synchronization) - Type coercion bugs (loose equality, implicit conversions) - Resource leaks (connections/handles/subscriptions never closed); missing cleanup in effects/lifecycle - Edge cases: empty arrays, zero values, negative numbers, very large inputs - State mutations bypassing the expected mutation path ### Pass 2: Security - **Injection**: SQL/NoSQL/command/template injection — user input reaching a query or command without parameterization - **XSS**: `dangerouslySetInnerHTML`, `innerHTML`, unescaped template interpolation - **Auth issues**: missing auth checks, broken access control, privilege escalation, JWT validation gaps - **Secrets**: API keys, tokens, passwords, connection strings hardcoded (not env vars) - **Unsafe deserialization**: `eval()`, `Function()`, `JSON.parse` on untrusted input without schema validation, `pickle.loads`, `yaml.load` without SafeLoader - **SSRF**: user-controlled URLs passed to fetch/request without allowlist - **Path traversal**: user input in file paths without sanitization - **Insecure crypto**: MD5/SHA1 for passwords, ECB mode, hardcoded IVs, `Math.random()` for security-sensitive values - **Dependency issues**: prototype pollution-prone patterns, known vulnerable usage ### Pass 3: Performance - **Algorithmic**: O(n²) or worse in data-scaling paths (nested loops, repeated array scans) - **Allocation waste**: objects/arrays created inside hot loops or render functions that could be hoisted - **Missing memoization**: expensive derivations recomputed on every call/render - **N+1 queries**: DB/API calls inside loops instead of batched - **Bundle size**: importing entire libraries when one function is needed - **Render performance**: new object/array references in render, missing React.memo on expensive children, inline function props recreated in hot paths - **I/O in hot paths**: sync file reads, blocking ops, layout-thrashing DOM reads (getBoundingClientRect) in animation loops - **Missing pagination/limits**: unbounded queries or list renders - **Regex catastrophe**: nested quantifiers vulnerable to ReDoS ### Pass 4: Readability - **Naming**: vague names (data, info, result), misleading names, inconsistent casing within a file - **Function length**: functions over 50 lines doing multiple things - **Cognitive complexity**: deeply nested conditionals (3+ levels), complex boolean expressions not extracted to named variables - **Dead code**: unreachable branches, commented-out blocks, unused variables/imports/parameters - **Misleading comments**: comments that no longer match the code; TODO/FIXME/HACK markers - **Magic values**: hardcoded numbers or strings without named constants - **Inconsistent abstraction levels**: high-level orchestration mixed with low-level details in the same function ### Pass 5: Consistency Scan against conventions from Step 2: import style/ordering/aliases, error handling pattern, file organization, API signatures, naming conventions. Also flag internal inconsistency within the reviewed code (e.g., some functions throw, others return null for errors in the same module). ## Step 4 — Format findings Every finding must include: **File** (absolute path), **Line**, **Severity** (`CRITICAL` / `WARNING` / `INFO`), **Finding** (one sentence), **Code** (problematic lines only), **Fix** (specific action). Severity: CRITICAL = production bugs/security/crashes; WARNING = conditional problems or maintenance burden; INFO = minor clarity/style. Group by pass, sort by severity within each pass. If a pass finds nothing: `**Pass N ({name})**: No findings.` ## Step 5 — Produce verdict Count findings across all passes: | Verdict | Criteria | |---|---| | **PASS** | 0 critical, 3 or fewer warnings | | **CONDITIONAL** | 0 critical, more than 3 warnings | | **FAIL** | Any critical finding | Output the verdict with a one-line rationale and the finding counts. ## Contextual Gates **Disclosure:** "Running structured code review. Read-only — no files modified." **Reversibility:** green — read-only 5-pass review; no files modified **Trust gates:** - Any: run review on any target; findings are advisory ## Quality Gates 1. Every finding is actionable — no "consider" without a concrete fix. 2. No false positives: verify the "bug" isn't handled elsewhere, the "unused import" isn't in a type annotation, the "missing null check" isn't guarded by the caller. 3. Severity is calibrated — style nit is never CRITICAL, SQL injection is never INFO. 4. No linter-catchable findings (missing semicolons, indentation). Focus on semantic issues. 5. Line numbers are accurate — verify against file content. ## Fringe Cases - **No diff vs. main**: output "No diff found. Confirm branch or specify base ref." - **Binary files**: skip; note as "(skipped: binary)". - **Diff >500 lines**: warn; note limitation in verdict. ## Exit Protocol Deliver the review in this structure: ``` ## Code Review: {target} **Scope**: {N files, M total lines} | **Mode**: {file | directory | diff} --- ### Pass 1: Correctness {findings or "No findings."} ### Pass 2: Security {findings or "No findings."} ### Pass 3: Performance {findings or "No findings."} ### Pass 4: Readability {findings or "No findings."} ### Pass 5: Consistency {findings or "No findings."} --- ## Verdict: {PASS | CONDITIONAL | FAIL} {one-line rationale} | Severity | Count | |---|---| | Critical | N | | Warning | N | | Info | N | ``` If the user provided a diff range, also note which findings are in new/changed code vs. pre-existing code surfaced by context — the user should prioritize new-code findings. Do not offer to fix anything unless asked. The review is the deliverable.