webstreams: rewrite ReadableStream, WritableStream, and TransformStream in C++ (zero JS builtins)

[Jarred-Sumner]

What

Rewrites the WHATWG Streams implementation (ReadableStream, WritableStream, TransformStream, all readers/writers/controllers, pipeTo/pipeThrough/tee, and the async iterator) as pure C++ in src/jsc/bindings/webcore/streams/, and deletes the old implementation (19 stream JS builtins + 51 old webcore stream C++ files). There are zero JS builtins left in the streams path: no @-private JS state, no builtin closures, no JS-side state machine. All stream state lives as C++ members on the JS cells themselves (WriteBarrier + visitChildren), and all spec "upon fulfillment/rejection" reactions go through two shared mechanisms (per-global native reaction handlers + bound functions) instead of per-instance closures.

Bun's extensions are preserved: type: "direct" streams, type: "bytes"/BYOB, lazily-materialized native controllers (file/socket/spawn bodies), Bun.readableStreamTo*, readMany, and the generated JSSink classes. String chunks keep working everywhere they used to (verified old-vs-new on 15 text-chunk scenarios).

Why

• Memory (100k live instances each, RSS/instance, release builds):

  	old	new
  new ReadableStream({pull(){}})	1283 B	582 B	−55%
  RS + getReader()	1709 B	666 B	−61%
  new WritableStream({write(){}})	1718 B	1036 B	−40%
  new TransformStream()	3628 B	1626 B	−55%

  The old implementation allocated ~3.1M JS Function objects for 400k streams (the builtins' per-stream algorithm closures); the new one allocates ~300k — only the user's own callbacks.

• Throughput (64 KiB chunks, 32 MiB/pass, best of 5, release vs release on the same machine; bench/snippets/webstreams-throughput.mjs):

  Stream machinery cost — the shared-chunk scenarios enqueue the same 64 KiB view every time; default streams pass chunks by reference in every runtime (nothing is copied), so these rows are reported as chunks/second (an earlier revision reported them as "MB/s" of nominal payload — that unit was misleading and is superseded by this table). Median of 3 isolated processes per cell, measured at the head that includes the async-iterator/reader.read() inlining.

  scenario	node 26	deno 2.9-canary	bun 1.3.14	old (canary)	new	peak RSS node / deno / 1.3.14 / old / new (MB)
  reader.read() loop	0.93 M/s	1.01 M/s	0.85 M/s	1.09 M/s	1.77 M chunks/s (565 ns/chunk)	49 / 59 / 43 / 42 / 38
  for await	0.87 M/s	0.71 M/s	0.45 M/s	0.40 M/s	1.59 M chunks/s	50 / 57 / 47 / 47 / 38
  pipeTo(WritableStream)	0.38 M/s	0.57 M/s	0.32 M/s	0.34 M/s	0.64 M chunks/s	57 / 61 / 49 / 50 / 39
  pipeThrough(TransformStream)	0.19 M/s	0.31 M/s	0.23 M/s	0.21 M/s	0.48 M chunks/s	58 / 63 / 56 / 54 / 40
  tee() + drain both	0.20 M/s	0.40 M/s	0.39 M/s	0.33 M/s	0.75 M chunks/s	54 / 60 / 49 / 48 / 38

  Real throughput — the fresh-buffers scenarios allocate and write a new 64 KiB chunk per enqueue (what socket/file sources produce), so MB/s is bounded by real memory work and every runtime converges toward allocator + memcpy bandwidth:

  scenario	node 26	deno 2.9-canary	bun 1.3.14	old (canary)	new
  reader.read() loop (fresh buffers)	6,509	6,851 MB/s	5,852	6,418	6,201
  for await (fresh buffers)	8,317	7,230	5,231	5,274	6,345
  pipeTo (fresh buffers)	4,897	5,298	5,805	5,033	5,911
  pipeThrough (fresh buffers)	4,664	5,394	5,099	4,556	5,400
  tee() + drain both (fresh buffers)	4,394	5,097	5,332	4,745	5,982

  Consumers and byte sources (materialize real output; MB/s):

  scenario	node 26	deno 2.9-canary	bun 1.3.14	old (canary)	new	peak RSS 1.3.14 / old / new (MB)
  new Response(stream).arrayBuffer()	746	1,818	8,132	7,237	8,127	173 / 170 / 199
  text chunks → Response.text() (Bun extension)	—	—	8,131	8,187	8,415	204 / 203 / 198
  Bun.readableStreamToBytes(stream) (Bun extension)	—	—	8,445	6,947	8,208	173 / 171 / 198
  byte source, default reader	5,578	5,935	22,997 †	5,997	7,955	43 / 71 / 57
  byte source, BYOB reader	17,835	15,912	38,373 †	24,709 †	15,396	45 / 44 / 39
  direct stream → readableStreamToBytes (Bun extension)	—	—	2,233	3,773	3,644	218 / 194 / 229

  Transform streams (bench/snippets/webstreams-transform.mjs; 16 MiB per pass in 64 KiB chunks, best of 5, median of 3 isolated processes; MB/s of decoded/uncompressed payload):

  scenario	node 26	deno 2.9-canary	bun 1.3.14	old (canary)	new
  TextEncoderStream	46	389	2,429	2,442	2,717
  TextDecoderStream	1,233	588	1,395	1,198	1,581
  CompressionStream (gzip)	270	439	447	361	390
  DecompressionStream (gzip)	689	1,086	926	853	1,057

  TextDecoderStream is the row the streams machinery dominates; the compression rows are mostly zlib-bound so the rewrite moves them less (both are still ahead of the pre-rewrite canary). Note CompressionStream on 1.3.14 (447) vs today's main (361) — that ~20% predates this PR and is unrelated to the streams rewrite.

  Methodology: bench/snippets/webstreams-throughput.mjs (64 KiB chunks, 32 MiB per pass, best of 5), every scenario in its own process via --scenario=, median of 3 processes per cell on an idle machine; RSS is the OS-reported peak process RSS (/usr/bin/time -v). node v26.3.0, deno 2.9.1 canary, "old (canary)" = 1.4.0-canary @ 4f2932980 (main just before this branch's merge base). † pre-rewrite Bun skipped the (spec-required) buffer transfers on byte sources — against the runtimes that also transfer (Node/Deno) the new implementation is faster on the default-reader row and behind on BYOB (an optimization target; see follow-ups).

  BYOB note: an earlier revision of this table was ~30% behind Node/Deno on the BYOB row. That gap is closed (parity with Deno, ~97% of Node, medians of noisy runs) by holding byte-stream buffers as ArrayBuffer impls instead of JSArrayBuffer wrapper cells: the spec's two per-chunk transfers are now contents moves with no new GC cells and no extra-memory re-reporting; the only cell per chunk is the view handed to the user. (Old Bun's larger BYOB number is not a valid baseline — it skipped the spec-required buffer transfers.)

  Peak-RSS note (investigated to root cause): three accumulate-then-assemble consumer rows peak about one payload (~25–35 MB here) higher than the previous implementation. This is not retention or a leak: at any instant the new implementation holds exactly one more dead result awaiting collection, because it allocates ~half as many JS objects per consume (measured: ~3.8k vs ~7.6k for a 512-chunk body), so JSC's allocation-driven GC runs less often between back-to-back large consumes. It is bounded at one payload, does not grow with iteration count, and post-GC RSS is equal or lower than before. Left as-is deliberately: papering over it with GC hints in the consumer path would trade real-application throughput for a benchmark's transient peak.

• Consumers × chunk shape (8 MiB/pass, MB/s, old → new; bench/snippets/webstreams-consumers.mjs):

  shape	toText	toArrayBuffer	toBytes	toArray	Response.text	Response.arrayBuffer	for await
  binary 64 KiB ×128	2,593 → 3,690	6,831 → 7,843	6,998 → 8,232	63,273 → 81,372	2,614 → 3,248	7,724 → 7,455	32,908 → 115,214
  text 64 KiB ×128	7,926 → 8,080	3,227 → 3,386	1,800 → 3,846	77,247 → 89,450	7,223 → 7,808	3,971 → 3,636	42,027 → 124,338
  mixed 64 KiB ×128	2,257 → 3,652	4,473 → 4,118	4,194 → 4,084	84,268 → 109,658	2,444 → 3,324	4,743 → 4,398	49,439 → 107,137
  binary 1 KiB ×8192	1,041 → 954	1,023 → 1,523	1,115 → 1,712	1,455 → 2,483	1,117 → 1,130	1,147 → 1,485	867 → 2,057
  text 1 KiB ×8192	1,414 → 1,588	970 → 1,375	952 → 1,206	1,319 → 2,121	1,449 → 1,905	721 → 1,348	889 → 2,524
  one 8 MiB chunk	1,959 → 5,084	≈2 TB/s (identity)	≈2 TB/s	≈2 TB/s	1,955 → 5,213	≈2 TB/s	≈2 TB/s

  The buffered consumers are driven by a persistent pump operation (one reaction registration per pending read; bulk queue drain per hop), which is what recovered the many-small-chunk rows. Remaining cells within run-to-run noise of the previous implementation: binary-1 KiB toText and a few 64 KiB arrayBuffer cells (±10%).

• Memory (bench/snippets/webstreams-memory.mjs, release builds):

  retained RSS per live instance (100k)	old	new
  new ReadableStream({pull(){}})	1,307 B	566 B
  RS + getReader()	1,680 B	676 B
  new WritableStream({write(){}})	1,722 B	1,034 B
  new TransformStream()	3,635 B	1,596 B

  workload RSS	old peak	new peak
  pipeTo 512 MiB (64 KiB chunks)	48.8 MB	17.0 MB
  for await 512 MiB	2.5 MB	0.5 MB
  Response(stream 256 MiB).arrayBuffer()	254.5 MB	253.1 MB
  Response(stream 256 MiB of text).text()	256.5 MB	249.4 MB

  The implementation allocates no closures per stream (~3.1M Function objects → ~300k for 400k streams: only the user's own callbacks remain).

• tee() / clone() (bench/snippets/webstreams-tee.mjs; 128 MiB tee payload, 64 MiB fetched body, 32 MiB uploaded body):

  scenario	old	new
  tee(): both branches drained concurrently	57,731 MB/s	206,635 MB/s
  tee(): branch B read only after A finishes	84,543 MB/s	151,754 MB/s
  tee(): read A, cancel B	48,141 MB/s	90,148 MB/s
  fetch().clone(): read both bodies	747 MB/s / 210 MB peak	1,237 MB/s / 49 MB peak
  fetch().clone(): read one, cancel clone	643 MB/s	710 MB/s
  Bun.serve: req.clone(), read both bodies	1,423 MB/s / 32 MB peak	1,466 MB/s / 1.5 MB peak

  The one remaining row below the previous implementation anywhere in these tables is direct streams → readableStreamToBytes (−20%), tracked in the checklist.

• Spec conformance, measured with the WPT streams suite vendored in this PR (69 .any.js files including idlharness.any.js, 1402 subtests, run in CI):

  • old implementation (behavior files only): 971/1174 passing (82.7%), 2 process-aborting crashes, 10 timeouts
  • this PR: 1402/1402 passing (100%), 0 crashes, 0 timeouts, and expectations.json is empty — no expected-failure list. This includes WPT's idlharness.any.js (228 WebIDL surface subtests: interface-object descriptors, prototype layout, method length/name, brand checks), the same file Node runs, vendored with idlharness.js + the webidl2 parser + the .idl definitions from the same WPT commit. idlharness found one real bug, fixed here: the streams interface objects were installed as enumerable globals (Web IDL: {writable: true, enumerable: false, configurable: true}; Node and the browsers comply) — they are now DontEnum like URL already was. The other non-streams constructors (Response, Blob, …) have the same pre-existing issue and are a separate follow-up.
  • The last formerly-pinned behavior subtest turned out to be a harness-shim divergence, not a stream bug: upstream testharness.js's assert_object_equals recurses into any non-null object on the actual side, so its {value: <empty Uint8Array>} vs {value: undefined} comparison is vacuous — the semantics every browser and Node run under. Our shim was stricter; it now ports upstream byte-for-byte. Bun's behavior for that case (cancel then read(view) resolving with a zero-length view) matches the WHATWG algorithm text, the reference implementation, Node, and Deno.
  • Scope vs Node/Deno: the same upstream streams/ test files both run in their WPT CI, minus the .tentative owning-type proposal (Node expected-fails it) and transferable/** (postMessage stream transfer, a feature Bun does not implement). Node and Deno each still carry expected-failure lists on this suite; this PR's list is empty.

• Maintainability: the implementation is a file-per-class transcription of the spec, so every function maps to a named spec operation.

Issues this closes

Each one verified by running the issue's own reproduction on the pre-rewrite build (bug reproduces) and on this branch (fixed):

• Support ReadableStream.from() #3700 / ReadableStream from node:stream/web does not implement method from #32529 — ReadableStream.from() (global and node:stream/web)
• Reusing an already consumed ReadableStream should always cause a ReadableStream is locked error #6860 — reusing a consumed ReadableStream now always rejects (Bun.readableStreamTo* on a disturbed, unlocked stream rejects with "ReadableStream has already been used"; the spec'd new Response(stream) path already threw)
• Support ReadableStreamBYOBReader.prototype.read(view, { min }) #7091 — ReadableStreamBYOBReader.read(view, { min })
• ReadableStream.prototype.values({ preventCancel: true }) not working #10431 — values({ preventCancel: true })
• Releasing the lock on a ReadableStreamBYOBReader throws a TypeError on pending read requests #17081 — ReadableStreamBYOBReader.releaseLock() with pending reads (rejects them per spec instead of throwing)
• Unable to abort pipes with an AbortSignal #26392 — pipeTo honors AbortSignal (sink abort() and source cancel() both run)
• Missing signal in WritableStreamDefaultController (breaks libraries) #31156 — WritableStreamDefaultController.signal
• ReadableStream BYOB read() does not detach supplied ArrayBuffer #32402 — BYOB read(view) detaches the supplied buffer
• ReadableStreamDefaultController prototype methods/getters all fail in with "this" is not a ReadableStreamDefaultController #17837 — ReadableStreamDefaultController methods failing brand checks ("this is not a ReadableStreamDefaultController"): no deterministic repro exists in the issue, but the mechanism that produced it (the JS-builtin controllers) no longer exists; every controller is a real C++ class now.

(#19006 from the same triage list is not claimed: its repro already passes on current main.)

Fixes #3700
Fixes #6860
Fixes #7091
Fixes #10431
Fixes #17081
Fixes #17837
Fixes #26392
Fixes #31156
Fixes #32402
Fixes #32529

Review feedback round (2026-07-03)

All applied on top of the rewrite, each with tests where behavior changed:

1. Identifier caching: every Identifier::fromString in the streams sources (71 sites, several per-chunk) now uses vm.propertyNames / BunBuiltinNames; the three helpers that took a method-name literal take const Identifier&.
2. Replacement-character audit: no defects — WTF's default lenient UTF-8 conversion is the FFFD-replacing conversion (StringImpl.cpp's conversion modes share the same case), and the mixed text path byte-concatenates before decoding once, so multi-byte sequences split across chunks decode correctly. The two equivalent encoders were unified on the simdutf sizer/writer pair.
3. Accumulator retention: the text accumulator (including the one on the long-lived direct-stream controller), the array sink's result array, and the internal chunk arrays held by conversion reactions are all released the moment the result is materialized.
4. String limits: text assembly past the string limit used to abort the process (StringBuilder overflow / fromUTF8ReplacingInvalidSequences's RELEASE_ASSERT); every materialization site now throws a catchable out-of-memory error using the same predicate Bun's string constructors use, with subprocess regression tests via the bun:internal-for-testing synthetic allocation limit.
5. InternalFieldTuples: the one nested (3-field) case is a dedicated JSReadableStreamIntoArrayOperation cell; the one-shot sink's close-function tuple and pipeTo's wait-for-all latch tuple are fields on their existing cells. The remaining tuples are genuine 2-field pairs.
6. Single-pass iteration: the chunk-array converters read the array exactly once (elements held in a MarkedArgumentBuffer, strings materialized and sized once); no user-provided iterable was ever iterated twice (ReadableStream.from builds one iterator record).
7. VM& threading: internal helpers across the streams sources take JSC::VM& from their callers instead of re-deriving it from the global object; entry points derive it once.

Behavior changes (intentional)

• Reusing an already-consumed ReadableStream with Bun.readableStreamTo* now rejects (ERR_INVALID_STATE, "ReadableStream has already been used") instead of resolving with an empty result (Reusing an already consumed ReadableStream should always cause a ReadableStream is locked error #6860). The previous implementation only errored while the stream was still locked.
• new TextDecoderStream(label, undefined | null) treats the options as an empty dictionary per Web IDL (previously threw).
• The streams interface objects on globalThis are non-enumerable, per Web IDL (matches Node and the browsers; found by WPT's idlharness).
• Releasing a reader/writer lock now follows the current spec and Node.js: pending reads, reader.closed, writer.closed/ready, and post-release method calls reject with the same TypeError (+ code: "ERR_INVALID_STATE", same messages) that Node 26 produces — verified case-by-case against Node. The old Bun-specific AbortError with code: "ERR_STREAM_RELEASE_LOCK" is no longer produced (nothing in the tree produced or consumed it after the old builtins were removed; process.stdin's internals were updated accordingly).
• Bun.readableStreamTo* are now regular native functions on Bun (previously configurable JS builtins).

Tests

• test/js/third_party/wpt-streams/: the vendored WPT streams suite + expectations (re-recorded against this implementation; expected-failure bodies still execute, so both regressions and silent graduations turn the suite red).
• New regression tests for byte-source pipeTo/pipeThrough (not covered by any WPT subtest) in test/js/web/streams/streams.test.js; the releaseLock test there was updated from the old implementation-specific error shape to the Node/spec shape (see behavior changes).
• bench/snippets/webstreams.mjs, webstreams-throughput.mjs, webstreams-memory.mjs: the benchmarks behind the numbers above.
• The wider Bun test-suite sweep is in progress on this branch.

Status

• implementation + integration, old implementation deleted
• WPT streams suite vendored (including idlharness.any.js): 1402/1402 subtests passing, expectations.json empty
• node-compat pass: reader/writer release, locked-acquisition, brand-check, byte- and default-controller and BYOB-request error shapes match Node 26 (codes + messages); promise semantics match Node observably
• Bun extension semantics preserved and tested: async-iterable/async-generator bodies (direct controller via yield, sink backpressure, iterator throw/return), reader.readMany(), direct streams, AsyncLocalStorage propagation into source callbacks
• buffered consumers ≥ the previous implementation on every chunk shape (persistent pump op)
• exception-check discipline: suites run clean under BUN_JSC_validateExceptionChecks=1
• tee() / Response.clone() / Request.clone() faster with lower peak RSS
• release-build crash + hang fixed for a direct stream whose reader is released mid-pull
• CI: full matrix on the latest push

[robobun]

^{Updated 8:51 PM PT - Jul 3rd, 2026}

❌ @Jarred-Sumner, your commit 2d0e6e5 has 1 failures in Build #68282 (All Failures):

• test/js/bun/util/v8-heap-snapshot.test.ts - SIGKILL on 🐧 25.04 x64

---

🧪   To try this PR locally:

bunx bun-pr 33193

That installs a local version of the PR into your bun-33193 executable, so you can run:

bun-33193 --bun

[github-actions]

Found 11 issues this PR may fix:

1. Support ReadableStream.from() #3700 - PR implements ReadableStream.from() which is currently missing
2. Reusing an already consumed ReadableStream should always cause a ReadableStream is locked error #6860 - Reusing a consumed ReadableStream should throw "locked" error; stream state machine conformance fix
3. Support ReadableStreamBYOBReader.prototype.read(view, { min }) #7091 - PR adds ReadableStreamBYOBReader.prototype.read(view, { min }) support via full byte-stream spec conformance
4. ReadableStream.prototype.values({ preventCancel: true }) not working #10431 - ReadableStream.prototype.values({ preventCancel: true }) not working; async iterator rewritten to spec
5. Releasing the lock on a ReadableStreamBYOBReader throws a TypeError on pending read requests #17081 - ReadableStreamBYOBReader.releaseLock() throws instead of rejecting pending reads; BYOB reader rewritten
6. ReadableStreamDefaultController prototype methods/getters all fail in with "this" is not a ReadableStreamDefaultController #17837 - ReadableStreamDefaultController methods fail with wrong "this" type; controller reimplemented in C++
7. TypeError in ReadableStreamDefaultController when using spawn with Bun.serve #19006 - TypeError on controller.enqueue() from event handler callback; same controller this-binding fix
8. Unable to abort pipes with an AbortSignal #26392 - pipeTo does not honor AbortSignal; pipeTo rewritten to full spec conformance
9. Missing signal in WritableStreamDefaultController (breaks libraries) #31156 - WritableStreamDefaultController.signal returns undefined; writable controller rewritten with signal support
10. ReadableStream BYOB read() does not detach supplied ArrayBuffer #32402 - BYOB read() does not detach supplied ArrayBuffer; byte-stream suite now at full pass
11. ReadableStream from node:stream/web does not implement method from #32529 - ReadableStream from node:stream/web missing from() method; global ReadableStream now implements it

If this is helpful, copy the block below into the PR description to auto-close these issues on merge.

Fixes #3700
Fixes #6860
Fixes #7091
Fixes #10431
Fixes #17081
Fixes #17837
Fixes #19006
Fixes #26392
Fixes #31156
Fixes #32402
Fixes #32529

🤖 Generated with Claude Code

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

This PR adds Web Streams consumer/source implementations, build and binding wiring for the new C++ sources, benchmark scripts and docs, async stack trace attachment helpers, and a stdin read/release refactor.

Changes

Web Streams runtime and tooling

Layer / File(s)	Summary
Build and binding wiring scripts/build/unified.ts, scripts/glob-sources.ts, src/codegen/generate-jssink.ts, src/jsc/bindings/BunObject.h, src/jsc/bindings/BunObject.cpp, src/jsc/bindings/ZigGlobalObject.cpp, src/jsc/bindings/bindings.cpp, src/jsc/bindings/webcore/JSDOMConvertNumbers.cpp	Updates source collection and unified-build exclusions, rewires stream-related bindings and generated hooks, and adjusts integer conversion cleanup.
Streams consumers and sources src/jsc/bindings/webcore/streams/*, src/js/builtins/AsyncIterableStream.ts, src/jsc/STREAMS.md	Adds Web Streams consumer/source implementations, async-iterator wrapping, and updated Streams architecture notes.
Builtin declarations, names, and docs src/js/builtins.d.ts, src/js/builtins/BunBuiltinNames.h, src/js/builtins/Fifo.ts, src/js/README.md, src/js/CLAUDE.md	Prunes stream intrinsics and builtin names, adds FIFO wiring, and updates builtin examples.
Web Streams benchmarks and local notes bench/snippets/webstreams*.mjs, .gitignore	Adds memory, throughput, consumer, and tee benchmark scripts and ignores the local /specs/ notes directory.

Async stack trace attachment

Layer / File(s)	Summary
Async stack collection src/jsc/bindings/AsyncStackTrace.cpp, src/jsc/bindings/AsyncStackTrace.h	Adds promise-reaction async stack collection and the exported wrapper used to attach collected frames to errors.
Bindings removal src/jsc/bindings/bindings.cpp	Removes the older async-stack attachment implementation from the bindings unit.

stdin stream lifecycle

Layer / File(s)	Summary
stdin stream disown/read lifecycle src/js/builtins/ProcessObjectInternals.ts	Removes the disown flag, changes reader release handling, and updates in-flight read retry logic to follow reader presence.

Possibly related PRs

• oven-sh/bun#29540: Both PRs adjust src/js/builtins/BunBuiltinNames.h and related builtin declaration surfaces.
• oven-sh/bun#29545: Both PRs change scripts/build/unified.ts for unified-source build behavior.
• oven-sh/bun#33123: Both PRs modify src/js/builtins/ProcessObjectInternals.ts around stdin reader release and in-flight read handling.

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Title check	✅ Passed	The title accurately summarizes the main change: a C++ rewrite of the streams implementation with JS builtins removed.
Description check	✅ Passed	The description covers the rewrite, rationale, verification, tests, and status; it is substantively complete despite using custom headings.

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 19

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@specs/ARCH-REVIEW.md`:
- Around line 11-137: Mark the three critical findings in this section as
v1-only since ARCHITECTURE.md already defines the Transform/SinkKind plumbing
and shared promise-reaction approach elsewhere. Update the headings and prose
around the JSCell virtual-method issue, the SourceKind/SinkKind gap, and the
no-closures mechanism so they are clearly labeled historical notes or rewritten
to match the current design. Use the existing section references and symbols
like SourceKind, SinkKind, JSReadRequest, and WebStreamsInternals.h to keep the
edits aligned with the frozen text.

In `@specs/ARCH-SELF-REVIEW.md`:
- Around line 13-15: Update the architecture note to use the same enum spellings
as the ABI spec: replace the current SinkKind::Transform / SourceKind::Transform
wording with the exact TransformSink / TransformSource names used in
specs/OP-SIGNATURES.md. Keep the SinkKind and SourceKind references aligned with
the ABI table so the enums in this section match the spec terminology exactly.

In `@specs/ARCHITECTURE.md`:
- Around line 196-205: The readable-stream controller exception is
underspecified because not every `stream.[[controller]]` consumer is listed.
Update the `JSReadableStream::m_controller` documentation in ARCHITECTURE.md to
enumerate the full erased-controller dispatch table, explicitly covering the
generic release path and all controller-typed call sites in addition to
`readableStreamCancel`, and make clear how each `ControllerKind` arm (`None`,
`Default`, `Byte`, `Direct`, `NativeSink`) is handled. Use the
`JSReadableStream::m_controller` and `ControllerKind` symbols to anchor the
description.
- Around line 79-84: Special-case JSReadableStreamAsyncIterator in the
public-class registration rules so it is excluded from the DOMConstructorID/lazy
getter plumbing described in §2. Update the architecture spec around the class
list and constructor-registration guidance to explicitly note that Class 14 has
no globalThis constructor, no lazy getter, and is created internally via
values()/[Symbol.asyncIterator](), while still keeping the prototype behavior
described elsewhere.
- Around line 118-120: The architecture entry for Bun stream consumers is
missing ownership of the generic text accumulator helper, so update the
`BunStreamConsumers.cpp` row to also name `readableStreamIntoText` and
`withoutUTF8BOM` alongside `Bun.readableStreamToText` / the other
`readableStreamTo*` paths. Keep the surrounding
`ReadableStream.prototype.{text,json,bytes,blob}` references intact, and ensure
the `BunStreamConsumers.cpp` coverage explicitly includes the internal text
accumulation path so Phase B has a clear owner.

In `@specs/BUN-EXTENSIONS.md`:
- Around line 39-40: The JSSink controller surface in BUN-EXTENSIONS.md is
overstated because it lists a user-visible .sink getter even though the
controller.sink contract is currently undefined; update the section describing
the native JSSink controller generated by generate-jssink.ts and the path (A)
pull surface to omit .sink from the public API, keeping it as internal/private
only. Align the documented methods with the actual controller wrapper surface
(write, end, flush, close, start) and ensure the later controller.sink ===
undefined description remains consistent.

In `@specs/check-streams.py`:
- Around line 69-79: The streams probe in check-streams.py uses a fixed
/tmp/streams_header_probe.cpp path, which can be overwritten or symlinked by
another process. Update the probe creation inside the not targets branch to use
a unique temporary file or directory via tempfile.NamedTemporaryFile or
TemporaryDirectory, keep the existing run(clangxx, ...) flow in
check-streams.py, and ensure the temporary probe is removed after compilation.
- Around line 26-35: Normalize the compile database entry path before searching
for REFERENCE_TU in reference_flags(), since entry["file"] may use native
separators and fail the endswith check on Windows. Update the matching logic to
compare a normalized path form from the compile_commands.json entries so the
JSCookie.cpp reference TU is found reliably across platforms.
- Around line 31-47: The flag parsing in reference_flags is losing information
by re-tokenizing entry["arguments"] with shlex.split, which can alter valid
quoted or spaced compiler args. Update reference_flags to use entry["arguments"]
directly when that field exists, and only fall back to splitting
entry["command"] when arguments is missing; keep the rest of the filtering logic
in reference_flags unchanged.

In `@specs/digest/03-writable.md`:
- Around line 154-157: Update the `[[stream]]` slot description in the
`WritableStreamDefaultWriter` internal slots table so it refers to the writer,
not a reader. Keep the other slot entries unchanged, and adjust the wording in
the spec text near `[[closedPromise]]`, `[[readyPromise]]`, and `[[stream]]` to
consistently describe `[[stream]]` as the `WritableStream` instance owned by the
default writer.

In `@specs/digest/04-transform-queuing-support.md`:
- Around line 701-709: The CanCopyDataBlockBytes predicate is checking detached
state backwards, so it currently rejects the normal attached-buffer case. Update
the detached-buffer guards in CanCopyDataBlockBytes so it only returns false
when either toBuffer or fromBuffer is detached, and otherwise allows copying;
keep the existing self-copy check intact. Verify the logic around
IsDetachedBuffer in the CanCopyDataBlockBytes algorithm matches the intended
byte-copy behavior.

In `@specs/OP-SIGNATURES.md`:
- Around line 525-531: Fix the coverage math in the summary table by reconciling
the internal-method row counts for `01-readable-classes.md` and the total.
Update the `Readable` controller entries so the counts match the audit’s stated
5 internal-method rows, and make the “Total” row reflect the corrected sum
across all digests. Use the digest table and the
`CancelSteps`/`PullSteps`/`ReleaseSteps` references to verify the row totals are
internally consistent.
- Line 111: The `ReadableByteStreamControllerConvertPullIntoDescriptor`
signature note currently implies only OOM/alloc failure, but the
`readableByteStreamControllerConvertPullIntoDescriptor` path can also fail via
`TransferArrayBuffer` before constructing the intrinsic view. Update the spec
entry to explicitly mention this non-OOM abrupt-completion path, keeping the
`JSReadableByteStreamController.cpp`/`JSC::JSArrayBufferView*
readableByteStreamControllerConvertPullIntoDescriptor(...)` reference intact so
callers don’t treat it as allocation-only.

In `@specs/PLUMBING.md`:
- Around line 84-91: Update the AbortSignal plumbing note to match the existing
removable GC-visited API already provided by
AbortSignal::addAbortAlgorithmToSignal and
AbortSignal::removeAbortAlgorithmFromSignal, and remove the speculative
requirement for a new AbortAlgorithm/visitJSFunction contract. The spec should
point readers to the existing WebCore::AbortSignal implementation and the
JSAbortSignalCustom visitation path, and describe that native stream code should
register, retain the returned id, and explicitly remove it on
close/error/finalize using the existing API.

In `@specs/probes/adversarial-smoke.js`:
- Around line 2-4: Make the probe actually fail the process instead of only
logging errors: in adversarial-smoke.js, update the
process.on("unhandledRejection") and process.on("uncaughtException") handlers to
increment failures (and set a non-zero process.exitCode), and ensure the final
result in the main probe flow uses that accumulated failure state rather than a
fixed 50 ms tail sleep. Use the existing withTimeout helper and the
VERIFY_FAIL/exit path near the end of the file so process-level exceptions are
counted and the script exits non-zero when any probe step fails.

In `@specs/probes/sync-throw-matrix.js`:
- Around line 1-10: The probe script only logs results, so mismatches like
“resolved!?”, “wrong:...”, and “HANG” do not fail the run. Update the `t` helper
and the top-level `await t(...)` cases in `sync-throw-matrix.js` to track any
non-success outcome and set `process.exitCode = 1` when a case is not
“rejected-correctly” or “ctor-threw-correctly”, including timeout/HANG results.

In `@specs/review-cpp/ReadableStreamOperations-A.md`:
- Around line 18-76: `ReadableStream.from` is using an iterator helper that
incorrectly rejects primitive strings, so the `readableStreamFromIterable` path
does not match `GetIterator(async)` semantics. Update
`readableStreamFromIterable` to use a lookup that works on primitives (or relax
`getAsyncIteratorExported`/`getAsyncIteratorImpl` to only reject nullish
values), and ensure the fallback sync-iterator path still calls the iterator
with the original value as `this`. Also add the string iterable case to the
`ReadableStream.from` WPT coverage so `"ab"` produces code points instead of
throwing.

In `@specs/review-cpp/TransformStreamOperations-A.md`:
- Around line 17-70: The documented context contract for the reaction handlers
is inconsistent with the actual registrations in TransformStreamOperations,
especially the onTSSinkAbortCancel* and onTSSourceCancel* paths. Update the
contract comment near JSStreamsRuntime.h so it explicitly distinguishes the bare
JSTransformStream cases from the InternalFieldTuple{transformStream, reason}
cases used by the cancel/abort handlers, and keep the tuple field meanings
aligned with the corresponding handler names and registration sites.

In `@specs/review-cpp/TransformStreamOperations-B.md`:
- Around line 17-47: The three fulfilled handlers in TransformStreamOperations-B
are calling resolvePromise and returning immediately without observing
exceptions, which breaks exception-check discipline. Update the resolvePromise
sites in onTSSinkAbortCancelFulfilled, onTSSinkCloseFlushFulfilled, and
onTSSourceCancelFulfilled to match the existing safe patterns already used in
this file, using scope.assertNoException() or RETURN_IF_EXCEPTION before
returning. Keep the fix localized to those handlers so the [[finishPromise]]
settlement path is exception-safe and consistent with the other resolvePromise
call sites.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 15facdfa-6c24-44d1-a728-b26628093c4a

📥 Commits

Reviewing files that changed from the base of the PR and between d816daf and 15724f0.

📒 Files selected for processing (285)

• scripts/build/unified.ts
• scripts/glob-sources.ts
• specs/ARCH-REVIEW.md
• specs/ARCH-SELF-REVIEW.md
• specs/ARCHITECTURE.md
• specs/BASELINE.md
• specs/BUN-EXTENSIONS.md
• specs/BUN-LAYER-DESIGN.md
• specs/BUN-LAYER-REVIEW-FIDELITY.md
• specs/BUN-LAYER-REVIEW-GC.md
• specs/CONSUMERS.md
• specs/CPP-SURFACE.md
• specs/HEADER-REVIEW-1.md
• specs/HEADER-REVIEW-2.md
• specs/HEADER-REVIEW-3.md
• specs/OP-SIGNATURES.md
• specs/PHASE-A-NOTES.md
• specs/PHASE-B-LOG.md
• specs/PHASE-C-BLOCKERS.md
• specs/PHASE-D-NOTES.md
• specs/PLUMBING.md
• specs/SLOT-TABLES.md
• specs/TEST-SURFACE.md
• specs/WPT-BASELINE.md
• specs/check-streams.py
• specs/compile-errors/round1.txt
• specs/compile-errors/round2.txt
• specs/compile-errors/round3.txt
• specs/digest/01-readable-classes.md
• specs/digest/02-readable-abstract-ops.md
• specs/digest/03-writable.md
• specs/digest/04-transform-queuing-support.md
• specs/probes/adversarial-smoke.js
• specs/probes/sync-throw-matrix.js
• specs/review-cpp/CONTRACT-AUDIT.md
• specs/review-cpp/DISCIPLINE-SWEEP.md
• specs/review-cpp/JSReadableByteStreamController-A.md
• specs/review-cpp/JSReadableStream-A.md
• specs/review-cpp/JSStreamPipeToOperation-A.md
• specs/review-cpp/JSTransformStreamDefaultController-AB.md
• specs/review-cpp/ReadableStreamOperations-A.md
• specs/review-cpp/TransformStreamOperations-A.md
• specs/review-cpp/TransformStreamOperations-B.md
• specs/review-cpp/WritableStreamOperations-A.md
• specs/review-cpp/WritableStreamOperations-B.md
• specs/streams-baseline.js
• specs/streams-spec.bs
• specs/streams-spec.html
• specs/streams-spec.txt
• src/codegen/generate-jssink.ts
• src/js/README.md
• src/js/builtins.d.ts
• src/js/builtins/BunBuiltinNames.h
• src/js/builtins/ByteLengthQueuingStrategy.ts
• src/js/builtins/CountQueuingStrategy.ts
• src/js/builtins/Fifo.ts
• src/js/builtins/ReadableByteStreamController.ts
• src/js/builtins/ReadableByteStreamInternals.ts
• src/js/builtins/ReadableStream.ts
• src/js/builtins/ReadableStreamBYOBReader.ts
• src/js/builtins/ReadableStreamBYOBRequest.ts
• src/js/builtins/ReadableStreamDefaultController.ts
• src/js/builtins/ReadableStreamDefaultReader.ts
• src/js/builtins/ReadableStreamInternals.ts
• src/js/builtins/StreamInternals.ts
• src/js/builtins/TextDecoderStream.ts
• src/js/builtins/TextEncoderStream.ts
• src/js/builtins/TransformStream.ts
• src/js/builtins/TransformStreamDefaultController.ts
• src/js/builtins/TransformStreamInternals.ts
• src/js/builtins/WritableStreamDefaultController.ts
• src/js/builtins/WritableStreamDefaultWriter.ts
• src/js/builtins/WritableStreamInternals.ts
• src/js/internal/sql/query.ts
• src/js/internal/streams/native-readable.ts
• src/jsc/STREAMS.md
• src/jsc/bindings/BunObject.cpp
• src/jsc/bindings/JS2Native.cpp
• src/jsc/bindings/ZigGlobalObject.cpp
• src/jsc/bindings/ZigGlobalObject.h
• src/jsc/bindings/bindings.cpp
• src/jsc/bindings/js_classes.ts
• src/jsc/bindings/webcore/DOMClientIsoSubspaces.h
• src/jsc/bindings/webcore/DOMConstructors.h
• src/jsc/bindings/webcore/DOMIsoSubspaces.h
• src/jsc/bindings/webcore/InternalWritableStream.cpp
• src/jsc/bindings/webcore/InternalWritableStream.h
• src/jsc/bindings/webcore/JSByteLengthQueuingStrategy.cpp
• src/jsc/bindings/webcore/JSByteLengthQueuingStrategy.h
• src/jsc/bindings/webcore/JSCountQueuingStrategy.cpp
• src/jsc/bindings/webcore/JSCountQueuingStrategy.h
• src/jsc/bindings/webcore/JSReadableByteStreamController.cpp
• src/jsc/bindings/webcore/JSReadableByteStreamController.h
• src/jsc/bindings/webcore/JSReadableStream.cpp
• src/jsc/bindings/webcore/JSReadableStream.h
• src/jsc/bindings/webcore/JSReadableStreamBYOBReader.cpp
• src/jsc/bindings/webcore/JSReadableStreamBYOBReader.h
• src/jsc/bindings/webcore/JSReadableStreamBYOBRequest.cpp
• src/jsc/bindings/webcore/JSReadableStreamBYOBRequest.h
• src/jsc/bindings/webcore/JSReadableStreamDefaultController.cpp
• src/jsc/bindings/webcore/JSReadableStreamDefaultController.h
• src/jsc/bindings/webcore/JSReadableStreamDefaultReader.cpp
• src/jsc/bindings/webcore/JSReadableStreamDefaultReader.h
• src/jsc/bindings/webcore/JSReadableStreamSink.cpp
• src/jsc/bindings/webcore/JSReadableStreamSink.h
• src/jsc/bindings/webcore/JSReadableStreamSource.cpp
• src/jsc/bindings/webcore/JSReadableStreamSource.h
• src/jsc/bindings/webcore/JSReadableStreamSourceCustom.cpp
• src/jsc/bindings/webcore/JSTextDecoderStream.cpp
• src/jsc/bindings/webcore/JSTextDecoderStream.h
• src/jsc/bindings/webcore/JSTextEncoderStream.cpp
• src/jsc/bindings/webcore/JSTextEncoderStream.h
• src/jsc/bindings/webcore/JSTransformStream.cpp
• src/jsc/bindings/webcore/JSTransformStream.h
• src/jsc/bindings/webcore/JSTransformStreamDefaultController.cpp
• src/jsc/bindings/webcore/JSTransformStreamDefaultController.h
• src/jsc/bindings/webcore/JSWritableStream.cpp
• src/jsc/bindings/webcore/JSWritableStream.h
• src/jsc/bindings/webcore/JSWritableStreamDefaultController.cpp
• src/jsc/bindings/webcore/JSWritableStreamDefaultController.h
• src/jsc/bindings/webcore/JSWritableStreamDefaultWriter.cpp
• src/jsc/bindings/webcore/JSWritableStreamDefaultWriter.h
• src/jsc/bindings/webcore/JSWritableStreamSink.cpp
• src/jsc/bindings/webcore/JSWritableStreamSink.h
• src/jsc/bindings/webcore/ReadableStream.cpp
• src/jsc/bindings/webcore/ReadableStream.h
• src/jsc/bindings/webcore/ReadableStreamDefaultController.cpp
• src/jsc/bindings/webcore/ReadableStreamDefaultController.h
• src/jsc/bindings/webcore/ReadableStreamSink.cpp
• src/jsc/bindings/webcore/ReadableStreamSink.h
• src/jsc/bindings/webcore/ReadableStreamSource.cpp
• src/jsc/bindings/webcore/ReadableStreamSource.h
• src/jsc/bindings/webcore/WritableStream.cpp
• src/jsc/bindings/webcore/WritableStream.h
• src/jsc/bindings/webcore/WritableStream.idl
• src/jsc/bindings/webcore/WritableStreamSink.h
• src/jsc/bindings/webcore/streams/BunStandaloneTextSink.h
• src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
• src/jsc/bindings/webcore/streams/BunStreamConsumers.h
• src/jsc/bindings/webcore/streams/BunStreamSource.cpp
• src/jsc/bindings/webcore/streams/BunStreamSource.h
• src/jsc/bindings/webcore/streams/CrossRealmTransform.cpp
• src/jsc/bindings/webcore/streams/JSByteLengthQueuingStrategy.cpp
• src/jsc/bindings/webcore/streams/JSByteLengthQueuingStrategy.h
• src/jsc/bindings/webcore/streams/JSCountQueuingStrategy.cpp
• src/jsc/bindings/webcore/streams/JSCountQueuingStrategy.h
• src/jsc/bindings/webcore/streams/JSCrossRealmTransformState.cpp
• src/jsc/bindings/webcore/streams/JSCrossRealmTransformState.h
• src/jsc/bindings/webcore/streams/JSDirectSinkCloseState.h
• src/jsc/bindings/webcore/streams/JSDirectStreamController.cpp
• src/jsc/bindings/webcore/streams/JSDirectStreamController.h
• src/jsc/bindings/webcore/streams/JSOneShotDirectSink.h
• src/jsc/bindings/webcore/streams/JSPullIntoDescriptor.cpp
• src/jsc/bindings/webcore/streams/JSPullIntoDescriptor.h
• src/jsc/bindings/webcore/streams/JSReadRequest.cpp
• src/jsc/bindings/webcore/streams/JSReadRequest.h
• src/jsc/bindings/webcore/streams/JSReadStreamIntoSinkOperation.h
• src/jsc/bindings/webcore/streams/JSReadableByteStreamController.cpp
• src/jsc/bindings/webcore/streams/JSReadableByteStreamController.h
• src/jsc/bindings/webcore/streams/JSReadableStream.cpp
• src/jsc/bindings/webcore/streams/JSReadableStream.h
• src/jsc/bindings/webcore/streams/JSReadableStreamAsyncIterator.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamAsyncIterator.h
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.h
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBRequest.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBRequest.h
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultController.h
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.h
• src/jsc/bindings/webcore/streams/JSReadableStreamReaderBase.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamReaderBase.h
• src/jsc/bindings/webcore/streams/JSResumableSinkPumpOperation.h
• src/jsc/bindings/webcore/streams/JSStreamAlgorithmContexts.cpp
• src/jsc/bindings/webcore/streams/JSStreamAlgorithmContexts.h
• src/jsc/bindings/webcore/streams/JSStreamPipeToOperation.cpp
• src/jsc/bindings/webcore/streams/JSStreamPipeToOperation.h
• src/jsc/bindings/webcore/streams/JSStreamTeeState.cpp
• src/jsc/bindings/webcore/streams/JSStreamTeeState.h
• src/jsc/bindings/webcore/streams/JSStreamsRuntime.cpp
• src/jsc/bindings/webcore/streams/JSStreamsRuntime.h
• src/jsc/bindings/webcore/streams/JSTextDecoderStream.cpp
• src/jsc/bindings/webcore/streams/JSTextDecoderStream.h
• src/jsc/bindings/webcore/streams/JSTextEncoderStream.cpp
• src/jsc/bindings/webcore/streams/JSTextEncoderStream.h
• src/jsc/bindings/webcore/streams/JSTransformStream.cpp
• src/jsc/bindings/webcore/streams/JSTransformStream.h
• src/jsc/bindings/webcore/streams/JSTransformStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSTransformStreamDefaultController.h
• src/jsc/bindings/webcore/streams/JSWritableStream.cpp
• src/jsc/bindings/webcore/streams/JSWritableStream.h
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultController.h
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultWriter.cpp
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultWriter.h
• src/jsc/bindings/webcore/streams/ReadableStreamOperations.cpp
• src/jsc/bindings/webcore/streams/StreamConstructor.h
• src/jsc/bindings/webcore/streams/StreamQueue.h
• src/jsc/bindings/webcore/streams/StreamsForward.h
• src/jsc/bindings/webcore/streams/TransformStreamOperations.cpp
• src/jsc/bindings/webcore/streams/WebStreamsExports.cpp
• src/jsc/bindings/webcore/streams/WebStreamsInternals.h
• src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp
• src/jsc/bindings/webcore/streams/WritableStreamOperations.cpp
• test/js/third_party/wpt-h2/run.test.ts
• test/js/third_party/wpt-h2/testharness-shim.ts
• test/js/third_party/wpt-streams/RESULTS.md
• test/js/third_party/wpt-streams/UPSTREAM.md
• test/js/third_party/wpt-streams/common/gc.js
• test/js/third_party/wpt-streams/expectations.json
• test/js/third_party/wpt-streams/streams/piping/abort.any.js
• test/js/third_party/wpt-streams/streams/piping/close-propagation-backward.any.js
• test/js/third_party/wpt-streams/streams/piping/close-propagation-forward.any.js
• test/js/third_party/wpt-streams/streams/piping/error-propagation-backward.any.js
• test/js/third_party/wpt-streams/streams/piping/error-propagation-forward.any.js
• test/js/third_party/wpt-streams/streams/piping/flow-control.any.js
• test/js/third_party/wpt-streams/streams/piping/general-addition.any.js
• test/js/third_party/wpt-streams/streams/piping/general.any.js
• test/js/third_party/wpt-streams/streams/piping/multiple-propagation.any.js
• test/js/third_party/wpt-streams/streams/piping/pipe-through.any.js
• test/js/third_party/wpt-streams/streams/piping/then-interception.any.js
• test/js/third_party/wpt-streams/streams/piping/throwing-options.any.js
• test/js/third_party/wpt-streams/streams/piping/transform-streams.any.js
• test/js/third_party/wpt-streams/streams/queuing-strategies.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/bad-buffers-and-views.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/construct-byob-request.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/crashtests/tee-locked-stream.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/enqueue-with-detached-buffer.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/general.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/non-transferable-buffers.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/patched-global.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/read-min.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/respond-after-enqueue.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/tee.any.js
• test/js/third_party/wpt-streams/streams/readable-byte-streams/templated.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/async-iterator.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/bad-strategies.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/bad-underlying-sources.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/cancel.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/constructor.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/count-queuing-strategy-integration.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/crashtests/garbage-collection.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/default-reader.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/floating-point-total-queue-size.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/from.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/garbage-collection.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/general.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/patched-global.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/reentrant-strategies.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/tee.any.js
• test/js/third_party/wpt-streams/streams/readable-streams/templated.any.js
• test/js/third_party/wpt-streams/streams/resources/recording-streams.js
• test/js/third_party/wpt-streams/streams/resources/rs-test-templates.js
• test/js/third_party/wpt-streams/streams/resources/rs-utils.js
• test/js/third_party/wpt-streams/streams/resources/test-utils.js
• test/js/third_party/wpt-streams/streams/transform-streams/backpressure.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/cancel.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/errors.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/flush.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/general.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/lipfuzz.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/patched-global.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/properties.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/reentrant-strategies.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/strategies.any.js
• test/js/third_party/wpt-streams/streams/transform-streams/terminate.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/aborting.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/bad-strategies.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/bad-underlying-sinks.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/byte-length-queuing-strategy.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/close.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/constructor.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/count-queuing-strategy.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/crashtests/garbage-collection.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/error.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/floating-point-total-queue-size.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/garbage-collection.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/general.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/properties.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/reentrant-strategy.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/start.any.js
• test/js/third_party/wpt-streams/streams/writable-streams/write.any.js
• test/js/third_party/wpt-streams/wpt-streams.test.ts
• test/js/third_party/wpt-testharness-shim.ts

💤 Files with no reviewable changes (12)

• src/js/builtins/ReadableByteStreamInternals.ts
• src/js/builtins/ByteLengthQueuingStrategy.ts
• src/js/builtins/CountQueuingStrategy.ts
• src/js/builtins/ReadableStreamBYOBReader.ts
• src/js/builtins/ReadableStreamDefaultController.ts
• src/js/builtins/ReadableStreamBYOBRequest.ts
• src/js/builtins/ReadableStreamDefaultReader.ts
• src/codegen/generate-jssink.ts
• src/js/builtins/BunBuiltinNames.h
• src/js/builtins/ReadableStream.ts
• src/js/builtins/ReadableByteStreamController.ts
• src/js/builtins.d.ts

[coderabbitai]

📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Mark these findings as v1-only.

ARCHITECTURE.md:252-419 already pins down the Transform/SinkKind plumbing and the shared promise-reaction mechanism, so these opening criticals no longer describe the frozen design. Please mark them as historical v1 notes or rewrite them against the current text.

🧰 Tools

🪛 LanguageTool

[style] ~35-~35: ‘exact same’ might be wordy. Consider a shorter alternative.
Context: ...es” idea, drop C++ virtual. Use the exact same device §4 already uses for algorithms: ...

(EN_WORDINESS_PREMIUM_EXACT_SAME)

🪛 markdownlint-cli2 (0.22.1)

[warning] 11-11: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/ARCH-REVIEW.md` around lines 11 - 137, Mark the three critical findings
in this section as v1-only since ARCHITECTURE.md already defines the
Transform/SinkKind plumbing and shared promise-reaction approach elsewhere.
Update the headings and prose around the JSCell virtual-method issue, the
SourceKind/SinkKind gap, and the no-closures mechanism so they are clearly
labeled historical notes or rewritten to match the current design. Use the
existing section references and symbols like SourceKind, SinkKind,
JSReadRequest, and WebStreamsInternals.h to keep the edits aligned with the
frozen text.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Name the generic text accumulator path too.

As noted in specs/BUN-LAYER-REVIEW-FIDELITY.md:67-99, Bun.readableStreamToText still has a separate readableStreamIntoText / withoutUTF8BOM path. If this section only names the public readableStreamTo* wrappers, Phase B still has no owner for that helper.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/ARCHITECTURE.md` around lines 118 - 120, The architecture entry for Bun
stream consumers is missing ownership of the generic text accumulator helper, so
update the `BunStreamConsumers.cpp` row to also name `readableStreamIntoText`
and `withoutUTF8BOM` alongside `Bun.readableStreamToText` / the other
`readableStreamTo*` paths. Keep the surrounding
`ReadableStream.prototype.{text,json,bytes,blob}` references intact, and ensure
the `BunStreamConsumers.cpp` coverage explicitly includes the internal text
accumulation path so Phase B has a clear owner.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Make probe failures fail the process.

unhandledRejection/uncaughtException are only logged, and Line 24 still exits 0 even after VERIFY_FAIL. That makes this verification probe false-green in automation or bisects. Count process-level failures in failures and set process.exitCode from the final result instead of relying on a fixed 50 ms tail sleep.

Also applies to: 17-24

🧰 Tools

🪛 ast-grep (0.44.0)

[warning] 3-3: Avoid using the initial state variable in setState
Context: setTimeout(() => rj(new Error("STEP_TIMEOUT " + name)), ms)
Note: [CWE-710] Improper Adherence to Coding Standards. Security best practice.

(setstate-same-var)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/probes/adversarial-smoke.js` around lines 2 - 4, Make the probe
actually fail the process instead of only logging errors: in
adversarial-smoke.js, update the process.on("unhandledRejection") and
process.on("uncaughtException") handlers to increment failures (and set a
non-zero process.exitCode), and ensure the final result in the main probe flow
uses that accumulated failure state rather than a fixed 50 ms tail sleep. Use
the existing withTimeout helper and the VERIFY_FAIL/exit path near the end of
the file so process-level exceptions are counted and the script exits non-zero
when any probe step fails.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Turn mismatches and hangs into a failing exit status.

Right now every case only prints a line, so "resolved!?", "wrong:...", and "HANG" still leave the script green. If this probe is used for verification, it needs to accumulate non-success results and set process.exitCode = 1 on any mismatch.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/probes/sync-throw-matrix.js` around lines 1 - 10, The probe script only
logs results, so mismatches like “resolved!?”, “wrong:...”, and “HANG” do not
fail the run. Update the `t` helper and the top-level `await t(...)` cases in
`sync-throw-matrix.js` to track any non-success outcome and set
`process.exitCode = 1` when a case is not “rejected-correctly” or
“ctor-threw-correctly”, including timeout/HANG results.

[coderabbitai]

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Fix ReadableStream.from for primitive iterables.

getAsyncIteratorExported rejects primitive strings, so ReadableStream.from("ab") fails instead of streaming code points. Switch this path to real GetIterator(async) semantics (or loosen the helper to only reject null/undefined) and add the WPT string case.

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 18-18: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

---

[warning] 22-22: Headings should be surrounded by blank lines
Expected: 1; Actual: 0; Below

(MD022, blanks-around-headings)

---

[warning] 23-23: Ordered list item prefix
Expected: 1; Actual: 2; Style: 1/1/1

(MD029, ol-prefix)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/review-cpp/ReadableStreamOperations-A.md` around lines 18 - 76,
`ReadableStream.from` is using an iterator helper that incorrectly rejects
primitive strings, so the `readableStreamFromIterable` path does not match
`GetIterator(async)` semantics. Update `readableStreamFromIterable` to use a
lookup that works on primitives (or relax
`getAsyncIteratorExported`/`getAsyncIteratorImpl` to only reject nullish
values), and ensure the fallback sync-iterator path still calls the iterator
with the original value as `this`. Also add the string iterable case to the
`ReadableStream.from` WPT coverage so `"ab"` produces code points instead of
throwing.

[coderabbitai]

📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win

Align the reaction-handler contract with the actual context types.

JSStreamsRuntime.h still documents most handlers as taking a bare JSTransformStream, but these registrations pass InternalFieldTuple{transformStream, reason} for the abort/source-cancel paths. That mismatch can mislead future call sites into an unchecked-downcast bug. Update the header contract to match the tuple-based handlers.

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 21-21: Fenced code blocks should have a language specified

(MD040, fenced-code-language)

---

[warning] 36-36: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

[warning] 37-37: Fenced code blocks should be surrounded by blank lines

(MD031, blanks-around-fences)

---

[warning] 68-68: Spaces inside emphasis markers

(MD037, no-space-in-emphasis)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/review-cpp/TransformStreamOperations-A.md` around lines 17 - 70, The
documented context contract for the reaction handlers is inconsistent with the
actual registrations in TransformStreamOperations, especially the
onTSSinkAbortCancel* and onTSSourceCancel* paths. Update the contract comment
near JSStreamsRuntime.h so it explicitly distinguishes the bare
JSTransformStream cases from the InternalFieldTuple{transformStream, reason}
cases used by the cancel/abort handlers, and keep the tuple field meanings
aligned with the corresponding handler names and registration sites.

[coderabbitai]

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Observe the resolvePromise result before returning.

These three fulfilled handlers settle [[finishPromise]] and immediately return; if resolvePromise(...) throws, the file will violate exception-check discipline and can leave the caller hanging. Reuse the existing scope.assertNoException() / RETURN_IF_EXCEPTION pattern already shown later in the file.

🧰 Tools

🪛 markdownlint-cli2 (0.22.1)

[warning] 17-17: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3

(MD001, heading-increment)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@specs/review-cpp/TransformStreamOperations-B.md` around lines 17 - 47, The
three fulfilled handlers in TransformStreamOperations-B are calling
resolvePromise and returning immediately without observing exceptions, which
breaks exception-check discipline. Update the resolvePromise sites in
onTSSinkAbortCancelFulfilled, onTSSinkCloseFlushFulfilled, and
onTSSourceCancelFulfilled to match the existing safe patterns already used in
this file, using scope.assertNoException() or RETURN_IF_EXCEPTION before
returning. Keep the fix localized to those handlers so the [[finishPromise]]
settlement path is exception-safe and consistent with the other resolvePromise
call sites.

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/snippets/webstreams-memory.mjs`:
- Line 4: The GC fallback in the webstreams memory benchmark is silently
becoming a no-op when neither Bun.gc nor an exposed globalThis.gc exists, which
makes the RSS measurements unreliable. Update the setup near the gc constant to
explicitly detect the absence of a usable GC function and surface a clear
warning or fail fast before running the benchmark, so the measurement only
proceeds when forced garbage collection is actually available.

In `@src/js/builtins/ProcessObjectInternals.ts`:
- Around line 149-155: Use a per-read release sentinel in ProcessObjectInternals
so internalRead() does not rely on the mutable reader field to detect a
disown/release. Capture the specific reader (or a release token) used by the
current read and have the rejection path around internalRead()/disown() only
treat that exact release as expected, instead of checking just !reader. Add a
regression test covering quick disown→own to verify reader A’s rejected
continuation does not destroy the stream when reader B has already been
acquired.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 1f9e0851-2583-47e4-bb4d-12ac5d0b7cac

📥 Commits

Reviewing files that changed from the base of the PR and between 15724f0 and 8b98d59.

📒 Files selected for processing (18)

• .gitignore
• bench/snippets/webstreams-memory.mjs
• bench/snippets/webstreams-throughput.mjs
• bench/snippets/webstreams.mjs
• src/js/builtins/ProcessObjectInternals.ts
• src/jsc/bindings/webcore/streams/JSReadableStreamAsyncIterator.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.cpp
• src/jsc/bindings/webcore/streams/JSStreamPipeToOperation.cpp
• src/jsc/bindings/webcore/streams/JSStreamPipeToOperation.h
• src/jsc/bindings/webcore/streams/JSStreamsRuntime.h
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultWriter.cpp
• src/jsc/bindings/webcore/streams/ReadableStreamOperations.cpp
• src/jsc/bindings/webcore/streams/WebStreamsInternals.h
• src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp
• test/js/third_party/wpt-streams/RESULTS.md
• test/js/third_party/wpt-streams/expectations.json
• test/js/web/streams/streams.test.js

[coderabbitai]

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (3)

src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp (3)

421-424: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Do not strip BOM from later mixed-path string chunks.

In the mixed buffer/string path, bytes may already contain earlier chunks, so stripping rope[0] removes a legitimate U+FEFF that appears mid-stream. This contradicts the stated mixed-path behavior and loses user data.

Proposed fix

     if (accumulator.rope.length()) {
         WTF::String rope = accumulator.rope.toString();
-        if (rope[0] == 0xFEFF)
-            rope = rope.substring(1);
         WTF::CString utf8 = rope.utf8();

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp` around lines 421 -
424, The mixed buffer/string path in BunStreamConsumers::arrayBufferAsString is
stripping a leading BOM from rope even when bytes already contains earlier
chunks, which can remove a legitimate U+FEFF from the middle of the stream.
Update the BOM handling in the accumulator.rope branch so it only removes a BOM
when this is the first and only chunk being materialized, and preserve rope
contents unchanged for later mixed-path chunks.

---

308-310: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return an ArrayBuffer for single string chunks.

convertChunksToArrayBuffer() currently returns encodeStringToUint8Array() for a one-string stream, so Bun.readableStreamToArrayBuffer() can resolve to a Uint8Array only on this fast path. Let it fall through to concatenateChunks(..., false) or add an ArrayBuffer-specific string encoder.

Proposed fix

-        if (chunk.isString())
-            RELEASE_AND_RETURN(scope, encodeStringToUint8Array(globalObject, chunk));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp` around lines 308 -
310, The fast path in convertChunksToArrayBuffer currently returns
encodeStringToUint8Array() for a single string chunk, which makes
Bun.readableStreamToArrayBuffer() resolve to a Uint8Array instead of an
ArrayBuffer. Update this path so it produces an ArrayBuffer consistently, either
by letting the single-string case fall through to concatenateChunks(..., false)
or by adding an ArrayBuffer-specific string encoding helper, and keep the change
localized to convertChunksToArrayBuffer/BunStreamConsumers logic.

---

721-748: 🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift

Finish fallible setup before locking and disturbing the stream.

After Line 721 clears m_directUnderlyingSource and marks the stream locked/disturbed, the subsequent underlyingSource->get(...) calls and bound-method setup can still throw. That leaves the direct stream unusable instead of returning a clean rejection. Move the property reads and method setup before mutating stream state, or restore state on every abrupt completion. As per coding guidelines, anything that can run user JS can synchronously free or mutate state, so perform observable operations before state transitions.

Sketch

-    stream->m_directUnderlyingSource.clear();
-    stream->m_bunMode = BunStreamMode::Default;
-    stream->m_lockedWithoutReader = true;
-    stream->m_disturbed = true;
-
     JSObject* startOptions = constructEmptyObject(globalObject);
     bool hasNumericHighWaterMark = stream->m_bunHighWaterMarkIsNumber || !std::isnan(stream->m_bunHighWaterMark);
     startOptions->putDirect(vm, Identifier::fromString(vm, "highWaterMark"_s), hasNumericHighWaterMark ? jsNumber(stream->m_bunHighWaterMark) : jsUndefined());
@@
     auto* closeContext = InternalFieldTuple::create(vm, domGlobalObject->internalFieldTupleStructure(), sink, closeFunction);
     installOneShotMethods(globalObject, sink, closeContext);
     RETURN_IF_EXCEPTION(scope, {});
+
+    stream->m_directUnderlyingSource.clear();
+    stream->m_bunMode = BunStreamMode::Default;
+    stream->m_lockedWithoutReader = true;
+    stream->m_disturbed = true;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp` around lines 721 -
748, The setup in JSOneShotDirectSink creation is mutating stream state before
all fallible JS-facing work is finished. Move the underlyingSource property
reads and installOneShotMethods-related setup ahead of clearing
m_directUnderlyingSource and setting
m_bunMode/m_lockedWithoutReader/m_disturbed, or make sure any exception path
restores the previous state in BunStreamConsumers handling. Use the
JSOneShotDirectSink and underlyingSource flow as the anchor when updating the
order so synchronous throws do not leave the stream permanently unusable.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`:
- Around line 421-424: The mixed buffer/string path in
BunStreamConsumers::arrayBufferAsString is stripping a leading BOM from rope
even when bytes already contains earlier chunks, which can remove a legitimate
U+FEFF from the middle of the stream. Update the BOM handling in the
accumulator.rope branch so it only removes a BOM when this is the first and only
chunk being materialized, and preserve rope contents unchanged for later
mixed-path chunks.
- Around line 308-310: The fast path in convertChunksToArrayBuffer currently
returns encodeStringToUint8Array() for a single string chunk, which makes
Bun.readableStreamToArrayBuffer() resolve to a Uint8Array instead of an
ArrayBuffer. Update this path so it produces an ArrayBuffer consistently, either
by letting the single-string case fall through to concatenateChunks(..., false)
or by adding an ArrayBuffer-specific string encoding helper, and keep the change
localized to convertChunksToArrayBuffer/BunStreamConsumers logic.
- Around line 721-748: The setup in JSOneShotDirectSink creation is mutating
stream state before all fallible JS-facing work is finished. Move the
underlyingSource property reads and installOneShotMethods-related setup ahead of
clearing m_directUnderlyingSource and setting
m_bunMode/m_lockedWithoutReader/m_disturbed, or make sure any exception path
restores the previous state in BunStreamConsumers handling. Use the
JSOneShotDirectSink and underlyingSource flow as the anchor when updating the
order so synchronous throws do not leave the stream permanently unusable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 3fa8a498-0164-40dc-97cf-03a2f65573cf

📥 Commits

Reviewing files that changed from the base of the PR and between 8b98d59 and 46e937f.

📒 Files selected for processing (5)

• bench/snippets/webstreams-consumers.mjs
• src/jsc/bindings/BunObject.cpp
• src/jsc/bindings/BunObject.h
• src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
• test/js/web/streams/streams.test.js

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@bench/snippets/webstreams-tee.mjs`:
- Line 7: The benchmark’s GC hook is falling back to a no-op when neither Bun.gc
nor globalThis.gc is available, which silently disables the memory cleanup
checkpoints. Update the webstreams-tee benchmark setup around the gc constant to
explicitly detect the missing GC capability and either fail fast or skip the
memory/RSS portion with a clear message, instead of using an empty fallback
function. Keep the change localized to the gc initialization and any checkpoint
logic that relies on it.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`:
- Around line 982-1002: The fast path in BunStreamConsumers should preserve
promise-based async semantics for all non-text consumers: in the branch handling
convertChunks() and arrayPromise->result(), ensure failures are returned as
rejected promises instead of being synchronously rethrown, and use
promiseResolvedWith() for the fulfilled object path rather than directly
creating a JSPromise and calling fulfill(). Update the logic around
convertChunks, throwException, and the final fulfillment in the relevant
consumer helper so readableStreamToArrayBuffer()/readableStreamToBytes() always
settle through the normal Promise contract.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 409f351e-8010-4c4d-8d7b-ed71d414580a

📥 Commits

Reviewing files that changed from the base of the PR and between 46e937f and a766855.

📒 Files selected for processing (18)

• bench/snippets/webstreams-memory.mjs
• bench/snippets/webstreams-tee.mjs
• src/jsc/bindings/webcore/JSDOMConvertNumbers.cpp
• src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
• src/jsc/bindings/webcore/streams/JSReadableByteStreamController.cpp
• src/jsc/bindings/webcore/streams/JSReadableStream.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.cpp
• src/jsc/bindings/webcore/streams/JSStreamsRuntime.h
• src/jsc/bindings/webcore/streams/JSTransformStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSWritableStream.cpp
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultWriter.cpp
• src/jsc/bindings/webcore/streams/ReadableStreamOperations.cpp
• src/jsc/bindings/webcore/streams/TransformStreamOperations.cpp
• src/jsc/bindings/webcore/streams/WebStreamsInternals.h
• src/jsc/bindings/webcore/streams/WritableStreamOperations.cpp
• test/js/web/fetch/body-clone.test.ts
• test/js/web/streams/streams.test.js

[Jarred-Sumner]

Addressed the review feedback in 28f593f:

• enqueueValueWithSize allocating under cellLock(): real — the RangeError throw is a GC allocation. The size check (and throw) now runs before the lock; only the queue mutation is under it, and callers no longer hold their own locker around the call.
• strategy size() result not run through the Web IDL unrestricted double conversion: real — Node (and the writable controller in this PR) coerce via ToNumber, so size() => ({ valueOf: () => 2 }) worked there and threw here. Fixed on the readable path with the same completion-record recovery as a throwing size(), plus a test.
• assignToStream using bare scope.clearException(): switched to clearExceptionExceptTermination() to match the sibling export and the subsystem rule.
• process.stdin release-rejection sentinel race (both comments): the catch now keys on the reader that issued the in-flight read instead of the current reader, so pause() followed by an immediate resume() can no longer destroy stdin with the release TypeError; covered by a pause/resume churn test.
• stale JS-builtin declarations in builtins.d.ts, stale example in src/js/CLAUDE.md, dangling references in src/jsc/STREAMS.md: removed/updated — the doc no longer refers to anything outside the tree.
• benchmark GC fallback silently no-op (webstreams-memory.mjs, webstreams-tee.mjs): they now fail fast when no GC hook is available.

The remaining comments point at files under specs/, which were removed from this PR earlier (they were working notes, now gitignored), so there is nothing in the tree for them to apply to; the two that described real code behavior (the queue lock and the size conversion) are the first two items above.

[claude]

Additional findings (outside current diff — PR may have been updated during review):

• 🟡 src/js/builtins/ProcessObjectInternals.ts:242-245 — The ERR_STREAM_RELEASE_LOCK error-code declaration is now dead: this PR deleted its only producer (the $ERR_STREAM_RELEASE_LOCK(...) calls in the removed ReadableStreamInternals.ts) and its only consumer (the err?.code === 'ERR_STREAM_RELEASE_LOCK' check replaced here). Per the "delete dead code in the same PR that makes it dead" rule, the entry at src/jsc/bindings/ErrorCode.ts:249 and the manual switch arm at src/jsc/bindings/ErrorCode.cpp:2642-2643 should be removed too (ErrorCode.rs regenerates from the .ts).

  Extended reasoning...

  What was left behind

  This hunk replaces the old err?.code === 'ERR_STREAM_RELEASE_LOCK' check in internalRead's catch handler with if (!reader). The PR description's "Behavior changes" section explicitly states: "The old Bun-specific AbortError with code: 'ERR_STREAM_RELEASE_LOCK' is no longer produced (nothing in the tree produced or consumed it after the old builtins were removed)."

  That statement is accurate — but the error-code declaration itself was left in place:

  • src/jsc/bindings/ErrorCode.ts:249 — ["ERR_STREAM_RELEASE_LOCK", Error, "AbortError"] (the source-of-truth entry that generates the $ERR_STREAM_RELEASE_LOCK builtin, the ErrorCode::ERR_STREAM_RELEASE_LOCK C++ enum value, and the Rust constant)
  • src/jsc/bindings/ErrorCode.cpp:2642-2643 — the manual case ErrorCode::ERR_STREAM_RELEASE_LOCK: arm in the message-formatting switch
  • src/jsc/ErrorCode.rs:532-533,1003,1344 — auto-generated from ErrorCode.ts, so removing the .ts entry regenerates these away

  Step-by-step proof

  1. Producers before this PR: The only sites that ever constructed this error were $ERR_STREAM_RELEASE_LOCK("Stream reader cancelled via releaseLock()") calls inside readableStreamDefaultReaderRelease and readableStreamReaderGenericRelease in src/js/builtins/ReadableStreamInternals.ts. This PR deletes that file in full (visible in the diff: 2,644 lines removed).
  2. Consumers before this PR: The only site that read this code was if (err?.code === "ERR_STREAM_RELEASE_LOCK") in src/js/builtins/ProcessObjectInternals.ts, which this hunk replaces with if (!reader).
  3. Post-PR grep: grep -rn 'STREAM_RELEASE_LOCK' src/ test/ packages/ returns exactly six hits — the ErrorCode.ts declaration, the ErrorCode.cpp switch arm, and the four auto-generated ErrorCode.rs lines. No producers, no consumers, no tests.
  4. Node.js has no ERR_STREAM_RELEASE_LOCK code (Node uses ERR_INVALID_STATE for the equivalent condition, which is exactly what this PR now produces per the description), so this is not a compat surface being preserved.

  Why nothing else prevents removal

  The ErrorCode.ts entry generates a $ERR_STREAM_RELEASE_LOCK link-time-constant builtin function and a C++/Rust enum value. With no callers of the builtin and no code that constructs or matches the enum value, it's pure dead surface: an unused generated JS function, an unreachable C++ switch arm, and unused Rust constants. It compiles and links fine, so nothing structural forces its removal.

  Impact

  Zero runtime impact — the generated $ERR_STREAM_RELEASE_LOCK function is never called, and the switch arm is unreachable. This is purely incomplete cleanup. Per CLAUDE.md's "Delete dead code in the same PR that makes it dead (required scope — name the deletions in the description)", and consistent with this PR already deleting ~50 dead BunBuiltinNames.h entries and builtins.d.ts declarations for the same reason, the ErrorCode entry belongs in the same sweep.

  Fix

  Two-line change: delete line 249 of src/jsc/bindings/ErrorCode.ts and the two-line case ErrorCode::ERR_STREAM_RELEASE_LOCK: at src/jsc/bindings/ErrorCode.cpp:2642-2643. Rebuild regenerates ErrorCode.rs without the four derived lines.

[Jarred-Sumner]

Two more findings came in after my last pass; both are handled:

• JSDirectStreamController::onClose null deref (release build) when a direct stream's reader is released while an async pull() is still running: confirmed with the exact repro (debug assertion at readableStreamGetNumReadRequests). Fixed by guarding the flushed-bytes path on readableStreamHasDefaultReader (mirroring onFlush), and the same scenario exposed one more layer: releasing a direct reader left its in-flight read() promise pending forever (a direct read lives on the controller, not in the reader's request queue), so the release operation now settles it with the same ERR_INVALID_STATE rejection the default reader produces. Regression test covers the crash, the rejection, and the follow-up reader receiving the flushed final chunk; the previous implementation hangs on the same input.
• ERR_STREAM_RELEASE_LOCK registry entry: declining this one. The premise that src/jsc/ErrorCode.rs is regenerated from ErrorCode.ts is not correct — no generator writes it; it is a checked-in mirror whose entries carry explicit, index-aligned discriminants (ErrorCode(237), …). Removing a mid-table entry means hand-renumbering every later discriminant across the enum, the alias table, and the name table for zero runtime effect (the entry is unreachable but harmless). If we want it gone, that's a standalone, mechanical PR where an alignment mistake can't hide inside a large diff.

[claude]

🟡 This hunk removes the last consumer of ERR_STREAM_RELEASE_LOCK (the err?.code === "ERR_STREAM_RELEASE_LOCK" check), and the PR deletes all producers (the $ERR_STREAM_RELEASE_LOCK(...) throws in the removed stream builtins). The registry entry at src/jsc/bindings/ErrorCode.ts:249 (["ERR_STREAM_RELEASE_LOCK", Error, "AbortError"]) and the hand-written switch arm at ErrorCode.cpp:2642-2643 are now dead — per the repo's "delete dead code in the same PR that makes it dead" rule, they should be removed here.

Extended reasoning...

What was left behind

This PR intentionally retires the Bun-specific ERR_STREAM_RELEASE_LOCK error code. The PR description says so explicitly under "Behavior changes (intentional)": "The old Bun-specific AbortError with code: 'ERR_STREAM_RELEASE_LOCK' is no longer produced (nothing in the tree produced or consumed it after the old builtins were removed)." However, the error-code registry entry and its downstream artifacts were not deleted:

• src/jsc/bindings/ErrorCode.ts:249 — ["ERR_STREAM_RELEASE_LOCK", Error, "AbortError"]
• src/jsc/bindings/ErrorCode.cpp:2642-2643 — the hand-written zero-argument $ERR_STREAM_RELEASE_LOCK() switch arm with the default message "Stream reader cancelled via releaseLock()"
• src/jsc/ErrorCode.rs:532/1003/1344 — generated from the .ts entry (will disappear on rebuild once the .ts line is removed)

Why it's dead

Producers. Before this PR, $ERR_STREAM_RELEASE_LOCK(...) was thrown at three sites, all in the deleted JS builtins: ReadableStreamInternals.ts (readableStreamDefaultReaderRelease and readableStreamReaderGenericRelease). This PR deletes those files entirely. Grep of the post-PR tree for ERR_STREAM_RELEASE_LOCK returns only the five registry lines above — zero $ERR_STREAM_RELEASE_LOCK( call sites and zero ErrorCode::ERR_STREAM_RELEASE_LOCK throw sites remain.

Consumers. The only consumer was the err?.code === "ERR_STREAM_RELEASE_LOCK" check in ProcessObjectInternals.ts's internalRead catch handler, which this exact hunk replaces with a reader-identity comparison (readerForThisRead !== reader). Grep confirms zero remaining string comparisons against "ERR_STREAM_RELEASE_LOCK" in src/, test/, or packages/.

Step-by-step proof

1. git grep -n ERR_STREAM_RELEASE_LOCK on this branch returns exactly five hits: ErrorCode.ts:249, ErrorCode.cpp:2642-2643, and the three generated ErrorCode.rs lines.
2. None of those five hits throws the error — ErrorCode.ts:249 is the registry declaration, ErrorCode.cpp:2642-2643 is the switch arm reachable only when a $ERR_STREAM_RELEASE_LOCK() builtin call routes through Bun::createErrorWithCode, and the .rs entries are the generated enum/table.
3. None of those five hits consumes the error — there is no .code === comparison and no catch predicate anywhere in the tree.
4. The new C++ implementation produces ERR_INVALID_STATE TypeErrors on reader/writer release (per the PR description's Node-compat pass), so nothing new reuses this code.
5. Therefore the registry entry, the .cpp switch arm, and the generated .rs constants are unreachable dead code created dead by this PR.

Impact

None at runtime — an unused ErrorCode enum member costs one entry in a static table and one unreachable switch arm. This is purely a "delete dead code in the same PR that makes it dead" cleanup item per CLAUDE.md, hence nit severity: it does not block the merge, cause incorrect behavior, or regress anything.

Fix

Remove line 249 from src/jsc/bindings/ErrorCode.ts and the case ErrorCode::ERR_STREAM_RELEASE_LOCK: arm at src/jsc/bindings/ErrorCode.cpp:2642-2643. The ErrorCode.rs entries are generated from ErrorCode.ts and will disappear on the next bun bd. Two-line source change.

[Jarred-Sumner]

Latest round of review findings, all handled in 7bb3c94 and the two commits before it:

• JSDirectStreamController::onClose null deref and the two sibling sites in JSReadableStreamDefaultController (shouldCallPull, enqueue): confirmed — Bun's widened isReadableStreamLocked doesn't imply a reader exists, so all readableStreamGetNumReadRequests callers now guard on readableStreamHasDefaultReader, matching the byte controller and the old builtins' null-safe checks. The onClose scenario also exposed that releasing a direct reader left its in-flight read pending forever; that read is now settled by the release operation (regression test covers the crash input, the rejection, and the flushed final chunk).
• Stale StreamQueue.h lock-discipline comments after enqueueValueWithSize became self-locking: rewritten to state the one exception precisely.
• Dead convertUnderlyingSourceDict: deleted (the ReadableStream constructor uses its own converter that also handles type: "direct").
• ERR_STREAM_RELEASE_LOCK registry entry (re-reported): same answer as before — ErrorCode.rs is not generated (no generator exists; it's a checked-in mirror with explicit index-aligned discriminants), so removing a mid-table entry means hand-renumbering every later discriminant. Best done as its own mechanical PR.

Separately, from the CI failure list: restored Bun's async-iterable/async-generator body semantics (yield → direct controller, sink backpressure, iterator throw/return; async-iterator-stream.test.ts back to green from 24 failures), restored AsyncLocalStorage propagation into a source's cancel, and aligned the default controller's state errors with Node (ERR_INVALID_STATE), which is what test/regression/issue/19661.test.ts asserts.

[Jarred-Sumner]

Two more review findings triaged; everything through d95685e is pushed:

• Reader constructors skipping materialization: confirmed and worse than the report says — new ReadableStreamDefaultReader(Bun.file(...).stream()).read() hung forever on this branch (the previous implementation threw). Both reader constructors now materialize the stream before locking, exactly like getReader(), with a regression test that pumps a Bun.file(...).stream() through a directly-constructed reader.
• $markPromiseAsHandled in AsyncIterableStream.ts: refuted — markPromiseAsHandled is a JavaScriptCore link-time constant (LinkTimeConstant.h:56, BuiltinNames.h:81), not something defined by the deleted Bun builtin, so the private name resolves; the converter's 86 tests execute that function. No change.

Also from the CI failure list on the last build: the WPT from.any.js/strategies.any.js failures were release-only — errors created inside our native promise reactions had no JavaScript frames and therefore no stack in release. Per the suggestion, the async-stack machinery was extracted out of bindings.cpp into AsyncStackTrace.{h,cpp} and readableStreamError now borrows the awaiting async function's frames from the pending read() promise, so those rejections report at async <caller> in release builds (and the test shim now matches testharness.js exactly instead of additionally requiring stack). The asan-lane validator hit in spawn-stdin-readable-stream.test.ts was a real bug of the same FFI class — ReadableStream__cancel could return to Rust with a pending exception — and is fixed alongside its sibling. Request now also rejects keepalive: true with a stream body (TypeError("keepalive"), before the usability check), matching Node.

[coderabbitai]

Actionable comments posted: 5

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (5)

src/js/builtins/ProcessObjectInternals.ts (1)

246-252: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

Avoid rearming internalRead() while another read is already pending. If own() has already replayed needsInternalReadRefresh, the stale rejection from the old reader can call triggerRead() again and start a second read on the new reader, which can duplicate or reorder stdin chunks. Gate the re-arm so only one read is active per reader.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/builtins/ProcessObjectInternals.ts` around lines 246 - 252, Avoid
rearming internalRead() when a newer read is already pending, because the stale
rejection path in ProcessObjectInternals can start a second read on the new
reader and duplicate or reorder stdin chunks. Update the triggerRead() re-arm
logic in the reader mismatch branch so it checks the current pending read/reader
state before calling triggerRead.$call(stream, undefined), and only schedules a
new read when no read is already active for that reader.

src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp (2)

367-368: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Return an ArrayBuffer for single string chunks.

Line 368 returns encodeStringToUint8Array(...), so Bun.readableStreamToArrayBuffer() returns a Uint8Array only for the single-string fast path, while the fallback path returns an ArrayBuffer.

Proposed fix

-        if (chunk.isString())
-            RELEASE_AND_RETURN(scope, encodeStringToUint8Array(globalObject, chunk));
+        if (chunk.isString()) {
+            auto* bytes = encodeStringToUint8Array(globalObject, chunk);
+            RETURN_IF_EXCEPTION(scope, {});
+            RELEASE_AND_RETURN(scope, bytes->possiblySharedJSBuffer(globalObject));
+        }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp` around lines 367 -
368, The single-string fast path in
BunStreamConsumers::readableStreamToArrayBuffer is returning a Uint8Array via
encodeStringToUint8Array, which makes it inconsistent with the fallback path
that returns an ArrayBuffer. Update the chunk.isString() branch to produce an
ArrayBuffer in the same way as the non-fast-path logic, keeping the return type
consistent for Bun.readableStreamToArrayBuffer().

---

906-923: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Delay direct-stream state mutation until fallible setup succeeds.

After Lines 906-909 clear the direct source and mark the stream locked/disturbed, ArrayBufferSink.start and the pull/close property gets can throw. That escapes synchronously and leaves the stream consumed/locked instead of returning a rejected consumer promise. Wrap this setup like consumeDirectStream() does, or move the fallible work before mutating stream state.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp` around lines 906 -
923, The direct-stream setup in BunStreamConsumers should not mutate stream
state before fallible work completes. In the consume path around the
ArrayBufferSink.start call and the pull/close property accesses, either wrap the
setup in the same try/fallible flow used by consumeDirectStream() or move the
stream->m_directUnderlyingSource.clear(), m_bunMode, m_lockedWithoutReader, and
m_disturbed updates until after those operations succeed. Ensure any exception
from start, pull, or close results in a rejected consumer promise rather than
leaving the stream consumed/locked.

src/jsc/bindings/webcore/streams/BunStreamSource.cpp (2)

1313-1315: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Use presence checks, not truthiness, for chunks and errors.

Falsy chunks (0, false, "") are valid stream values and are currently skipped; falsy thrown reasons are also treated as “no error”. Use !error.isEmpty() for stored errors, and write the chunk whenever done is false. As per coding guidelines, empty, zero, and unset are distinct states.

Proposed direction

-    bool hasTruthyError = !error.isEmpty() && error.toBoolean(globalObject);
+    bool hasError = !error.isEmpty();

-    bool hasChunk = chunk.toBoolean(globalObject);
     if (isDone) {
         op->m_closed = true;
-        if (hasChunk) {
-            MarkedArgumentBuffer args;
-            args.append(chunk);
-            ASSERT(!args.hasOverflowed());
-            invokeMethod(globalObject, op->m_sink.get(), Identifier::fromString(vm, "write"_s), args);
-            RETURN_IF_EXCEPTION(scope, );
-        }
         op->m_reading = false;
         RELEASE_AND_RETURN(scope, resumableEnd(globalObject, op, jsUndefined(), false));
     }
-    if (hasChunk) {
+    {
         MarkedArgumentBuffer args;
         args.append(chunk);

Also applies to: 1371-1384, 1448-1450

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamSource.cpp` around lines 1313 -
1315, The stream handling in BunStreamSource is using truthiness checks for
stored errors and chunk values, which incorrectly drops valid falsy values like
0, false, and empty strings. Update the error path in the relevant read methods
to use !error.isEmpty() instead of toBoolean-based checks, and in the
chunk-writing logic always write the chunk whenever done is false, regardless of
its truthiness. Make the same presence-check adjustment in the affected
ReadableStreamSource helpers so empty, zero, and unset remain distinct states.

Source: Coding guidelines

---

447-452: 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win

Clamp the JS-returned sizes before casting to size_t.
result.asNumber() and startResult.toNumber(globalObject) can still be NaN, +Infinity, negative, or larger than size_t, so the static_cast<size_t>(...) paths here can overflow or hit UB. The autoAllocateChunkSize input is already range-checked upstream.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/webcore/streams/BunStreamSource.cpp` around lines 447 - 452,
Clamp the JS numeric results before converting them in BunStreamSource, since
result.asNumber() and startResult.toNumber(globalObject) can still be NaN,
infinite, negative, or exceed size_t. Update the size handling in the relevant
stream-reading path (including nativeAdjustChunkSize and the chunk-length min
logic) to validate and bound the values before any static_cast<size_t>
conversion, using the existing range-safe behavior already established for
autoAllocateChunkSize as a guide.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/js/builtins/AsyncIterableStream.ts`:
- Around line 71-78: The cleanup block in AsyncIterableStream’s close path
currently uses a nested finally that always rethrows closingError, which can
mask a different failure from iter.throw?.(closingError). Update the logic
around the iter.throw?.(closingError) call so that any exception/rejection from
the iterator cleanup is preserved and surfaced instead of being overwritten by
the original closingError, while still clearing iter to undefined in the
shutdown path.

In `@src/jsc/bindings/AsyncStackTrace.cpp`:
- Around line 22-30: Shorten the oversized comment blocks in AsyncStackTrace by
reducing the inline rationale to 3 lines max and keeping only the local
invariant near the relevant logic in the affected sections around the async
stack trace walk and combinator handling. Move the detailed algorithm
explanation from the top-of-file and other long comment blocks into separate
docs or a more concise note, and keep the remaining comments tied to the nearby
symbols such as the async stack trace collection logic in AsyncStackTrace.
- Around line 94-102: The promise-reaction traversal in AsyncStackTrace should
not stop after inspecting only the first JSPromiseReaction from payloadCell().
Update the logic around the reaction-walking code to keep iterating through the
full sibling chain of JSPromiseReaction nodes, checking each entry with
unwrapGeneratorFromContext(JSPromiseReaction::tryGetContext(...)) before falling
back to reaction->promise(). This ensures prepended reactions don’t hide an
awaiting async generator and preserves async stack frames regardless of
attachment order.

In `@src/jsc/bindings/AsyncStackTrace.h`:
- Around line 1-4: The file header comment in AsyncStackTrace.h exceeds the
3-line limit and should be compressed into a shorter invariant summary. Update
the top-of-file comment near the AsyncStackTrace header so it keeps the same
meaning but fits within 3 lines, removing extra detail while still referencing
the async stack recovery behavior and the AsyncStackTrace.cpp implementation.

In `@src/jsc/STREAMS.md`:
- Around line 71-72: The syntax-check example is incorrect because
compile_commands.json is not a clang response file, so the shown @<flags from
build/debug/compile_commands.json> usage will not work. Update the STREAMS.md
guidance to say that the TU’s compile command should be extracted from
compile_commands.json and then rerun with -fsyntax-only. Refer to the
syntax-check command example in the STREAMS.md section and keep the instruction
focused on using the existing TU command rather than trying to pass the JSON
file directly to clang++.

---

Outside diff comments:
In `@src/js/builtins/ProcessObjectInternals.ts`:
- Around line 246-252: Avoid rearming internalRead() when a newer read is
already pending, because the stale rejection path in ProcessObjectInternals can
start a second read on the new reader and duplicate or reorder stdin chunks.
Update the triggerRead() re-arm logic in the reader mismatch branch so it checks
the current pending read/reader state before calling triggerRead.$call(stream,
undefined), and only schedules a new read when no read is already active for
that reader.

In `@src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp`:
- Around line 367-368: The single-string fast path in
BunStreamConsumers::readableStreamToArrayBuffer is returning a Uint8Array via
encodeStringToUint8Array, which makes it inconsistent with the fallback path
that returns an ArrayBuffer. Update the chunk.isString() branch to produce an
ArrayBuffer in the same way as the non-fast-path logic, keeping the return type
consistent for Bun.readableStreamToArrayBuffer().
- Around line 906-923: The direct-stream setup in BunStreamConsumers should not
mutate stream state before fallible work completes. In the consume path around
the ArrayBufferSink.start call and the pull/close property accesses, either wrap
the setup in the same try/fallible flow used by consumeDirectStream() or move
the stream->m_directUnderlyingSource.clear(), m_bunMode, m_lockedWithoutReader,
and m_disturbed updates until after those operations succeed. Ensure any
exception from start, pull, or close results in a rejected consumer promise
rather than leaving the stream consumed/locked.

In `@src/jsc/bindings/webcore/streams/BunStreamSource.cpp`:
- Around line 1313-1315: The stream handling in BunStreamSource is using
truthiness checks for stored errors and chunk values, which incorrectly drops
valid falsy values like 0, false, and empty strings. Update the error path in
the relevant read methods to use !error.isEmpty() instead of toBoolean-based
checks, and in the chunk-writing logic always write the chunk whenever done is
false, regardless of its truthiness. Make the same presence-check adjustment in
the affected ReadableStreamSource helpers so empty, zero, and unset remain
distinct states.
- Around line 447-452: Clamp the JS numeric results before converting them in
BunStreamSource, since result.asNumber() and startResult.toNumber(globalObject)
can still be NaN, infinite, negative, or exceed size_t. Update the size handling
in the relevant stream-reading path (including nativeAdjustChunkSize and the
chunk-length min logic) to validate and bound the values before any
static_cast<size_t> conversion, using the existing range-safe behavior already
established for autoAllocateChunkSize as a guide.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: f74541a7-cf40-48fd-8d2e-71e68a24d177

📥 Commits

Reviewing files that changed from the base of the PR and between a766855 and d95685e.

📒 Files selected for processing (52)

• bench/snippets/webstreams-memory.mjs
• bench/snippets/webstreams-tee.mjs
• bench/snippets/webstreams-throughput.mjs
• src/js/CLAUDE.md
• src/js/builtins.d.ts
• src/js/builtins/AsyncIterableStream.ts
• src/js/builtins/ProcessObjectInternals.ts
• src/jsc/STREAMS.md
• src/jsc/bindings/AsyncStackTrace.cpp
• src/jsc/bindings/AsyncStackTrace.h
• src/jsc/bindings/ZigGlobalObject.cpp
• src/jsc/bindings/bindings.cpp
• src/jsc/bindings/webcore/streams/BunStandaloneTextSink.h
• src/jsc/bindings/webcore/streams/BunStreamConsumers.cpp
• src/jsc/bindings/webcore/streams/BunStreamSource.cpp
• src/jsc/bindings/webcore/streams/JSByteLengthQueuingStrategy.cpp
• src/jsc/bindings/webcore/streams/JSCountQueuingStrategy.cpp
• src/jsc/bindings/webcore/streams/JSDirectStreamController.cpp
• src/jsc/bindings/webcore/streams/JSReadStreamIntoSinkOperation.h
• src/jsc/bindings/webcore/streams/JSReadableByteStreamController.cpp
• src/jsc/bindings/webcore/streams/JSReadableStream.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamAsyncIterator.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBReader.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamBYOBRequest.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSReadableStreamDefaultReader.cpp
• src/jsc/bindings/webcore/streams/JSStreamPipeToOperation.cpp
• src/jsc/bindings/webcore/streams/JSStreamsRuntime.h
• src/jsc/bindings/webcore/streams/JSTextDecoderStream.cpp
• src/jsc/bindings/webcore/streams/JSTextEncoderStream.cpp
• src/jsc/bindings/webcore/streams/JSTransformStream.cpp
• src/jsc/bindings/webcore/streams/JSTransformStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSWritableStream.cpp
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultController.cpp
• src/jsc/bindings/webcore/streams/JSWritableStreamDefaultWriter.cpp
• src/jsc/bindings/webcore/streams/ReadableStreamOperations.cpp
• src/jsc/bindings/webcore/streams/StreamQueue.h
• src/jsc/bindings/webcore/streams/TransformStreamOperations.cpp
• src/jsc/bindings/webcore/streams/WebStreamsExports.cpp
• src/jsc/bindings/webcore/streams/WebStreamsInternals.h
• src/jsc/bindings/webcore/streams/WebStreamsMisc.cpp
• src/jsc/bindings/webcore/streams/WritableStreamOperations.cpp
• src/runtime/webcore/Body.rs
• src/runtime/webcore/ReadableStream.rs
• src/runtime/webcore/Request.rs
• test/js/bun/http/serve.test.ts
• test/js/bun/util/readablestreamtoarraybuffer.test.ts
• test/js/node/process/process-stdin.test.ts
• test/js/third_party/wpt-testharness-shim.ts
• test/js/web/fetch/body.test.ts
• test/js/web/fetch/fetch.stream.test.ts
• test/js/web/streams/streams.test.js

[coderabbitai]

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Nested finally discards the real error from iter.throw().

if (closingError) {
  try {
    await iter.throw?.(closingError);
  } finally {
    iter = undefined;
    throw closingError;
  }
}

If iter.throw?.(closingError) itself throws/rejects with a different error (e.g. the generator's own cleanup fails), that error is silently discarded and only the original closingError is ever surfaced. This matches Biome's noUnsafeFinally flag at line 77 and is a real (not false-positive) case here, since the inner try can genuinely produce a distinct exception.

🐛 Suggested fix

       if (closingError) {
         try {
           await iter.throw?.(closingError);
+        } catch (throwError) {
+          // Prefer the error raised while notifying the iterator (it may
+          // carry more diagnostic context), but never lose the original.
+          closingError = throwError ?? closingError;
         } finally {
           iter = undefined;
-          // eslint-disable-next-line no-throw-literal
-          throw closingError;
         }
+        // eslint-disable-next-line no-throw-literal
+        throw closingError;
       } else {

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change

	if (closingError) {
	try {
	await iter.throw?.(closingError);
	} finally {
	iter = undefined;
	// eslint-disable-next-line no-throw-literal
	throw closingError;
	}
	if (closingError) {
	try {
	await iter.throw?.(closingError);
	} catch (throwError) {
	closingError = throwError ?? closingError;
	} finally {
	iter = undefined;
	}
	// eslint-disable-next-line no-throw-literal
	throw closingError;
	}

🧰 Tools

🪛 Biome (2.5.1)

[error] 77-77: Unsafe usage of 'throw'.

(lint/correctness/noUnsafeFinally)

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/js/builtins/AsyncIterableStream.ts` around lines 71 - 78, The cleanup
block in AsyncIterableStream’s close path currently uses a nested finally that
always rethrows closingError, which can mask a different failure from
iter.throw?.(closingError). Update the logic around the
iter.throw?.(closingError) call so that any exception/rejection from the
iterator cleanup is preserved and surfaced instead of being overwritten by the
original closingError, while still clearing iter to undefined in the shutdown
path.

Source: Linters/SAST tools

[coderabbitai]

📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win

Shorten the long inline rationale blocks.

These new comments exceed Bun’s 3-line comment limit. Keep the invariant locally and move detailed algorithm notes to docs if needed.

As per coding guidelines, “Keep code comments to 3 lines max.”

Also applies to: 54-65, 158-162

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/jsc/bindings/AsyncStackTrace.cpp` around lines 22 - 30, Shorten the
oversized comment blocks in AsyncStackTrace by reducing the inline rationale to
3 lines max and keeping only the local invariant near the relevant logic in the
affected sections around the async stack trace walk and combinator handling.
Move the detailed algorithm explanation from the top-of-file and other long
comment blocks into separate docs or a more concise note, and keep the remaining
comments tied to the nearby symbols such as the async stack trace collection
logic in AsyncStackTrace.

Source: Coding guidelines

[Jarred-Sumner]

Latest review round, handled in 734b10b:

• Reader released by a patched Object.prototype.then during read-result resolution (both 🔴 reports): confirmed — reproduced the BYOB respond(0)-in-closed-state shape as a debug assertion (release null deref). Fixed at the source of the class rather than per call site: readableStreamGetNumRead{Into,}Requests are now null-safe (no reader → 0) and readableStreamFulfillRead{Into,}Request return early when the reader is gone (its remaining requests were already rejected by releaseLock()), which is exactly the deleted builtins' ?. semantics. Regression test covers both reported shapes.
• BYOB reader constructor materialization: correct call — getReader({ mode: "byob" }) intentionally never materializes, so the constructor half of d95685e was spurious; reverted (the default-reader half, which fixed a real hang, stays).
• AsyncIterableStream.ts throw-in-finally: restructured to keep the exact previous semantics (the original error is rethrown; the iterator's own cleanup failure is subsumed) without throwing from a finally.
• Stale specs/ reference in RESULTS.md, the compile_commands.json phrasing in STREAMS.md, over-long header comment in AsyncStackTrace.h: all fixed. The longer comment blocks inside AsyncStackTrace.cpp are pre-existing text moved verbatim out of bindings.cpp, not new prose.

WPT 1174/1174, streams suite 115/115 (including the new regression tests), async-iterator 86/86 on the pushed head.

[Jarred-Sumner]

Confirmed and fixed in b815c9e: reader.cancel() on a direct stream with an in-flight async pull left the pending read() hanging forever — exactly the traced mechanism (readableStreamCancel closes the stream, so the direct controller's onClose early-returns without settling the promise-kind read it holds, which never sits in the reader's request queue). The cancel path now settles it as done, mirroring the earlier releaseLock() fix; regression test included. Notably, the same input hangs forever on the previous implementation too (the probe reports HANG on the current canary), so this closes a pre-existing hang class rather than a rewrite regression — and since a stranded read means a handler that never resolves, it is a strong candidate for the load-dependent serve-body-leak fixture stalls CI has been showing.

[claude]

🔴 Actual location: JSDirectStreamController.cpp:409-423 × JSReadableStreamDefaultReader.cpp:123-131 (anchored here because those files' diffs are collapsed). When a type:'direct' stream has its final chunk armed (set by onClose at :593-594 when there are flushed bytes but no reader — the exact fallthrough introduced by this PR's fix for the earlier onClose null-deref), a subsequent read via a non-Promise-kind read request (pipeTo, tee, for await) silently drops the chunk: readableStreamDefaultReaderRead at :125 queues the readRequest, then :127 calls onPull, whose m_finalChunkArmed branch (:413-419) creates a fulfilled promise (adopted only by the Promise-kind path at :121) and calls readableStreamCloseIfPossible — which dispatches closeSteps() on the just-queued request. The returned promise is discarded at :129 (m_pendingRead is null on this branch). The regression test at streams.test.js:637-653 covers the Promise-kind path (rs.getReader().read()); the for await/pipeTo/tee siblings drop the chunk. Fix: in onPull's m_finalChunkArmed branch, when a readRequest is queued, deliver via readableStreamFulfillReadRequest(stream, chunk, false) before closing — mirroring onClose at :587-590.

Extended reasoning...

What the bug is

When a type: 'direct' stream has its final chunk armed via m_finalChunkArmed (set by onClose at JSDirectStreamController.cpp:593-594 when there are flushed bytes but no reader / no pending read requests — the fallthrough this PR introduced when fixing the onClose null-deref in commit 219e226b), a subsequent read via a non-Promise-kind read request (ReadRequestKind::PipeTo, DefaultTee, ByteTee, AsyncIterator) silently drops the chunk: the read request receives closeSteps() (i.e. {done: true}) instead of chunkSteps(finalChunk).

Code path

readableStreamDefaultReaderRead (JSReadableStreamDefaultReader.cpp:107-131) for a Direct controller with a non-Promise-kind readRequest:

1. Line 125: readableStreamAddReadRequest(vm, stream, readRequest) — queues the request in reader->m_readRequests.
2. Line 127: controller->onPull(globalObject).
3. onPull at :409: m_finalChunkArmed is true → :413-416 creates a promise fulfilled with {value: chunk, done: false}, then :419 readableStreamCloseIfPossible(globalObject, stream) → readableStreamClose → detaches reader->m_readRequests and calls closeSteps() on the request queued at step 1 → :422 returns the fulfilled promise. m_pendingRead is never set on this branch.
4. Back at :129: !hadPendingRead && controller->m_pendingRead — m_pendingRead is null, condition false → the returned promise carrying the chunk is discarded.

Result: the pipeTo/tee/async-iterator read request sees {done: true} and never receives the buffered bytes. The Promise-kind path at :111-121 works because it adopts the returned promise via resolvePromise(readPromise, pulled) at :121.

Why nothing prevents it

The comment at :123-124 says "Other read-request kinds wait in [[readRequests]]; the pump's unobserved head-of-line promise for this read is dropped so delivery reaches the request" — but the m_finalChunkArmed branch (:409-423) doesn't put the chunk anywhere the queued readRequest can see it: it delivers only via the returned promise (which the non-Promise arm intentionally drops), and its only side effect is readableStreamClose, which runs closeSteps on queued requests — not chunkSteps. The branch was written assuming only the Promise-kind caller (which is also what its sibling onClose at :575-582 handles via m_pendingRead), and the non-Promise arm at :123-131 was written assuming onPull delivers via m_pendingRead or via a queued read-request fulfillment — neither of which the m_finalChunkArmed branch does.

Regression from the deleted JS builtin

The old onCloseDirectStream (ReadableStreamInternals.ts) armed the final chunk by swapping this.$pull = $onCloseDirectStreamFinalPull.bind(...), and the old readableStreamDefaultReaderRead returned controller.$pull(controller) — a promise resolving to {value: flushed, done: false}. The old pipeTo/tee/for-await all consumed via that returned promise, so the chunk was delivered regardless of consumer type. The C++ rewrite split reads into Promise-kind (adopts the returned promise at :121) and non-Promise-kind (queued, waits for chunkSteps/closeSteps), and the m_finalChunkArmed branch was written for only the former.

Step-by-step proof

The setup exactly matches the author's own regression test at streams.test.js:637-653, differing only in the follow-up consumer:

const rs = new ReadableStream({
  type: 'direct',
  async pull(c) { await Promise.resolve(); c.write(new Uint8Array(10)); c.end(); },
});
const reader = rs.getReader();
reader.read().catch(() => {});     // triggers onPull; m_deferClose reset to 0 when it returns
reader.releaseLock();               // clears stream->m_reader AND controller->m_pendingRead (per 219e226b)
await Bun.sleep(0);                 // async pull body runs: c.write(10) then c.end() → onClose
                                    //   → readableStreamHasDefaultReader(stream) is false (no reader)
                                    //   → :593-594 sets m_finalChunkArmed = true, m_finalChunk = 10-byte chunk
                                    //   → :595 returns without closing; state stays Readable, stream unlocked
// Existing test does rs.getReader().read() here → Promise kind → adopts onPull's returned promise → works.
// Non-Promise consumers instead:
let seen = 0;
for await (const c of rs) seen += c.byteLength;   // → seen = 0 (chunk dropped)
// or: await rs.pipeTo(new WritableStream({ write(c){ seen += c.byteLength; } }));  → seen = 0
// or: const [a, b] = rs.tee(); for await (const c of a) seen += c.byteLength;      → seen = 0

Reachability of each consumer: the stream is unlocked (m_reader cleared, m_lockedWithoutReader false) and state is Readable (:595 returned without closing), so values()/pipeTo/tee all succeed in acquiring a reader. JSReadableStreamAsyncIterator.cpp:160-161 creates ReadRequestKind::AsyncIterator; JSStreamPipeToOperation.cpp:139-141 creates ReadRequestKind::PipeTo; ReadableStreamOperations.cpp:910 creates ReadRequestKind::DefaultTee — all call readableStreamDefaultReaderRead and reach the :123-131 arm.

Impact

Silent data loss (the worst failure mode — no error, no hang, just missing bytes) in code this PR wrote, in exactly the scenario class the author is actively fixing on this PR (release-reader-during-async-pull on direct streams; commits 219e226b and 7bb3c94), and the PR's own regression test at streams.test.js:637-653 covers the Promise-kind sibling but not the for await/pipeTo/tee siblings. This is the missed sibling of those fixes per CLAUDE.md's "fix the whole class in the same PR". Not a duplicate of any prior comment on this PR — the resolved onClose null-deref (which created the m_finalChunkArmed fallthrough) and the still-open reader.cancel() hang at BunStreamConsumers.cpp:500 are the same scenario setup with different failure sinks.

Fix

In onPull's m_finalChunkArmed branch (JSDirectStreamController.cpp:409-423), before creating the fulfilled promise, check for a queued readRequest and deliver via readableStreamFulfillReadRequest — exactly what onClose at :587-590 already does:

if (m_finalChunkArmed) {
    m_finalChunkArmed = false;
    JSValue chunk = m_finalChunk.get();
    m_finalChunk.clear();
    auto* stream = m_stream.get();
    // A non-Promise-kind read (pipeTo/tee/for-await) queued its readRequest before
    // calling onPull; deliver via chunkSteps rather than the returned promise.
    if (stream && readableStreamHasDefaultReader(stream) && readableStreamGetNumReadRequests(stream) > 0) {
        readableStreamFulfillReadRequest(globalObject, stream, chunk, false);
        RETURN_IF_EXCEPTION(scope, {});
        readableStreamCloseIfPossible(globalObject, stream);
        RETURN_IF_EXCEPTION(scope, {});
        return jsUndefined();
    }
    JSObject* result = createIteratorResultObject(globalObject, chunk, false);
    ...
}

A ~6-line addition. The regression test at :637-653 should be extended with for await/pipeTo/tee variants (same setup, different follow-up consumer).

[claude]

🟡 The new pause() and resume() churn regression test at test/js/node/process/process-stdin.test.ts:305-332 (added in 28f593f for this internalRead fix) spawns with stderr: "pipe" (:321) but only awaits Promise.all([proc.stdout.text(), proc.exited]) (:329) — stderr is never drained, unlike the two neighboring tests in the same file (:277-278, :301-302). If the child ever writes to stderr the diagnostic is lost (and in the limit, an unread pipe can deadlock the child) — add proc.stderr.text() to the Promise.all. Minor: "x".repeat(1024) at :325 → Buffer.alloc(1024, "x").toString() per test/CLAUDE.md.

Extended reasoning...

What the issue is

The regression test "pause() and resume() churn while data is in flight never destroys stdin" at test/js/node/process/process-stdin.test.ts:305-332 was added in commit 28f593f as coverage for this PR's internalRead reader-identity fix in ProcessObjectInternals.ts. It spawns a child with stderr: "pipe" (line 321) but then only awaits:

const [stdout, exitCode] = await Promise.all([proc.stdout.text(), proc.exited]);

at line 329 — proc.stderr is never read. This diverges from the root CLAUDE.md rule "Subprocess tests: drain pipes concurrently. Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]) — an unread pipe fills the ~64KB OS buffer and deadlocks the child", and from the two immediately-preceding tests in the same file, which both drain stderr (lines 277-278 and 301-302 each do expect(await proc.stderr.text())).

Step-by-step

1. Line 321 configures the spawn with stderr: "pipe", so the child's fd 2 is a pipe the parent must read.
2. The child's -e script writes only to stdout on success, so in the happy path stderr stays empty and the un-read pipe never fills — the test passes.
3. On the failure path this test is designed to catch — the pre-fix bug where process.stdin is destroyed with TypeError [ERR_INVALID_STATE]: Invalid state: Reader released — the child's line 313 does console.log("ERROR " + ...); process.exit(1), which goes to stdout, so that specific failure is still visible. But any other failure mode (a debug-build warning, an uncaught-exception dump, an ASAN report, a native assertion) writes to stderr, and that diagnostic is silently discarded: the test just reports expect(stdout.trim()).toBe("TOTAL 20480") failing with an empty or truncated string and no explanation.
4. In the limit, if a future regression made the child write >64 KB to stderr (e.g. repeated warnings inside the 5 ms setInterval churn loop), the child would block on write(2, ...) and the test would hang until the harness timeout — the exact scenario the CLAUDE.md rule exists to prevent. await using proc bounds that hang, but only after the test times out.

Why it's a nit, not a blocker

The child never writes to stderr in normal operation, so there's no deadlock in practice today; await using proc provides a backstop; and this is test-quality only, with zero runtime impact. It doesn't justify blocking a merge. But it's new code in this PR, it's a one-line fix, and it's the exact shape Bun reviewers routinely catch (per CLAUDE.md's "Landing PRs" → "Subprocess tests: drain pipes concurrently").

Secondary items

• "x".repeat(1024) at :325: test/CLAUDE.md says "To create a repetitive string, use Buffer.alloc(count, fill).toString() instead of "A".repeat(count). "".repeat is very slow in debug JavaScriptCore builds." At 20×1 KB this is negligible in wall-clock terms, so purely a convention nit.
• await Bun.sleep(10) × 20 at :326: three verifiers noted this against test/CLAUDE.md's "never wait for time to pass", but I'm not pressing it — this is a bounded-window "X does not happen" race test where the sleeps space parent writes so the child's 5 ms setInterval pause/resume churn can interleave, and the neighboring test at :297 uses Bun.sleep(1000) for the same class of assertion. Root CLAUDE.md explicitly allows "For 'X does not happen', poll a bounded window".

Fix

const [stdout, stderr, exitCode] = await Promise.all([proc.stdout.text(), proc.stderr.text(), proc.exited]);
expect(stdout.trim()).toBe(`TOTAL ${20 * 1024}`);
expect(stderr).toBe("");
expect(exitCode).toBe(0);

and optionally proc.stdin.write(Buffer.alloc(1024, "x").toString()) at :325.

(Anchored at ProcessObjectInternals.ts because the test file's diff hunk is collapsed by GitHub.)

[Jarred-Sumner]

f554ff82d1ef adds the piece this thread's flush questions pointed at, measured end to end:

Finding. The HTTP response sink already flushes buffered writes at the end of the tick (it registers with the event loop's deferred-task queue, which runs right after every microtask drain) — but the JS-facing direct-stream sink had no equivalent, so a producer that wrote and then suspended (an async-generator Response body read by a JS reader, or any type: "direct" source without an explicit flush()) delivered nothing until the stream ended. The old async-iterable converter masked exactly this with a per-batch setImmediate(... flush(true)).

Change. The direct controller now arms a one-shot end-of-tick flush on the same deferred queue when a write buffers data (new Bun__EventLoop__postDeferredTask / unregisterDeferredTask exports; unregistered on destruction), so writes batch within a tick and are delivered when it ends — and the converter's setImmediate is deleted, leaving it pure iterator driving + backpressure (write() < 0 → await flush(true)).

Measured (slow async generator yielding every 120 ms):

consumer	before, timer removed	after
HTTP socket (syscall timestamps)	already same-tick	same-tick
JS reader on Response(gen()).body	one concatenated chunk at stream end	per-yield delivery

Regression tests cover both shapes (a direct source writing inside a never-resolving pull(), and per-yield generator delivery). Streams 118/118, WPT 1174/1174, body-stream 9086/9086, fetch.stream 110/110 on the pushed head.