child_process: match node's spawn-failure event contract

[robobun]

Repro

const { spawn, spawnSync } = require("child_process");

const c = spawn("definitely-not-a-binary");
c.on("error", () => {});
c.on("close", (code, signal) => console.log(code, signal, c.exitCode));
// node: -2 null -2
// bun:  -2 undefined null

console.log(spawnSync("echo", ["hi"], { stdio: ["pipe", "pipe", "pipe", "ignore", "ignore"] }).output.length);
// node: 5    bun: 3

try {
  spawn("echo", [], { stdio: "bogus" });
} catch (e) {
  console.log(e.code);
}
// node: ERR_INVALID_ARG_VALUE    bun: ERR_INVALID_OPT_VALUE (removed in Node 15)

Cause

All three live in src/js/node/child_process.ts.

1. Spawn-failure lifecycle. The deferred-error branch in ChildProcess#spawn() emitted close directly with a single argument:

process.nextTick(() => {
  this.emit("error", ex);
  this.emit("close", (ex as SystemError).errno ?? -1);
});

so listeners got signal === undefined, and this.exitCode was never assigned. Node's _handle.onexit(err) sets this.exitCode = err (the negative errno) and then calls maybeClose(), which emits close with (this.exitCode, this.signalCode). The divergence is not ENOENT-specific; it covers every code in the deferred list (EACCES gives close(-13, undefined) with exitCode === null).

This matters for supervision code, which is exactly the code that reads these fields: if (signal === null && code !== 0) takes the wrong branch on undefined, and child.exitCode ?? fallback hides the errno.

2. spawnSync().output. The result was hardcoded to three entries regardless of how many stdio slots the caller asked for, so output.length disagreed with stdio.length and any slot past stderr vanished.

3. Retired error code. ERR_INVALID_OPT_VALUE was removed from Node in v15. Node's getValidStdio/stdioStringToArray throw ERR_INVALID_ARG_VALUE for an unknown stdio string and for a stdio value that is neither a string nor an array. (validateMaxBuffer/validateTimeout already used $ERR_OUT_OF_RANGE correctly.)

Fix

• The deferred-error branch sets this.exitCode = errno inside the nextTick (same ordering as Node: the error listener already sees it) and emits close through #maybeClose(), which supplies (exitCode, signalCode). signalCode stays null, and no exit event is emitted, matching Node.
• spawnSync() builds output with one entry per normalized stdio slot. Slots the parent does not read from are null, which is what Node reports for ignore/inherit/fd entries.
• normalizeStdio() throws $ERR_INVALID_ARG_VALUE, whose message is already byte-identical to Node's (The argument 'stdio' is invalid. Received 'bogus'). The now-dead local ERR_INVALID_OPT_VALUE helper is deleted.

Not fixed here

A literal pipe at stdio[3] or beyond still reports null in output, because Bun.spawnSync does not buffer the extra pipes (the pre-existing TODO above output). That slot also deadlocks today once the child writes more than the pipe buffer, so closing the gap properly needs native work in Bun.spawnSync rather than a change in this layer. Every other stdio shape (ignore, inherit, fd numbers) now matches Node exactly.

Verification

New tests in test/js/node/child_process/child_process.test.ts:

• spawn() failure lifecycle > ENOENT / > EACCES: assert the event order is ["error", "close"] (no exit), that exitCode === err.errno both when error fires and after close, that close receives (errno, null), and that signalCode / killed are unchanged.
• spawnSync() > output has one entry per stdio slot and > output has three entries for the default stdio.
• spawn() > stdio > an unknown stdio string throws ERR_INVALID_ARG_VALUE (async and sync) and > stdio that is neither a string nor an array throws ERR_INVALID_ARG_VALUE, asserting the code and Node's exact message.

# fail-before (released bun):
$ USE_SYSTEM_BUN=1 bun test test/js/node/child_process/child_process.test.ts -t "failure lifecycle"
- "exitCode": -13
+ "exitCode": null
  "closeArgs": [ -13, -     null, ]
 0 pass  2 fail

