feat(world): commit-ordered event positions (event_{seq}) for world-postgres and world-local - #3269
Draft
pranaygp wants to merge 29 commits into
Draft
feat(world): commit-ordered event positions (event_{seq}) for world-postgres and world-local#3269pranaygp wants to merge 29 commits into
pranaygp wants to merge 29 commits into
Conversation
…process A replay-context event creation previously described its snapshot with a single watermark, which only proves no event landed above it. It cannot detect a *missing* event below it, so a replay working from a log with a hole still committed events derived from that hole — and because correlation IDs are positional ordinals of one seeded sequence, a one-event difference renames every downstream entity and corrupts the log. Creations now also send the snapshot's event count and its cursor, and a rejection restarts the replay inside the same invocation instead of re-posting the rejected payload (whose IDs the corrected log invalidates) or paying a queue round trip. A world may attach the missing events to its 412, in which case the first restart needs no event-log request. Also guards the suspension `attr_set` write, and re-sorts a merged event log by event ID when an append arrives out of order. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Also makes the v4 event tests derive their mock origin from the override like the rest of the file already does, so a non-empty override does not fail unit tests. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The matching world-vercel guard shipped and is live in production, so the e2e lanes exercise both halves against the default endpoint. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Resolves the overlap with #3110, which introduced the same event-log merge consolidation this branch had added as `mergeEvents`: `appendUniqueEvents` now carries the optional id set from main plus the out-of-order re-sort and warning, and `mergeEvents` is gone. Main's `withPreconditionRetry` edit drops out with the function itself. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The event-log merge no longer re-sorts by event id. A World's canonical order is its own: world-vercel orders by event id, but world-local orders by (createdAt, eventId) and deliberately re-mints keys (dominant-event and claim canonicalization) so the two diverge. Re-sorting by event id there produced an order no ordered load would ever return, reordering a terminal event ahead of an accepted hook and breaking concurrent hook-token arbitration. The merge was only sorting so the snapshot could read its watermark off the tail, so read the maximum ULID time across the log instead. That removes the ordering dependency entirely and is exact rather than merely safe: every loaded event is at or below the maximum, so stateEventCount is still events.length whatever order the World returned. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…p/autoincrement-event-sort-ids * origin/peter/event-count-guard: Derive the precondition watermark from the log's maximum, not its tail Remove the temporary backend URL override Temporarily point CI at the backend branch preview Gate event creation on the loaded event count and restart replays in-process
…tirement Four failing tests against the barrier primitives in private.ts: - the idle safety net (private.ts:461) retires a barrier whose delivery is still parked inside awaitEarlierDeliveries, because pendingDeliveries only counts the hydration window (released at step.ts:322, before the detached continuation at step.ts:324); - one idle tick retires EVERY live barrier, not just abandoned ones; - a later-in-log delivery consequently computes an empty deferral set, skips the macrotask yield at private.ts:392, and is handed to workflow code before an earlier-in-log one; - the same inversion without the idle net at all: markDelivered() runs one statement before resolve(), so the registry stops mentioning a delivery while the branch it woke is still hops away from its next useStep. Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
End-to-end reproduction on current main, from the event log alone — no artificial hydration latency, no shared ReplayPayloadCache, failing at every consumer hop count from 0 to 16 with the production error shape: Replay divergence: step event step_created for step_<ULID> belongs to "afterHook", but the current step consumer is "afterStep" An unclaimed buffered hook payload makes resolvesOnItsOwn (private.ts:296-319) report false for itself, and transitively for the armed wait barrier that defers behind it. A later step result therefore skips BOTH via the kind === 'step' escape at private.ts:368-371, computes an empty deferral set, and resolves on microtasks while the wait is still parked — so the step branch draws the ULID the log assigns to the hook branch. Verified mechanism: neutralising that skip makes all six cases pass (and keeps the 72 existing delivery-ordering assertions green), so the skip is the cause. No existing suite covers this because every hook case in step-delivery-ordering / step-delivery-hop-count / delivery-barrier-coverage registers its awaiter before the drain, taking the armed path in hook.ts rather than claim(). Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
…ivery traffic Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
The 12 tests that fail on main are right-reason failures documenting the delivery-barrier idle collapse/starvation windows and the buffered-hook claim-ordering race. Mark them it.fails so the suites merge as executable documentation (the #3137 -> #3139 convention); the fix PR flips the markers back to it. The 4 controls that pass on main keep plain it. Also adds an empty changeset (test-only change). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
…le writers Events gain an optional dense per-run seq (EventSchema.seq) assigned at the commit point. world-postgres serializes every append behind a run-scoped advisory lock taken as the first lock of a single transaction that spans the entity mutation and the event insert; event ids are re-minted inside that critical section to dominate the run's tail, so seq order == id order == commit order == visibility order and a cursor reader can never skip a late-committing event (the CORRUPTED_EVENT_LOG storage class). Creates that carry the precondition snapshot (stateEventCount/stateCursor, from the merged event-count-guard SDK machinery) are checked against a transactional currency fence: a snapshot the log has moved past is rejected with 412 and the whole transaction — entity mutation included — rolls back, so a replay derived from a superseded view can never commit a decision. Sibling creates of one suspension share a snapshot and are credited through (writerSnapshot/writerBaseCount on the run row) instead of fencing each other. The runtime asserts seq contiguity on every event load so any remaining hole fails loudly instead of surfacing as a replay divergence hundreds of events later. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The tail-based currency fence livelocked runs under steady inbound load: every hook_received landing between a replay's load and its next write 412'd the write and forced a full replay restart, and the facts kept coming (measured ~41 restarts/run in the step-storm repro; every attempt timed out as stuck while zero corrupted). Mirror Temporal's buffered-events model instead: facts (creates without a precondition snapshot) get a commit-ordered position but never invalidate a decision; only a foreign *decision* — a snapshot-carrying create from a different snapshot — fences. The run row tracks lastFencedSeq (the last decision's position); a fenced create is rejected iff a foreign decision sits past its snapshot and the sibling credit doesn't match. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Storm forensics (fence_count audit column) attributed every corrupted run to a credit hole, not engine nondeterminism: two invocations that loaded the identical prefix present byte-identical stateCursor+stateEventCount, so the snapshot-keyed credit admitted both writers' decision batches and their interleaved sets baked an order no replay derives (correlation ordinals inverted against commit order at seqs the writers' fence_counts prove they never saw). The runtime now mints a random writerId per invocation delivery and sends it with every precondition snapshot; the world's credit compares writerId (falling back to stateCursor only for runtimes that don't send one). A second writer with the same snapshot 412s and restarts against the corrected log. Also adds the fence_count forensics column and a WORKFLOW_POSTGRES_EVENT_FENCE=tail|decision experiment switch. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Three defects in the delivery-barrier registry could hand branch-deciding deliveries to workflow code out of event-log order, surfacing as CORRUPTED_EVENT_LOG on replay. All three are fixed here, and the twelve expected-fail repros that documented them are flipped back to plain `it`. Barriers no longer retire on a global idle tick. #3198 stopped the idle CHECK from observing idle while a committed delivery is parked; this stops the same predicate from retiring the barriers themselves. `pendingDeliveries` tracks only the host-side hydration window — released inside the promiseQueue slot, before the detached continuation that hands the value over, and never touched at all by a wait_completed — so one idle tick used to retire every live barrier while their deliveries were still parked in awaitEarlierDeliveries. Retirement moves to its own poll, which retires only an UNARMED barrier: an unclaimed buffered hook payload, the one delivery that can be abandoned at the root. Every other kind is committed and retires from its own chain. Both polls get a deadline. Without one neither has any escape from continuous unrelated traffic: an abandoned barrier under a stream of deliveries to a never-read hook starved indefinitely, observed live as a run making no progress for 3m19s across 228 pokes. The barrier deadline is counted in raw ticks, so the traffic keeping the system busy cannot stretch it; scheduleWhenIdle's is counted in poll rounds, each of which waits out a full promiseQueue drain, because that function also schedules suspensions, where firing early preempts data delivery. A retired delivery stays visible to the registry for one more macrotask. markDelivered() resolved a barrier one statement before the resolve() that wakes the branch, so anything reading the registry in between — a delivery consumed in a later drain window, or a buffered payload's claim() — computed an empty deferral set and overtook the branch it was meant to follow. The live registry still drops the entry immediately; a second short-lived map carries ordering visibility across the gap. A step result's buffered-hook skip is narrowed to the payload itself. A step still never gates on an unclaimed payload, where the claim commonly sits downstream of the step result, but it now gates on an earlier armed wait or hook whatever that delivery is itself waiting for. Skipping those as well, via the transitive resolvesOnItsOwn walk, was the buffered-claim corruption: in Promise.all([step, sleep-then-read-hook]) the wait completion is precisely what wakes the branch that goes on to claim the payload. awaitEarlierDeliveries no longer consults that walk, leaving the per-delivery path a single linear pass; the walk itself survives only in hasParkedCommittedDelivery, which gates suspensions rather than deliveries. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
…nt-sort-ids * origin/main: [benchmarks] Split STSO by inline vs queue-hop steps, add distribution diffs vs main (#3213) feat(core): emit faas.instance span attribute for compute instance identity (#2989) [world-local] Retry transient EPERM unlink failures on Windows (#3215) [world-vercel] Raise H2 receive windows on the events agent (#3212) Version Packages (beta) (#3185) Align Streams UI with trace viewer (#3197) fix(core): don't observe idle while a committed delivery is parked behind its deferral (#3198) [world-vercel] Make HTTP/2 actually multiplex on the events path (#3190) [RFC] feat(nitro): embed observability dashboard in-process at /_workflow (#2548)
…rked deliveries as in-flight Follow-up to the previous commit, from review of #3196 and from an e2e regression that review work exposed. Retirement by the safety net is now UNARMED-only. The abandon deadline used to fire for any barrier, which reintroduced the collapse at a longer timescale: production hydration (object-storage fetch plus decrypt) runs 10-500ms, far past any tick budget worth setting, so the slowest deliveries would have been exactly the ones force-retired mid-flight. Only an unclaimed buffered hook payload — the one delivery that can be abandoned at the root — is retirable by the net; every other barrier leaves the registry from its own delivery chain. All five call sites were audited against that invariant: step_completed, step_failed, wait_completed, the waiting-consumer and claim() hook paths, and the abort hook all attach their chain unconditionally. Deliveries parked in awaitEarlierDeliveries are now counted, and the count gates scheduleWhenIdle. A delivery whose only remaining gate is a recentlyDeliveredBarriers entry is invisible to both pendingDeliveries (already released in its hydration slot) and to a scan of the live registry (already empty), so a suspension armed in that window preempted it and the run suspended carrying none of the work the delivery was about to create. That is how the second payload of the hookWithSleepWorkflow e2e went missing. Guarded by a unit test that fails without the counter. Review fixes: - Starvation suite: the poke storm held pendingDeliveries above zero with a single unmatched increment, so it modelled a stuck hydration rather than overlapping traffic. It now overlaps genuinely, leaks nothing, and asserts both properties. The header no longer attributes the observed 3m19s stall to this mechanism, which the suite does not establish. - Added the liveness guard asked for in review: 228 unclaimed payloads gating a later delivery still drain in a handful of ticks. - Added the mirrored-log control asked for in review: the interleaving where the step result legitimately precedes the wait completion must keep replaying cleanly, so the ordering assertions cannot be satisfied by flipping the bias. - Below-watermark suite: the read shape is not a world-postgres class. world-vercel paginates DynamoDB by event ULID with an `eid:` cursor over a strictly-after range, and workflow-server documents its IDs as monotonic only intra-instance. Recorded in the header; the second test is renamed to say it characterizes current behavior rather than pinning a contract. - Call-site comments that documented idle retirement of parked barriers as a deliberate tradeoff now describe what the net actually does. - Stale private.ts / step.ts line references replaced with symbol names. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
…imed An unarmed delivery barrier is retired on the inference that no consumer has claimed the payload. That inference is not available until a consumer could have: `claim()` cannot hand anything over until the payload's own hydration resolves, so while it is still hydrating, "nobody has claimed it" carries no information. Left unguarded, the two retirement conditions conspire during that window. The payload's own hydration holds `pendingDeliveries` above zero, so the idle route cannot fire, and the abandon deadline retires the barrier because its hydration was slow. Against a production hydration (object storage fetch plus decrypt, 10-500ms) and a deadline of a few dozen `setTimeout(0)` ticks, that is the common case for a buffered payload rather than an edge one. `registerDeliveryBarrier` now takes `abandonableAfter` and does not start the retirement poll until it settles; `workflow/hook.ts` passes the buffered payload's own hydration. The waiting-consumer path is unaffected: it registers armed, and an armed barrier leaves the poll on its first check either way. This is the same slow-hydration trap b58fdcf had on the armed side, found by re-auditing the deadline against production latency after that commit's storm run. The regression test holds hydration open well past the deadline and asserts both that the barrier survives and that a later step result still orders behind the claim. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
Counted flat, `IDLE_POLL_DEADLINE_ROUNDS` is not a deadline but a cap on how many events one drain window may deliver. Delivering a window in log order costs one macrotask per deferring delivery, so a window of K events needs K poll rounds to clear. Past the budget `scheduleWhenIdle` fires anyway, into the middle of the batch — and for a hook consumer that callback raises a `WorkflowSuspension`, so the run suspends carrying none of the work the remaining deliveries were about to create. A new test in `delivery-barrier-idle-starvation.test.ts` puts 40 deliveries in one window and shows the callback landing after exactly 16 of them. The budget now restarts whenever a delivery reaches workflow code, tracked as `deliveryProgress` and incremented by `markDelivered()` only. Excluding the abandonment safety net's retirements is what keeps the escape hatch working: a stream of pokes to a hook nobody reads retires barrier after barrier and delivers nothing, so the budget still runs out and the suspension still fires. That is the 3m19s stall the deadline was added for, and its test still passes. Ordering machinery that slows delivery down is only safe if the liveness timers around it measure progress rather than elapsed rounds; this is the one that did not. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Pranay Prakash <pranay.gp@gmail.com>
* pgp/autoincrement-event-sort-ids: Key the fence's sibling credit on a per-invocation writerId Fence decisions on foreign decisions, not on facts Assign commit-ordered event positions in world-postgres and fence stale writers Derive the precondition watermark from the log's maximum, not its tail Remove the temporary backend URL override Temporarily point CI at the backend branch preview Gate event creation on the loaded event count and restart replays in-process # Conflicts: # packages/core/src/runtime/helpers.test.ts # packages/core/src/runtime/step-executor.ts # packages/world-vercel/src/events.ts
…sts' into pgp/event-seq * origin/pgp/replay-engine-determinism-tests: fix(core): reset the idle poll's budget on delivery progress fix(core): do not start the abandon clock before a payload can be claimed fix(core): never retire a committed delivery on a timer, and count parked deliveries as in-flight fix(core): make delivery-barrier retirement deterministic test(core): mark known-failing determinism repros as expected failures test(core): reproduce delivery-barrier starvation under unrelated delivery traffic test(core): characterize divergence from a below-watermark event test(core): reproduce ReplayDivergenceError from a buffered hook payload test(core): reproduce delivery-barrier idle collapse and premature retirement
…s append lock Assign every event a dense per-run seq and a tail-dominant event key at its publish point, under an on-disk per-run append serializer (the filesystem analogue of world-postgres's advisory transaction lock), and enforce the stateEventCount decision fence under the same lock. Extend the SDK's contiguity assertion to the pre-replay convergence point so the inline write-response delta and run_started preload are covered too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ffect A fenced create whose snapshot is stale was rejected at the publish point — after entity writes, claim files, and the terminal marker had already landed. Unlike postgres, those writes cannot roll back: a rejected step_created stranded a step entity whose step executes with no step_created in the log (an unreplayable completion), and a rejected run_completed left the terminal marker behind, rejecting every later hook resume. Admit or reject the session while the append is still a pure no-op. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The run_started preload and the step-terminal inline delta are events-directory scans whose latency grows with the directory; holding the per-run append serializer across them starved the run's other writers and, under load, tripped the stale-lock breaker on a healthy holder — minting duplicate positions. Attach both read pages after the lock releases (their point-in-time semantics are unchanged), and err the stale threshold far to the late side: breaking early corrupts, breaking late merely stalls a crashed run's writers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add isDecisionEvent and gate the stateEventCount fence on it in world-local and world-postgres: facts (completions, receipts, non-lazy step_started claims) pass unfenced and never bump the fence even when the runtime attaches a snapshot. A stale fact is byte-identical to a fresh one and takes its meaning from its commit-assigned log position, so fencing it prevents nothing and converts steady traffic into replay-restart churn (measured: 776 wait_completed + 1107 step_started rejections in one 48-run storm). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Contributor
🦋 Changeset detectedLatest commit: 94943d2 The changes in this PR will be included in the next version bump. This PR includes changesets to release 21 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
VaguelySerious
removed
the
event-log-race-repro
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
2 participants
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.
Summary
Implements #3201 (
event_{seq}: commit-assigned per-run sequence as canonical log order, with fenced appends) for world-postgres and world-local, plus the SDK reader/fence half, and validates it by running the event-log-race-repro storm locally against both worlds, seq branch vsmain(32ac8e7).Headline: on world-postgres the storm's corruption collapses — hook-storm (the production shape) goes 5 corrupted → 0 corrupted with 24/24 completing, and step-storm goes 20 corrupted / 4 completed → 2 corrupted / 22 completed. On world-local the storage-ordering class is verifiably gone (invariant sweeps below) and hook-storm corruption halves, but the engine wake-order class #3201 explicitly scopes out still dominates there — the in-process queue makes world-local the harshest concurrent-replay amplifier.
This branch combines:
seqcontiguity assertion),pgp/replay-engine-determinism-tests),Storm matrix
24 step-storm + 24 hook-storm attempts per condition, concurrency 6, one machine,
nextjs-turbopackproduction server. "slow" = alive and appending but past the harness's 240s per-run cap (reported asstuck), not wedged.¹ main-postgres landed 40/48 before the launch budget cut it; rates are per-landed.
Storage invariants under storm (seq branch):
seq, 0 gaps/duplicates, 0 id-order-vs-seq violationsEvery corrupted run on the seq branch has a perfectly ordered, dense log: the residual is not a storage problem.
What's new on top of the two merged branches
world-local commit-ordered appends
.locks/runs/{runId}.append.lock, exclusive-create; in-process promise chain in front; stale-break with log rescan).seqand a tail-dominant event key are allocated at the publish point, so(createdAt, eventId)order == filename-ULID order == seq order == commit order == visibility order. Cursor readers can no longer skip a late-committing event.stateEventCountdecision fence is enforced under the same lock, before any side effect.mintRunDominantEventKey(the terminal-event re-mint) is deleted — publish-point allocation under the lock dominates by construction.SDK
assertEventSequenceContiguityis re-asserted at the pre-runWorkflowconvergence point, covering the two append paths that bypass the paginated loader (inline write-response delta,run_startedpreload). A World ordering bug now fails fast asWORLD_CONTRACT_ERRORinstead of surfacing three replays later as corruption — this gate caught two real implementation bugs during this work, both within minutes.Fence semantics: fence only decision writes
isDecisionEventin@workflow/world; both worlds fence/bump decisions only (child-entity creations incl. lazy step_started, terminal transitions, attr_set). Facts (wait_completed,step_completed,hook_received, non-lazystep_startedclaims) pass unfenced even when the runtime attaches a snapshot: a stale fact is byte-identical to a fresh one and takes its meaning from its commit-assigned position. Fencing facts prevented nothing and converted steady traffic into replay-restart churn (measured: 776wait_completed+ 1,107step_startedrejections in one 48-run storm). This also matches the vercel-world server contract, wherewait_completedis guarded but never bumps the marker.Hard-won implementation gotchas (additions to #3201's list)
step_createdever entering the log — an unreplayable completion) and, worse, a 412'drun_completedleft the terminal marker behind, rejecting every later hook resume and wedging the run.run_startedpreload / inline-delta scans starved writers and tripped the stale-lock breaker on a healthy holder — minting duplicate positions. (The contiguity gate caught this immediately asseq 36 followed by 36.)The residual class, isolated
With storage ordering proven clean, the remaining CORRUPTED_EVENT_LOG failures have one signature:
Replay could not consume event: step_created(...). Forensics on a live capture: two invocations with complete views of the same log derived different ULID-draw assignments (one writer'sfinalizeStepordinals at…KWSS, the other's at…KWSX+— a 4-draw skew) and both flushed, interleaving two derivations into one perfectly-ordered log. The fence cannot reject either writer — neither is stale. This is precisely #3201's "What this does NOT fix": wake-order nondeterminism within a replay, which needs #3179 (call-site correlation ids) and/or Phase D (deliver live resolutions in commit order).Two observations for that follow-up work:
UNHANDLED_COMMANDanalogue) would convert the mixture-write into a clean divergence retry.Validation
event-ordering.test.ts: dense positions under concurrent + cross-instance appends, cursor-skip impossibility, terminal ordering, pre-seq adoption, fence semantics)event-ordering.test.tsPart of #3201.