Review Agent PRs That Retry Non-Idempotent Writes

The checkout service froze after a double charge report. A Friday agent pull request had shipped a retry helper. The helper wrapped a POST that created paid orders.

Support later found two rows for one cart. This pattern shows up in agent diffs every week. The model sees a timeout and reaches for retries.

The suite stays green because tests never replay the write. Reviewers need a tighter loop than a green check. Treat the retry helper as untrusted until a replay test fails.

The scene inside the diff

The agent named the change make checkout resilient. The patch added a small withRetry utility around fetch. It caught network failures and ran the same POST again.

Comments claimed the change followed production best practice. The production handler was not idempotent at all. Each success inserted an order and captured a payment.

A retry after a slow 201 created a second capture. The UI showed one confirmation and two receipts. Finance only noticed during the next settlement window.

What to trust in the agent patch

Trust the diagnosis of a real timeout class. Network blips and slow gateways do exist. Trust a extracted helper only when the helper stays pure.

Trust tests that fail on a second insert. Trust a unique index more than a comment. Trust a stored idempotency key over a retry count.

Do not trust comments that cite resilience without a key. Do not trust a green suite with one happy path. Do not trust backoff copied from a generic blog snippet.

What to revert on sight

Revert retries around POST, PUT, PATCH, or DELETE writes. Revert helpers that recapture payment after a timeout. Revert sleeps that hide duplicate side effects in tests.

Revert changelog lines that call the handler idempotent. Keep a retry only when the operation is proven safe. Safe means the same key yields one business effect.

Safe also means tests prove the second call is a no-op. Anything else is a revert, not a nit. Write that revert reason in the review thread.

A concrete bad patch

The following TypeScript is a typical agent result. Treat it as an unexecuted example. It is a review fixture, not production code.

// unexecuted example — agent retry around a write
type CheckoutBody = {
  cartId: string;
  amountCents: number;
};

async function withRetry<T>(
  fn: () => Promise<T>,
  attempts = 3
): Promise<T> {
  let lastError: unknown;
  for (let i = 0; i < attempts; i += 1) {
    try {
      return await fn();
    } catch (err) {
      lastError = err;
      await new Promise((r) => setTimeout(r, 50 * (i + 1)));
    }
  }
  throw lastError;
}

export async function checkout(
  body: CheckoutBody
): Promise<{ orderId: string }> {
  return withRetry(async () => {
    const res = await fetch("http://orders.internal/orders", {
      method: "POST",
      headers: { "content-type": "application/json" },
      body: JSON.stringify(body),
    });
    if (!res.ok) {
      throw new Error(`checkout failed: ${res.status}`);
    }
    return res.json();
  });
}

The loop looks careful and the backoff looks adult. The POST still creates a new order on every success. A lost 201 still retries as a fresh write.

A safer shape to request

Ask the agent to send an idempotency key. Store the key with the order row. Reject a second insert for the same key.

Retry only the read of the stored result. Application checks alone lose a race. The database must enforce one row per key.

// unexecuted example — keyed write, then read on retry
import { randomUUID } from "node:crypto";

type CheckoutBody = {
  cartId: string;
  amountCents: number;
  idempotencyKey: string;
};

export async function checkout(
  body: CheckoutBody
): Promise<{ orderId: string }> {
  const key = body.idempotencyKey || randomUUID();
  const existing = await findOrderByKey(key);
  if (existing) {
    return { orderId: existing.id };
  }
  const created = await insertOrderOnce({
    cartId: body.cartId,
    amountCents: body.amountCents,
    idempotencyKey: key,
  });
  return { orderId: created.id };
}

Pair that handler with a unique constraint. Without the constraint the race still doubles charges. Reviewers should ask for the migration in the same PR.

-- unexecuted example — one business effect per key
ALTER TABLE orders
  ADD CONSTRAINT orders_idempotency_key_unique
  UNIQUE (idempotency_key);

Review workflow in numbered steps

Follow this sequence on every retry helper PR. Skip no step when money moves. Record the answers in the review comment.

1. List every HTTP method the helper wraps.
2. Mark each method as read or write.
3. Demand an idempotency key on every write.
4. Confirm a unique index exists for that key.
5. Search tests for a replay of the same body.
6. Fail the review if replay creates a second row.
7. Fail the review if payment capture runs twice.
8. Merge only after the replay test stays a no-op.

Use ripgrep before reading the story comments. Agents often hide retries one file away. The commands below keep that scan short.

git diff origin/main...HEAD --stat
rg -n "withRetry|retries|attempt" src test
rg -n "method: \"POST\"|method: 'POST'" src
rg -n "idempotency" src db

Reproducible test artifact

The next test is the actual review gate. Run it against a local fake insert path. The test should fail on the agent patch above.

