Code Review

---
name: code-review
description: Review a diff, PR, or set of changed files for correctness bugs, security issues, and unnecessary complexity. Use when the user asks to review code, check a PR, or "look over" a change before it merges -- not for general code questions.
---

# Code Review

You are reviewing a *change*, not grading a codebase. Every finding must be
something introduced or made worse by this diff, or a pre-existing landmine
that this diff walks directly into. Don't review-drift into the rest of the
file.

## Scope

1. Establish the diff: `git diff main...HEAD` (or the PR's changed files if
   given a PR number/URL). If no diff exists yet, review the working tree
   changes (`git diff` + `git diff --staged`).
2. Read each changed file with enough surrounding context to understand what
   calls it and what it depends on -- a five-line hunk in isolation hides
   the bugs that matter most.
3. Skip generated files, lockfiles, and vendored code unless the user
   specifically asks about them.

## What to look for, in priority order

1. **Correctness** -- will this produce a wrong result, crash, or silently
   corrupt data under some real input? Off-by-one, null/undefined access,
   wrong operator, inverted condition, race condition, resource leak,
   swallowed exception, incorrect error handling on a partial failure.
2. **Security** -- injection (SQL/command/template), unsanitized input
   reaching a sink, broken auth/authz check, secret committed in plaintext,
   unsafe deserialization, missing bounds check on user-controlled size.
3. **Data integrity** -- a migration or write path that can leave the store
   in an inconsistent state; a retry that isn't idempotent; a cache that can
   serve stale/wrong data after this change.
4. **Test coverage** -- does a new branch or edge case introduced by this
   diff have no test that would catch a regression in it? Flag the gap, not
   "please add more tests" in general.
5. **Simplification / reuse** -- only when it's clearly cheaper and no
   riskier: an existing helper that does the same thing, an abstraction that
   isn't earning its complexity, dead code this diff should have removed.

Do **not** flag: formatting/whitespace, naming preferences without a
correctness angle, "add a comment here," or anything a linter/formatter
already enforces in this repo.

## Verify before reporting

For each candidate finding, re-read the actual code path end to end and
state the concrete input or sequence of events that triggers it. If you
can't articulate a specific failing scenario, it's not a finding -- drop it
or downgrade it to a question. This step removes more false positives than
anything else in this file.

## Output format

Order findings most-severe first. For each:

- **File:line** of the anchor
- **One-sentence summary** of the defect
- **Failure scenario**: concrete input/state -> wrong output/crash
- **Suggested fix**, if it's a one-liner; otherwise describe the shape of
  the fix without writing the whole patch

If nothing survives the verify step, say so explicitly -- "no correctness
or security issues found in this diff" is a valid and useful review.

## Guardrails

- Never invent a vulnerability class that doesn't apply to this language or
  framework (don't flag SQL injection in a diff with no SQL).
- Never soften a real finding to be polite, and never inflate a nitpick to
  sound serious -- match tone to actual severity.
- If the diff is large, review it in logical chunks (by file or by feature)
  rather than skimming the whole thing at low resolution.

+function getDiscount(user) {
+  if (user.plan = 'pro') {
+    return 0.2;
+  }
+  return 0;
+}