/code-review

name: code-review
description: "Review code for actionable defects. Correctness is the core; performance and security are optional sub-cases of the same engine. Anchors on agreements between participants across a boundary, forces a violating execution, and refutes every candidate before reporting. Use when asked to review changes, find bugs, audit a codebase, or check whether a fix is safe."

Code Review

Purpose

Most review output is noise: a list of things that *look* wrong, unranked, unrefuted, and impossible to act on. This skill produces the opposite — a small number of findings, each with a required behaviour, a feasible trigger, a concrete contradiction, an observable consequence, and the strongest counterargument already checked.

Its central bet: **the defects reviewers miss are rarely visible inside one file.** They are disagreements between two participants that each look reasonable alone — a caller and a callee, a producer and a consumer, a writer and a later reader, two branches that should establish the same state. A checklist applied file-by-file cannot see those, because the two halves are never in view at the same time. So the unit of review here is the **agreement**, not the file.

Structure: one engine, three anchors

Code review is the skill. **Correctness is its core** — the dimension generic tooling covers worst, and the one described in full below. **Performance and security are sub-cases**: the same engine, the same refutation discipline, the same report contract, with a different anchor and one or two extra rules each.

| Sub-case | Anchor | Where its rules live | |---|---|---| | **Correctness** *(core, default)* | Agreements between participants across a boundary | This file + `references/correctness-taxonomy.md` | | **Performance** | Workload → resource demand → growth or contention → consequence | `references/performance-review.md` | | **Security** | Source → trust boundary → sink, with an attacker controlling the source | `references/security-review.md` |

Read a sub-case's file only when that sub-case is selected. Each is short on purpose: it states what *differs*, and the rest of this file still applies.

Sub-cases are independently *activated*, not mutually exclusive. One root cause can carry correctness and security impact — report it once, with both impacts.

Invocation

/pm-ai-shipping:code-review
/pm-ai-shipping:code-review dimensions=correctness scope=changes
/pm-ai-shipping:code-review dimensions=performance,security
/pm-ai-shipping:code-review dimensions=all

Claude Code ships its own bundled `/code-review`. Use the plugin-qualified form above when you mean this one.

These are instruction arguments, not shell flags.

• **Default: `correctness`.** Bare "review this" or "find bugs" means correctness only.
• An explicit list selects exactly those sub-cases; `all` selects three. Never silently reinterpret

an unknown or empty selection — ask.

• **Scope:** use what was asked. Otherwise review working changes if present, else the repository.
• **State the selected dimensions, the scope and the comparison baseline before investigating.**
• Reviewing changes means following dependencies *beyond* the changed lines, and distinguishing

defects the change **introduced** from defects it merely **revealed**.

• Review and report. Apply fixes only when asked.

Shared engine

Every sub-case uses one skeleton. Only the anchor and the refutation rules differ.

**Map a flow → identify an obligation → inspect every participant → construct a violating execution → trace the consequence → attempt refutation → report.**

Build one minimal map first: inputs, major execution flows, who owns which state, external dependencies, observable effects. Each selected sub-case enriches it — do not build three maps, and do not make a security-only run wait on correctness mapping.

Correctness: the agreement engine

A *boundary* is semantic, not a file split. It separates a caller and a callee, two callbacks, two executions of the same function, a producer and a consumer, or a value written now and read later.

For each consequential agreement, hold these in working notes — not in the report:

Participants:
Value, entity or effect exchanged:
Authority (who decides the real answer):
Identity and lifetime/version:
Required relationship:
Evidence for that relationship:
Relevant transitions or orderings:
Observable consumer or consequence:

**Establish the obligation without inventing intent.** Evidence comes from specifications, documented contracts, language or protocol semantics, tests that encode an expectation, or a necessary producer/consumer relationship. A consumer's implementation alone does not prove the consumer is right. Where participants disagree, say why the disagreement produces a *wrong outcome* — sometimes the contradiction is certain while which side should change is genuinely open. Missing documentation is a limitation, not automatically a finding.

**Start where agreements are most likely to break:** values transformed or negotiated, identities reassigned, work becoming asynchronous, state persisted and reloaded, several effects that must agree. Then do a local pass over ordinary decisions, arithmetic, boundaries and error branches — the anchor must not become a filter that discards plain bugs.

Force a violating execution

A suspicion is not a finding until you construct the execution that breaks it. Where the implementation permits:

• make a **requested** value differ from the **accepted or effective** one;
• keep two operations live at once and vary their completion order;
• change the relevant identity or generation between observation and use;
• compare distinct transitions that should end in equivalent state;
• inject failure between effects, and interruption before completion;
• exercise empty, exact-boundary and adjacent-boundary inputs.

Establish that each case is actually reachable. Do not assume it.

Two