stdio: enforce write-after-end contract for piped process.stdout/stderr

[robobun]

What

For a piped (non-TTY) process.stdout/process.stderr, calling write() after end() succeeds, reports no error, and delivers the bytes to the pipe reader. That violates the node:stream Writable contract: a write after end() must fail with ERR_STREAM_WRITE_AFTER_END and must not reach the underlying fd.

The stream's own state already reports writableEnded: true, writable: false, so the public state contradicts what the fd does.

Repro

// child: parent spawns it with a piped stdout and reads it
const so = process.stdout, ev = [];
so.on("error", e => ev.push("err:" + (e?.code ?? e?.name)));
so.write("A");
so.end("B");
const ret = so.write("C", e => { /* err expected here */ });

Before this change the pipe reader receives "ABC", write() returns true, and the callback gets no error. Node (and this change) deliver "AB", write() returns false, and the callback plus an 'error' event fire with ERR_STREAM_WRITE_AFTER_END.

Cause

Piped stdio is backed by a node:fs WriteStream in "fast path" mode, whose write is replaced by writeFast in src/js/internal/fs/streams.ts. writeFast pushes data straight into the FileSink and never consults the Writable state, so the ERR_STREAM_WRITE_AFTER_END guard that lives in Writable.prototype.write is bypassed.

Fix

writeFast now delegates to Writable.prototype.write when the stream is ending (end() has been called). That path already implements the correct contract (callback + 'error' with ERR_STREAM_WRITE_AFTER_END, no delivery), so the bytes never reach the sink.

Scope note: this covers the end() path, which is the reachable write-after-termination mode for process.stdout/process.stderr. destroy() is intentionally a no-op on stdio (the streams override _destroy to call _undestroy() for Node parity), so it does not latch destroyed and ERR_STREAM_DESTROYED is not reachable there.

Verification

bun bd test test/js/node/process/process-stdout-write-after-end.test.ts passes for both stdout and stderr; the same tests fail on the released binary ("ABC" delivered).

[robobun]

^{Updated 6:20 PM PT - Jul 6th, 2026}

⏳ @robobun, your commit 921d8a0 is still building in Build #69513, but has 1 failures so far (All Failures):

• test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js - SIGABRT on 🐧 13 x64-asan

[coderabbitai]

Warning

Review limit reached

@robobun, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 41 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 5c395e16-8aa4-4cb8-b354-3bbc621fbcea

📥 Commits

Reviewing files that changed from the base of the PR and between 23b7826 and 921d8a0.

📒 Files selected for processing (3)

• src/js/internal/fs/streams.ts
• test/js/node/process/process-stdout-write-after-end-fixture.mjs
• test/js/node/process/process-stdout-write-after-end.test.ts

Walkthrough

The writeFast fast-path in fs streams now checks writable state before writing, delegating to Writable.prototype.write when the stream is ending, ended, destroyed, or errored. A new test fixture and test case verify that writes after end() produce ERR_STREAM_WRITE_AFTER_END and are not delivered to the pipe.

Changes

Write-after-end fix and verification

Layer / File(s)	Summary
writeFast writable-state guard src/js/internal/fs/streams.ts	Adds a check on _writableState in writeFast to route to Writable.prototype.write for ending/ended/destroyed/errored states instead of the fast-path sink.
Fixture script for write-after-end test/js/node/process/process-stdout-write-after-end-fixture.mjs	New fixture writes, ends, writes again with callback, captures error events, and reports writable state and results as JSON via stderr.
Test asserting write-after-end is not delivered test/js/node/process/process-stdio.test.ts	New test spawns the fixture, confirms the post-end write is dropped from the piped stdout, and validates the stderr JSON report against the expected error shape and successful exit.

Related PRs: None identified.

Suggested labels: bug, node.js, streams, tests

Suggested reviewers: None identified.

Poem
A rabbit typed, then hit "the end,"
No stray write could sneak past that bend.
Fast-path checked its state with care,
Errors caught before they'd dare.
Stdout stays clean, "AB" the whole—
A tidy stream, a job well-toled. 🐇

🚥 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 clearly summarizes the main change: enforcing write-after-end behavior for piped stdio streams.
Description check	✅ Passed	The description covers what the PR does and how it was verified, with clear repro, cause, fix, and test evidence.

---

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

[coderabbitai]

Actionable comments posted: 3