# pass-after (debug build):
$ bun bd test test/js/node/child_process/child_process.test.ts
 44 pass  3 fail (all three fail identically on main in this container: two
                  5s timeouts under debug+ASAN and a missing default shell)

Node differential, before and after

                                               node            bun (before)           bun (after)
spawn(missing)   error           ENOENT, exitCode=-2    ENOENT, exitCode=null   ENOENT, exitCode=-2
                 close              (-2, null)            (-2, undefined)          (-2, null)
spawn(noexec)    error           EACCES, exitCode=-13   EACCES, exitCode=null   EACCES, exitCode=-13
                 close              (-13, null)           (-13, undefined)         (-13, null)
spawnSync stdio ["pipe","pipe","pipe","ignore","ignore"]
                 output.length          5                       3                      5
spawnSync stdio ["pipe","pipe","pipe", 1]
                 output.length          4                       3                      4
spawn stdio "bogus"              ERR_INVALID_ARG_VALUE  ERR_INVALID_OPT_VALUE  ERR_INVALID_ARG_VALUE
spawn stdio 5                    ERR_INVALID_ARG_VALUE  ERR_INVALID_OPT_VALUE  ERR_INVALID_ARG_VALUE

Node's own ported tests still pass: every test/js/node/test/parallel/test-child-process-*.js that passes on main still passes, including test-child-process-spawn-error.js (which touches child.stdin/stdout/stderr/stdio synchronously on a failed spawn, the case where #maybeClose()'s #closesNeeded bookkeeping could have swallowed the close).

---

no test proof · iteration 1 · Platform-specific test(s) that do not run on this machine. Deferring to CI, which covers all platforms: test/js/node/child_process/child_process.test.ts

[robobun]

^{Updated 11:25 PM PT - Jul 9th, 2026}

❌ @robobun, your commit 498f8ff has 6 failures in Build #71315 (All Failures):

• test/js/bun/http/proxy-stress-concurrent.test.ts - code 1 on 🍎 14 x64
• test/regression/issue/26030.test.ts - code 1 on 🐧 3.23 x64
• test/js/node/test/sequential/test-net-localport.js - code 1 on 🐧 3.23 x64-baseline
• test/js/node/test/sequential/test-net-localport.js - code 1 on 🐧 3.23 x64
• test/napi/napi.test.ts - code 1 on 🪟 2019 x64-baseline
• test/js/sql/postgres-invalid-message-length.test.ts - code 1 on 🪟 2019 x64-baseline
• test/js/sql/postgres-binary-array-bounds.test.ts - code 1 on 🪟 2019 x64-baseline

---

🧪   To try this PR locally:

bunx bun-pr 33304

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

bun-33304 --bun

[github-actions]

Found 1 issue this PR may fix:

1. Differences in child_process.spawnSync return shape and error normalization between Node.js and Bun #31767 - Reports multiple spawnSync return shape differences vs Node (output array, status, pid, stdout/stderr); this PR fixes the output array length to match the number of stdio slots and corrects spawn-failure error normalization

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

Fixes #31767

🤖 Generated with Claude Code

[robobun]

Not adding Fixes #31767 here: this PR does not fix that issue.

#31767 is about the spawnSync result shape when the spawn fails (status, pid, output, stdout, stderr on a doesnotexist binary). That path is untouched by this PR, and it still diverges on this branch:

$ bun-debug -e 'const r = require("child_process").spawnSync("doesnotexist"); console.log({ status: r.status, output: r.output, pid: r.pid, stdout: r.stdout })'
{ status: undefined, output: [ null, null, null ], pid: undefined, stdout: null }
# node: { status: null, output: null, pid: 0, stdout: undefined }

