fix(core): serialize a DataView as its viewed bytes - #4443
Conversation
`DataView` had no reducer, so it fell through to devalue's built-in encoding: the whole backing `ArrayBuffer` plus the view's offset and length. Node hands out small `Buffer`s as windows onto a shared 8 KiB pool, so a four-byte `DataView` over a `Buffer.allocUnsafe(4)` serialized 8 KiB of unrelated allocations — and step returns are written to the run's event log, where that residue outlives the process that leaked it. Add a `DataView` reducer alongside the typed-array ones, base64 of the viewed range only, in all three reducer sets that share the wire format (host, VM twin, QuickJS handle-space) so a value round-trips identically whichever side serializes it. Revivers accept a non-string payload as well: devalue hands a custom reviver the hydrated referent, so a pre-reducer payload arrives as the backing `ArrayBuffer` rather than base64. Without that branch, old event logs would decode an `ArrayBuffer` as if it were base64 (or throw, in the `atob` implementations). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 379d1de 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 |
🧪 E2E Test Results✅ All tests passed
|
| Passed | Failed | Skipped | Total | |
|---|---|---|---|---|
| ✅ ▲ Vercel Production | 3878 | 0 | 739 | 4617 |
| ✅ 💻 Local Development | 4238 | 0 | 550 | 4788 |
| ✅ 📦 Local Production | 4238 | 0 | 550 | 4788 |
| ✅ 🐘 Local Postgres | 4238 | 0 | 550 | 4788 |
| ✅ 🪟 Windows | 340 | 0 | 2 | 342 |
| ✅ 🌐 Cross-language Conformance | 68 | 0 | 84 | 152 |
| ✅ vercel-http-transport | 873 | 0 | 153 | 1026 |
| ✅ vercel-multi-region | 27 | 0 | 0 | 27 |
| ✅ vercel-ws-transport | 591 | 0 | 93 | 684 |
| Total | 18491 | 0 | 2721 | 21212 |
Details by Category
✅ ▲ Vercel Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-node | 141 | 0 | 30 |
| ✅ astro-quickjs | 141 | 0 | 30 |
| ✅ example-node | 141 | 0 | 30 |
| ✅ example-quickjs | 141 | 0 | 30 |
| ✅ express-node | 141 | 0 | 30 |
| ✅ express-quickjs | 141 | 0 | 30 |
| ✅ fastify-node | 141 | 0 | 30 |
| ✅ fastify-quickjs | 141 | 0 | 30 |
| ✅ hono-node | 141 | 0 | 30 |
| ✅ hono-quickjs | 141 | 0 | 30 |
| ✅ nest-node | 141 | 0 | 30 |
| ✅ nest-quickjs | 141 | 0 | 30 |
| ✅ nextjs-turbopack-node | 168 | 0 | 3 |
| ✅ nextjs-turbopack-quickjs | 168 | 0 | 3 |
| ✅ nextjs-webpack-node | 168 | 0 | 3 |
| ✅ nextjs-webpack-quickjs | 168 | 0 | 3 |
| ✅ nitro-node | 141 | 0 | 30 |
| ✅ nitro-quickjs | 141 | 0 | 30 |
| ✅ nuxt-node | 141 | 0 | 30 |
| ✅ nuxt-quickjs | 141 | 0 | 30 |
| ✅ python-node | 66 | 0 | 105 |
| ✅ sveltekit-node | 160 | 0 | 11 |
| ✅ sveltekit-quickjs | 160 | 0 | 11 |
| ✅ tanstack-start-node | 141 | 0 | 30 |
| ✅ tanstack-start-quickjs | 141 | 0 | 30 |
| ✅ vite-node | 141 | 0 | 30 |
| ✅ vite-quickjs | 141 | 0 | 30 |
✅ 💻 Local Development
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 142 | 0 | 29 |
| ✅ astro-stable-quickjs | 142 | 0 | 29 |
| ✅ express-stable-node | 142 | 0 | 29 |
| ✅ express-stable-quickjs | 142 | 0 | 29 |
| ✅ fastify-stable-node | 142 | 0 | 29 |
| ✅ fastify-stable-quickjs | 142 | 0 | 29 |
| ✅ hono-stable-node | 142 | 0 | 29 |
| ✅ hono-stable-quickjs | 142 | 0 | 29 |
| ✅ nest-stable-node | 142 | 0 | 29 |
| ✅ nest-stable-quickjs | 142 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 170 | 0 | 1 |
| ✅ nitro-stable-node | 142 | 0 | 29 |
| ✅ nitro-stable-quickjs | 142 | 0 | 29 |
| ✅ nuxt-stable-node | 142 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 142 | 0 | 29 |
| ✅ sveltekit-stable-node | 161 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 161 | 0 | 10 |
| ✅ tanstack-start-node | 142 | 0 | 29 |
| ✅ tanstack-start-quickjs | 142 | 0 | 29 |
| ✅ vite-stable-node | 142 | 0 | 29 |
| ✅ vite-stable-quickjs | 142 | 0 | 29 |
✅ 📦 Local Production
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 142 | 0 | 29 |
| ✅ astro-stable-quickjs | 142 | 0 | 29 |
| ✅ express-stable-node | 142 | 0 | 29 |
| ✅ express-stable-quickjs | 142 | 0 | 29 |
| ✅ fastify-stable-node | 142 | 0 | 29 |
| ✅ fastify-stable-quickjs | 142 | 0 | 29 |
| ✅ hono-stable-node | 142 | 0 | 29 |
| ✅ hono-stable-quickjs | 142 | 0 | 29 |
| ✅ nest-stable-node | 142 | 0 | 29 |
| ✅ nest-stable-quickjs | 142 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 170 | 0 | 1 |
| ✅ nitro-stable-node | 142 | 0 | 29 |
| ✅ nitro-stable-quickjs | 142 | 0 | 29 |
| ✅ nuxt-stable-node | 142 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 142 | 0 | 29 |
| ✅ sveltekit-stable-node | 161 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 161 | 0 | 10 |
| ✅ tanstack-start-node | 142 | 0 | 29 |
| ✅ tanstack-start-quickjs | 142 | 0 | 29 |
| ✅ vite-stable-node | 142 | 0 | 29 |
| ✅ vite-stable-quickjs | 142 | 0 | 29 |
✅ 🐘 Local Postgres
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ astro-stable-node | 142 | 0 | 29 |
| ✅ astro-stable-quickjs | 142 | 0 | 29 |
| ✅ express-stable-node | 142 | 0 | 29 |
| ✅ express-stable-quickjs | 142 | 0 | 29 |
| ✅ fastify-stable-node | 142 | 0 | 29 |
| ✅ fastify-stable-quickjs | 142 | 0 | 29 |
| ✅ hono-stable-node | 142 | 0 | 29 |
| ✅ hono-stable-quickjs | 142 | 0 | 29 |
| ✅ nest-stable-node | 142 | 0 | 29 |
| ✅ nest-stable-quickjs | 142 | 0 | 29 |
| ✅ nextjs-turbopack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-stable-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-canary-quickjs | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-node | 170 | 0 | 1 |
| ✅ nextjs-webpack-stable-quickjs | 170 | 0 | 1 |
| ✅ nitro-stable-node | 142 | 0 | 29 |
| ✅ nitro-stable-quickjs | 142 | 0 | 29 |
| ✅ nuxt-stable-node | 142 | 0 | 29 |
| ✅ nuxt-stable-quickjs | 142 | 0 | 29 |
| ✅ sveltekit-stable-node | 161 | 0 | 10 |
| ✅ sveltekit-stable-quickjs | 161 | 0 | 10 |
| ✅ tanstack-start-node | 142 | 0 | 29 |
| ✅ tanstack-start-quickjs | 142 | 0 | 29 |
| ✅ vite-stable-node | 142 | 0 | 29 |
| ✅ vite-stable-quickjs | 142 | 0 | 29 |
✅ 🪟 Windows
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack-node | 170 | 0 | 1 |
| ✅ nextjs-turbopack-quickjs | 170 | 0 | 1 |
✅ 🌐 Cross-language Conformance
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ python | 68 | 0 | 84 |
✅ vercel-http-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 141 | 0 | 30 |
| ✅ express | 141 | 0 | 30 |
| ✅ hono | 141 | 0 | 30 |
| ✅ nextjs-turbopack | 168 | 0 | 3 |
| ✅ nitro | 141 | 0 | 30 |
| ✅ vite | 141 | 0 | 30 |
✅ vercel-multi-region
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ nextjs-turbopack | 27 | 0 | 0 |
✅ vercel-ws-transport
| App | Passed | Failed | Skipped |
|---|---|---|---|
| ✅ example | 141 | 0 | 30 |
| ✅ express | 141 | 0 | 30 |
| ✅ nextjs-turbopack | 168 | 0 | 3 |
| ✅ vite | 141 | 0 | 30 |
📊 Workflow Benchmarkscommit Backend:
Streams
📈 STSO distribution vs main (inline / queue-hop histograms)1020 steps (inline) Cumulative STSO time: main 141689ms → this run 183890ms (Δ +42201ms, +30%) 📈 CRTT drill-down vs main (RTT distributions & profiles)RTT over stream progress (avg per tenth of stream, bars scaled min→max): RTT by chunk size (avg per log size bin, ~160B → ~12KB serialized, bars scaled min→max): Delivery jitter over stream progress (avg positive CDV per tenth of stream, bars scaled min→max): 📜 Previous results (2)f337c3fMon, 28 Sep 2026 21:21:17 GMT · run logs
Streams
239f996Sat, 26 Sep 2026 15:41:12 GMT · run logs
Streams
ℹ️ Metric definitions & methodologyStreams: first-chunk RTT (the stream-open path, before any buffering/backpressure), CRTT percentiles, and worst delivery stall (CDV max). Cells are medians across iterations; per-run values in the artifacts. No 🔴/🟢 marks until targets attach. The collapsed STSO distribution section above buckets every step gap, split inline (same warm process — pure framework overhead) vs queue-hop (fresh process — dispatch, reinit, replay). The collapsed CRTT drill-down: per-variant RTT histograms (fixed log bins, 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) · 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) · CRTT: chunk round-trip time (per-chunk write → read latency, one clock domain: deployment → stream backend → same deployment) · CDV: chunk delay variation / delivery jitter (inter-arrival gap minus inter-write gap per seq-adjacent pair; skew-free; the row is each run's MAX positive value, so one stall moves it) 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 · paced control (100/s, 60B): the control: 300 tiny (~60B) deltas metronome-paced at 100/s — zero workload structure, so it reads the transport floor and flush cadence, and disambiguates transport-wide vs workload-specific when a replay row moves · size sweep (100/s, 160B-12KB): same pacing as the control with deltas padded in rotation across seven log-spaced sizes (~160B–12KB) — rotation decouples size from stream position, so it isolates whether chunk size causes latency · replay gateway-gpt-5.4-nano-2000t (1x): raw provider SSE cadence captured at the AI gateway boundary (gpt-5.4-nano, the most popular gateway model; per-token deltas p50 208B = the modal production chunk size), replayed exactly as measured — the typical customer's workload; its CDV is the typical customer's real delivery jitter · replay eve-gpt-5.6-sol-2000t (1x): a captured eve turn (gpt-5.6-sol, the most-used demanding eve model; ~2000 output tokens = production p50 turn length) replayed exactly as measured — eve's envelope protocol re-ships the cumulative message so sizes ramp 142B→13KB; the demanding outlier tenant's reality · replay eve-gpt-5.6-sol-2000t (2x): the same eve capture at 2x — the headroom/stress row; real fast-tier models emit the same chunk sizes at proportionally higher rate, so time compression is a faithful speed model · first chunk (pooled): every run's seq-0 RTT pooled across all stream scenarios — the first chunk precedes any workload differentiation, so pooling samples one shared stream-open path with exact percentiles Replay cadences (semantic sha256) — eve-gpt-5.6-sol-2000t 🔴 marks a percentile over its target (within target is left unmarked). Targets (p75/p90/p99, ms) — TTFS 200/300/600 All timestamps are deployment-side; runs are triggered in-deployment, so the CI runner and api.vercel.com sit outside every measured window. TTFS = Cold starts stay in the numbers (real bursty-workload latency, inflates P75+); Best is the warm floor. |
Sim WorldSimulated world deterministic testing for races. Traces 🟠 world-sim scenario book — 1 fail of 42 total
Full trace: |
About these numbersSizes are gzip; parentheses show the change against
|
alangenfeld
left a comment
There was a problem hiding this comment.
The reducer is the right fix, and it's applied consistently across host, VM, QuickJS and web-shared. One blocker, inline: registering a custom DataView reviver changes how payloads written before this PR revive.
The E2E Vercel failures look unrelated. The one I opened (example-node) is a 90s timeout in createHook({ experimental_force: true }) › declines to take a token from a run whose runtime predates involuntary disposal, and the branch is well behind main's spec-version changes, so a rebase will probably clear it.
This fixes a data leak, so it's a reasonable candidate for backporting to stable.
Local agent review (anthropic/claude-opus-5.5)
| new global.BigInt64Array(reviveArrayBuffer(value, global)), | ||
| BigUint64Array: (value: string) => | ||
| new global.BigUint64Array(reviveArrayBuffer(value, global)), | ||
| DataView: (value: string | ArrayBufferLike) => |
There was a problem hiding this comment.
Registering a custom DataView reviver changes how payloads written before this PR revive. They recorded the view's bounds as well as its buffer (["DataView",2,4464,4]), and on main devalue's built-in branch restores them via fromViewInfo(tag, buffer, value[2], value[3]). Once a custom reviver exists for the tag, devalue skips that branch and value[2]/value[3] are dropped.
With devalue 5.9.2 directly, reviving the same old payload:
legacy = [['DataView',1,1,2],['ArrayBuffer',2],'AQIDBA==']
main → byteOffset 1, byteLength 2, [2, 3]
PR → byteOffset 0, byteLength 4, [1, 2, 3, 4]
Old runs replay on their own deployment's code, so this mostly affects new code reading old logs. In the o11y UI and CLI it shows the whole 8 KiB pooled slab where main shows the 4 viewed bytes, i.e. it now displays the residue this PR is keeping out of new logs. Runs moved across deployments (deploymentId: 'latest', runs raised to a newer spec version) would also replay different bytes.
Suggestion: emit the new encoding under its own tag (e.g. DataViewBytes) and don't register a DataView reviver at all. Old ["DataView", …] tuples keep going through devalue's built-in path with their bounds, in every reader (QuickJS's fromViewInfo already handles DataView), and the four "accept an ArrayBuffer too" branches go away. New payloads stay unreadable by older SDKs either way.
Local agent review (anthropic/claude-opus-5.5)
There was a problem hiding this comment.
AI: Agreed, and adopted the suggestion — f337c3f8d.
I'd looked at the lost bounds and talked myself into them being acceptable, on the grounds that the whole buffer is what the old payload recorded. That reasoning only holds for replay. You're pointing at readers, and it doesn't hold there at all: web-shared and cli ship independently of the deployment that wrote the log, so this would have made the o11y UI render the pooled slab on exactly the payloads the PR exists to keep out of new logs. That's the wrong direction for a leak fix.
So: the reducer now emits DataViewBytes and no DataView reviver is registered anywhere. Old tuples keep taking devalue's built-in branch with their bounds in all four readers — I checked each one rather than assuming, since the built-in branch routes through a fromViewInfo parse operation and two of them override it: QuickJS's handles the tag and forwards byteOffset/length, and hydrateData uses devalue's default ops. Deleting the four "accept an ArrayBuffer too" branches and their two helpers makes this commit net-negative on lines.
One knock-on worth naming: a payload under the new tag is unreadable by an SDK predating it either way, but it now fails with Unknown type DataViewBytes instead of a TypeError from indexing values with a base64 string — a better failure.
Pinned in both directions: a legacy ["DataView",1,1,2] tuple revives as its recorded [2, 3] with byteOffset 1 / byteLength 2 (host + QuickJS), and a test asserts no reducer or reviver set claims the DataView tag.
| const hydrated = hydrateData(legacy, getCommonRevivers()) as DataView; | ||
|
|
||
| expect(hydrated).toBeInstanceOf(DataView); | ||
| expect([...new Uint8Array(hydrated.buffer)]).toEqual([1, 2, 3, 4]); |
There was a problem hiding this comment.
The legacy tuple has bounds (1, 2), so the value it recorded is [2, 3], which is what main revives. This assertion pins the lost-bounds behavior; it should expect [2, 3] with byteOffset/byteLength checked (see the comment on common.ts).
Local agent review (anthropic/claude-opus-5.5)
There was a problem hiding this comment.
AI: Right — that assertion was pinning the bug. Fixed in f337c3f8d.
With the encoding moved to its own tag the test now asserts what the tuple actually recorded: byteOffset 1, byteLength 2, and the bytes [2, 3]. I also renamed it from "still revives payloads written before the DataView reducer existed" to "revives a pre-existing DataView payload with its bounds", since the old name described the weaker property it was checking, and added the equivalent case to quickjs-serde.test.ts so the fromViewInfo path is covered too.
Alongside it there is now a test that a new subview payload is devl[["DataViewBytes",1],"AgM="], so the two encodings are pinned as distinguishable rather than just individually correct.
| BigInt: string; // string representation of bigint | ||
| BigInt64Array: string; // base64 string | ||
| BigUint64Array: string; // base64 string | ||
| DataView: string; // base64 string of the viewed bytes only |
There was a problem hiding this comment.
Nit: every DataView reviver here accepts string | ArrayBufferLike, so this type doesn't describe what the revivers take. Moot if the new encoding moves to its own tag.
Local agent review (anthropic/claude-opus-5.5)
There was a problem hiding this comment.
AI: Moot as you predicted — f337c3f8d. The key is now DataViewBytes and every reviver takes string, so the type describes them exactly.
Registering a custom reviver for `DataView` took devalue's built-in parse branch out of reach, and with it the bounds that branch restores from `["DataView", buf, offset, length]`. Those tuples are already in event logs, and the o11y UI and CLI read them with current code — so the previous commit made them render the whole pooled slab where `main` renders the few viewed bytes, surfacing exactly the residue it exists to keep out of new logs. Emit the base64 form under `DataViewBytes` instead and register no `DataView` reviver anywhere. Old payloads keep taking the built-in branch with their bounds in every reader (QuickJS's `fromViewInfo` already handles the tag), new payloads still carry only the viewed range, and the four "accept an ArrayBuffer too" reviver branches and their helpers go away. A payload written under the new tag remains unreadable by an older SDK, as it was before, but now fails with `Unknown type DataViewBytes` rather than a TypeError. Tests pin both directions: a legacy tuple revives as its recorded [2, 3] with byteOffset 1 / byteLength 2, and no reducer or reviver set claims the `DataView` tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com>
alangenfeld
left a comment
There was a problem hiding this comment.
f337c3f8d resolves the blocker. Older ["DataView", buf, offset, length] tuples now go through devalue's built-in branch with their bounds in every reader: host, QuickJS (its fromViewInfo forwards them) and web/CLI. New payloads are pinned byte for byte, and the removed ArrayBuffer branches leave less to maintain. One non-blocking test gap, inline.
FYI, not for this PR: vercel-py's devalue parser reads only devalue's built-in DataView, so a DataView sent from TS to a Python workflow now fails with Unknown type DataViewBytes. The other custom typed-array tags already fail there the same way, so it belongs with vercel-py's handling of the workflow reducer tags.
Pre-existing and unchanged here: an old DataView revived into the node:vm workflow realm comes back as a host-realm DataView, because devalue's default fromViewInfo constructs from globalThis.
Local agent review (anthropic/claude-opus-5.5)
| BigInt64Array: [['BigInt64Array', 1], '.'], | ||
| BigUint64Array: [['BigUint64Array', 1], '.'], | ||
| Class: [['Class', 1], { classId: 2 }, 'class//Example'], | ||
| DataViewBytes: [['DataViewBytes', 1], '.'], |
There was a problem hiding this comment.
The o11y UI and CLI are the readers that motivated the tag split, but the guard for it only lives in core (common-vm.test.ts, serialization/serialization.test.ts). Nothing here fails if someone later adds a DataView entry to getWebRevivers() or getCLIRevivers(), and this fixture set doesn't exercise it either: a DataView key with a fixture would pass this file.
Suggest adding, for both reviver sets:
it('revives a pre-existing DataView payload with its bounds', () => {
const legacy = [['DataView', 1, 1, 2], ['ArrayBuffer', 2], 'AQIDBA=='];
for (const revivers of [getWebRevivers(), getCLIRevivers()]) {
expect(revivers).not.toHaveProperty('DataView');
const dv = hydrateData(legacy, revivers) as DataView;
expect([dv.byteOffset, dv.byteLength]).toEqual([1, 2]);
expect([dv.getUint8(0), dv.getUint8(1)]).toEqual([2, 3]);
}
});Local agent review (`anthropic/claude-opus-5.5`)
* fix(core): serialize a DataView as its viewed bytes `DataView` had no reducer, so it fell through to devalue's built-in encoding: the whole backing `ArrayBuffer` plus the view's offset and length. Node hands out small `Buffer`s as windows onto a shared 8 KiB pool, so a four-byte `DataView` over a `Buffer.allocUnsafe(4)` serialized 8 KiB of unrelated allocations — and step returns are written to the run's event log, where that residue outlives the process that leaked it. Add a `DataView` reducer alongside the typed-array ones, base64 of the viewed range only, in all three reducer sets that share the wire format (host, VM twin, QuickJS handle-space) so a value round-trips identically whichever side serializes it. Revivers accept a non-string payload as well: devalue hands a custom reviver the hydrated referent, so a pre-reducer payload arrives as the backing `ArrayBuffer` rather than base64. Without that branch, old event logs would decode an `ArrayBuffer` as if it were base64 (or throw, in the `atob` implementations). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com> * fix(core): emit the new DataView encoding under its own tag Registering a custom reviver for `DataView` took devalue's built-in parse branch out of reach, and with it the bounds that branch restores from `["DataView", buf, offset, length]`. Those tuples are already in event logs, and the o11y UI and CLI read them with current code — so the previous commit made them render the whole pooled slab where `main` renders the few viewed bytes, surfacing exactly the residue it exists to keep out of new logs. Emit the base64 form under `DataViewBytes` instead and register no `DataView` reviver anywhere. Old payloads keep taking the built-in branch with their bounds in every reader (QuickJS's `fromViewInfo` already handles the tag), new payloads still carry only the viewed range, and the four "accept an ArrayBuffer too" reviver branches and their helpers go away. A payload written under the new tag remains unreadable by an older SDK, as it was before, but now fails with `Unknown type DataViewBytes` rather than a TypeError. Tests pin both directions: a legacy tuple revives as its recorded [2, 3] with byteOffset 1 / byteLength 2, and no reducer or reviver set claims the `DataView` tag. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com> --------- Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com>
|
Backport PR opened against |
* fix(core): serialize a DataView as its viewed bytes `DataView` had no reducer, so it fell through to devalue's built-in encoding: the whole backing `ArrayBuffer` plus the view's offset and length. Node hands out small `Buffer`s as windows onto a shared 8 KiB pool, so a four-byte `DataView` over a `Buffer.allocUnsafe(4)` serialized 8 KiB of unrelated allocations — and step returns are written to the run's event log, where that residue outlives the process that leaked it. Add a `DataView` reducer alongside the typed-array ones, base64 of the viewed range only, in all three reducer sets that share the wire format (host, VM twin, QuickJS handle-space) so a value round-trips identically whichever side serializes it. Revivers accept a non-string payload as well: devalue hands a custom reviver the hydrated referent, so a pre-reducer payload arrives as the backing `ArrayBuffer` rather than base64. Without that branch, old event logs would decode an `ArrayBuffer` as if it were base64 (or throw, in the `atob` implementations). * fix(core): emit the new DataView encoding under its own tag Registering a custom reviver for `DataView` took devalue's built-in parse branch out of reach, and with it the bounds that branch restores from `["DataView", buf, offset, length]`. Those tuples are already in event logs, and the o11y UI and CLI read them with current code — so the previous commit made them render the whole pooled slab where `main` renders the few viewed bytes, surfacing exactly the residue it exists to keep out of new logs. Emit the base64 form under `DataViewBytes` instead and register no `DataView` reviver anywhere. Old payloads keep taking the built-in branch with their bounds in every reader (QuickJS's `fromViewInfo` already handles the tag), new payloads still carry only the viewed range, and the four "accept an ArrayBuffer too" reviver branches and their helpers go away. A payload written under the new tag remains unreadable by an older SDK, as it was before, but now fails with `Unknown type DataViewBytes` rather than a TypeError. Tests pin both directions: a legacy tuple revives as its recorded [2, 3] with byteOffset 1 / byteLength 2, and no reducer or reviver set claims the `DataView` tag. --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: vercel[bot] <35613825+vercel[bot]@users.noreply.github.com> Co-authored-by: Nathan Rajlich <71256+TooTallNate@users.noreply.github.com>
The problem
DataViewwas the oneArrayBufferViewwithout a reducer, so it fell through to devalue's built-in encoding. That encoding emits the whole backingArrayBufferplus the view's offset and length:Node hands out small
Buffers as windows onto a shared 8 KiB pool (Buffer.allocUnsafe, andBuffer.frombelowBuffer.poolSize >>> 1), andallocUnsafedoes not zero what it returns. So a four-byteDataViewover a pooled buffer serialized the whole slab of unrelated prior allocations. Step returns are written to the run's event log, so that residue is persisted (encrypted, but persisted) and outlives the process that leaked it.Measured on
main, aDataViewover aBuffer.allocUnsafe(4)holding[1, 2, 3, 4]:Base64-decoding that payload yields the source text of an unrelated module that happened to be in the pool. Typed arrays were never exposed: they have had reducers all along, which claim the value before devalue's classification runs.
The fix
Add a reducer that claims
DataView— base64 of the viewed range only, via the existingviewToBase64/viewInfohelpers, so the range is still read through hardened internal-slot getters and a shadowedbyteOffsetcannot widen it.After:
The tag is
DataViewBytes, notDataView, and noDataViewreviver is registered anywhere — see "Older event logs" below for why.The same reducer/reviver pair is added to all three sets that share this wire format, so a
DataViewround-trips identically whichever side serializes it:serialization/reducers/common.tscodec-devalue-vm)serialization/reducers/common-vm.tsruntime/quickjs-serde.tsThis is not just tidiness: the workflow VM serializes step arguments that the host step executor deserializes. Claiming the tag on one side only would hand the other side's reviver a shape it does not expect.
SerializableSpecialgainsDataViewBytes, which the repo's own guard tests then require downstream — a reviver inpackages/web-shared(the CLI reuses core's) and a fixture inserializable-revivers.test.ts. Without the web reviver the o11y UI would throwUnknown type DataViewByteson any payload containing one.Deliberately not done: bumping
devalue. The encoding above is devalue behaving as documented — aDataViewis defined by a buffer and a window onto it, and devalue preserves both. The leak is that a workflow event log is the wrong place to preserve the rest of that buffer, which is ours to decide, in a reducer.Older event logs
This is why the new encoding gets its own tag, and it is the one thing in this PR that is not obvious. Credit to @alangenfeld's review for catching it — the first revision of this PR did register a
DataViewreviver, and was wrong.devalue skips its built-in branch entirely for any tag that has a custom reviver, and hands that reviver the hydrated referent rather than the tuple. Payloads already in event logs encode a
DataViewas["DataView", buf, offset, length], and only the built-in branch can read those bounds back (fromViewInfo(tag, buffer, value[2], value[3])); a custom reviver never sees them. Registering one underDataViewwould therefore have widened every such payload to its whole backing buffer:Replay is not the exposure — a run replays on the deployment it started from. Readers are:
web-sharedandcliship independently of whatever wrote the log, so the o11y UI would have started rendering the whole 8 KiB pooled slab for exactly the payloads this PR exists to keep out of new logs. Under a distinct tag the built-in branch stays reachable and those payloads revive unchanged, in all four readers — QuickJS'sfromViewInfoalready handlesDataView, andhydrateDatauses devalue's default operations.Note the converse, unchanged either way: a payload written by this change is not readable by an SDK that predates it, as for any reducer addition. It now fails with
Unknown type DataViewBytesrather than a TypeError from indexingvalueswith a base64 string.Tests
serialization.test.ts— a pooledDataViewthroughdehydrateStepReturnValue/hydrateStepReturnValue, asserting the exact wire bytes (devl[["DataViewBytes",1],"AQIDBA=="]) and that the payload names noArrayBuffer. The test asserts its own precondition (buffer.byteLength > byteLength) so it cannot pass vacuously ifBufferstops pooling. Plus a subview onto a larger buffer, a zero-length view, and revival into the workflow VM realm.["DataView",1,1,2]tuple revives as its recorded[2, 3]withbyteOffset1 /byteLength2 (host and QuickJS), a new subview payload isdevl[["DataViewBytes",1],"AgM="], and no reducer or reviver set in core claims theDataViewtag.serialization/serialization.test.ts— reducer and reviver units, including the pooled case and the non-DataViewrejections.reducers/common-vm.test.ts— the host and VM reducers agree byte for byte on the same subview. The two read the view's range through different primitives, so this is worth pinning.runtime/quickjs-serde.test.ts— three cases added to the guest↔host wire-parity table, plus round trips for a new subview and a legacy tuple.Verification
@workflow/core(2801),@workflow/web-shared(236),@workflow/cli(156) pass, as doerrors,utilsandworld.pnpm typecheckis clean across all 43 tasks. Biome reports two pre-existingnoNonNullAssertionerrors inweb-shared/src/lib/hydration.ts, present onmainand untouched here.A whole-repo
pnpm testfails in this devbox for reasons unrelated to the change — 15 concurrent vitest tasks exhaust it, andcargois absent for the Rust plugin. A control run on a clean tree fails the same way; the per-package runs above are the signal.Docs Preview
The supported-types list gains
DataView.foundations/serializationThese links require Vercel team access — the preview sits behind deployment protection.
🤖 Generated with Claude Code