🤖 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 `@test/js/node/process/process-stdio.test.ts`:
- Around line 162-185: The current coverage in process-stdio.test only exercises
process.stdout with write-after-end and ERR_STREAM_WRITE_AFTER_END, so expand
the matrix to include the destroy() path and process.stderr as well. Add sibling
tests around the existing process.stdout - write after end() errors and is not
delivered (piped) case, using the relevant fixture(s) and assertions for
ERR_STREAM_DESTROYED, and mirror the same behavior checks for process.stderr so
the fix is verified across both streams and both termination modes. Reference
the existing process.stdout test structure and the related fixture names when
adding the new cases.
- Around line 171-183: The test in process-stdio.test.ts is assuming stderr
contains only JSON, but spawned subprocesses can emit harmless ASAN/debug noise
first. Update the assertion around proc.stderr.text() in this scenario to ignore
any extra benign lines and parse only the last non-empty line before checking
the report object, so the test remains stable across debug/ASAN builds.

In `@test/js/node/process/process-stdout-write-after-end-fixture.mjs`:
- Around line 9-16: The test in process-stdout-write-after-end-fixture.mjs uses
fixed setTimeout sleeps around so.write, which makes it flaky and violates the
test guidance. Remove the unnecessary pre-write delay since end() already sets
the ending state synchronously, and replace the post-write timeout with awaiting
the actual completion condition for the write callback/error handling to settle.
Use the existing so.write callback path and the surrounding test helpers in this
fixture to wait for the expected callback/error event instead of sleeping.

🪄 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: 1f5c2277-f648-4eae-8878-b035b86571d5

📥 Commits

Reviewing files that changed from the base of the PR and between d363fa6 and 23b7826.

📒 Files selected for processing (3)

• src/js/internal/fs/streams.ts
• test/js/node/process/process-stdio.test.ts
• test/js/node/process/process-stdout-write-after-end-fixture.mjs

[claude]

The fix looks correct and all earlier test-quality nits are addressed, but it adds a _writableState read + .ending getter to every writeFast call — the hot path for piped process.stdout/stderr — so a maintainer should confirm the per-write overhead is acceptable.

Extended reasoning...

Overview

This PR adds a 6-line guard to writeFast in src/js/internal/fs/streams.ts so that writes after end() on piped process.stdout/process.stderr delegate to Writable.prototype.write, which correctly produces ERR_STREAM_WRITE_AFTER_END instead of silently delivering bytes to the sink. It adds a fixture and a test.each(["stdout","stderr"]) test that verifies the post-end write is rejected, the callback receives the error, an 'error' event fires, and the byte never reaches the pipe reader. Over three review rounds the tests were tightened (no setTimeout, noise-tolerant JSON parsing, distinctive POST_END_MARKER sentinel) and the guard was narrowed from four conditions to just state.ending — the only demonstrably-live one for stdio.

Security risks

None. This is a Node.js Writable-contract compat fix; no auth, crypto, path handling, or untrusted-input parsing is involved.

Level of scrutiny

Medium-high. The logic is trivially correct — Writable.prototype.write already implements the exact contract at src/js/internal/streams/writable.ts:453-454, and the delegation pattern matches the existing kWriteMonkeyPatchDefense line immediately above. But writeFast backs every piped process.stdout.write (i.e. every console.log when stdout is a pipe), and the file itself flags the function as known-tricky ("This function implementation is not correct."). The new guard adds one this._writableState property read plus a .ending bitmap-getter call to every write. That's almost certainly negligible next to the FileSink write itself, but hot-path additions like this are exactly what CLAUDE.md's Performance section says a maintainer should sign off on.

Other factors

The bug hunter found nothing. All prior inline comments (mine and CodeRabbit's) are resolved, and the author's responses were substantive — including correctly demonstrating that destroy()/ERR_STREAM_DESTROYED is unreachable for stdio because _destroy calls _undestroy(). The behavior change is Node-parity-correct but could theoretically surprise code that relied on the buggy write-after-end delivery; that seems unlikely in practice since Node has always errored here. No CODEOWNERS entry covers this path.

[robobun]

The diff is ready. The remaining red CI lanes are unrelated to this change:

• darwin 26 aarch64 test-bun (both shards): buildkite-agent artifact download timed out after 120s for step 'darwin-aarch64-build-bun'. The download of the build artifact timed out, so no tests ran there at all. Same failure on two consecutive runs (builds 69472 and 69513).
• debian 13 x64-asan test-bun: test/js/node/test/parallel/test-worker-message-port-transfer-terminate.js SIGABRT on ASSERTION FAILED: !scope.exception() || !hasSlot in JSCJSValuePropertyInlines.h:51, a JSC exception-scope assertion in worker termination. Nothing in this PR touches workers or JSC bindings.

test/js/node/process/process-stdout-write-after-end.test.ts and src/js/internal/fs/streams.ts do not appear in any failure annotation on either build. The test passes locally on debug+ASAN and on release, and fails on the released binary (the fail-before proof is in the PR description).