[vishkaty]

Concurrent requests that share one Idempotency-Key each persist their own checkout and each
answer 201, so the key names one of several stored checkouts rather than a single result, and a
later retry of the key replays whichever request wrote last. Expected: one stored checkout per
key, every identical retry answered with that one result, and a retry carrying different
parameters answered 409.

The text. checkout-rest.md, Specific Header Requirements, lines 1241 to 1245: when a key
is provided the server MUST store it with the operation result, MUST return the cached
result for duplicate keys, and MUST return 409 Conflict if the key is reused with different
parameters. That is the wording at a25a4a24, the commit the v2026-04-08 tag named before it
was moved; at 9c9d6fce, the commit it names today and the head of release/2026-04-08 since
2026-10-22, items 2 and 3 say the same of a duplicate key whose request body matches the
original and of a mismatched body, and link to the Idempotency Key Requirements table in
signatures.md (lines 826 to 835 there): on a duplicate with a matching payload return the
cached response and do not re-execute, on a duplicate with a mismatched payload reject with
409 Conflict. Neither file uses the word concurrent at either commit. Measured by the state
the server is left in, both items fail: eight requests carrying different parameters under one
key all answered 201 and none answered 409, and eight requests carrying one body under one key
each executed and each stored a checkout, which the signatures.md row forbids in as many
words at both commits (a25a4a24 line 485, 9c9d6fce line 833: return the cached response,
do not re-execute). Measured request by request, both items need a reading, because neither
text says what the cached result or the first use is when the requests are simultaneous; if
the intent is that only sequential reuse is covered, this is a gap in the reference rather than
a conformance defect, and the right outcome is to say so. The server declares 2026-04-08
(src/utils/config.ts:15).

The window, on main fe191a3f. createCheckout reads the idempotency record
(src/api/checkout.ts:637), then awaits the agent profile fetch (:721, the fetch at :138
inside parseAgentProfile), then writes: saveCheckout at :764, saveIdempotencyRecord at
:767. updateCheckout has the same shape: read at :811, await at :852, saveCheckout at
:916, saveIdempotencyRecord at :919. The fetch is the only await between the read and the
write in either handler, so the window is reachable over the wire whenever the agent sends a
profile over http, which is the required and documented form of the header: checkout-rest.md
lines 1237 to 1240, UCP-Agent MUST be present carrying the platform profile URI as
profile="https://platform.example/profile", and overview.md, Business Requirements, item 1,
businesses MUST fetch and validate the platform profile unless already cached. The record
statement is INSERT OR REPLACE (src/data/transactions.ts:162) on key TEXT PRIMARY KEY
(src/data/db.ts:77), so every racer claims the key in turn and none is refused, while each has
already stored a checkout of its own. Neither handler changed between e7c9c124, the base of the
first head of this branch, and fe191a3f: #258 changed completeCheckout and the import
block only.

Observed over a socket against the reference booted from a fresh clone of main fe191a3f
(node v22.14.0, with src/index.ts:177 reading the listen port from the environment and nothing
else changed), eight concurrent creates under one key with identical bodies and a profile served
by a local stub that answers once all eight have asked: eight checkouts persist, eight 201
responses return naming eight different ids, and a GET on each id answers 200, in three trials
of three. With differing bodies: eight 201 responses, eight checkouts, no 409. Two controls on
the same server do not race: with no UCP-Agent header, and with a data: profile, which
parseAgentProfile decodes without a fetch (:131 to :136), identical bodies give one
checkout and differing bodies give one 201 and seven 409. updateCheckout behaves the same way:
eight handler executions (rows in request_logs) and eight 200 responses with the http profile,
one execution and 1x200 plus 7x409 in both controls. The same measurement on e7c9c124 gave the
same figures.

Why the suite did not catch it. No existing test issues two requests under one key
concurrently; the idempotency tests are sequential, and sequentially this path is correct.

