fix(node): preserve stdio fds for sync child processes

[Tiancheng-Xu]

Summary

• Preserve file descriptors exposed directly by Deno's process stdio streams when converting Node.js child-process stdio options.
• Avoid calling pause() on writable process stdout/stderr streams.
• Run piped stdin as CommonJS when process.execPath is spawned without Node arguments, matching Node's no-argument stdin semantics.
• Add a regression test for spawnSync() with process stdio streams.

Fixes #36834.

User impact and root cause

Passing process.stdout or process.stderr to spawnSync() previously lost the underlying file descriptor. Preserving it exposed a separate compatibility issue: Deno translated a no-argument Node invocation to deno run -A -, which interpreted piped stdin as ESM and failed on CommonJS require(). The follow-up keeps the stdio fix and adds --ext=cjs only for that no-argument stdin path.

Testing

Current head:

• ./x test-node child_process_test (passed)
• focused reproduction of spawnSync(process.execPath, [], { input, stdio: [...] }) (exit status 0)
• ./x build (passed with a temporary local Apple-linker configuration workaround)
• direct rustfmt --check for ext/node/ops/node_cli_parser.rs (passed)
• git diff --check for the follow-up change (passed)

Earlier unchanged validation:

• ./x lint-js
• deno x --yes npm:pagefind --help (exits 0 and prints help output)

The repository-wide format check was attempted, but the local environment could not start the Rust formatter process. Upstream CI for the new head is pending.

AI tools assisted with implementation and review. I verified the final changes and the validation results reported above.

[Copilot]

🔵 Needs a closer look

It modifies Node.js compatibility process-spawning stdio handling, which warrants manual validation across platforms and stdio configurations beyond what can be proven from the diff alone.

Pull request overview

This PR fixes Deno’s Node.js child_process stdio option conversion so that when callers pass Deno’s process.stdin/stdout/stderr streams (which expose their fd via stream.fd), the child process inherits those OS file descriptors instead of falling back to JS-level piping. It also avoids calling pause() on writable stdio streams, and adds a regression test covering spawnSync() with process stdio streams.

Changes:

• Teach streamHandleFd() to read stream.fd (in addition to _handle.fd) so Deno’s process stdio streams can be passed through as real fds.
• Guard pause() / readStop() calls so writable stdio streams don’t throw when used in stdio conversion.
• Add a unit regression test asserting spawnSync() returns null stdout/stderr when using process stdio streams (i.e., they’re inherited, not piped).

File summaries

File	Description
ext/node/polyfills/internal/child_process.ts	Preserves fds from Deno’s process stdio streams during stdio conversion and avoids calling pause() on non-readable streams.
tests/unit_node/child_process_test.ts	Adds a regression test ensuring spawnSync() inherits process stdio streams (so stdout/stderr are null).

Review details

• Files reviewed: 2/2 changed files
• Comments generated: 0
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[Tiancheng-Xu]

The completed CI run has seven specs jobs failing on the same unrelated check_node_builtin_modules snapshot mismatch (the actual readFileSync diagnostic uses ReadFileSyncOptions, while the expected output was updated by #36848). The failures span debug platforms plus release Linux x86_64; 125 other jobs passed, and this PR does not touch the Node type declarations or those snapshots. I cannot rerun upstream workflows with contributor permissions. Could a maintainer rerun the failed jobs to refresh the affected build caches?