oak-adversarial-review

Adversarial Review

Overview

Green tests show only that the tests pass. This review starts from the opposite assumption: the change is broken somewhere, and the job is to find where. It adds a coverage contract and a fact-check to the canonical review, so that no changed file goes unexamined and no finding survives on a misreading.

The coverage contract and the drop grounds in Stage 4 follow alibaba/open-code-review (Apache-2.0).

Stance and context boundary

• Attack; do not praise. A finding that cannot be demonstrated is not a finding.
• Every finding names a file, a line and a concrete input or sequence of steps that breaks it.
• Build your view from the task artifacts (spec, acceptance criteria, plan) and the diff. Do not read the author's notes, the developer summary, run events or an autonomy log until your findings are written. Checking run status is allowed.
• Run in a fresh subagent context, never in the session that wrote the change.

Stage 1 Coverage

1. List every changed file as (path, status), where status is added, modified, deleted or renamed.
2. Every file ends as reviewed or skipped. A skip needs a concrete reason, such as "generated file" or "lockfile". "Looked minor" is not a reason.
3. Reviewing a file does not cover its counterpart. A changed interface, test or config is its own entry.
4. Publish the counts: files in the change, reviewed and skipped.

Stage 2 Attack plan

For each risk you suspect, write down before reading further:

• the expected severity;
• the stake: where it happens, what breaks and who would notice;
• exactly what you must read to confirm or refute it.

Then read that, and confirm or drop the risk. Do not pad the plan with risks you have no reason to suspect.

Stage 3 Dimensions

Attack each reviewed file along these nine dimensions:

1. Assumptions. Does the code assume a value exists, is non-empty, is unique, is ordered or arrives only once?
2. State preconditions. What state must hold before this runs, and what happens when it does not?
3. Authorization. Is access checked, including on every new read path?
4. Input handling. Empty, whitespace-only, maximum length, wrong type, encoding, unicode, and injection into a query, a shell, a template or a log.
5. Concurrency and idempotency. Two callers at once; the same request twice.
6. Error paths. What does the caller actually see when this fails?
7. Spec drift. Behavior the spec asked for that is missing, and behavior the diff adds that nobody asked for.
8. Conventions. Consistency with the conventions the repository documents.
9. Architecture. Conformance with the project's own declared rules only, never with rules you would prefer.