adversarial-reviewer

A normal review looks for obvious problems. This review looks with the eyes of someone who wants the code to fail, someone who has never seen it, and someone who is paid to find holes in it. Each persona must report at least one finding. If a persona genuinely cannot find a problem after a thorough examination, that is itself a result - state exactly what was checked and why each candidate turned out safe.

Before starting

Read the code in full without judging it. Understand what it is trying to do and what invariants it relies on. Only then switch to the adversarial personas, one at a time.

If a diff or specific files are in scope, focus there. If the scope is the current change, also read the surrounding code that the change interacts with.

Persona 1: The Saboteur

The Saboteur wants the code to fail at runtime. They probe for:

• Edge cases: what happens with empty input, zero, negative numbers, very large values, unicode, null, undefined, or a missing key in an object?
• Sequence and ordering: what if events arrive out of order? What if two calls happen concurrently? Is there a race between reads and writes?
• Type coercion and implicit conversion: does == hide a type mismatch? Does a numeric string arrive where a number is expected and gets treated as one?
• Failure paths: what happens when a network call fails, a file is missing, a lock times out, or a dependency throws? Are errors swallowed? Does the system end up in a partially-updated state?
• Off-by-one and boundary arithmetic: are loop bounds correct? Are slices and ranges inclusive or exclusive where they need to be?

The Saboteur reports findings as: "Saboteur finds: [precise scenario] at [file:line] causes [concrete failure]."

Persona 2: The New Hire

The New Hire is reading this code for the first time, on their second week. They flag:

• Naming confusion: identifiers that mean something different from what they look like, or two things that sound the same but differ in a subtle way.
• Undocumented assumptions: preconditions that callers must satisfy but are never stated, state that must be initialized before a function is called, or ordering guarantees that the code depends on but does not enforce.
• Missing tests for non-obvious behavior: any path that a reader would not predict from the function's name or signature but that is load-bearing.
• Why is this here? Code that does something surprising without explanation. Side effects that are not obvious from the call site.
• Complexity that could be simpler: nested conditionals that could be flattened, a loop doing the work of a standard library function, state that is tracked manually when a different data structure would make it automatic.