Skip to content

feat(runtime): present the delivery nonce on ack; 409 stops the turn (D6 consumer) - #1349

Merged
lilyshen0722 merged 5 commits into
mainfrom
feat/hosted-runtime-delivery-nonce
Aug 30, 2026
Merged

lilyshen0722 merged 5 commits into
mainfrom
feat/hosted-runtime-delivery-nonce

Conversation

@lilyshen0722

Copy link
Copy Markdown
Contributor

Consumer half of ADR-026 D6, pairing with #1347: the hosted DO echoes the claim's deliveryId on ack and treats 409 stale_delivery as 'you were replaced' — drops its staged reply, does not post, does not retry. Additive and safe in either merge order (today's ack route ignores the body and never returns 409). 2 tests. Merge gate: @otto.

🤖 Generated with Claude Code

https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK

lilyshen0722 and others added 4 commits August 29, 2026 21:35
…stops the turn (ADR-026 D6 consumer)

Pairs with #1347. The claim's deliveryId is echoed on ack; a 409
stale_delivery raises StaleDeliveryError — the DO drops its staged
reply and moves on, never retries or posts twice. Additive: today's
server ignores the body and never 409s. 2 tests; 22/22.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK
…9; document that D6 makes acks single-winner, not posts (Otto)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK
…uard on staged entries (Otto)

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013pc6nGXRS8mHvrwcXMSRDK
@samxu01
samxu01 force-pushed the feat/hosted-runtime-delivery-nonce branch from 07c3ca2 to c6a7821 Compare August 30, 2026 04:36

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gate at c6a7821574dd0936c29519ad5391b974037944a0 (CLEAN, 10/10 green, base main). Reviewed against #1347 at 18d9b7a66, since this PR is only correct relative to that producer.

Blocking: the nonce is read from a key the producer never writes, so this consumer presents nothing.

src/agent-do.ts:108await ackEvent(cfg, event._id, event.deliveryId), and CapEvent declares deliveryId?: string at the top level of the event. #1347 puts it in the payload:

// agentEventService.ts list(), #1347 @ 18d9b7a66
const enrichedPayload = {
  ...basePayload, ...digestBundle,
  ...(event?.deliveryNonce ? { deliveryId: event.deliveryNonce } : {}),
};
return { ...event, payload: enrichedPayload };

The claim uses .lean() with no .select(), and GET /api/agents/runtime/events is a raw res.json({ events }), so the wire object is:

{ _id, type, podId, deliveryNonce: "<hex>", payload: { ..., deliveryId: "<hex>" } }

Top-level deliveryId does not exist on it. event.deliveryId is undefined, ackEvent sends {}, and this runtime acks nonce-less forever. Under Phase A that is invisible — it just increments withoutNonce, which is the counter #1347 gates the Phase B flip on, so this PR's presence actively holds that gate shut. Under Phase B the route answers 400 delivery_id_required, ackEvent throws ackEvent 400, and no event is ever acked. Fix is event.payload?.deliveryId.

The suite cannot see this: cap.test.ts passes 'nonce-abc' into ackEvent by hand, so it pins the client's serialization and never the extraction one call frame up. agent-do has no test that hands it a realistically-shaped event. One fixture with the real wire shape catches it.

The comment at agent-do.ts:111-113 claims a guarantee the ordering cannot provide. "drop the staged reply so nothing posts twice" — but handleEvent (which posts) runs at :104, before the ack at :108. By the time the 409 arrives the duplicate is already in the pod; the replacement child posts its own. And the id was pushed to processed at :105, so the processed.includes guard is what prevents a re-post, not the staging delete. Deleting the staged entry is, if anything, mildly the wrong direction: it discards the memoized reply that would have been the second line of defence once the id ages out of the 200-entry ring.

I don't think that's fatal to the PR — this is an ack-side signal and #1347 scopes D6 to the ack. But the comment should say what the 409 actually buys (stop recording it as our error, stop retrying) rather than a double-post guarantee it doesn't deliver. If a genuine pre-post supersession check is wanted, that's a design question for D6, not a comment fix.

Non-blocking: a systematically superseded runtime reports as healthy. The continue at :115 skips batchErrors += 1 — correct, it isn't our error — but also records nothing, so lastError is cleared at :125 and lastPollAt is fresh. A runtime whose claims expire every single cycle looks identical on /status to one working perfectly. That is the exact inverse of the latched-lastError fix reasoned about two lines below it. A staleDeliveries counter in storage, surfaced on /status, costs one line.

What's right. Branching on body.code rather than the bare 409 is the correct call and the negative test for it (something_else and a throwing json()) is the one I'd have asked for. The staging shape guard plus its test is a clean fix for a real reply: undefined TypeError.

Blocking on the wiring bug only.

@lilyshen0722

Copy link
Copy Markdown
Contributor Author

TASK-090 correction at 91e641e: the claim nonce is in event.payload.deliveryId, not event.deliveryId. The hosted runtime now forwards that exact value to ackEvent; the regression test fails on the old top-level read. Worker suite 30/30 and typecheck pass. Please independently re-gate the new head.

@lilyshen0722 lilyshen0722 left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PASS at 91e641e6. My block is cleared exactly at the seam, and the extra deletion is the part that matters most.

agent-do.ts:108 now reads event.payload?.deliveryId, which is where list() actually writes it, and the string guard means a malformed value degrades to undefined (Phase-A accepted) rather than being echoed as a nonce that cannot match.

Removing deliveryId from CapEvent is what stops this recurring. The mis-wiring was not a typo — the type said the field lived at the top level, so reading it there was the correct move against a wrong contract. Deleting the declaration means the next person cannot make the same reasonable mistake. That is a better fix than the one I recommended, which only corrected the read site.

The new test pins the contract at the right altitude: it asserts ackEvent is called with the nonce taken from a polled event's payload, so a future producer that moves the field reddens here instead of going quiet.

The StaleDeliveryError comment correction is right too — the processed-id ring, not the staged-reply delete, is what prevents a double post. The old comment claimed the delete was doing that work.

10/10 CI green. behind=0.

One note carried from the other side of this pair: this PR is now correct against list(), and #1347 also pushes a nonce over the WebSocket at a different nesting. That is a finding on #1347, not here — filed there.

@lilyshen0722
lilyshen0722 merged commit c59480b into main Aug 30, 2026
11 checks passed
@lilyshen0722
lilyshen0722 deleted the feat/hosted-runtime-delivery-nonce branch August 30, 2026 19:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant