Backport #2266 + #2946: precondition guard for event creation - #3079
Conversation
Backports the stateUpdatedAt optimistic-concurrency guard for event creation (#2266) together with its default-on follow-up (#2946), adapted to the stable runtime. - @workflow/errors: PreconditionFailedError (HTTP 412) - @workflow/world: CreateEventParams.stateUpdatedAt + backend contract - @workflow/core: guard helpers (latestEventStateUpdatedAt, withPreconditionRetry), on by default (opt out WORKFLOW_PRECONDITION_GUARD=0); wired into the wait_completed / run_completed replay paths and every create in handleSuspension; exhausted 412 schedules an immediate re-invocation ({ timeoutSeconds: 0 }, stable has no turbo/reinvoke) - @workflow/world-vercel: send stateUpdatedAt on the wire; map 412 to PreconditionFailedError (events-v4 + makeRequest) Adaptation notes vs main: uses stable's getWorkflowRunEvents (no MutableEventLog/loadWorkflowRunEvents rename); run_completed does a single guarded create (never retried in place) and escalates on 412; SAFE_MODE / sequential-replays cleanup and docs from the source PRs are omitted (not present on stable). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
🦋 Changeset detectedLatest commit: 49ad6e2 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 |
🧪 E2E Test Results❌ Some tests failed Summary
❌ Failed Tests▲ Vercel Production (1 failed)vite (1 failed):
🌍 Community Worlds (102 failed)redis (19 failed):
turso (83 failed):
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
❌ 🌍 Community Worlds
✅ 📋 Other
❌ Some E2E test jobs failed:
Check the workflow run for details. |
TooTallNate
left a comment
There was a problem hiding this comment.
Reviewed against both upstream PRs (#2266 and #2946, which I reviewed on main), focusing on the guard's semantics surviving the port into stable's older runtime. Approving.
Function-level parity verified rather than eyeballed: latestEventStateUpdatedAt (including the non-decodable-ULID debug log), isPreconditionGuardEnabled (default-on !== '0'), and PRECONDITION_MAX_RELOAD_RETRIES = 2 are byte-identical to the post-#2946 state on main. withPreconditionRetry differs in exactly one line — getWorkflowRunEvents instead of main's renamed loadWorkflowRunEvents — and I checked stable's helper has the same contract (paginates to completion from the cursor, returns {events, cursor}), so the reload-and-merge behavior is unchanged.
The one substantive adaptation is correct, and it's the interesting one. Main's exhausted-412 path calls reinvoke(0); the backport returns { timeoutSeconds: 0 }. I traced why that's right rather than a downgrade:
- Main needed ack-and-reenqueue specifically because turbo mode has already acked the message by that point — rethrowing stalled runs for the queue's ~300s visibility timeout (the bug the forced-ON e2e round caught).
- Stable has no turbo mode at all (zero
isTurboEnabledreferences), so that hazard doesn't exist here. And{ timeoutSeconds: 0 }isn't a rethrow: world-vercel's handler sends a fresh no-delay message before acking the current one — the same crash-safe ack+enqueue shapereinvoke(0)provides. So both branches get a fresh replay through the same mechanism, expressed in each branch's idiom.
Also verified: run_failed remains unguarded (sends no snapshot) on both branches — behavior parity with main's deliberate fail-open asymmetry; 412 maps to PreconditionFailedError on both of stable's request paths (the v3 utils.ts status ladder and events-v4.ts), so the guard can't silently no-op depending on which wire a deployment uses; the CreateEventParams.stateUpdatedAt backend-contract JSDoc came across intact (the externally-originated-vs-replay-origin marker rule — the piece custom-World authors need); and the pre-merge blocker classes are clean (WORKFLOW_SERVER_URL_OVERRIDE is '', no TEMP(/REVERT BEFORE MERGE markers).
Verified locally: core 787 (+3 pre-existing expected-fails) and world-vercel 173 green on the branch, including the ported precondition-guard-replay tests. Combining the two upstream PRs into one backport is the right call — #2946 flips on what #2266 adds, so splitting them would land a dead-code intermediate state on stable.
CI: the 4 failures are the three Community World lanes (Redis/Turso/MongoDB) and their summary — the known stable-branch baseline set (they've failed on every stable backport I've reviewed, and per the repo's own CI notes community worlds target an older spec version). Nothing touching event creation.
One rollout note, not blocking: default-ON means stable's 412-recovery path goes production-hot on the next 4.x release. Same ask I made on #2946 — confirm the backend guard support is fully rolled out before this ships in a published 4.x, since 4.x users won't have opted in. The failure mode on an unsupporting backend is a no-op (stateUpdatedAt ignored), so the risk is bounded either way.
Resolves conflicts with the precondition-guard backport (#3079) and the world-testing per-server data-dir isolation fix (#3055): - packages/core/src/runtime.ts: keep both error imports (MaxEventsExceededError + PreconditionFailedError); both are used. - packages/world-testing/src/util.mts: keep both spawn env entries; WORKFLOW_LOCAL_DATA_DIR first so opts.env can still override it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Backports the
stateUpdatedAtoptimistic-concurrency guard for event creation tostable:Combined into one PR (the two upstream PRs are tightly coupled — #2946 flips on the guard #2266 added), ending with the guard on by default (opt out with
WORKFLOW_PRECONDITION_GUARD=0).What it does
Replay-context event creations send a
stateUpdatedAtsnapshot (the ULID time of the latest event the runtime has loaded). A supporting backend records the per-run latest out-of-band event time and rejects a stale create with 412PreconditionFailedError. On rejection the runtime reloads the event log from its cursor and retries (up to twice); if still stale it schedules an immediate re-invocation for a fresh replay.run_completedis never retried in place — a stale rejection forces a fresh replay rather than committing a result computed from the stale snapshot.Backward-compatible and fails open: backends without guard support ignore
stateUpdatedAt; the guard is best-effort.Changes
@workflow/errors—PreconditionFailedError(HTTP 412)@workflow/world—CreateEventParams.stateUpdatedAt+ backend-contract JSDoc@workflow/core— guard helpers (latestEventStateUpdatedAt,withPreconditionRetry), on by default; wired into thewait_completed/run_completedreplay paths and every create insidehandleSuspension; exhausted 412 escalates to an immediate re-invocation@workflow/world-vercel— sendsstateUpdatedAton the wire; maps 412 →PreconditionFailedError(v4 frame path +makeRequest)Adaptation notes (vs. the source PRs on
main)stablehas diverged, so this is a manual adaptation rather than a cherry-pick:getWorkflowRunEvents(main renamed it toloadWorkflowRunEventsand addedMutableEventLog).reinvoke(); the exhausted-412 escalation returns{ timeoutSeconds: 0 }(stable's existing re-invocation signal).throwForErrorResponse+makeRequest), predating main's sharederrorForResponse; the 412 branch was added in both.stable): theWORKFLOW_SAFE_MODEremoval, theWORKFLOW_SEQUENTIAL_REPLAYScleanup, and the docs pages from the source PRs (stable has no runtime-tuning/config docs page to host the flag).Tests
helpers.test.ts—latestEventStateUpdatedAt+withPreconditionRetry(opt-in gating, default-on, reload+retry, exhaustion rethrow, non-precondition passthrough).precondition-guard-replay.test.ts— drives the realworkflowEntrypointreplay loop with real ULID event ids: a 412-rejectedwait_completedreloads from the held cursor and retries with the newer snapshot; a 412-rejectedrun_completedis attempted exactly once, resolves into an immediate re-invocation, and never becomesrun_failed. (The wait scenario asserts the guard's reload/retry contract; the downstream replay-branch assertion from the source PR was adapted — stable's replay engine handles post-reload continuation differently.)world-vercel— 412 →PreconditionFailedErrormapping;stateUpdatedAtpresent/absent in the wire body.All core (790) and world-vercel (173) unit tests pass locally; typecheck and lint clean.
🤖 Generated with Claude Code