Add world.snapshots storage interface (local, postgres, vercel) - #3250
Add world.snapshots storage interface (local, postgres, vercel)#3250TooTallNate wants to merge 24 commits into
Conversation
🦋 Changeset detectedLatest commit: 374dbaa 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 |
🧪 E2E Test Results✅ All tests passed E2E Test SummarySummary
Details by Category✅ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ vercel-multi-region
|
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
a4e1f58 to
c935825
Compare
… assertions (analytics listing can omit attempt entirely)
…ed (encp) hook payloads open Main's sealed-box work (#3096) makes cross-deployment resumeHook() seal hook payloads to the target run's published X25519 public key. The shared start() path publishes that key regardless of engine, so QuickJS runs receive sealed payloads too — but the QuickJS entrypoint resolved only the bare symmetric key via importKey(), which cannot open encp envelopes. The first sealed hook payload wedged the run right after hook_received, timing out every hook/webhook e2e on Vercel prod (node:vm legs were fine — the node engine resolves the full capability via memoizeEncryptionKey). Resolve deriveRunPayloadKeys() in the entrypoint instead and widen the runtime's key types from CryptoKey to DecryptionKey. Writes stay symmetric (encrypt() with RunPayloadKeys takes the encr path). Regression test seals a payload exactly as resumeHook does and round-trips it through the VM.
…import, VM-leak guard, telemetry namespace, eval-string escaping - Deterministic crypto.getRandomValues/randomUUID in the VM bootstrap, drawing from the seeded Math.random (identical sequences to the node engine's vm/index.ts implementations); all crypto.subtle methods throw with step-function guidance. process.env exposed as a frozen copy, matching node. - Intl: throwing constructors (no ICU in QuickJS), and toLocale*-family methods (incl. localeCompare) throw when given an explicit locale so cross-engine divergence is loud instead of silently writing different values into the event log. No-argument forms keep working. - runtime.ts lazy-imports the QuickJS entrypoint at dispatch, keeping the ~1.3MB embedded WASM assets out of node-engine deployments. - runQuickJSWorkflow wraps the per-run phase so an exceptional exit disposes the VM instead of leaking it in a reused compute instance; corrected the misleading fail-loud comment (run_failed, not retry); warn when the event drain loop exhausts its iteration bound. - Telemetry attributes renamed quickjs.* → workflow.vm.* to stay in the file's workflow.* namespace. - Eval-string correlation-id interpolation uses JSON.stringify instead of quote-only escaping. - common-vm.test.ts pins the reducer/reviver superset invariant against common.ts so the duplicated sets can't silently drift. - Docs enumerate the remaining global-surface differences (subtle.digest, Intl, WebAssembly, Atomics); quickjs-entrypoint documents the known precondition-guard gap.
…tion + resumeId dedup) #1834 made resumeHook() fall back to enqueueing the run with a hookInput payload when the direct hook_received write fails transiently, with the runtime materializing the missing event on delivery. Only the node:vm path implemented it — the QuickJS dispatch returned before the node block, so the resilient payload was silently dropped and the new e2e timed out on every quickjs leg. - runtime.ts threads hookInput into runWorkflowWithQuickJS; the entrypoint materializes the missing hook_received after loading the event log (resumeId-keyed dedup, occurredAt from the resumeId ULID, local eventData substitution for lazy/ref responses, EntityConflict / HookNotFound handling) — mirroring the node block. - processEvents drops duplicate hook_received rows sharing a resumeId (first-in-log wins), matching the node engine's EventsConsumer dedup; the seen-set lives in the VM heap so it is deterministic per replay. Verified against the dev server with WORKFLOW_VM=quickjs: the resilient resume e2e passes and the materialization is observable in the logs; all 27 hook e2e tests green.
… WASM module caching
…loop event ceiling - Inline steps now claim via a lazy step_started carrying the input (step_created deferred, atomic create-claim in the world), with ownerMessageId stamped and authoritativeAttempt=1 — a concurrent invocation racing on the same fresh step loses with EntityConflictError and skips instead of both bare-starting the step and double-running the body. This also removes the stepsCreatedByUs set, whose 'created by us' invariant didn't survive the swallowed create-race conflict; redelivery backstops now key on hasCreatedEvent. - dispatchPendingOps' createdAttributeEvent/createdGetConflictHook signals are consumed again: when the loop exits suspended without ever reading back a self-written attr_set / getConflict hook_created (eventually-consistent listing lag), the entrypoint requeues immediately instead of parking the run awaiting_external with its unblocking event already written. - The server-supplied event ceiling is re-checked at the top of every continuation-loop turn (seenEventIds.size), so a single invocation fanning out inline can no longer grow the log arbitrarily past the operator's limit. The quickjs dispatch in runtime.ts converts MaxEventsExceededError into run_failed / MAX_EVENTS_EXCEEDED — the guard's throw previously nacked forever, parking runaway runs in 'running'. - Documented the deliberate decision that the platform function timeout is the only bound on inline chaining (budget parked per batch), matching the node engine.
…l implementations
…torage
The security bot flagged path traversal: an unvalidated runId was
interpolated directly into {basedir}/snapshots paths, letting ../ (or /,
\, NUL, absolute fragments) escape the snapshots dir on save/load/delete.
Apply the same two-layer scheme as the other world-local storages:
assertSafeEntityId as the primary defense plus resolveWithinBase
containment at the join. Hostile-runId test covers all three operations.
ff2c6e4 to
968cfa1
Compare
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 158558ms → this run 128545ms (Δ -30013ms, -19%) 📜 Previous results (3)0584471Mon, 10 Aug 2026 19:29:25 GMT · run logs
d2a115bTue, 04 Aug 2026 00:41:43 GMT · run logs
968cfa1Fri, 31 Jul 2026 23:24:59 GMT · run logs
ℹ️ Metric definitions & methodologyThe collapsed STSO distribution section above buckets every step gap of the sequential-steps run (not a sampled window), split by whether the step ending the gap ran inline — in the same warm process as the step before it, so the gap is pure framework overhead — or after a queue-hop — the first step of a fresh process, which pays queue dispatch, client reinit and event-log replay. Bars overlay the two runs: Best/P75/P90/P99 deltas compare against the most recent benchmark run on Metrics — TTFS: time to first step body (in-deployment start() → first step body, deployment clocks) · STSO: step-to-step overhead (gap between consecutive step bodies) · WO: workflow overhead (whole-run time outside step bodies, in-deployment anchored) · SL: stream latency (in-deployment write → read propagation, readAt - writtenAt) · SO: stream overhead (end-to-end write+consume time beyond the modelled generation window) Scenarios — step: one trivial no-op step, no stream; no hooks, so the run stays in turbo mode (in-process fast path) · stream: one streaming step; no hooks, so the run stays in turbo mode (in-process fast path) · hook + stream: registers a hook before one step, which exits turbo mode (dispatch path) · 1020 steps: 1020 trivial sequential steps; STSO is measured between consecutive steps in the given step ranges, and WO is the whole-run overhead outside step bodies · stream latency: parallel reader/writer steps on a dedicated stream; SL is the in-deployment write->read propagation (readAt - writtenAt) · stream overhead (text): writer streams 300 variable-length text token deltas paced at 100/s for 3s (a haiku-size LLM's token throughput) while a parallel reader drains the whole stream; SO is the end-to-end write+consume time beyond the 3s generation window (overhead/backpressure) · stream overhead (structured): same workload as stream overhead (text), but each delta is an AI-SDK-style structured object ({ type: 'text-delta', id, text }) instead of a raw string, so the SO gap vs the text scenario is the added serialization cost 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 · SL 50/60/125 · SO 250/500/1000 All metrics are measured from deployment-side timestamps only. Runs are triggered by an in-deployment route that stamps the anchor ( Cold starts are kept in the numbers on purpose — they are part of real bursty-workload latency. The workbench deployment cold-starts the |
pranaygp
left a comment
pranaygp
left a comment
There was a problem hiding this comment.
Reviewed the incremental diff (16 files, +838). Mergeable with fixes — nothing here runs until #3251, but three inline findings become live correctness bugs the moment it does, and the changeset/semver one ships breakage to community worlds on merge.
Beyond the inline comments:
- Orphaned snapshots on postgres cleanup: the legacy
run_cancelledpath instorage.tsdeletes hooks and waits but not snapshots; there are zero FKs/cascades inschema.tsand no retention job, and rows are 5–15 MBbytea— unbounded growth. Other cleanup sites worth checking:storage.ts:948, 1000, 1045, 1601. - All three vercel paths bypass
makeRequest, losing transient-error →WorkflowWorldError{code:'TRANSPORT'}mapping (retryability),getRequestTimeoutMs(), and the http logging. Theundici.request()choice forsaveis well-justified (undici#3288 — Buffer body survives RetryAgent retries) but argues for teachingmakeRequesta raw-body mode, not three hand-rolled paths. - Older-server compat:
load's 404→null degrades gracefully by accident, butsave/deletethrow against a workflow-server without the endpoints — #3251 must treatsaveas best-effort or this becomes a hard version coupling. - Minor: world-local snapshots is the only sub-storage not given the
tag(breaks multi-world basedir isolation); noload/deletetest coverage on the vercel client (the 503-retry save test is genuinely good); nothing exercises the postgres table beyond the migration applying;eventsCursorrides an HTTP header unencoded (constrains future cursor formats). - Migration note: it's
0018_add_snapshots_table.sql(the PR description says the stack docs referenced 0016).
Path-traversal hardening in the runId-validation follow-up commit is solid (two independent layers, well-tested) — no findings there.
| metadata: SnapshotMetadata | ||
| ): Promise<void> { | ||
| await ensureDir(snapshotsDir); | ||
| await Promise.all([ |
There was a problem hiding this comment.
Torn save pair: write() is atomic per file (temp + rename), but this Promise.all of two renames is not — a crash between them, or a concurrent load interleaving, pairs a .bin from one suspension with an eventsCursor from another. That's the worst failure mode for this feature: restore replays from the wrong log position and silently diverges instead of erroring. Suggest a single atomic write (metadata header + data in one file) or data-then-metadata with the metadata carrying a digest of the data it describes, as the commit marker.
There was a problem hiding this comment.
Fixed in 374dbaa by restructuring the storage to a single self-describing blob: new encodeSnapshotEnvelope/decodeSnapshotEnvelope helpers in @workflow/world pack the metadata and bytes into ONE file ({runId}.snapshot), written with the existing atomic temp+rename. The torn window is structurally gone — there is no second file to pair wrongly. A corrupt/truncated envelope decodes to null (clean miss → full replay), never torn state. Test added: corrupt-envelope-as-miss + single-file assertion.
| const data = new Uint8Array(buffer); | ||
|
|
||
| const eventsCursor = | ||
| response.headers.get('X-Snapshot-Events-Cursor') || null; |
There was a problem hiding this comment.
load fabricates metadata instead of failing loudly. (a) save writes metadata.eventsCursor ?? '' (L51), so a server that drops the header is indistinguishable from a snapshot taken at cursor-null — i.e. "replay from the beginning", the silent-wrong-answer direction. (b) createdAt falls back to new Date(), inventing a timestamp. Unlike local/postgres, this path never runs SnapshotMetadataSchema.parse. Treat a missing cursor header as malformed (throw, or return null so the caller cold-starts) and parse through the schema.
There was a problem hiding this comment.
Fixed in 374dbaa: the envelope (see the torn-save thread) is now the GET/PUT body, and load decodes metadata exclusively from it — schema-validated via SnapshotMetadataSchema inside decodeSnapshotEnvelope. A body that doesn't decode returns null (cold start), even when plausible-looking headers are present; nothing is ever fabricated from headers or wall time. The X-Snapshot-* headers are still sent on save as observability-only denormalized copies. Tests added: full-metadata round-trip with NO headers, and undecodable-body-as-miss with headers present.
| }; | ||
| }, | ||
|
|
||
| async delete(runId: string): Promise<void> { |
There was a problem hiding this comment.
Delete throws on 404 while local (force: true) and postgres (plain DELETE) are idempotent. The interface JSDoc says delete runs at terminal state — exactly the path most likely to retry, run twice, or run for a run that never snapshotted. Add 404 to the success set, or document non-idempotence and make #3251 tolerate the throw.
There was a problem hiding this comment.
Fixed in 374dbaa: DELETE now treats 404 as success, matching local's force: true and postgres's plain DELETE, and the interface JSDoc now states the idempotence requirement explicitly. Tests added for both 404-is-success and non-retryable-error-still-throws (403 — the shared RetryAgent retries 5xx, which is orthogonal).
| // Diagnostic: actual on-the-wire snapshot bytes and the HTTP-PUT | ||
| // cost, grep-able by runId alongside @workflow/core's QUICKJS_VM | ||
| // diagnostics. | ||
| console.debug('[Workflow] WORLD_SNAPSHOT_DIAG', { |
There was a problem hiding this comment.
Ungated console.debug on a path that runs on every suspension/resume (also at L139 and the load path). Route through the package's existing httpLog/debug gating.
There was a problem hiding this comment.
Fixed in 374dbaa: all three WORLD_SNAPSHOT_DIAG sites route through a helper gated on the package's existing HTTP_DEBUG_ENABLED flag (same gate as httpLog).
| * data so that on restore, only events created after the snapshot need | ||
| * to be fetched. | ||
| */ | ||
| snapshots: { |
There was a problem hiding this comment.
Breaking change labeled minor. This is a required member of Storage, and the changeset itself says community worlds must add it — that's a major for @workflow/world. worlds-manifest.json lists 13 out-of-repo implementations (@workflow-worlds/*, @fantasticfour/world-*, @platformatic/world, workflow-world-jazz); all fail to typecheck, and none are in CI, so this is invisible until they upgrade. In-repo mocks dodge it via as unknown as World, which is why CI is green.
Pick deliberately: make it snapshots?: and have #3251 feature-detect (keeps this a true minor, and a world that can't store blobs should be allowed to simply not — #3251 already needs a no-snapshot cold-start path), or keep it required and bump major. I'd favor optional. The bump type also determines the semver step on any stable backport, so the mislabel isn't cosmetic.
There was a problem hiding this comment.
Fixed in 374dbaa — went with your preferred option: snapshots?: is now optional on Storage, so this is a true minor and the 13 out-of-repo worlds keep typechecking. #3251's entrypoint feature-detects (world.snapshots absent ⇒ threshold forced to 0 ⇒ pure full replay, always correct). The changeset text was updated to say community worlds are unaffected. The interface JSDoc also now spells out the two new contract points this review surfaced: metadata must round-trip losslessly AND atomically with the bytes it describes (the envelope helpers make a plain blob store satisfy that with one write), and delete must be idempotent.
…hreads Merge resolution — main's #3048 finals carried into the inline-loop architecture: - namespace + run-origin nextTraceCarrier threaded through runWorkflowWithQuickJS into every publish (step handoffs, hook_conflict requeue, wait continuations, immediate requeues) - suspended-exit requeues converted to FRESH messages (never { timeoutSeconds } visibility-redelivery of the current message — the hookInput redelivery trap fixed on #3048); exit wait sweep enqueues the continuation for the soonest unscheduled wait directly - entrypoint-side hookInput materialization dropped in favor of main's engine-agnostic prologue re-ensure in runtime.ts (with #3230's (runId, resumeId) claim protocol); dispatch stays inside the replay loop's try so engine failures classify into run_failed - interrupt handler keeps the perf branch's per-burst mutable budget, with main's configurable getReplayTimeoutMs() as the ceiling Review fixes (PR #3049 threads): - CRITICAL overflow wedge: overflow steps are handed to the queue in the same turn their step_created is written, BEFORE the event feed — the feed always observes those writes and continued the loop, so the old handoff was unreachable on the only turn that classified the steps as fresh (the cause of promiseRaceStressTestWorkflow hanging in the quickjs CI legs) - backstop gating: the deliveryAttempt > 1 gate (common case on worlds that advance attempts on routine redeliveries) is replaced with the node engine's ownership decision table — lease-active steps owned by another message arm a DELAYED backstop for the lease remainder under an epoch-scoped key; owner redeliveries and expired/unstamped steps dispatch immediately under the bare-correlationId key. Ownership is derived host-side from observed step_started/step_retrying events - ack-without-requeue: inline step terminals the feed has not surfaced raise the requeue signal, so the loop never acks with durably written terminals and nothing scheduled to consume them - idempotency keys bucketed by purpose (dispatch / backstop:<epoch> / retry:<n>) so worlds that retire used keys cannot swallow a later publish for the same step - live-feed terminal buffering: step/wait/attr terminals arriving before this VM constructs the corresponding resolver are buffered (__terminalBuffer, mirroring __hookPayloadBuffer) and settle the promise at construction — the single-scan continuation path previously dropped them and the await never settled Validated: core 1888 passed, full e2e 136/136 under WORKFLOW_VM=quickjs (nextjs-turbopack dev, world-local).
# Conflicts: # packages/core/src/runtime/quickjs-entrypoint.ts # packages/core/src/runtime/quickjs-runtime.ts # packages/world-postgres/src/drizzle/migrations/meta/_journal.json # packages/world-vercel/src/trace-propagation.test.ts
…ent delete, gated diagnostics - @workflow/world: snapshots interface is now OPTIONAL on Storage (a World that can't store blobs simply omits it; consumers feature-detect and fall back to full replay) — keeps this a true minor for community worlds. New encodeSnapshotEnvelope/decodeSnapshotEnvelope helpers pack metadata + bytes into ONE self-describing blob; decode validates via the schema (passthrough for forward compat) and returns null for anything torn/corrupt/foreign — never fabricated metadata. - world-local: single envelope file per run replaces the .bin/.json pair — the torn-save window (crash or concurrent load between two renames pairing bytes with another suspension's cursor) is structurally gone. - world-postgres: data column stores the envelope; full metadata round-trips losslessly (new fields need no migration). Columns remain as denormalized observability copies. - world-vercel: envelope is the PUT/GET body, so the full metadata round-trips without any workflow-server change; loads never invent metadata from headers/wall time (undecodable body = clean miss). DELETE treats 404 as success (idempotent like local/postgres). WORLD_SNAPSHOT_DIAG logs gated behind the package's HTTP debug flag.
Note
Supersedes #3050, which GitHub auto-marked as merged (and auto-deleted the head branch) when a restacking mistake briefly force-pushed the head branch to the same commit as its base. Same content, freshly rebased on the stack.
Summary
PR 3 of the QuickJS VM roadmap: the
world.snapshotsstorage interface and its implementations, ported from the original snapshot-runtime branch (#1300). Inert until PR 4 — nothing in the runtime calls these APIs yet.What's included
@workflow/world:Storage.snapshotsinterface —save(runId, data, metadata)/load(runId)/delete(runId)— plusSnapshotMetadata(eventsCursor,createdAt). The cursor is storage-layer metadata stored alongside the opaque snapshot bytes so a restore only fetches events recorded after the snapshot.@workflow/world-local: filesystem storage under{basedir}/snapshots/({runId}.bin+{runId}.jsonsidecar), with round-trip unit tests.@workflow/world-postgres:workflow_snapshotstable (migration0016, renumbered from the original branch to follow main's0012–0015).@workflow/world-vercel: client for the workflow-server snapshot endpoints (PUT/GET/DELETE /v2/runs/:runId/snapshot, binary body + metadata headers — endpoints are live in workflow-server since vercel/workflow-server#339). Includes:undici.request()(notfetch()) PUT path so Buffer bodies survive RetryAgent retries (Retry Handler Fails When Request Has a Body nodejs/undici#3288),makeRequest, per the repo's trace-propagation rule), covered by newtrace-propagation.test.tscases.Notes
@workflow/core's responsibility (PR 4's save pipeline); worlds must not add their own compression (ciphertext doesn't compress).Storageis a breaking change for community World implementations — appropriate for the v5 beta line; called out in the changeset.WORLD_SNAPSHOT_DIAGwarn logging from the original branch was demoted toconsole.debug.