---
name: code-review
description: Perform an automated code review on recent changes. Checks for security issues, performance problems, error handling gaps, and code quality. Outputs a structured report with severity levels.
---

# Code Review

Perform a thorough automated code review on recent code changes and produce a structured report.

## Steps

### 1. Identify the Changes to Review

Determine what to review based on context:

- If there are staged changes: `git diff --cached`
- If the user mentions a specific commit range: `git diff <range>`
- Otherwise default to the last commit: `git diff HEAD~1`

Also run `git diff --stat` (with the same range) to get an overview of files changed.

If no changes are found, inform the user and stop.

### 2. Read the Full Diff

Read the complete diff output. For each changed file, also read the full file content using the Read tool so you can understand the surrounding context, not just the changed lines.

### 3. Analyze Against Review Checklist

Evaluate every change against each category below. Only report genuine issues -- do not fabricate problems or flag idiomatic code.

#### Security (CRITICAL priority)
- **Injection vulnerabilities**: SQL injection, command injection, template injection. Look for string concatenation in queries or shell commands.
- **XSS risks**: Unescaped user input rendered in HTML, `dangerouslySetInnerHTML`, `innerHTML`.
- **Hardcoded secrets**: API keys, passwords, tokens, private keys in source code. Check for patterns like `key=`, `secret=`, `password=`, `token=`, base64 blobs.
- **Path traversal**: User input used in file paths without sanitization.
- **Insecure dependencies**: Known vulnerable patterns (e.g., `eval()`, `Function()`, `child_process.exec` with user input).
- **Authentication/authorization gaps**: Missing auth checks on new endpoints, privilege escalation.
- **CORS misconfiguration**: Overly permissive `Access-Control-Allow-Origin`.

#### Performance (WARNING priority)
- **N+1 queries**: Database calls inside loops.
- **Missing pagination**: Unbounded list queries.
- **Memory leaks**: Event listeners not cleaned up, growing arrays/maps without bounds, unclosed streams.
- **Unnecessary re-renders** (React): Missing `useMemo`/`useCallback` where it matters, new object/array literals in render.
- **Blocking operations**: Synchronous I/O in async contexts, `fs.readFileSync` in server handlers.
- **Missing indexes**: New database queries on unindexed columns (if schema is visible).

#### Error Handling (WARNING priority)
- **Swallowed errors**: Empty catch blocks, catch blocks that only log.
- **Missing error handling**: Unhandled promise rejections, no try/catch around I/O operations.
- **Generic error responses**: Leaking stack traces or internal details to clients.
- **Missing input validation**: Endpoints accepting unvalidated user input.
- **Unchecked null/undefined**: Accessing properties on potentially null values without guards.

#### Code Quality (INFO priority)
- **Naming**: Variables/functions that are unclear, misleading, or inconsistent with project conventions.
- **Duplication**: Copy-pasted logic that should be extracted into a shared function.
- **Complexity**: Functions over 50 lines, deeply nested conditionals (>3 levels), functions with >4 parameters.
- **Dead code**: Unreachable code, unused imports, commented-out blocks.
- **Type safety**: Missing types in TypeScript, `any` overuse, incorrect type assertions.
- **API design**: Inconsistent response formats, missing HTTP status codes, unclear endpoint naming.

#### Testing (INFO priority)
- **Missing test coverage**: New public functions/endpoints without corresponding tests.
- **Brittle tests**: Tests relying on implementation details, hardcoded timestamps, or flaky async patterns.
- **Missing edge cases**: Only happy-path tested, no error/boundary testing.

### 4. Generate the Review Report

Output the report in this exact format:

```
## Code Review Report

**Files reviewed**: <count> files, <additions>+ / <deletions>-
**Scope**: <brief description of what changed>

---

### CRITICAL

> Issues that must be fixed before merging.

- **[SECURITY]** `<file>:<line>` - <description>
  **Fix**: <specific remediation>

---

### WARNING

> Issues that should be addressed but are not blocking.

- **[PERFORMANCE]** `<file>:<line>` - <description>
  **Fix**: <specific remediation>

- **[ERROR_HANDLING]** `<file>:<line>` - <description>
  **Fix**: <specific remediation>

---

### INFO

> Suggestions for improvement.

- **[QUALITY]** `<file>:<line>` - <description>
  **Suggestion**: <improvement>

---

### Summary

| Category       | Critical | Warning | Info |
|---------------|----------|---------|------|
| Security       |    0     |    0    |   0  |
| Performance    |    0     |    0    |   0  |
| Error Handling |    0     |    0    |   0  |
| Code Quality   |    0     |    0    |   0  |
| Testing        |    0     |    0    |   0  |

**Verdict**: APPROVE / REQUEST_CHANGES / NEEDS_DISCUSSION
```

Rules for the verdict:
- **APPROVE**: Zero critical issues, at most 2 warnings.
- **REQUEST_CHANGES**: Any critical issue, or 3+ warnings.
- **NEEDS_DISCUSSION**: Architectural concerns or ambiguous intent that requires team input.

### 5. Offer Follow-Up

After the report, ask the user:

> Would you like me to fix any of these issues automatically?

If yes, apply the fixes using the Edit tool, re-run the review on the fixed code, and confirm resolution.

## Edge Cases

- **Very large diffs (>1000 lines)**: Focus on the most impactful files first. Summarize trivial changes (renames, formatting) without deep analysis.
- **Generated code**: If files appear auto-generated (e.g., `*.generated.ts`, lockfiles, migration files), note them but skip detailed review.
- **Binary files**: Skip binary files, note their presence.
- **No issues found**: Still output the report structure with "No issues found" and an APPROVE verdict. Mention 1-2 things that were done well.