feat(agents): replica-safe wake claims and dones — the backend is the claim authority - #4794
Open
adityavkk wants to merge 6 commits into
Open
feat(agents): replica-safe wake claims and dones — the backend is the claim authority#4794adityavkk wants to merge 6 commits into
adityavkk wants to merge 6 commits into
Conversation
…ed session streams
…ssion streams With fencedSessionStreams on, the append path forwarded the claim's write token to the backend but still required the in-memory ClaimWriteTokenStore to know it first. That store is process memory: a claim adopted by another server instance, or by this one before a restart, was refused 401 while the backend still considered it live — the process-local write authority electric-sql#4286 describes, one layer up. In fenced mode the backend minted the token and verifies it atomically with the append, so the server now defers to it and does not consult the store on the append path; with the flag off, validation is unchanged. The token stays opaque (spec §4): nothing inspects its shape. Because a backend without the extension ignores Write-Fence and creates the stream unfenced, a client that requires fencing must check for the `Write-Fence: true` echo before treating the stream as fenced [WF-02]. Fenced creates now HEAD for the echo (the client library does not surface the PUT response) and fenced forks check the PUT response, and both fail instead of yielding a stream the fence never protects. The pin against DurableStreamTestServer flips from "silently unfenced" to "refused".
…leased
The ClaimWriteTokenStore only ever shrank on a done or a kill handled by
the same process. A server instance that received a wake's delivery but
not its claim kept the delivered token forever, and one that minted or
refreshed a claim whose done reached a sibling instance kept the claim
entry — and kept answering isValid() for it — forever.
Entries now carry an expiry of the default claim lease plus the grace a
backend keeps a write token alive past it (PROTOCOL §7 lease_ttl_ms,
30s + 5s), refreshed by every mint and by a new renew() the heartbeat
path calls when the backend answers without re-minting, so a claim this
process is the sole authority for lives as long as it heartbeats. A lazy
sweep on the paths that add entries drops what expired. The public
methods keep their signatures; the constructor takes an optional
{ ttlMs, now } for tests.
…stances With more than one server instance behind one address, the runtime's claim callback for a wake need not reach the instance that received the delivery, and its done need not reach the one that handled the claim. Three defects share that root, all in the wake-callback route: - The claim was answered locally and never reached the backend. On the instance that had not seen the delivery, takeDelivered() found nothing and the store minted a random token; with fencedSessionStreams on, the runtime's first append was refused 401, the wake failed, and its done — sent at the delivery's tail offset — acked the message away with nothing on the stream. Silent loss, not a retry. - Webhook wakes never materialised a consumer_claims row, so a done on an instance that had not minted the token found neither entityCleared nor an owning store entry and left the entity `running` (or `stopping`, from which nothing else ever leaves). - The status write was a read of entity.status followed by updateStatus(), which a done racing the next wake's delivery could clobber. A claim is now forwarded like a heartbeat: a non-done callback that renews the lease and, on a backend with the Write Fencing extension, returns the wake's write token (PROTOCOL §7.1), which the claim adopts over anything in local memory. A done carries a wake id too, so the claim path excludes it explicitly — routed as a claim, a done would be refused for a write token no backend mints for it and would never release the claim. In fenced mode a claim the backend issues no token for is refused (502 WRITE_TOKEN_UNAVAILABLE) rather than answered with a token the backend would reject; the runtime drops the wake unclaimed and the lease lapse re-wakes it. Without fencing the store is still the only write authority, so nothing is gained by holding the wake on the backend's answer: a claim it refuses, or never answers, falls back to a local mint and the wake runs exactly as it did before claims were forwarded. The delivered token and the store's own mint remain the fallback when fencing is off. Once the backend has accepted the claim it is materialised in consumer_claims (webhook wakes only — pull-wake claims already were), so materializeReleasedClaim's compare-and-clear yields entityCleared on any instance. The same insert fences a redelivered wake's second claim: an active row for the same (consumerId, epoch) answers 409 WAKE_ALREADY_CLAIMED before any token is minted, so the duplicate never claims, runs, or acks. The backend's own (generation, wake_id) fence rejects lapsed and re-armed wakes before this point, so an active row can only mean a wake another runtime is still processing. The status transition is one conditional statement — running → idle, stopping → stopped, nothing else touched — behind the unchanged `entityCleared || stillOwnsClaim` gate. It is a statement of its own rather than part of the release: the next wake can be delivered in between and set `running` again, which this write would report `idle` under until that wake's own done settles it. The done clears everything the consumer holds in the store (clearConsumer, consumer-scoped so a newer wake's entry survives), which also drops the delivered token an instance kept for a claim it never handled. A runtime refusing a forwarded wake reverts the entity to idle, as a failed forward already did, but only while it is still `running` — a rolling runtime restart refuses wakes in bulk, and an unconditional write would report a stopping entity, or one a sibling has already woken again, wrong.
…ched the stream A wake whose appends fail (a deposed or lapsed write token, a lost connection) records WRITE_FAILED and appends an error event — through the same producer, so when the failure is a dead token the error event is lost too. The done then still acked safeAckOffset, and the backend consumed the triggering message with nothing on the stream to say why no run answered it. When a producer batch fails after the error event was appended and no append has landed since, the done now carries no acks: PROTOCOL §7.1 idles the claim without moving the cursor, and the pending work re-wakes the entity — the same shape as a crash mid-wake, and a duplicate run rather than a silent loss. Batches are sent concurrently, so a batch queued before the failure can report its own error afterwards; the producer's last written offset is what separates that from the error event itself being lost, and a wake whose error event did land keeps acking as before. The ack-less done is logged as a warning, because it is a full re-run of the handler and a write failure that is not clearing turns it into a wake loop. Two smaller guards on the same path: a redelivery of a wake already in flight for the same stream and generation (the backend's retry of a delivery whose 2xx it never saw) is acknowledged without starting a second wake, which would otherwise claim the same generation — the backend accepts that while the lease is live — and run the handler twice; and a `write_token` delivered with the wake notification is adopted when the claim callback returns none, for a server that passes the field through without adopting it.
…lace Syncing a manifest entry's wake registration unregistered the entry's rows and then registered the replacement. A source event evaluated between the two statements matched nothing, and nothing ever re-evaluates it: a `runFinished` wake was lost whenever a spawned child's run completed in the few milliseconds after its parent's run wrote the child's manifest entry. The parent's end-of-run batch and the child's `run` update reach the server together, so the two race every time; forwarding claims to the backend (01825eb) moved the child by a claim round trip, enough to land its completion in that gap in a quarter to a half of runtime-dsl F1 runs. The registry now replaces the registration: the replacement is upserted first and only then are the entry's other rows removed, so an event in between matches the old row or the new one, never neither. A manifest that re-describes the registration spawn already made — the usual case — resolves to that same row through `uq_wake_registration`, and nothing is deleted or re-created at all; `include_response`, the one field outside the constraint, is adopted on the row.
✅ Deploy Preview for electric-next ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
adityavkk
marked this pull request as ready for review
September 2, 2026 21:34
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #4792 (its two commits are included in this diff until it merges; the four commits after
9e9f593are this PR).Problem
With two Agents Server instances behind one load balancer, the runtime's claim callback can land on an instance that never saw the wake's webhook.
ClaimWriteTokenStoreis process memory, so that instance mints a random token; with fenced session streams the first append is refused 401, and because the runtime had already claimed, itsdoneacks the inbox message away: silent loss of roughly half of all wakes at two instances. Adonelanding on the non-minting instance also left the entityrunningforever, because webhook mode never wrote the durable claim row the release path clears. Both are the same root cause: the process, not the backend, was the authority for the claim.Approach
{wake_id, generation, acks: []}), and thewrite_tokenthe backend re-mints on it is what the runtime receives — on any instance. In fenced mode a claim the backend does not confirm is refused (502 WRITE_TOKEN_UNAVAILABLE), never minted locally; with fencing off the local mint remains as the fallback.consumer_claims/entity_dispatch_statelike the pull path always did, so adoneon any instance clears dispatch state and settles the status through a conditionalUPDATE(stopping → stopped,running → idle, only from those states). A redelivered wake's second claim is fenced409 WAKE_ALREADY_CLAIMEDbefore the runtime acks anything; the done clears the store per consumer; a refused runtime forward revertsrunning.WebhookNotificationcarries the deliveredwrite_token; a wake whose write failure never reached the stream sends itsdonewith no acks so the backend re-wakes the message instead of consuming it silently; a redelivered wake for a(stream, generation)already in flight on the same process is not started twice.replaceByManifestKey): re-describing an existing registration upserts it in place instead of delete-then-recreate. The round trip's timing exposed a pre-existing window in which an event evaluated against no registration and its wake was lost (runtime-dslF1 flaked ~40%); the registration is now never absent, pinned by an invariant test that evaluates the event at every statement boundary.Compatibility
fencedSessionStreamson, a backend that does not returnwrite_tokenon acks now refuses the claim instead of letting the runtime run with a token the backend will reject.PostgresRegistry.materializeActiveClaimreturnsbooleanand acceptsunlessActive; newupdateStatusAfterDoneandrevertRunningStatus;ClaimWriteTokenStoregains an options object andrenew();WakeRegistry.replaceByManifestKey.Verification
Two instances behind one proxy were exercised end to end against a backend implementing the Write Fencing extension (wake on A / claim on B, claim on A / done on B), passing after this change and failing before it.