The change. A new claimIdempotencyAndSaveCheckout in src/data/transactions.ts claims the
key with INSERT ... ON CONFLICT(key) DO NOTHING and stores the checkout inside one
transaction, through the runInTransaction helper that #258 added, so either both happen or
neither does. saveIdempotencyRecord now reports whether it inserted; a caller that did not
insert lost the race and answers with the recorded response, or 409 when the request hashes
differ, through one replayIdempotent helper that the two handlers also use for the read at the
top. Both writes are synchronous, so unlike the read at the top of the handler they can share a
transaction.

The transaction matters for a second reason. An earlier version of this change claimed the key
before storing the checkout without a transaction, and that left a record behind for a checkout
that was never stored: a retry answered 201 from the record while a direct read of the checkout
answered 404. One of the tests below covers precisely that; it passes on the unfixed tree, so it
guards the fix rather than the original fault.

completeCheckout and cancelCheckout call the same saveIdempotencyRecord. Neither has an
await between its idempotency read and its record write, so neither reaches the changed
statement with the key already claimed, and their answers to a race are the same with and
without this change. In completeCheckout the read is at :947 and, since #258, the record is
written inside the completion transaction (:1171 to :1205, the record at :1198) before the
webhook delivery at :1209, with the status check at :982 to :991 between the two. In
cancelCheckout the read is at :1233, saveCheckout at :1270 and the record at :1273,
with no transaction around the two writes; this change wraps them in runInTransaction the way
completeCheckout does, so a storage failure between the two cannot leave a canceled checkout
whose key was never recorded, which on main answers every retry of that key 409
CHECKOUT_NOT_MODIFIABLE (Cannot cancel a canceled checkout session) instead of replaying the
cancel. Measured over the wire on the same two servers, eight concurrent identical completes of
one ready checkout under one key, with a dead webhook, give 8x200, one handler execution and one
order, in two trials each on main and on this branch. Two different checkouts completed
concurrently under one key, measured in process with the fetch mocked to hold the delivery open,
give 1x200 and 1x409 IDEMPOTENCY_CONFLICT with the record naming the first, on main and on
this branch alike. Both figures come from #258, which records the key before delivering the
webhook as the Python reference does (rest/python/server/services/checkout_service.py:938,
then :949). On e7c9c124 the same eight completes gave 1x200 and 7x409
CHECKOUT_NOT_MODIFIABLE with eight executions, and two checkouts under one key both completed
with the record naming the last writer.

One adjacent window is deliberately not touched: the not modifiable check in updateCheckout
(:836 to :846) also sits before the same profile fetch, so an update held at that fetch while
a complete lands on the same checkout stores the update over the completed checkout (measured in
process on main fe191a3f and on this branch: the complete answers 200 with status completed,
the released update answers 200, and the stored checkout afterwards has status
ready_for_complete and no order). That is not an idempotency question, it is the same before
and after this change, and it is noted so the scope is explicit.

Tests. test/idempotency_race.test.ts, five cases in the existing node:test idiom with in
memory databases. Three hold the mocked profile fetch open with a counter gate until every racer
has reached it: identical concurrent creates must name one checkout; differing concurrent creates
must give one 201 and the rest 409; the same for updates. Every 409 in those two cases must carry
messages[0].code IDEMPOTENCY_CONFLICT, since the server answers CHECKOUT_NOT_MODIFIABLE
and OUT_OF_STOCK with 409 too. The fourth plants a storage failure with a trigger on
checkouts and checks that a key is never recorded for a checkout that was not stored; the fifth
plants the same failure on idempotency_keys and checks that a cancel whose key could not be
recorded leaves the checkout open, so the retry cancels it and answers 200. Against main
fe191a3f with the test file added the first three fail, reporting eight checkouts where one is
required, 8x201 where 1x201, 7x409 is expected, and 8x200 where 1x200, 7x409 is
expected; the fourth passes; the fifth fails with the retry answered 409
CHECKOUT_NOT_MODIFIABLE. With the change all five pass. Eleven variants of the change were
then run with the whole suite kept, and each fails at least one of the five: INSERT OR REPLACE restored, the insert count ignored, and the checkout stored before the key is claimed
each fail the three race cases; the hash check removed fails the two differing cases; the hash
check inverted fails all three race cases; the winner ignored in create fails the two create
cases; the winner ignored in update fails the update case; no transaction around the claim fails
only the fourth case; no transaction around the cancel writes fails only the fifth; a lost race
answered with a 409 of another code, and the conflict code swapped in replayIdempotent, each
fail the two differing cases. The two hash check variants also fail three and five of the
existing sequential idempotency tests, and the swapped code fails the sequential envelope test.
Before the code assertion was added, the lost race variant passed the whole suite as it stood,
175 from main plus the four race cases, 179 of 179.

