fix: resume open streams from tail cursors - #3430
Conversation
Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
🦋 Changeset detectedLatest commit: cc1cb8e The changes in this PR will be included in the next version bump. This PR includes changesets to release 16 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 |
📊 Workflow Benchmarkscommit Backend:
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 194368ms → this run 184115ms (Δ -10253ms, -5%) 📜 Previous results (5)32fdf03Thu, 13 Aug 2026 00:15:56 GMT · run logs
9704155Wed, 12 Aug 2026 20:58:21 GMT · run logs
62fa152Wed, 12 Aug 2026 17:34:02 GMT · run logs
103e15eTue, 11 Aug 2026 21:58:08 GMT · run logs
1b6891bTue, 11 Aug 2026 20:59:14 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) · Fan-out TTFS: fan-out time to first step (in-deployment start() → first of the parallel step bodies to complete) · Fan-out TTLS: fan-out time to last step (in-deployment start() → last of the parallel step bodies to complete, i.e. when the Promise.all resolves) · 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 · Promise.all(100 steps): 100 trivial no-op steps started together in a single Promise.all; Fan-out TTFS is the first of them to complete and Fan-out TTLS the last, both from the in-deployment clientStart, so their gap is the spread the runtime adds across the fan-out · 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 |
🧪 E2E Test Results❌ Some tests failed ❌ Failed E2E Tests▲ Vercel Production (27 failed)astro-node (1 failed):
astro-quickjs (1 failed):
example-node (1 failed):
example-quickjs (1 failed):
express-node (2 failed):
express-quickjs (1 failed):
fastify-node (1 failed):
fastify-quickjs (1 failed):
hono-node (1 failed):
hono-quickjs (1 failed):
nest-node (1 failed):
nest-quickjs (1 failed):
nextjs-turbopack-node (1 failed):
nextjs-turbopack-quickjs (1 failed):
nextjs-webpack-node (1 failed):
nextjs-webpack-quickjs (1 failed):
nitro-node (1 failed):
nitro-quickjs (1 failed):
nuxt-node (1 failed):
nuxt-quickjs (1 failed):
sveltekit-node (1 failed):
sveltekit-quickjs (1 failed):
tanstack-start-node (1 failed):
tanstack-start-quickjs (1 failed):
vite-node (1 failed):
vite-quickjs (1 failed):
E2E Test SummarySummary
Details by Category❌ ▲ Vercel Production
✅ 💻 Local Development
✅ 📦 Local Production
✅ 🐘 Local Postgres
✅ 🪟 Windows
✅ vercel-multi-region
|
Sim WorldSimulated world deterministic testing for races. Traces 🟠 Mint-ordered log — 3 fail of 41 total
Full trace: 🟢 Append-only log — 0 fail of 41 total
Full trace: |
Signed-off-by: Nathan Colosimo <110621881+NathanColosimo@users.noreply.github.com>
VaguelySerious
left a comment
There was a problem hiding this comment.
AI review: blocking issues found
| if (newChunks.length > 0 && mounted) { | ||
| setChunks((prev) => [...prev, ...newChunks]); | ||
| serverCursorRef.current = | ||
| pollResult.cursor ?? serverCursorRef.current; |
There was a problem hiding this comment.
AI Review: Blocking
Dropping skipFrames is the correct pairing for the World contract in #3475, and the initial-load and poll paths here are internally consistent. The problem is that it is correct only against that contract, and @workflow/web and the World packages are separately versioned, so users can land on a mismatched pair.
Against a World still on the old contract, the open tail returns cursor: null. readStreamChunksServerAction then reports cursor: null, this hook leaves serverCursorRef.current at null, and the next poll re-reads from index 0 with nothing skipping the frames already rendered, so the whole stream is appended again every POLL_INTERVAL_MS. I ran the new pair against an old-contract World: 3 chunks render, one poll leaves 7 rendered entries, a second leaves 12. It grows quadratically in poll count for the life of the run, and the failure mode is duplicated output rather than an error, so nothing surfaces it.
"Old contract" is not hypothetical here. It is any pinned @workflow/world-local or @workflow/world-postgres older than #3475, plus every community World implementing what docs/content/worlds/v4/building-a-world.mdx and the v5 copy currently document.
Two ways out. Make the cursor change additive in #3475, adding a new resumeCursor field and leaving cursor alone, so both pairings work and this hook can prefer the new field when present. Or declare the coupling: a peer or version floor on the World packages, and a minor rather than a patch. As written the changeset is a patch on @workflow/web alone, which is the one thing that cannot express the dependency.
Worth noting the reverse skew is already handled well: readStreamChunksServerAction only throws when hasMore && !result.cursor, which an old-contract World never produces. So the tolerance is there, it just is not enough on its own.
| cursor = page.cursor; | ||
| if (!page.hasMore) { | ||
| hasMore = page.hasMore; | ||
| if (!hasMore) { |
There was a problem hiding this comment.
AI Review: Note
if (!hasMore) expect(page.done).toBe(true) asserts an implication the contract explicitly disclaims. The getChunks docs already say new chunks may appear in future requests even after hasMore is false, and this PR's premise is that hasMore: false on an open stream is a normal terminal state for a page loop. The assertion only passes here because the stream is already closed before the loop runs.
That makes it a trap for whoever copies this loop against a live stream, and it does not add coverage beyond the expect(paginatedChunks).toHaveLength(streamChunks.length) below it. I would drop it, or hoist it to a single assertion after the loop with a comment that the stream was closed first.
Since this is the loop shape users will copy, it would also be worth asserting the open-stream case somewhere: read a stream that is still open, confirm hasMore goes false with a non-null cursor, write another chunk, and confirm the cursor resumes after the delivered chunks. That is the behavior the whole stack turns on and the e2e suite does not cover it yet.
| for (const chunk of allChunks) { | ||
| totalSize += chunk.length; | ||
| } | ||
| cursor = result.cursor ?? cursor; |
There was a problem hiding this comment.
AI Review: Note
cursor = result.cursor ?? cursor is the load-bearing line of the new contract. It is what stops an idle poll, which returns an empty page with cursor: null, from resetting the reader to index 0 and replaying the stream. Same for cursor: streamDone ? null : (cursor ?? null) below.
There is no test for either. The web suite covers readStream transport and hook status, not this retention. A test that polls twice with no new chunks in between and asserts the returned cursor is unchanged would pin down the behavior everything else depends on. I wrote that case against world-local while reviewing and it passes, so this is about locking it in rather than a defect: it is the most likely thing to regress silently later, because the symptom is duplicated output rather than a failure.
| if (newChunks.length > 0 && mounted) { | ||
| setChunks((prev) => [...prev, ...newChunks]); | ||
| serverCursorRef.current = | ||
| pollResult.cursor ?? serverCursorRef.current; |
Description
Update core and dashboard stream consumers to page while hasMore is true, then retain the final open-tail cursor for the next poll.
The consumers no longer request an overlapping page or skip duplicate frames. Each returned chunk is consumed exactly once, and a later poll resumes after the last delivered chunk.
This PR consumes the versioned World contract and matching package constraints from #3475.
Stacked on #3475.
How did you test your changes?
PR Checklist - Required to merge