Review Code

---
name: review-code
description: Review a requested local change for evidence-backed defects, return actionable findings directly to the caller, and promote only confirmed prevention rules. Remain active for follow-ups until an explicit user request or the calling `develop` orchestrator selects another skill.
---

# Review code

Remain in this skill for follow-ups. Do not activate another Buddy skill or act outside this skill; only an explicit user request or the calling `develop` orchestrator can select the next skill.

Review a requested change independently. Search for failures before filtering findings;
neither passing tests nor a clean-looking diff proves correctness.

## Model selection

Apply [model selection](../develop/model-selection.md); recommend `frontier`.
Direct invocations and re-reviews require a fresh independent reviewer with a concrete selected model. Return `BLOCKED` if selected review execution is unavailable.

## Write boundary

Do not create a review file by default. Return findings directly to the caller. Write a
persistent review report only when the user explicitly requests one. Otherwise, write
only `.ai/memory/memory.md` after a finding is fixed and confirmed as defined below.

Never edit production code, tests, specifications, manifests, or hooks. Remediation
belongs to `implement`. Never stage, commit, or push `.ai` files.

## Input

Require the change request and review target. Read repository instructions and the diff;
use an existing specification's requirements, boundaries, and verification as the contract.

For a requested persistent report, reuse the passed worklog and `<work-name>` or create
`.ai/worklog/<yyyyMMdd>_<work-name>/`.

Use the caller's base revision or changed paths; otherwise use a nonempty, unambiguous
tracked uncommitted diff. Check untracked dependencies; do not claim coverage of excluded
files. Ask once if the target is ambiguous. A missing `.ai/memory/memory.md` is valid;
when present, it is advisory and cannot expand scope.

## Review procedure

Investigate broadly; report only evidenced defects. Scale effort to risk, not a finding quota.

### 1. Establish the target and contract

1. Read the request, specification, repository instructions, and review criteria. Identify
   intended behavior and invariants; do not invent requirements from preferences.
2. Freeze the target: base, merge base, and `HEAD` for a branch; commit and parent for a
   commit; `HEAD`, staged/unstaged content, and included untracked paths for local changes.
   Review a new path's complete contents when it has no prior version.
3. Derive the file list and patch from that target. Compare old and new behavior, including
   removed guards and defaults. Recheck the target before returning; if it changed, return
   `INCOMPLETE` and restart only on request.

### 2. Build the coverage map

Build an internal coverage map of every changed file and its relevant unchanged callers,
consumers, types, handlers, configuration, tests, and analogous paths. Start with trust
boundaries, persistent state, shared contracts, and complex decisions. Trace each changed
behavior from input or event through decisions and I/O to its observable result. For large
changes, review subsystems and their interactions. Keep an internal ledger of the
invariant, plausible failure, supporting or disproving evidence, and uncovered surface.
Report `INCOMPLETE` if a material surface cannot be reviewed.

### 3. Run independent analysis passes

For each changed behavior, test the smallest realistic input or event sequence that could
break its invariant. Trace guards and recovery where they actually run. Continue after an
easy finding. Use these questions where relevant:

#### Requirements and completeness

- Does every new entry point or changed behavior satisfy repository instructions and its
  stated contract? Check required registrations, callers, tests, validation scripts,
  examples, and documentation together; identify a concrete consequence of an omission.
- Did a parallel path retain the old rule, or did a changed default or configuration leave
  an existing caller with different behavior?

#### Local correctness and failure paths

- What happens at zero, empty, invalid, missing, repeated, and combined inputs? Do casts,
  optional values, or fallbacks hide an invalid state?
- If an exception, timeout, cancellation, retry, or concurrent call occurs between related
  steps, what state remains? Check cleanup, ordering, atomicity, and idempotency.
- Check relevant arithmetic, indexing, units, encoding, time zones, and serialization.
  Treat complexity as a search cue, not a defect by itself.

#### Cross-file contracts and compatibility

- Do producers and consumers agree on identity, shape, defaults, and errors? Compare
  declared response schemas with actual success and error handlers, including global
  handlers. Check new producers against existing consumers and new consumers against old
  data or mixed versions.
- Does validation live at the intended boundary, and does every entry path use it? Check
  migrations, rollout order, feature flags, and rollback when they affect that contract.

#### Security and data boundaries