The output change here is on the success path: it makes output 1:1 with the stdio array (output.length === stdio.length, null for slots the parent does not read) instead of always three entries. The spawn-failure shape in #31767 is already covered by #31768, which early-returns Node's "never started" shape from the catch, so that issue should close with that PR rather than this one.

The two PRs touch adjacent lines in spawnSync() but not the same ones.

[github-actions]

This PR may be a duplicate of:

1. child_process: match Node's spawnSync result shape when spawn fails #31768 - Both fix spawnSync() result shape when spawn fails, overlapping on the output array construction for failed spawns

🤖 Generated with Claude Code

[robobun]

Not a duplicate of #31768. The two are adjacent in spawnSync() but they fix disjoint paths, and the headline fix here isn't in spawnSync() at all.

What #31768 does: early-returns Node's "never started" shape from the catch in spawnSync() (output: null, status: null, pid: 0, stdout/stderr: undefined), i.e. the result shape when Bun.spawnSync throws. It leaves the success-path output: line as context.

What this PR does:

1. ChildProcess's deferred spawn-error branch (async spawn(), a different class entirely): sets exitCode to the negative errno and emits close with (exitCode, signalCode) instead of the errno alone, so the signal argument is null rather than undefined. This is the main fix and has nothing to do with spawnSync.
2. spawnSync().output gets one entry per stdio slot on the path where the child actually ran (output.length === stdio.length instead of always 3).
3. normalizeStdio() throws ERR_INVALID_ARG_VALUE instead of the retired ERR_INVALID_OPT_VALUE.

Neither PR makes the other redundant, and they compose: after both land, a failed spawn returns output: null via #31768's early return (so the loop added here never runs), and a successful spawn returns an output array sized to the stdio array.

They do both edit the const result = { ... } block, so whichever lands second needs a one-line rebase. Happy to take that side if this one is later.

[coderabbitai]

Walkthrough

This PR updates Bun’s Node.js child_process compatibility by changing spawnSync output shaping, deferred spawn() failure lifecycle handling, and stdio validation errors, with corresponding tests.

Changes

child_process spawn and spawnSync fixes

Layer / File(s)	Summary
spawnSync output shape src/js/node/child_process.ts, test/js/node/child_process/child_process.test.ts	spawnSync now builds result.output with Node-aligned slots and null placeholders for additional stdio entries; tests verify cardinality and values.
Deferred spawn failure lifecycle src/js/node/child_process.ts, test/js/node/child_process/child_process.test.ts	Deferred failures now set exitCode from errno, emit "error", and call #maybeClose(); tests cover ENOENT, EACCES, event ordering, and materialized stdio streams.
stdio validation errors src/js/node/child_process.ts, test/js/node/child_process/child_process.test.ts	normalizeStdio() now throws $ERR_INVALID_ARG_VALUE("stdio", stdio) for invalid inputs, removes the old helper, and has matching spawn and spawnSync tests.

🚥 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 concisely describes the main change: aligning child_process spawn failure behavior with Node's contract.
Description check	✅ Passed	The description explains the change and verification clearly, but it uses custom headings instead of the template's exact sections.

---

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

[coderabbitai]

Actionable comments posted: 1

Caution

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

⚠️ Outside diff range comments (1)

src/js/node/child_process.ts (1)

496-514: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Keep output aligned in the batch-file error path. This early return still hardcodes 3 slots, so spawnSync() can return a shorter output array than the caller’s normalized stdio when windowsBatchFileError hits with extra descriptors.

🤖 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/node/child_process.ts` around lines 496 - 514, The `spawnSync()` early
return in the `windowsBatchFileError` branch is hardcoding a 3-entry `output`
array, which can get out of sync with the normalized `stdio` length. Update the
`spawnSync`/`windowsBatchFileError` path in `child_process.ts` to build `output`
using the same normalized stdio size as the rest of `spawnSync()` instead of
returning a fixed three-slot array, while keeping the rest of the error object
unchanged.

🤖 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/child_process/child_process.test.ts`:
- Around line 377-395: The “stdio that is neither a string nor an array” test
only covers spawn, but it should also verify spawnSync since both paths use
normalizeStdio and should throw the same ERR_INVALID_ARG_VALUE. Update the
child_process test case to add a spawnSync expectation alongside the existing
spawn assertion, using the same invalid stdio value and matching error shape as
the neighboring “unknown stdio string” test.

