How to review an AI-written payment idempotency key change

To review an AI-written PR that adds an idempotency key to a payment endpoint, follow the key from the incoming request to the durable payment record and the external provider call. Check what happens when two requests arrive together, when a retry changes the payload, and when the provider succeeds but the service times out. A key in a header is only an input. The review question is whether every retry can cause at most one charge and return an honest result.

Imagine POST /payments accepts an amount, currency, and recipient. An agent adds an Idempotency-Key header, looks it up before calling the provider, and stores the response afterward. That handles a sequential retry after success. It does not yet handle two requests that both miss the lookup, or a process crash after the provider charges but before the response is stored.

Define the operation the key identifies

Start with the API contract. Ask whether the key is scoped to an account and operation, how long it is retained, and what happens if the same key is sent with a different amount, currency, or recipient. A replay with changed payment details must not silently return the first payment as though it matched the new request. Compare a stable fingerprint of the meaningful request fields, and define a clear mismatch response. Avoid comparing raw JSON bytes if field order or harmless defaults can differ.

Trace the key through the handler, queue, database, and provider adapter. If only one process keeps an in-memory map, a retry routed to another instance can still create a second payment. If the provider supports its own idempotency key, check whether the same stable operation identifier reaches that boundary too. A local database guard cannot by itself make an external side effect atomic with a local write.

In Review Assist, compare the stated request in problem.user_asks with the assumptions and anchored code tour in the Intent Document. The document can show why the agent chose a key scope or retention window. It does not establish that the choice matches your payment contract. Checking scope drift helps if the PR quietly changes retry behavior beyond the requested endpoint.

Inspect the race and the uncertain outcome

Look for one atomic way to claim the key, usually a unique constraint or an equivalent durable conditional write over the chosen scope. A read followed by an insert is not enough: both requests can read an empty slot. The losing request needs defined behavior while the first is in progress, such as a retryable in-progress response or a wait for the committed result. It must not start a second provider call.

Then walk through the order of writes and side effects. If the service records success before calling the provider, a crash can leave a false success. If it calls the provider first and only then records success, a timeout or crash leaves an unknown local outcome. That case needs reconciliation, often by querying the provider with the stable operation identifier before any new attempt. Do not label a timeout as a failed charge unless the provider has confirmed failure.

Retention matters after the happy path. A key that expires while clients may still retry can permit a second effect. Ask who sets the retry window and how the service treats an expired key. The exact period is a product and provider decision; the PR should make the boundary visible and test it.

Ask for tests that can expose a second charge

A useful test drives two identical requests at the service boundary at the same time. Hold the first at the provider call until the second has passed key lookup, then release both. Assert one provider invocation, one durable payment, and the documented response for each caller. This is stronger than calling the handler twice in sequence. Use a real uniqueness constraint or a faithful test database, since an in-memory mock can hide the race.

Add a same-key, different-payload case; a retry after a completed result; a provider timeout after an accepted charge; and a crash or restart between the external call and the local result write. For the uncertain case, assert that reconciliation finds the original payment and that the retry does not charge again. Tests should inspect the side effect and stored state, not only the HTTP status. The guide to untested paths is a way to turn any missing case into a focused request.

Check verification.performed and verification.not_verified against the current PR head. Review Assist anchors changed hunks and records what the agent says it exercised. Its coverage count means counted hunks have an explanation; it does not simulate concurrent calls or certify payment safety. What a green check means explains that limit.

Make the approval condition specific

A review comment could say: “The new lookup is before the provider call, but two requests can both miss it. Please show the durable unique claim, add a concurrent same-key test that asserts one provider call, and cover a provider success followed by a local timeout. What response do we return for a changed payload and an in-progress operation?”

Approve when the key's scope and lifetime are explicit, the claim is atomic, the provider boundary is protected, and tests cover concurrent duplicates and uncertain outcomes. If the provider cannot confirm an ambiguous charge, record that gap and its operational owner instead of treating a green check as proof of exactly-once processing.