// test/checkout.retry.test.js
const assert = require("node:assert/strict");
const { test } = require("node:test");

let inserts = 0;
const orders = new Map();

async function fakeInsert(body) {
  inserts += 1;
  const id = `ord_${inserts}`;
  const key = body.idempotencyKey || id;
  orders.set(key, { id, ...body });
  return { id };
}

async function checkoutWithRetry(body, attempts = 3) {
  let last;
  for (let i = 0; i < attempts; i += 1) {
    last = await fakeInsert(body);
  }
  return last;
}

test("replayed checkout must not insert twice", async () => {
  inserts = 0;
  orders.clear();
  const body = {
    cartId: "cart_1",
    amountCents: 1999,
    idempotencyKey: "idem_1",
  };
  await checkoutWithRetry(body, 3);
  assert.equal(inserts, 1);
  assert.equal(orders.size, 1);
});

Run the file with the built-in test runner. Keep the command in the review notes. A red log is the merge blocker, not a flake.

node --test test/checkout.retry.test.js

The assertion should fail until the insert path is unique. That red result is the review signal. A green suite without this case is not evidence.

Add a second case for missing keys. Agents often mint a new UUID on every retry. That habit defeats the unique index on paper.

test("retry must reuse the caller idempotency key", async () => {
  inserts = 0;
  orders.clear();
  const body = {
    cartId: "cart_2",
    amountCents: 5000,
    idempotencyKey: "idem_stable",
  };
  await checkoutWithRetry(body, 2);
  await checkoutWithRetry(body, 2);
  assert.equal(orders.size, 1);
  assert.equal([...orders.keys()][0], "idem_stable");
});

Decision table for the reviewer

Paste this table into the review comment. Agents respond better to a table than a lecture. Humans also merge faster with a binary action.

Signal in the diff	Trust	Action
Retry on GET of a stored order	Yes, timeout only	Keep with a cap
Retry on POST that inserts rows	No	Revert the helper
Comment says idempotent	No	Demand a unique index
Test hits the route once	No	Add a replay case
Unique key plus find-first	Conditional	Keep after a race test
Backoff without a key	No	Revert before merge
New UUID inside the retry loop	No	Require the client key

Free-server review loop

A small hosted loop keeps this check cheap. MonkeyCode is an open-source project with free model access and a free server option. Disclosure: This article was prepared as part of MonkeyCode's product outreach.

The operator also describes ten million free tokens for that model access. Use the server as a throwaway review box. Do not treat it as production hosting.

Do not paste live payment secrets into prompts. Do not load production dumps onto the free box. Keep fixtures synthetic and tiny.

Numbered setup for the box:

1. Clone the service onto the free server.
2. Add the replay test beside the agent patch.
3. Run node --test and save the failure log.
4. Send the diff plus the log to a free hosted model.
5. Ask only for revert lines and missing tests.
6. Re-run the suite after the model edits the patch.

A short prompt that stays in bounds looks like this. Keep the prompt boring and mechanical. The value is the failing test, not the chat.

Review this diff as a write-path change.
List HTTP methods that are not idempotent.
Name lines to revert.
Name one replay test that must fail today.
Do not add retries on POST.

git diff origin/main...HEAD > /tmp/pr.diff
node --test test/checkout.retry.test.js > /tmp/test.log 2>&1
wc -l /tmp/pr.diff /tmp/test.log

The model is a second pair of eyes on the table. Discard praise that is not backed by the replay test. Keep the unique constraint discussion in the same thread.

Limitations

This workflow does not prove end-to-end payment safety. It does not replace a ledger or a processor idempotency store. It does not model every timeout at the gateway.

The fake insert map is a local stand-in. It will miss database isolation bugs. It will miss webhook retries from the processor.

Teams still need a unique constraint and a dead-letter path. Free model output can still praise a bad retry. The test remains the source of truth.

Discard any suggestion that weakens the replay assertion. Discard any helper that retries capture itself. Discard any sleep added only to quiet CI.

Who should not use this approach

Do not use this loop for live card traffic. Do not use it as the only control in banking cores. Do not use it when the reviewer cannot read SQL constraints.

Skip the free server if the repo holds production secrets. Skip retries entirely when the write cannot be keyed. Skip agent refactors during an active incident.

Skip this article as a payment design guide. It is a review gate for agent diffs. Design still belongs with the ledger owner.

What good looks like after review

The merged handler accepts a client key. The table rejects a second row for that key. The retry, if any, only re-reads the stored order.

The suite replays the same body and still sees one insert. That is engineering on a noisy network. A retry without a key is only theater.

Treat the theater as a revert, not a polish pass. Reviewers can run the same gate on the next agent PR. The free server and free model access remain optional tools for that gate.