[bartlomieju]

Summary

• When child_process.spawn fails with EMFILE/ENFILE, set stdin/stdout/stderr/stdio to undefined instead of creating dummy streams, matching Node.js behavior where the OS couldn't allocate pipe file descriptors.
• Handle compound shell commands (e.g. ulimit -n 64 && deno file.js) in transformDenoShellCommand. Previously only commands starting directly with the deno executable were transformed, missing cases where deno appears after shell operators like &&, ||, or ;.

Fixes the node compat test test-child-process-emfile.js.

Test plan

• ./x test-compat test-child-process-emfile.js passes

[fibibot]

LGTM. Both fixes line up with what the upstream test/sequential/test-child-process-emfile.js asserts:

1. EMFILE/ENFILE → undefined stdio: the test explicitly checks proc.stdin === undefined, proc.stdout === undefined, etc. Pre-PR Deno installed dummy Writable/Readable placeholders that would have failed those strict-equality checks. The PR's isResourceError branch sets all four to undefined (matching Node's lib/internal/child_process.js), and skipping the nextTick(...maybeClose...) block matches Node's behavior of not emitting 'exit' (the test's mustNotCall('exit')) — 'close' follows 'exit' in Node's flow, so it correctly doesn't fire either.

2. Compound shell command transformation: the test's child_process.execSync('ulimit -Hn') and ulimit -n 64 && "${process.execPath}" "${__filename}" patterns need the &&-after-ulimit form to be transformed. The regex /^(.*?)\s*(&&|\|\||;)\s*/ + recursion on the rest correctly turns ulimit -n 64 && deno file.js into ulimit -n 64 && <real-deno-path> file.js. The if (transformedRest !== rest) guard ensures we only emit a transformed version when something actually changed downstream.

One non-blocking observation inline.

[fibibot]

Non-blocking edge case: the regex doesn't understand quoted strings, so echo "foo && bar" && deno file.js would split at the first && (inside the quotes), with prefix echo "foo and rest bar" && deno file.js. Recursion on the rest still finds deno, so we'd produce echo "foo && bar" && <deno-path> file.js (the unmodified prefix is reassembled with a space, which is fine), but for inputs where the operator is truly inside a string and there's no real subsequent deno invocation, the transformedRest !== rest guard correctly returns the original command. So the unbroken path is safe; the worst case is we re-emit a quoted operator with extra whitespace. Worth at least adding a code-comment that this assumes &&/||/; outside quotes.

[fibibot]

Two-part fix. Both halves are needed for the upstream sequential/test-child-process-emfile.js to pass on Deno — the test re-spawns itself via shell as ulimit -n 64 && "${process.execPath}" "${__filename}" and then exhausts FDs to provoke EMFILE on a child spawn, so without both (a) compound-shell rewriting of deno after && and (b) EMFILE-aware stdio handling, the test wouldn't even start. Substance reads correctly. CI is 133/136 with no failures.

Three concerns inline (well, in body — none anchor to a single bug line, they're more about scope and testability):

1. The fixed test isn't enabled in tests/node_compat/config.jsonc

The PR body says Fixes the node compat test test-child-process-emfile.js and the test plan checks the ./x test-compat invocation, but sequential/test-child-process-emfile.js is not added to the config in this diff (only parallel/test-memory-usage-emfile.js already exists, and that's a different test). That means CI on this PR never actually runs the test the fix targets — it only runs the existing parallel suite that wasn't broken. Future regressions in either of these two changes won't be caught.

If there's a reason the test can't be enabled (Linux-only via RLIMIT_NOFILE? sequential-mode coverage gap? ulimit-walks-up behavior issue?), worth saying so in the PR description so reviewers know the test plan is local-only by design. Otherwise enabling it (with whatever requires/flaky/os annotation fits) would let CI confirm both halves of the fix.

2. (this as any).stdin = undefined × 4 sidesteps the class type contract

The ChildProcess class members are typed (presumably Writable | null for stdin, Readable | null for stdout/stderr, Array for stdio). The fix sets them to undefined via as any casts to satisfy Node's behavior on EMFILE. That works, but it leaves a type lie that future readers will hit. Two cleaner shapes:

• Update the member types to Writable | null | undefined etc. (Node docs explicitly allow both null and undefined for these properties), no casts needed.
• Or use null instead of undefined — Node's runtime returns null for stdio that wasn't set up at all in some paths, and null matches the existing Writable | null typing. The user-facing test for "is there a stdin" is usually if (proc.stdin) which treats both the same.

Not blocking; just worth a follow-up since the // deno-lint-ignore no-explicit-any × 4 reads as a workaround the type system is fighting.

3. Compound shell rewrite handles only one deno per command

The new branch in transformDenoShellCommand splits on the first &&/||/; and recurses on the rest. Recursion only transforms if the recursive call's command starts with deno. So:

• ulimit -n 64 && deno run foo ✓ (the EMFILE test's case — works as intended)
• echo hi && deno run foo && deno run bar — first match: prefix=echo hi, rest=deno run foo && deno run bar; recursion sees deno-leading rest, the existing path transforms only the first deno and leaves && deno run bar as args. Second deno invocation isn't rewritten and the child process won't find deno on PATH.
• false || deno run foo ✓

The EMFILE test doesn't trigger the multi-deno case, so this is fine for the stated scope. Worth knowing it exists as a follow-up if/when other tests rely on chained deno invocations through a shell. The regex comment about quoted strings is honest about another corner case (e.g. echo "x && y" && deno foo would mis-split on the operator inside the string).

[lunadogbot]

1. Fixes the node compat test test-child-process-emfile.js is still only covered by the manual test plan. The diff changes both ChildProcess.spawn EMFILE/ENFILE stdio semantics and transformDenoShellCommand, but tests/node_compat/config.jsonc does not add sequential/test-child-process-emfile.js (only the existing parallel/test-memory-usage-emfile.js is present). Please enable the compat test in CI, or add an equivalent regression test if that upstream test cannot run reliably.

[fibibot]

1. tests/node_compat/config.jsonc still does not enable sequential/test-child-process-emfile.js, so CI is green without running the compat test this PR is meant to fix. The diff changes both EMFILE/ENFILE stdio semantics and compound shell rewriting; please add that upstream test to the config, or add an equivalent regression test if the upstream test cannot run reliably.

addressed