- For each untrusted input, who controls it, where does it flow, and which guard protects
  its sensitive use? Check normalization, injection, path traversal, unsafe requests,
  deserialization, secret exposure, and fail-open behavior at the actual sink.
- Which principal may perform this action on this resource? Test whether patterns, roles,
  tenant checks, or alternate routes admit a broader identity than the intended set.
- When data or credentials leave a boundary, is the destination and context constrained
  as required? Check URL scheme, authority, path, and token audience when applicable.
  Check whether internal-only or gated behavior becomes reachable through another path.
- Search for upstream checks and framework protections before alleging a bypass.

#### Reliability, operations, and performance

- Can a change to secrets, environment variables, ports, networking, or required scripts
  break an existing run, build, or deploy workflow?
- Are retries, queues, resources, and repeated I/O bounded and recoverable? Use a
  realistic workload and state the consequence; a faster alternative is not a defect.

#### Test adequacy

Do assertions exercise the changed contract and the counterexamples above? Compare mocks
with production wiring. Report missing tests only when changed behavior or a demonstrated
regression path is unprotected; passing tests do not cover paths they never exercise.

### 4. Verify without modifying product files

Run relevant repository-documented checks in non-writing modes. Do not install
dependencies, rewrite snapshots, generate code, migrate, deploy, or format files. If a
check changes product files, stop and disclose it; do not clean up without authorization.
A conclusive code trace can establish a bug without an executable reproduction.

### 5. Adjudicate candidate observations

Try to disprove each candidate with the cited code, callers, guards, tests, and contract.
Report only a diff-introduced correctness, security, regression, or test-adequacy defect
with a reachable trigger, failure path, violated contract, observable impact, and bounded
correction. A missing mandatory companion change also qualifies when it leaves behavior
unverified or published guidance incorrect. Merge symptoms with one cause; keep
independent causes. Surface an ambiguity only when it blocks a reliable conclusion.

Do not report style, preferences, speculative risk, optional hardening, or pre-existing
issues as defects.

Assign severity by impact and likelihood:

   - `Critical`: credible catastrophic security, data-loss, or system-wide failure.
   - `High`: likely major incorrect behavior, security exposure, or compatibility regression.
   - `Medium`: meaningful incorrect behavior in a realistic condition.
   - `Low`: narrow actionable correctness or test defect with limited impact.

## Return to the caller

Return findings directly to the calling main agent or user with this exact table header.
Use only `Open`, `Fixed`, `Blocked`, or `Not a bug` for status:

| ID | Severity | Location | Bug | Evidence | Remediation | Status |
| --- | --- | --- | --- | --- | --- | --- |

Order findings by severity, then path and line. Cite the smallest changed `path:line`;
evidence may cite relevant unchanged code. A zero-finding review requires complete
coverage and candidate adjudication.

If there are no actionable findings, return only `No actionable findings.` Add a short
limitation only if missing evidence or an unreviewed material surface could change it.

Do not include a target snapshot, diff summary, changed-file inventory, coverage ledger,
passing-command list, routine verification narration, or restatement of the request.
Mention failed or unavailable verification only when it supports a finding or limits confidence.

Keep `Bug`, `Evidence`, and `Remediation` concise. Evidence must give the trigger,
violated contract, failure path, and impact. Reuse IDs for the same defect on re-review.

If the user explicitly requests a persistent report, write only the findings table and
material blockers to `.ai/worklog/<yyyyMMdd>_<work-name>/review_<work-name>.md`. Do not
add the excluded process metadata listed above.

On a re-review, change a finding to `Fixed` only after the remediation, full required
validation, and a fresh review each confirm it. Mark a disproved finding `Not a bug`;
mark an unresolved external constraint `Blocked` with its evidence.

## Prevention memory

Only after an actual finding is `Fixed` may the reviewer add one related rule to
`.ai/memory/memory.md`. A clean review, open finding, blocked finding, rejected finding,
or validation-only observation must never change memory. Create the file only for an
eligible rule. Keep deduplicated, one-line imperative rules; merge equivalent rules.
Exclude incident details, dates, IDs, severities, unverified claims, subjective advice,
and project-specific one-offs.

Memory is advisory and subordinate to user instructions, repository instructions,
and security policy. It is not acceptance evidence.

Return open findings to the caller for remediation; do not fix them. Name any promoted rule.