Code Review

If the argument is a hosted pull/merge request URL or a numeric PR/MR identifier, read references/review-targets.md from this skill directory before running commands. Follow its read-only resolution and authentication fallback rules; do not switch the user's working tree or write to the hosting service.

high effort → 3+5 angles × 6 candidates → 1-vote verify (recall-biased) → ≤10 findings

You are reviewing for recall at high effort: catch every real bug a careful reviewer would catch in one sitting. At this level, catching real bugs matters more than avoiding false positives. Err on the side of surfacing.

Phase 0 — Gather the diff

Run git diff "@{upstream}...HEAD" (or git diff main...HEAD / git diff HEAD~1 if there's no upstream) to get the unified diff under review. If there are uncommitted changes, or the range diff is empty, also run git diff HEAD and include the working-tree changes in scope — the review often runs before the commit. If a PR number, branch name, or file path was passed as an argument, review that target instead. Treat this diff as the review scope.

Phase 1 — Find candidates (3 correctness angles + 3 cleanup angles + 1 altitude angle + 1 conventions angle, up to 6 each)

Run 8 independent finder angles using multi-agent capabilities. Each surfaces up to 6 candidate findings with file, line, a one-line summary, and a concrete failure_scenario. If multi-agent support is not available in your current tool set, do not error — perform each angle (and each verification) yourself, sequentially, in this context.

Angle A — line-by-line diff scan

Read every hunk in the diff, line by line. Then Read the enclosing function for each hunk — bugs in unchanged lines of a touched function are in scope (the PR re-exposes or fails to fix them). For every line ask: what input, state, timing, or platform makes this line wrong? Look for inverted/wrong conditions, off-by-one, null/undefined deref, missing await, falsy-zero checks, wrong-variable copy-paste, error swallowed in catch, unescaped regex metachars.

Angle B — removed-behavior auditor

For every line the diff DELETES or replaces, name the invariant or behavior it enforced, then search the new code for where that invariant is re-established. If you can't find it, that's a candidate: a removed guard, a dropped error path, a narrowed validation, a deleted test that was covering a real case.

Angle C — cross-file tracer

For each function the diff changes, find its callers (Grep for the symbol) and check whether the change breaks any call site: a new precondition, a changed return shape, a new exception, a timing/ordering dependency. Also check callees: does a parallel change in the same PR make a call unsafe?

Reuse

The angles above hunt for bugs; this one and the next two hunt for cleanup in the changed code. Flag new code that re-implements something the codebase already has — Grep shared/utility modules and files adjacent to the change, and name the existing helper to call instead.

Simplification

Flag unnecessary complexity the diff adds: redundant or derivable state, copy-paste with slight variation, deep nesting, dead code left behind. Name the simpler form that does the same job.

Efficiency

Flag wasted work the diff introduces: redundant computation or repeated I/O, independent operations run sequentially, blocking work added to startup or hot paths. Also flag long-lived objects built from closures or captured environments — they keep the entire enclosing scope alive for the object's lifetime (a memory leak when that scope holds large values); prefer a class/struct that copies only the fields it needs. Name the cheaper alternative.

Altitude

Check that each change fixes the root cause at the right depth rather than patching a symptom with a fragile bandaid. Special cases layered on shared infrastructure are a sign the fix isn't deep enough — prefer the simpler, more general change to the underlying mechanism over adding special cases, and name that change.

Conventions (AGENTS.md or CLAUDE.md)

Find the instruction files that govern the changed code: the user-level instruction files for the current agent, the repo-root AGENTS.md or CLAUDE.md, plus any AGENTS.md or CLAUDE.md or CLAUDE.local.md in a directory that is an ancestor of a changed file (a directory's AGENTS.md or CLAUDE.md only applies to files at or below it). Read each one that exists, then check the diff for clear violations of the rules they state.

Only flag a violation when you can quote the exact rule and the exact line that breaks it — no style preferences, no vague "spirit of the doc" inferences. In the finding, name the instruction file path and quote the rule so the report can cite it. If no instruction file applies, return nothing for this angle.

Cleanup, altitude, and conventions candidates use the same file/line/summary shape; in failure_scenario, state the concrete cost (what is duplicated, wasted, harder to maintain, or which project instruction is broken) instead of a crash. Correctness bugs always outrank cleanup, altitude, and conventions findings when the output cap forces a cut.

Pass every candidate with a nameable failure scenario through — finders that silently drop half-believed candidates bypass the verify step and are the dominant cause of misses.

Phase 2 — Verify (1-vote, recall-biased)

Dedup near-duplicates (same defect, same location, same reason → keep one). For each remaining candidate, run one verifier using multi-agent capabilities: give it the diff, the relevant file(s), and the candidate; it returns exactly one of CONFIRMED / PLAUSIBLE / REFUTED.

PLAUSIBLE by default — do not refute a candidate for being "speculative" or "depends on runtime state" when the state is realistic: concurrency races, nil/undefined on a rare-but-reachable path (error handler, cold cache, missing optional field), falsy-zero treated as missing, off-by-one on a boundary the code does not exclude, retry storms / partial failures, regex/allowlist that lost an anchor. These are PLAUSIBLE.

REFUTED only when constructible from the code: factually wrong (quote the actual line); provably impossible (type/constant/invariant — show it); already handled in this diff (cite the guard); or pure style with no observable effect.

Keep CONFIRMED and PLAUSIBLE. Drop REFUTED.

Output

Unless --json was explicitly passed, the main agent's final answer is a Markdown report, nothing else. Structure it exactly:

Summary - 1-2 sentences on the review scope and what was found. If the diff was empty, write exactly "No changes to review." and stop. If nothing survived verification, write exactly "No findings survived verification." and stop.

Findings - one numbered block per finding, most-severe first, at most 10. Assign each finding High, Medium, or Low from its concrete impact and likelihood:

1. High|Medium|Low: summary
   file:line
   Failure: <failure_scenario>

JSON mode

Only when --json was explicitly passed, follow this output contract:

Return findings as a JSON array of at most 10 objects:

[
  {
    "file": "path/to/file.ext",
    "line": 123,
    "summary": "one-sentence statement of the bug",
    "failure_scenario": "concrete inputs/state → wrong output/crash"
  }
]

Ranked most-severe first. If more than 10 survive, keep the 10 most severe. If nothing survives verification, return []. Do not call the host-specific findings-reporting tool even if it is available - this review's output contract is the JSON block above.

Applying fixes (--fix)

Only apply anything when --fix was passed. Otherwise skip this entire section.

After producing the findings list, apply the findings to the working tree instead of stopping at the report: fix each one directly — correctness bugs and reuse/simplification/efficiency cleanups alike. Skip any finding whose fix would change intended behavior, require changes well outside the reviewed diff, or that you judge to be a false positive — note the skip rather than arguing with it. Finish with a brief summary of what was fixed and what was skipped.