---

Outside diff comments:
In `@src/js/node/child_process.ts`:
- Around line 496-514: The `spawnSync()` early return in the
`windowsBatchFileError` branch is hardcoding a 3-entry `output` array, which can
get out of sync with the normalized `stdio` length. Update the
`spawnSync`/`windowsBatchFileError` path in `child_process.ts` to build `output`
using the same normalized stdio size as the rest of `spawnSync()` instead of
returning a fixed three-slot array, while keeping the rest of the error object
unchanged.

🪄 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: d4263a40-88f9-4c71-bb56-2a33f38273ad

📥 Commits

Reviewing files that changed from the base of the PR and between 1498d7b and 5b8caff.

📒 Files selected for processing (2)

• src/js/node/child_process.ts
• test/js/node/child_process/child_process.test.ts

[robobun]

Thanks @coderabbitai. Took the inline one, declining the outside-diff one.

Taken: spawnSync assertion for the non-string/non-array stdio case (17a1a04). Both entry points share normalizeStdio, so it belongs next to the async assertion.

Declined: padding output in the windowsBatchFileError early return.

That branch is a pre-spawn failure: the child never starts, and Node reports a never-started child with output: null, not an array of nulls sized to the stdio array. Padding it would move that path further from Node, not closer. (For reference, #31768 rewrites this exact block to Node's never-started shape, output: null / status: null / pid: 0 / stdout: undefined.)

It also isn't mechanically possible where the suggestion puts it: bunStdio is computed ~30 lines below that early return, after validateTimeout/validateMaxBuffer. Hoisting stdio normalization above the windowsBatchFileError check would change which error wins when both an invalid stdio value and a batch-file path are passed.

The output-length invariant in this PR is scoped to the path where the child actually ran, which is the only path where Node returns an output array at all.

[claude]

No issues found — the changes look correct and well-tested, but swapping the direct close emit for #maybeClose() in the deferred-error path relies on #closesNeeded staying at 1 there, which is a non-local invariant worth a maintainer's confirmation given how many packages depend on this event contract.

Extended reasoning...

Overview

Three targeted Node-compat fixes in src/js/node/child_process.ts plus tests:

1. The deferred spawn-error branch (ENOENT/EACCES/EAGAIN/EMFILE/ENFILE) now sets this.exitCode = errno and routes close through #maybeClose() instead of emitting it directly with one argument, so listeners receive (errno, null) and child.exitCode is populated — matching Node's _handle.onexit(err) behavior.
2. spawnSync().output is padded with null for stdio slots beyond stderr so output.length === stdio.length.
3. normalizeStdio() throws ERR_INVALID_ARG_VALUE instead of the retired ERR_INVALID_OPT_VALUE, and the dead helper is deleted.

Security risks

None. No untrusted-input parsing, auth, crypto, or filesystem-path handling is introduced; the changes only adjust event arguments, an output-array length, and an error code on paths that already existed.

Level of scrutiny

Moderate. Changes #2 and #3 are mechanical and low-risk. Change #1 is small but semantically meaningful: it replaces a direct emit("close", errno) with #maybeClose(), which only fires when #closesGot === #closesNeeded. I traced the invariant — in the deferred-error path #handle is nulled before any stdio getter can create a real pipe (so the #closesNeeded++ in #getBunSpawnIo never runs), and the IPC increment only executes after Bun.spawn() succeeds — so #closesNeeded stays at 1 and close fires. The PR description confirms this via test-child-process-spawn-error.js, which touches .stdin/.stdout/.stderr/.stdio synchronously on a failed spawn. Still, child_process event lifecycle is a load-bearing Node-compat surface that a lot of the npm ecosystem sits on, so a maintainer familiar with this file should sign off.

Other factors

• Bug-hunting system found nothing; CodeRabbit's one nit (spawnSync coverage for non-string/non-array stdio) was addressed in 17a1a04 and the thread is resolved.
• Tests follow harness conventions (tempDir, bunEnv, event-driven awaits, exact-value assertions) and include fail-before/pass-after verification.
• The author has already flagged a one-line rebase interaction with #31768 in the spawnSync result block; whichever lands second will need to reconcile.
• No CODEOWNERS entry for this path.

[robobun]

Fair call on the #closesNeeded invariant, @claude. It was verified by hand rather than by the suite, so I've encoded it instead (392cf34).

The risk is narrow: #maybeClose() emits only when #closesGot === #closesNeeded, and the single #closesNeeded++ lives in #getBunSpawnIo's case 1/2 after the if (!value) guard. On the deferred-error path #handle is nulled before spawn() returns, so every getter takes the !value branch and hands back a destroyed dummy stream without bumping the count. Nothing else can reach the counter there: the IPC #closesNeeded += 1 only runs after Bun.spawn() succeeds, and #getBunSpawnIo's default: arm (fd >= 3) returns null when handle is null.

That reasoning is now a test. close still fires after the stdio streams are materialized materializes stdin, stdout, stderr and the stdio array synchronously on a failed spawn, then asserts the full contract, for both default stdio and an extra pipe at stdio[3] (the default: arm):

# fail-before (released bun):
$ USE_SYSTEM_BUN=1 bun test child_process.test.ts -t "close still fires"
 0 pass  2 fail

# pass-after:
$ bun bd test child_process.test.ts -t "failure lifecycle"
(pass) ENOENT
(pass) EACCES
(pass) close still fires after the stdio streams are materialized (default stdio)
(pass) close still fires after the stdio streams are materialized (an extra pipe)
 4 pass  0 fail  12 expect() calls

If someone later adds a #closesNeeded++ reachable from a failed spawn, close stops firing and these tests hang out rather than passing quietly. test-child-process-spawn-error.js touches the same streams but only asserts the error fields, so it would not have caught it.

[claude]

I didn't find any bugs, but the switch from a direct emit("close", errno) to #maybeClose() on the deferred-error path makes close firing depend on #closesNeeded staying at 1 when Bun.spawn throws — I traced it and the tests cover it, but it's worth a maintainer's eye given how many packages depend on child_process failure semantics.

Extended reasoning...

Overview

Three targeted Node-compat fixes in src/js/node/child_process.ts, all with new tests in test/js/node/child_process/child_process.test.ts:

1. Deferred spawn-error lifecycle (the headline change): when Bun.spawn throws a deferred error (ENOENT/EACCES/EAGAIN/EMFILE/ENFILE), the catch block now sets this.exitCode = errno inside the nextTick and routes close through #maybeClose() instead of emitting it directly with a single argument. This makes close fire with (errno, null) instead of (errno, undefined) and populates child.exitCode.
2. spawnSync().output length: pads the output array with null for stdio slots beyond index 2, so output.length === stdio.length.
3. Retired error code: normalizeStdio() now throws $ERR_INVALID_ARG_VALUE instead of the local ERR_INVALID_OPT_VALUE helper (removed from Node in v15). The dead helper is deleted; I confirmed it had no other callers in this file.

Security risks

None. No untrusted input parsing changes, no privilege/permission logic. The changes only affect error-code strings, event-argument shape, and array padding with null.

Level of scrutiny

Medium-high. node:child_process spawn-failure semantics are load-bearing for process supervisors, task runners, and CI tooling. The subtle part is change (1): previously close was emitted unconditionally; now it goes through the #closesNeeded/#closesGot counter. I traced the failure path — #closesNeeded starts at 1, the IPC increment is inside the try after Bun.spawn succeeds so it never runs, and the stdio getters don't increment when #handle is null (they short-circuit to destroyed streams / null) — so the single #maybeClose() call correctly emits close. The new "close still fires after the stdio streams are materialized" test locks this in for both default stdio and an extra pipe, and the PR description reports Node's ported test-child-process-spawn-error.js still passes. That said, this is exactly the kind of counter-based event bookkeeping where a maintainer with domain context should confirm no other path can bump #closesNeeded before the nextTick.

Other factors

• The bug hunter found nothing.
• The one CodeRabbit inline nit (add spawnSync coverage for non-string/non-array stdio) was addressed in 17a1a04 and the thread is resolved.
• No CODEOWNERS for these files.
• The PR author flagged an adjacent-line interaction with #31768 (the spawnSync failed-spawn result shape); whichever lands second needs a small rebase of the const result = { ... } block.
• Changes (2) and (3) are mechanical and low-risk on their own.

[robobun]

CI status: the diff is green, the red lanes are repo-wide flake

(Updated for builds 71295 and 71315 on the rebased branch.)

Build 71315 on 498f8ff: 280 passed, 6 failed. Every child_process test passed on every lane that ran them. This PR's diff from its merge-base is exactly two files (src/js/node/child_process.ts + its test), and none of the failing tests load node:child_process.

Lane	Failure	Also failing on
alpine x64 + x64-baseline	test-net-localport.js EADDRINUSE on common.PORT	builds 71313, 71312, 71302, 71301, 71300
windows 2019 x64-baseline	postgres-binary-array-bounds.test.ts CONNECTION_REFUSED	build 71313
windows 2019 x64-baseline	postgres-invalid-message-length.test.ts CONNECTION_REFUSED	build 71312
windows 2019 x64-baseline	napi.test.ts "Condition not met after 100 GC attempts"	flaky annotation on 68193
alpine x64	26030.test.ts (mysql docker startup)	builds 71302, 71301
darwin 14 x64	proxy-stress-concurrent.test.ts 1/1200 requests dropped	build 71313

The previous run, build 71295, had 276 pass / 2 fail (same test-net-localport.js EADDRINUSE on both alpine lanes). I re-rolled once; the re-roll hit the same flake plus the Windows DB-container races.

The postgres/mysql failures are CONNECTION_REFUSED on a DB container that was not ready in time; #33822 (merged to main after this branch's rebase base, 2c12b165cb) bakes the data dirs so cold start drops to ~2-3s. This branch is behind that by three commits, which is why those races still hit here but not on 71310/71309.

Verification recap

$ bun bd test test/js/node/child_process/child_process.test.ts -t "failure lifecycle"
 4 pass  0 fail  12 expect() calls
$ bun bd test test/js/node/child_process/child_process.test.ts -t "output has"
 2 pass  0 fail
$ bun bd test test/js/node/child_process/child_process.test.ts -t ERR_INVALID_ARG_VALUE
 2 pass  0 fail
$ USE_SYSTEM_BUN=1 bun test test/js/node/child_process/child_process.test.ts -t "failure lifecycle"
 0 pass  4 fail

Both review bots are satisfied (claude[bot]: "no bugs found"; CodeRabbit: "No actionable comments"), and all review threads are resolved.

Ready for a maintainer. The one thing worth a second pair of eyes, raised by claude[bot] and now covered by the close still fires after the stdio streams are materialized test: routing close through #maybeClose() means it only fires while #closesNeeded stays at 1 on the spawn-failure path. #handle is nulled before spawn() returns, so the stdio getters short-circuit to destroyed streams without incrementing, and the IPC increment only runs after Bun.spawn() succeeds.

[robobun]

Rebased onto main (8209c7e). The only conflict was the two import lines in child_process.test.ts: main added fork to the node:child_process import, this branch added tempDir to the harness import. Kept both.

The src/ diff is byte-identical to before the rebase. The #closesNeeded invariant still holds on current main: the only #closesNeeded++ is still behind the if (!value) guard in #getBunSpawnIo's case 1/2, and #33707's change to the stdin case didn't add one.

# pass-after (debug build, rebased):
$ bun bd test child_process.test.ts -t "failure lifecycle"
 4 pass  0 fail
$ bun bd test child_process.test.ts -t "output has"
 2 pass  0 fail
$ bun bd test child_process.test.ts -t ERR_INVALID_ARG_VALUE
 2 pass  0 fail

# fail-before (released bun):
$ USE_SYSTEM_BUN=1 bun test child_process.test.ts -t "failure lifecycle"
 0 pass  4 fail

# ported node tests:
$ for f in test-child-process-spawn-error.js test-child-process-spawnsync.js \
           test-child-process-exec-error.js test-child-process-spawnsync-validation-errors.js \
           test-child-process-cwd.js; do bun-debug test/js/node/test/parallel/$f; done
 all pass

The same 3 container-environment failures in the full file (should allow us to set env, should allow us to spawn in the default shell, extra stdio pipes are not double-closed on GC) still reproduce with origin/main's src/js/node/child_process.ts and no diff applied.

[claude]

No bugs found. The change is small and well-tested, but routing the deferred spawn-error path through #maybeClose() depends on #closesNeeded staying at 1 when #handle is null — the author already flagged that as worth a maintainer glance, and I agree.

Extended reasoning...

Overview

Three Node.js compatibility fixes in src/js/node/child_process.ts, plus tests in test/js/node/child_process/child_process.test.ts:

1. The deferred spawn-error branch (ENOENT/EACCES/EAGAIN/EMFILE/ENFILE) now sets this.exitCode = errno and emits close via #maybeClose() so listeners receive (errno, null) instead of (errno, undefined), matching Node's _handle.onexit contract.
2. spawnSync().output is padded with null for every stdio slot past index 2, so output.length === stdio.length.
3. normalizeStdio() throws ERR_INVALID_ARG_VALUE instead of the retired ERR_INVALID_OPT_VALUE; the local helper is deleted.

Security risks

None. No auth, crypto, path handling, or untrusted-input parsing is touched. The changes only adjust event arguments, an output-array shape, and an error code on already-validated inputs.

Level of scrutiny

Moderate. child_process is widely used and the deferred-error path is exactly what process supervisors observe. Changes (2) and (3) are mechanical. Change (1) is small but relies on a non-local invariant: #maybeClose() only fires when #closesGot === #closesNeeded, so correctness depends on nothing bumping #closesNeeded when #handle is null. I traced it — the only increment is in #getBunSpawnIo case 1/2 after the if (!value) guard, which short-circuits when handle is null; the IPC += 1 is inside the successful-spawn try block. The new "close still fires after the stdio streams are materialized" tests encode that invariant for both default stdio and an extra pipe.

Other factors

• Fail-before / pass-after verified in the PR body and again after rebase.
• CI green on all lanes that run child_process tests; the 4 red lanes were shown to be unrelated (artifact-download timeout, a pty test, a node:net GC flake also failing on other branches).
• CodeRabbit's only nit (add spawnSync to the non-string/non-array stdio test) was applied; its outside-diff suggestion was correctly declined with reasoning.
• No CODEOWNERS entry for this path.

I'm not auto-approving because the author themselves called out the #maybeClose() routing as the one thing worth a second pair of eyes, and it's the kind of private-state coupling a maintainer should sign off on rather than a bot.

[robobun]

Closing as part of a cleanup of stale pull requests. This PR has had no new commits since 2026-07-10, it conflicts with main, and its last CI run failed. This is not a judgment on the fix itself. If the problem still reproduces on a current build, reopen this PR after a rebase or open a new one against main.