[core] Check a restarted replay against the counts the world reported - #3267
Draft
VaguelySerious wants to merge 14 commits into
Draft
[core] Check a restarted replay against the counts the world reported#3267VaguelySerious wants to merge 14 commits into
VaguelySerious wants to merge 14 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>
A 412's delta is untrusted data on a failure path, and the restart it feeds has to reload the log in full whenever that delta cannot be proven to complete it: - Reject a delta carrying an event of another run. It is merged straight into the replay's log, so a foreign event there is a corrupt log rather than a corrected one. - Reject a delta whose events carry a payload the JSON error body mangled. Payload fields are Uint8Array everywhere else in the client, but a 412 body is JSON, so resolved bytes arrive as a Buffer-shaped object that EventSchema happily accepts and the runtime would hydrate garbage from. - Bypass the delta at the inline step-claim site when a sibling claim in the same batch was accepted: that sibling wrote step events of its own, possibly after the World computed the delta. - Report `source: 'full-reload'` when a delta parsed but there was no cached log to merge it into. Beyond the delta: - Settle every write in a suspension phase before a rejection escapes. Promise.all leaves siblings in flight, and a sibling create landing after a 412 escaped commits an event minted from the abandoned replay's correlation-id sequence while racing the restart's reload. A 412 is preferred over a sibling rejection because it has a cheap recovery. - Bound the re-invocation chain per run on the queue message. A fresh enqueue resets the queue's delivery count, so a permanently fenced run had no run-level bound; it now fails with RUNTIME_ERROR once the budget is spent. - Label restarts from the main replay catch `replay-write`: it receives 412s for both run_completed and wait_completed and cannot tell them apart. Docs: the guard's fail-open behaviour (it rejects only on evidence, so 412 volume describes a workload rather than a deployment's health), the delta's same-run requirement for World authors, and the two re-invocation env vars. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… restart The replay payload cache resumes prewarming from the number of events it has already scanned, which is only sound while the log grows by appending. A restart after a 412 replaces the log with a corrected one, so the missing events appear below that length and were never scanned; the restart's full-reload branch now calls resetScan(). A restart also only logged that it happened. It now reports what the reload found, so a log that grew (the fence working) is distinguishable from one that came back unchanged, which means client and world disagree about the same set of events and every subsequent restart will be rejected too.
Both guard inputs are derived from the loaded set alone, so they cannot distinguish two replays that consumed the same events in different orders. That is what makes the one-sided-safety argument hold, and it bounds what 412 volume can report.
Resolved three conflicts against main's #3186 (computeInstanceId on step_started) and adapted its test to this branch's preconditionSnapshot API: - core/runtime/step-executor.ts: fold the three snapshot fields into main's bundled `startEventParams` object, spread as a unit at both step_started call sites. - core/runtime/step-executor.test.ts: the compute-instance test now passes the guard via `preconditionSnapshot` and asserts the whole snapshot survives alongside computeInstanceId. - world-vercel/src/events.ts: keep both main's computeInstanceId spread and this branch's three state* spreads. - core/runtime/helpers.test.ts: drop main's imports of `withPreconditionRetry` and `MutableEventLog`, both deleted here.
A stale-snapshot rejection can carry the comparison it was made on: `recordedAtOrBelow` (events the world had recorded at or below the client's watermark) and the `stateUpdatedAt` that watermark was. The v4 error decoder now picks those up alongside the inline delta, and `preconditionExpectation` reads them back off the error. After a full cursor-less reload, the runtime counts how many of the reloaded events sit at or below that watermark. Two outcomes mean the next restart would re-derive the same snapshot and earn the same rejection, so it escalates to a fresh invocation immediately instead: the reload came back unchanged, or it grew but is still short of what the world recorded. Only a full reload is treated as evidence. A delta-fed restart proves nothing about the log as a whole, and absent or malformed counts leave the reload unchecked rather than unsatisfied. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 9338543 The changes in this PR will be included in the next version bump. This PR includes changesets to release 20 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 |
Contributor
Contributor
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests▲ Vercel Production (4 failed)example (1 failed):
express (1 failed):
hono (1 failed):
nuxt (1 failed):
📦 Local Production (1 failed)nextjs-turbopack-stable (1 failed):
📋 Other (1 failed)e2e-vercel-prod-tanstack-start (1 failed):
E2E Test SummarySummary
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
❌ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 📋 Other
✅ vercel-multi-region
|
# Conflicts: # packages/core/src/runtime.ts # packages/core/src/runtime/step-executor.ts # packages/core/src/runtime/suspension-handler.ts # packages/world-vercel/src/events-v4.ts # packages/world-vercel/src/events.ts
…estart-evidence # Conflicts: # packages/core/src/runtime.ts
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 #3145.
#3145 gives an in-process replay restart a classification: after a stale-snapshot rejection it records whether the reload came back
unchanged,grew, orshrank. That is a measurement, but the restart does nothing with it — it spends the next restart regardless, and a run whose reload cannot produce a different write burns the full budget before escalating.This PR closes that loop using evidence the world already has.
A rejecting world may echo the comparison
A 412 can now carry the numbers it rejected on, on the error's
details:recordedAtOrBelow— how many events the world had recorded at or below the client's watermarkstateUpdatedAt— the watermark that count was taken atBoth or neither.
decodePreconditionDetailsin the v4 error path picks them up alongside the inline delta (each is decoded independently, so an unusable delta does not discard usable counts), andpreconditionExpectationreads them back offPreconditionFailedError. Additive on both sides: a world that reports nothing leaves each reload unchecked, never unsatisfied, and a client shipped today already drops fields it does not know.A restart that cannot progress escalates immediately
After a full cursor-less reload, the runtime counts the reloaded events at or below that watermark (
countEventsAtOrBelow). A run's log only grows and the watermark is fixed, so if the reload has not reachedrecordedAtOrBelow, re-deriving the replay produces the same snapshot and earns the same rejection. Two cases short-circuit the remaining in-process restarts:stalledReasonreload-unchangedreload-short-of-recordedEither one warns once (
skipping further in-process restarts) and hands straight to the delayed re-invocation, which is a fresh invocation and possibly a different region. The escalation ladder is unchanged in shape — the bound is now a ceiling rather than a quota.Deliberately narrow:
unchangedoutranksshort. An unchanged reload is the stronger statement, and it holds whether or not the world echoed anything.Tests
helpers.test.ts—countEventsAtOrBelow(equality inclusive, empty log, undecodable id) andpreconditionExpectation(both fields, zero accepted, one-of-two / negative / fractional / string / no details / wrong error type allnull)events-v4.test.ts— 412 counts surfaced with no delta attached; counts kept when the delta beside them is unusable; incomplete or out-of-range counts ignoredprecondition-guard-replay.test.ts— the rejection harness can now grow its log per rejection and echo counts, which separates the two behaviours: a static log stops after one restart withreload-unchanged; a log that grows but stays short stops withreload-short-of-recorded; a log that reaches the echoed count keeps restarting to the bound, with every reload assertingsatisfiedandoutcome: 'grew'packages/core(1690) andpackages/world-vercel(325) suites pass.🤖 Generated with Claude Code