Adversarial Review

Adversarial Review

/lean-agent:adversarial-review reviews one change as an adversary. The target is a pull request number or URL, a branch, or --wip for uncommitted work.

Three rules shape it:

1. The finder is never the judge. One agent looks for defects. A different agent, which never sees the first one's reasoning, tries to prove each one wrong. Only survivors are reported.
2. No proof, no finding. Every finding names the input, the code path at a line in the file as it is now, and the wrong result. If a runnable check would settle it in a minute, it is run.
3. The repository's rules beat general taste. The rules file is read before the code.

Steps

Step	What happens
1. Check out	A clean worktree of the branch, never a checkout holding other work.
2. Gate	Run the gate script. Any rule it prints means a full review. Nothing printed means a light review on the six questions only. See Review Rules.
3. Hunt	A fresh agent gets the diff, the description, the linked issue, the rules file and a checkout it can build. It reads whole files around each hunk and every caller of a changed symbol.
4. Refute	A second fresh agent gets each candidate alone and tries to make it go away.
5. Report	Survivors only, blockers first, at most ten.
6. Route	Findings go back to the agent that wrote the change, or onto the pull request as review threads.
7. Record	One line per review with log-review.sh.

Both agents run on the cheap model, started with the descriptions lean:hunt <owner/repo>#<pr> and lean:refute <owner/repo>#<pr> so lean-stats can tie their cost to the change. See Models and the Cascade.

The six questions

Asked on every review, each answered with the line that proves it.

1. Does every new test fail when the production line it depends on is reverted? Revert it and run it.
2. Is every new route in every inventory it belongs to, with the permission gate the issue asked for?
3. Does every session, query and cache key in the change name its tenant?
4. Does every new document, event, column or index have a migration, and does existing data survive it?
5. Does anything from an exception, a request or a user string reach a log or a response unexamined?
6. Does every read-then-write on a shared row name its lock, and every outbound call name its timeout and bound?

Plus the contract question, always: does the change alter what a consumer sees (a route, a field, a status code, stricter validation, a public type or function)?

And the kinds question, always: for each rule in the diff that branches on a type or a value, which kinds can it meet (value type, string, class, interface, abstract class, nullable, a member with a non-null initializer), and was each one checked? The hunter runs the smallest check for every kind the tests do not cover. It was added in 1.3.1 after three defects of one shape reached the review bots in two of three changes: a rule tested with one kind of type and broken on another.

Phrase checklist items as artefacts, not facts. "Did the holdout check run, and is its output on the change" can only be answered by looking. "Was the mutation reverted" can be answered from the description, and for two weeks it was. See Holdout Check.

How the refuter works

• Looks for the guard elsewhere: a caller that validates, a framework that escapes, a type that excludes the case, a test that already covers it.
• Confirms the change caused the defect.
• Runs the smallest check that settles it: a scratch test, the named test with the line reverted, a query plan, a request against a local build.

Each candidate ends as confirmed, refuted or unsure. An unsure finding is kept only if it is a blocker, marked unverified, with what would settle it.

The report

Severity	Means
blocker	Security hole, data loss, contract break with a consumer, wrong results on a normal path
concern	A real defect on an edge path
nit	Optional, labelled, never presented as required

Each finding carries its evidence, an effort mark (quick or heavy) and a proposed fix. The report ends with what was not checked. If nothing survives, it says so and lists what was checked.

In one session

With LEAN_AGENTS=one there are no fresh agents. The hunt and the refute run as two passes: write the candidates down first, then refute each one from the code and a check, not from the hunt's reasoning. The report says the finder and the judge shared a context.

The rules file

The critic reads docs/review-rules.md, then .claude/review-rules.md. It holds what only this codebase knows: which validation changes are breaking, which consumers accept which contract versions, what may not sit in a public record. skills/adversarial-review/review-rules.example.md shows the shape.

Run a review bot as well. The bot is good at general correctness patterns. The critic is good at what the rules file lists.

The log

log-review.sh <repo> <pr> <confirmed> <refuted> <blockers> "<categories>" "<caught elsewhere first>" [pre_push] [ci] [review] [escaped]

It appends to review-log.tsv, or to the path in REVIEW_LOG, and writes the same row to the lean-stats database. Measuring a Batch explains the columns.