Checks run locally on this branch. npm run build passes. npm test: 180 pass, none
skipped, on node 20.18.0, 22.14.0 and 24.22.1, against 175 on main fe191a3f under each. The
wire figures for this branch above were taken before the review changes were applied, on a
tree whose only difference under src/ from this head is the cancelCheckout transaction,
so the create, update and complete handlers measured are byte identical to the ones here. The
pinned pre-commit run --all-files passes all twelve hooks. The wire measurement above against
the patched server gives one checkout and 8x201 for identical bodies and 1x201 plus 7x409 for
differing bodies with the http profile, one checkout and 1x200 plus 7x409 for the update, and
the controls unchanged. One note on the node 20 leg that concerns the machine and not the
change: better-sqlite3 12.11.1 publishes prebuilt bindings for node ABIs 127, 137, 141 and 147
and none for ABI 115, so node 20 compiles the binding from source everywhere. The workflow job
did that on ubuntu for 341c2a3d, the first head of this branch, with plain npm ci and
passed. On macOS with the Command Line Tools alone it needed npm ci --ignore-scripts, then
node-gyp rebuild --release in node_modules/better-sqlite3 using node-gyp 11.0.0 with its
Xcode version lookup short circuited (one line in gyp/pylib/gyp/xcode_emulation.py) and
CXXFLAGS="-isystem <SDK>/usr/include/c++/v1", then npm rebuild esbuild fsevents; the
node-gyp 10.1.0 bundled with that npm stops at the Xcode version check. The change relies on
runInTransaction, which wraps db.transaction, and on run().changes from better-sqlite3,
on ON CONFLICT DO NOTHING from the SQLite the package bundles (3.53.2), and the test on
mock.method, t.after and the global fetch and Response.

Review. Of the three suggestions on the first head, 341c2a3d, two are taken here and one
is left for a follow up. The 409 code assertion closes a real hole: a variant that answers a
lost race with a 409 of another code passed the whole suite before it, 179 of 179, and fails
the two differing cases with it. The backstop timer in gateProfileFetch now has unref() called on it:
measured on this file with a planted failure that leaves the timer uncleared, the file finishes
in 361 ms instead of 2562 ms; in the case the timer exists for, a racer that never reaches the
fetch, the runner now reports the pending test as soon as the loop drains instead of waiting
for the timer, still red. cancelCheckout now pairs its two writes in a transaction, since #258
has merged and the reason for leaving the completion side separate is gone; the handler keeps
saveCheckout then saveIdempotencyRecord inside runInTransaction as completeCheckout
does, rather than the claim helper, because it has no await in which a claim could lose. The
inline request_hash checks at the top of completeCheckout and cancelCheckout are left as
they are: routing them through replayIdempotent, and reading response_status from the
record, changes no behaviour, and this diff stays the one that was approved plus the two
behavioural points above; a follow up can do that refactor on its own.

Relationship to other work. This is the nodejs counterpart of the python race fixed in #245,
merged 2026-10-19, which takes the same first writer wins contract; the two share no files. This
branch is based on main at fe191a3f, after #258 merged on 2026-10-29 as 93a6e71e, and adds
replayIdempotent beside the two helpers #253 added. #258 put a runInTransaction helper in
transactions.ts, and claimIdempotencyAndSaveCheckout now calls it. The helper is
getTransactionsDb().transaction(fn)(), the statement the first head 341c2a3d wrote in
place, so the swap changes no behaviour. #258 also moved the completion writes and the completion
idempotency record into one transaction, which is the alignment with the Python reference noted
above. #264, now also rebased onto fe191a3f, changes parseAgentProfile in the same
checkout.ts (it drops a profile webhook_url that is not an http or https URL without
credentials) and adds test/agent_profile.test.ts; it touches neither handler changed here nor
transactions.ts. A three way merge in either order gives the same tree, and the combined suite
passes, 191 of 191.

