Quality and Security
Adversarial Review: Trying to Disprove That a Change Is Correct
How to test an important claim about a change by trying to refute it with concrete counterexamples, and how to confirm a defect, withdraw a false…
After AI-assisted code review, it pays to take a step to one side: not only to look for errors, but also to check whether there is real ground for calling the reviewer's particular suspicion an error. What would have to happen, under supported conditions, for this change to turn out wrong? What evidence separates a confirmed defect from a guess?
Adversarial review is the careful testing of an important claim about a change against counterexamples. It does not require hostility towards the author, it does not require checking every conceivable combination, and it does not require finding something to object to. Its boundaries are the agreed requirements, your own (or an explicitly permitted) teaching example, and a specific risk. Nothing in this unit grants permission to probe other people's systems.
Test the claim, not the person
A change can have a well-named function, green checks and a convincing description and still break a product rule. The reverse also happens: a path that looks complicated may be safe because of a database constraint or a contract the reviewer did not notice at first.
Google's practical code-review guidelines recommend checking behaviour at edge cases and under concurrent execution, weighing technical facts rather than personal preferences, and not holding up a change for the sake of perfection or style. These are the guidelines of one particular engineering organisation, not a universal policy for every team; the requirements of the specific project and the risk of the change set the limits of a review. (Google Engineering Practices: The Standard of Code Review; What to Look For)
Before you look for a counterexample, write down the claim you are going to test. For example: "one slot in the schedule may have at most one active reservation", or "a user may modify only their own reservation". A rule of this kind, one that must remain true after the change, is called an invariant.
Then pin down three things:
- Conditions. Who performs the action, what state existed before it, which version of the code, and which agreements about the data apply?
- An independent criterion for the expected result. What defines correct behaviour: a requirement, a contract, an official guarantee of a tool, or a data invariant?
- A possible counterexample. What is the shortest permitted sequence of actions that would break the rule if the suspicion is right?
To test authorisation, for instance, the phrase "the user is not signed in" is not enough. The more important pair is "the user is signed in but tries to change another person's record". Signing in to a product proves who made the request; on its own it does not prove that this person is allowed to change a particular object.
If it is unclear whether a scenario belongs to the supported conditions, you may be looking at a gap in the requirements. Do not declare it a defect until you have established whether the product should support it and what risk the uncertainty creates.
Set the boundaries before you complicate the scenario
A list of edge cases without context grows without end. Different products can have different permitted roles, cancellation rules, database guarantees, time limits and ways of retrying a request. Fix the boundaries before adding a scenario to the report:
| Part of the check | Question |
|---|---|
| Change | Exactly which change and which base revision are being reviewed? |
| Rule | Which user-facing or technical claim must remain true? |
| Preconditions | Which roles, data, order of actions, client versions and failures are permitted? |
| Expected result | Which source defines it independently of the implementation? |
| Consequence | What will the user see, or what will change in stored data? |
| Missing information | Which fact must be clarified before drawing a conclusion? |
A poor scenario sounds like "there might be a race somewhere". A better one states a concrete interleaving: "two requests checked that the slot was free before either of them wrote a reservation; then both saved a separate record for the same slot". Now it is clear which preconditions are needed and which invariant must be tested.
Separately, check whether the test has an independent criterion for the expected result. If the code and the test rest on the same wrong assumption, a green test confirms only that the two agree with each other. For example, a test that only checks the "unauthorised" response for someone who has not signed in says nothing about a person with a valid session asking for someone else's record.
Worked example: signing in is not the right to modify a record
Consider a made-up teaching application programming interface (API) for reservations. The requirement says a user may change only their own reservation within their own organisation. This is an illustrative example; its code and tests have not been run.
Olena and Marko both belong to the organisation "North". Olena owns reservation 418 and Marko owns 527. Both are signed in. Marko sends a request to change record 418. The current handler first checks that the requester has a valid session and then looks up the reservation by its number across the whole table. It does not check who owns the record it finds.
The minimal counterexample:
- The store contains reservation 418, owned by Olena.
- Marko has a valid account but does not own 418.
- Marko changes the contact field in a request for record 418.
- The system saves the new contact and reports success.
This breaks the rule even though the route correctly rejects visitors who are not signed in. The sign-in was confirmed; the right to change the record was not.
The Open Worldwide Application Security Project (OWASP), in its API1:2023 entry, explains this distinction as object-level authorisation: when a request contains an object identifier and performs an action on that object, check that this particular current user has the right to perform this particular action. (OWASP API1:2023) Here it serves as an independent criterion for our teaching requirement, not as proof that every system has an identical role model.
A minimal fix must restrict the lookup to the current user and their organisation, or perform equivalent checks before the change. The test must confirm that Marko's request is rejected, that Olena's record is unchanged and that the response leaks no extra data; a positive check must allow Olena to change her own record. If a user can belong to several organisations, also test a request made in the wrong organisational context. A test without a sign-in covers a different rule.
If the product deliberately hides the existence of other people's records, the refusal may look like "not found" rather than "forbidden". In that case the criterion includes not only the absence of a change but also the absence of extra data in the response. Do not demand a specific response code unless the contract supports it.
Choose classes of action by risk
Once the main invariant is known, choose a small number of different ways to break it. Start from consequences, not from the number of scenarios:
- Value boundaries: an empty, minimum, maximum, malformed or missing identifier.
- A different participant: another owner, customer, role or organisation.
- Repetition: the client retried a request after a timeout and does not know whether the first attempt worked.
- A different order: cancellation, update or completion happens in an order the happy path does not expect.
- Concurrency: several permitted requests read the same state and try to change it.
- Partial failure: one part of the operation was saved, but the next write, notification or confirmation did not happen.
- Stale context: an older client, role or record is in play while the contract or access rules are changing.
The chosen scenario must match the program's actual path. Do not assume the client can send any value if the contract forbids it; at the same time, check that the server does not rely solely on an unverified restriction in the user interface. Do not add an external network, an attacker, special privileges or exotic failures just to make a finding sound more dramatic.
Worked example: two requests take the same slot
Suppose the schedule has pre-created slots, each with its own identifier. The requirement is at most one active reservation per slot. The current code first asks whether the slot is free and then creates the reservation. The two actions happen separately.
This is a made-up teaching example; the code shown and the checks mentioned have not been run. The example query is written in Structured Query Language (SQL).
A possible sequence:
| Step | Request A | Request B |
|---|---|---|
| 1 | reads: slot 815 is free | |
| 2 | reads: slot 815 is free | |
| 3 | creates reservation A for 815 | |
| 4 | creates reservation B for 815 |
Each request got a plausible answer to its own check. A transaction gives all-or-nothing behaviour for the changes inside it, but on its own it does not make a "check, then write" sequence safe from a race. At the Read Committed level, two clients can still both see the slot as free. To preserve the invariant you need protection against concurrent writes: for example, a uniqueness constraint, an explicit row lock, or a serializable transaction with error handling and retry. Here, the atomicity of a transaction means only that its own changes are committed together or rolled back together.
PostgreSQL uses the Read Committed isolation level by default: each individual statement sees data committed before that statement began. Two consecutive reads can see different states if a concurrent change was committed between them. So two independent checking requests can both read "free" before anyone saves a reservation. (PostgreSQL 18, Transaction Isolation)
If the table stores only active reservations for fixed slots, one possible safeguard is a uniqueness constraint on the slot identifier: the database rejects the second row. In PostgreSQL an insert can use ON CONFLICT DO NOTHING, which means "on conflict, do not create a second record":
INSERT INTO active_reservations (slot_id, customer_id)
VALUES (815, 29)
ON CONFLICT (slot_id) DO NOTHING
RETURNING id;
In this fragment the slot_id column must be NOT NULL and carry a UNIQUE constraint. If the insert returned an identifier, the request has reserved the slot. If the insert returned no row, the slot is already taken, and in this scenario the handler reports that the reservation failed. A repeat of the same request is handled by a separate rule for identifying requests. The handler must not silently report success when no row was created. PostgreSQL documents a uniqueness constraint as a rule that applies across all rows, and ON CONFLICT as a way to specify the action taken on a conflict. (PostgreSQL 18, Constraints; PostgreSQL 18, INSERT)
This solution fits only the model described. It assumes one reservation per fixed slot with its own stable identifier, not arbitrary time intervals. If the system keeps cancelled rows forever, a plain UNIQUE on slot_id will block rebooking after a cancellation: you would need a separate record for the active reservation, an index that enforces uniqueness only on active rows, or another agreed model. If time can be booked in arbitrary intervals, you need a guarantee against overlapping intervals, not just against identical identifiers.
A meaningful regression test must simulate two concurrent requests against the same isolated store, not merely replace the store with a mock. Both requests reach the attempt to write. The expectation: exactly one active reservation is committed, the second request receives a proper refusal, and clients never see a false double success. A barrier in the test helps check a specific order of actions; a random wait of a few milliseconds does not, on its own, guarantee a reproducible interleaving.
Using a higher isolation level also needs its own check. PostgreSQL's serializable level may reject one transaction with a serialization error; the application must be ready to retry the whole transaction. The name Serializable does not mean the error disappears or that a retry has already been implemented. In many models it is simpler and more precise to express the invariant as a data constraint.
Withdraw a false suspicion as rigorously as you confirm a defect
Imagine a reviewer claims: "redelivery of the same message will create a second reservation." It is a plausible hypothesis, but the word "redelivery" is not yet evidence.
This is also a made-up teaching example; the query and the related checks have not been run. Message processing begins by inserting a receipt keyed by the organisation, the source and the message identifier. All three fields are required, and the combination has a unique constraint. Writing the receipt and creating the reservation happen in one transaction:
INSERT INTO processed_events (tenant_id, source_id, event_id)
VALUES (12, 'calendar-provider', 'evt-731')
ON CONFLICT (tenant_id, source_id, event_id) DO NOTHING
RETURNING event_id;
The transaction creates a reservation only if the insert returned a row. If another delivery has already created the receipt, the insert returns nothing, so the handler does not create another reservation. This example proves only the absence of a duplicated stored reservation under the stated conditions; to return an identical response to a repeat, the model would have to store the result or the reservation identifier separately. Assume, too, that every path that creates a reservation has been checked: none bypasses this receipt, the key does not allow NULL, and both records are committed or rolled back together.
Under these conditions the conclusion "redelivering this message created a second reservation" is refuted for stored reservations. A good reply from the reviewer: "I initially missed the unique constraint and the shared transaction, so I'm withdrawing the finding about a second row. I did not check whether the external notification is repeated; if that is part of the requirement, it is a separate path and a separate check."
A database constraint does not prove that an email will not be sent twice, that an external provider will not charge twice, or that an action outside the transaction will not be repeated. An external effect may need the provider's own idempotency key, a send log, or an outbox table: records of events that must be delivered, which a separate process reads and sends with retries. A reviewer who points to such a different consequence may well be right. You have to state clearly which claim exactly has been refuted.
An old public report about Ruby on Rails illustrates this discipline well. The author of issue #13234 gave a reproduction in which two concurrent requests pass the uniqueness validation before either of them writes its result. The duplicate scenario is real; the original claim that this was a thread-safety problem inside the save method does not describe the precise cause. The current Rails documentation explicitly warns that application-level uniqueness validation does not guarantee the absence of duplicates and recommends a uniqueness constraint in the database. The right conclusion is neither "nothing breaks" nor "the whole defect is in Rails", but "separate reads and writes race; the invariant must be enforced at a level that sees both concurrent writes". (Rails, issue #13234; Rails API: uniqueness validation, concurrency and integrity)
A real case: a reported bug is confirmed
Public history can show how a hypothesis becomes a confirmed defect. In SQL, the order of keywords must follow the grammar of the specific database. In 2021, the author of Ruby on Rails issue #41480 reported that a migration using both the if_not_exists and algorithm: concurrently options generated an invalid PostgreSQL statement. The report included a short reproduction application, the versions (Rails 6.1.2.1, pg 1.2.3 and PostgreSQL 13.1) and the actual syntax error message.
The report can be checked against an independent criterion: PostgreSQL's official grammar for CREATE INDEX, which places CONCURRENTLY before IF NOT EXISTS. In the error message, Rails had generated the reverse order: IF NOT EXISTS CONCURRENTLY. There is a chain from precondition to result: the given combination of options → a wrong query string → a parser error.
In the related pull request (PR) #41490, the author of the fix changed the order and added a regression test that should catch the previous output. It was merged on 4 March 2021. This is stronger evidence than votes or the author's confidence: there is a reproduction, the correct syntax from the database vendor, a fix and a test. (Rails, issue #41480; Rails, pull request #41490; PostgreSQL 18, CREATE INDEX)
At the same time, this is a historical defect in a specific version and a specific combination of options. It does not follow that current Rails still generates the same error. Keeping the claim precisely bounded preserves the useful lesson without turning it into a generalisation about today's product.
Ask an AI reviewer for evidence, not a verdict
An artificial intelligence (AI) agent can quickly suggest suspicious paths, explain a sequence of events or write a test. Ask it to state:
- the exact file, line and revision;
- the rule it believes is broken;
- the preconditions and the shortest reproduction;
- the expected and the actual behaviour;
- the source of the independent criterion;
- which fact would refute the finding.
Then check for yourself whether that execution path exists, whether the source applies to the relevant version, and whether the test reproduces the system's real boundary. If the agent reports that it ran a command, compare that against the available execution log. Do not put into the conclusion a check that nobody ran.
As of 24 September 2026, OpenAI's official materials describe Codex review rules in the AGENTS.md file and recommend testing them on an example that should trigger, a safe counterexample and an irrelevant change. The same publication makes clear that tests, branch protection and required approvals remain the mechanisms that enforce requirements. For Claude Code, the documentation at that date describes managed Code Review as a research preview: it posts findings but neither approves nor blocks a pull request. Having many agents, or a product that verifies findings and filters false positives, does not guarantee that a particular finding is correct or that it checked the right version. Feature availability and names change; check current documentation rather than carrying capabilities over from one product to another. (OpenAI: Custom Code Review Rules for Codex, 20 July 2026; Anthropic: Claude Code Review)
When to stop reviewing
The goal is to give the team enough evidence for a decision, not to guarantee the absence of all future defects. For each risk, state one of these conclusions:
- Confirmed: a permitted scenario breaks the rule; show the reproduction, the consequence and the smallest appropriate fix.
- Not confirmed: the required preconditions do not match the supported contract, or an independent guarantee preserves the invariant; record exactly what was checked.
- Insufficient data: a requirement, version, configuration or accessible environment is missing; name the specific piece of evidence that would change the decision.
Stop searching when the claims critical to the change have appropriate checks, the most important permitted paths have left no open blocker, and further scenarios only repeat mechanisms you have already explored. Record the limits of your confidence and move on to the next risk. If the test environment cannot exercise concurrency, do not replace the missing evidence with a confident tone.
In Accelerate, the authors explain that smaller batches of change shorten the cycle and speed up feedback; in an appendix they also report organisation-level results in which teams with no approval process or with peer review reported higher delivery performance. As an editorial conclusion for this unit, these observations support early, focused review and a clear stopping point, but they do not show that an individual request should skip required checks or branch protection. (Nicole Forsgren, Jez Humble, Gene Kim, Accelerate, authorised excerpt)
A precise report often contains one confirmed finding, one withdrawn suspicion and one unknown precondition. The number of items does not measure the quality of a review. A review is finished not when every doubt has vanished, but when the team can explain what was checked, what was refuted, what remains unknown and why that is enough for the next decision.
Summary
Adversarial review starts with one important claim, its conditions and an independent criterion of correctness. The reviewer then builds the smallest permitted reproduction and separates fact from hypothesis. Access checks must distinguish a valid sign-in from the right to a particular record; concurrent invariants must rest on a guarantee from the data store, not merely on the sequence of steps in the application. A defect you find must be proven, a false suspicion withdrawn, and a lack of evidence named plainly.
Sources and further reading
Materials checked on 10 October 2026. PostgreSQL documentation and product documentation can change; for a specific implementation, verify the version and configuration.
- PostgreSQL 18, 5.5 Constraints —
UNIQUEconstraints, primary keys, exclusion constraints andNULLbehaviour; supports the explanation of which guarantees belong to the data schema. - PostgreSQL 18, 13.2 Transaction Isolation — isolation levels, the default read snapshot, re-evaluation of an
UPDATEcondition after a concurrent change, and the need to handle serialization errors. - PostgreSQL 18, INSERT and CREATE INDEX — insert conflict handling and index grammar, used in the teaching and real-world examples.
- Open Worldwide Application Security Project, API1:2023 Broken Object Level Authorization — checking the current user's permissions on every object an API accesses.
- Ruby on Rails, issue #41480, 17 February 2021 — a minimal reproduction of the wrong SQL keyword order, with the Rails, PostgreSQL adapter and database server versions.
- Ruby on Rails, PR #41490, merged 4 March 2021 — the generation order was fixed and a regression test added.
- Ruby on Rails, ActiveRecord uniqueness validation, API documentation — describes the race between application-level uniqueness validation and the write, and recommends a matching database constraint. Checked 24 September 2026.
- Ruby on Rails, issue #13234, 8 December 2013 — a public example in which the observed duplication is real, but the explanation as a thread-safety defect must be distinguished from a race between read and write.
- GitHub documentation, Pull request reviews and Status checks — reviews and automated checks play different roles; required approvals and checks depend on the rules of a specific branch.
- Google Engineering Practices, The Standard of Code Review and What to Look For — guidance on technical risks, edge cases, concurrency, maintainability, and not using personal style preferences as a blocker.
- Nicole Forsgren, Jez Humble, Gene Kim, Accelerate, authorised excerpt — research on fast feedback and small batches of change; the organisational results reported are not a rule for approving an individual request.
- OpenAI, Custom Code Review Rules for Codex, 20 July 2026 — repository instructions can refine review; the publication explicitly separates review from tests, branch protection and required approvals.
- Anthropic, Claude Code Review — documentation, current at the date checked, on review modes, product availability, research-preview status and whether managed review blocks a pull request.