From 647afe327b035952aa6dce3d5db9d142bd4d2dbb Mon Sep 17 00:00:00 2001 From: backryun Date: Sun, 26 Jul 2026 15:52:04 +0900 Subject: [PATCH] refactor(sse): declare the multipart/gRPC frame bodies as ArrayBuffer-backed (#8533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * refactor(sse): declare the multipart/gRPC frame bodies as ArrayBuffer-backed Seven TS2769 across three files, one root cause: a bare `Uint8Array` widens to `Uint8Array`, which admits `SharedArrayBuffer` and so is not assignable to `BodyInit`. Every one of the seven is a `fetch({ body })` call rejecting an otherwise-valid payload. They flow from exactly two producers: buildMultipartBody() audioTranscription.ts -> 5 sites here + 1 in audioTranslation.ts grpcWebFrame() windsurf.ts -> 1 site Both allocate with `new Uint8Array(length)`, which is always ArrayBuffer-backed, so declaring `Uint8Array` states what the code already guarantees. This is a narrowing of the declaration to match reality, not a cast at the call sites — no `as BodyInit` anywhere, and the seven call sites are untouched. 280 -> 273, zero new, on a line-number-agnostic diff of the full tsc error set. The runtime diff is two return-type annotations; nothing executable changed. Tests: buildMultipartBody already has four direct tests, but they assert the payload's *contents* (boundary, filename sanitization, MIME fallback) — none assert the backing, which is precisely what the new type guarantees and what a plausible switch to a pooled `Buffer.concat` would break. Added multipart-body-arraybuffer-backing.test.ts (3 tests): the buffer is a plain ArrayBuffer, the view spans it entirely at offset 0 (no pool remainder), and both hold for a 64 KiB payload past Node's Buffer-pool threshold. 258/258 across the 17 affected audio/windsurf suites. Not included: the 3 remaining TS2339 in audioTranscription.ts (`data?.data?.…` on an untyped poll result). Different root cause — grouped by kind, not by file, per #8484. * chore(quality): trim the buildMultipartBody doc so audioTranscription.ts stays under the 800 cap My doc comment on buildMultipartBody added 9 lines to a file with 8 to spare (792 -> 801 as check:file-size counts, cap 800), so this PR was failing the gate on growth I introduced. Condensed the comment to 4 lines; the file lands at 796. Rebuilt on top of the /green-prs sync merge (9973bf3bf) rather than force-pushing my own rebase over it — the release-tip sync there is @ikelvingo's work and had already been validated against this branch. Delta unchanged: 280 -> 273, 7 fixed, zero new. 68/68 on the audio/windsurf suites; typecheck:core and eslint clean; check:file-size now OK. --------- Co-authored-by: backryun Co-authored-by: ikelvingo --- open-sse/executors/windsurf.ts | 12 +++- open-sse/handlers/audioTranscription.ts | 7 ++- ...multipart-body-arraybuffer-backing.test.ts | 62 +++++++++++++++++++ 3 files changed, 78 insertions(+), 3 deletions(-) create mode 100644 tests/unit/multipart-body-arraybuffer-backing.test.ts diff --git a/open-sse/executors/windsurf.ts b/open-sse/executors/windsurf.ts index 19924afa2a..d87e245149 100644 --- a/open-sse/executors/windsurf.ts +++ b/open-sse/executors/windsurf.ts @@ -248,8 +248,16 @@ function buildGetChatMessageRequest( // ─── gRPC-web framing ──────────────────────────────────────────────────────── -/** Wrap a protobuf message in a 5-byte gRPC-web data frame. */ -function grpcWebFrame(payload: Uint8Array): Uint8Array { +/** + * Wrap a protobuf message in a 5-byte gRPC-web data frame. + * + * Returns `Uint8Array`, not bare `Uint8Array`: the frame is + * allocated with `new Uint8Array(length)`, which is always ArrayBuffer-backed, + * and only that narrower form satisfies `BodyInit` at the `fetch` call below. + * Bare `Uint8Array` widens to `Uint8Array`, which admits + * `SharedArrayBuffer` and is therefore rejected as a request body. + */ +function grpcWebFrame(payload: Uint8Array): Uint8Array { const frame = new Uint8Array(5 + payload.length); frame[0] = 0x00; // compression flag: no compression const view = new DataView(frame.buffer); diff --git a/open-sse/handlers/audioTranscription.ts b/open-sse/handlers/audioTranscription.ts index 55762eb4cf..f119860cad 100644 --- a/open-sse/handlers/audioTranscription.ts +++ b/open-sse/handlers/audioTranscription.ts @@ -72,11 +72,16 @@ function getUploadedFileName(file: Blob & { name?: unknown }): string { return typeof file.name === "string" && file.name.length > 0 ? file.name : "audio.wav"; } +/** + * `body` is `Uint8Array`, not bare `Uint8Array`: `new Uint8Array(n)` + * is always ArrayBuffer-backed, and only that narrower form satisfies `BodyInit` + * (the bare type widens to `ArrayBufferLike`, which admits `SharedArrayBuffer`). + */ export async function buildMultipartBody( file: Blob & { name?: unknown }, fields: Record, fileFieldName = "file" -): Promise<{ body: Uint8Array; contentType: string }> { +): Promise<{ body: Uint8Array; contentType: string }> { const boundary = "----OmniRouteAudioBoundary" + Date.now().toString(36); const parts: Uint8Array[] = []; const encoder = new TextEncoder(); diff --git a/tests/unit/multipart-body-arraybuffer-backing.test.ts b/tests/unit/multipart-body-arraybuffer-backing.test.ts new file mode 100644 index 0000000000..b267416f06 --- /dev/null +++ b/tests/unit/multipart-body-arraybuffer-backing.test.ts @@ -0,0 +1,62 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +const { buildMultipartBody } = await import("../../open-sse/handlers/audioTranscription.ts"); + +/** + * `buildMultipartBody` is handed straight to `fetch` as the request body by six + * call sites across `audioTranscription.ts` and `audioTranslation.ts`, so its + * result must satisfy `BodyInit` — which requires an `ArrayBuffer`-backed view, + * not merely "some Uint8Array". A `SharedArrayBuffer`-backed view, or a slice + * into a shared pool, is not a valid request body. + * + * The four existing `buildMultipartBody` tests in + * `audio-transcription-handler.test.ts` decode the payload and assert its + * *contents* (boundary, filename sanitization, MIME fallback). None of them + * assert the backing, which is the property the declared + * `Uint8Array` return type exists to guarantee — and the one a + * plausible "optimization" to a pooled `Buffer.concat` would break. + */ + +function audioFile(name = "clip.wav", type = "audio/wav") { + return Object.assign(new Blob([new Uint8Array([1, 2, 3, 4])], { type }), { name }); +} + +test("the assembled body is backed by a plain ArrayBuffer, never a shared one", async () => { + const { body } = await buildMultipartBody(audioFile(), { model: "whisper-1" }); + + assert.ok(body instanceof Uint8Array); + assert.ok( + body.buffer instanceof ArrayBuffer, + "a SharedArrayBuffer-backed view is not accepted as a fetch BodyInit" + ); +}); + +test("the assembled body owns its whole buffer — not a view into a shared pool", async () => { + const { body } = await buildMultipartBody(audioFile(), { model: "whisper-1" }); + + // `new Uint8Array(totalLength)` allocates exclusively. A pooled allocation + // (e.g. switching to Buffer.concat) would leave a non-zero byteOffset and a + // buffer larger than the payload, so the bytes handed to fetch would no + // longer be the whole buffer. + assert.equal(body.byteOffset, 0, "body must start at offset 0 of its own buffer"); + assert.equal( + body.byteLength, + body.buffer.byteLength, + "body must span its entire buffer, with no pooled remainder" + ); +}); + +test("the backing holds for a larger payload than a single pool slab", async () => { + // 64 KiB — comfortably past Node's 8 KiB Buffer pool threshold, so a pooled + // implementation would behave differently here than for the small case above. + const big = Object.assign(new Blob([new Uint8Array(64 * 1024)], { type: "audio/wav" }), { + name: "big.wav", + }); + const { body } = await buildMultipartBody(big, { model: "whisper-1" }); + + assert.ok(body.buffer instanceof ArrayBuffer); + assert.equal(body.byteOffset, 0); + assert.equal(body.byteLength, body.buffer.byteLength); + assert.ok(body.byteLength > 64 * 1024, "payload carries the file plus the multipart envelope"); +});