[damaz91]

Thank you @vishkaty for the exceptionally thorough write-up, wire reproduction notes, and clean fix!

Claiming the Idempotency-Key with INSERT ... ON CONFLICT(key) DO NOTHING and persisting the checkout inside a single synchronous better-sqlite3 transaction (claimIdempotencyAndSaveCheckout) cleanly closes the UCP-Agent profile-fetch race window in createCheckout and updateCheckout without holding locks across network I/O or risking orphaned idempotency records when storage writes fail. Replaying via replayIdempotent ensures concurrent identical retries receive the exact persisted checkout representation while concurrent differing payloads receive 409 (IDEMPOTENCY_CONFLICT), bringing the Node.js sample in line with #245. The barrier-gated tests in test/idempotency_race.test.ts are deterministic and well-crafted.

A few small, non-blocking suggestions for optional polish or follow-up:

• replayIdempotent reuse: completeCheckout and cancelCheckout still inline the duplicate request_hash check at the top of each handler — they could share this.replayIdempotent(...) as well (and replayIdempotent could optionally read record.response_status directly rather than taking status separately).
• cancelCheckout / completeCheckout write parity: cancelCheckout has no await between its read and write, though calling claimIdempotencyAndSaveCheckout there too would give it the same single-transaction guarantee on storage failure. (Agreed with leaving completeCheckout's pre-webhook transaction alignment separate so it composes cleanly with #258.)
• Test assertions: In test/idempotency_race.test.ts, calling .unref() on the backstop timer in gateProfileFetch avoids keeping the event loop open if a future change rejects a request before fetch, and asserting messages[0].code === "IDEMPOTENCY_CONFLICT" on the 409 responses would guard against another 409 error code masking an idempotency conflict.

LGTM!

[vishkaty]

Thanks for the review. Two of the three suggestions are in the pushed head and one is left for a follow up, so the diff stays the one you approved plus the two behavioural points.

Test assertions: both taken. Asserting messages[0].code === "IDEMPOTENCY_CONFLICT" on the 409s closes a real hole: a variant that answers a lost race with a CHECKOUT_NOT_MODIFIABLE 409 passed the whole suite before the assertion, 179 of 179, which was the 175 from main plus the four race cases at that point, and fails the two differing cases with it. The backstop timer now has unref() called on it; measured on this file with a planted failure that leaves the timer uncleared, the file finishes in 361 ms instead of 2562 ms, and in the case the timer exists for, a racer that never reaches fetch, the runner reports the pending test as soon as the loop drains, still red.

Write parity: taken for cancelCheckout, since #258 has merged and the reason for keeping the completion side separate is gone. The handler now runs saveCheckout and saveIdempotencyRecord inside runInTransaction, the shape completeCheckout has on main, rather than through the claim helper, because it has no await in which a claim could lose. A fifth test plants a storage failure on idempotency_keys during a cancel: on main the checkout is canceled without its key and the retry answers 409 CHECKOUT_NOT_MODIFIABLE; with the transaction the failed attempt leaves the checkout open and the retry cancels it.

replayIdempotent reuse in completeCheckout and cancelCheckout, and reading response_status from the record: left out of this head on purpose. Both are refactors that change no behaviour, and I would rather not widen an approved diff for them; happy to open a follow up with just that once this is in.

The branch is rebased from e7c9c124 onto fe191a3f and calls the runInTransaction helper #258 added. Eleven variants of the change each fail at least one of the five tests; 180 pass on node 20, 22 and 24 against 175 on main; the description is updated with the rebase, the cancel change and the figures.