From 27ace641fe071106a036c298d76d7372ec1088cb Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 27 Oct 2026 16:48:29 +0000 Subject: [PATCH] Replace the npm wrapper with the native binary using `execve` (#57513) ## Why Replacing the Node process lets the native binary receive signals and report its termination directly. ## What changed - Use `process.execve` on non-Windows platforms when available, preserving CLI arguments and the prepared environment. - Install a `SIGXFSZ` handler before replacement so the native binary gets the default signal disposition instead of inheriting Node's ignored signal. - Retain asynchronous spawning for Windows and older Node versions. ## Testing Extend the exit-status test to check that `execve` leaves no child process and that `SIGHUP` and `SIGXFSZ` terminate it with the original signal, while retaining coverage for the spawn fallback. GitOrigin-RevId: 7437f15af419fce5b10c1c00425c7d38160991fc --- codex-cli/bin/codex.js | 17 +++++++++------ codex-cli/tests/exit-status.test.mjs | 31 ++++++++++++++-------------- 2 files changed, 27 insertions(+), 21 deletions(-) diff --git a/codex-cli/bin/codex.js b/codex-cli/bin/codex.js index 9a58834de7..132509521e 100755 --- a/codex-cli/bin/codex.js +++ b/codex-cli/bin/codex.js @@ -112,12 +112,6 @@ function findCodexExecutable() { const binaryPath = findCodexExecutable(); -// Use an asynchronous spawn instead of spawnSync so that Node is able to -// respond to signals (e.g. Ctrl-C / SIGINT) while the native binary is -// executing. This allows us to forward those signals to the child process -// and guarantees that when either the child terminates or the parent -// receives a fatal signal, both processes exit in a predictable manner. - function isPnpmOwnedCodexInstall(nodeModulesDir) { if (!existsSync(path.join(nodeModulesDir, ".modules.yaml"))) { return false; @@ -239,6 +233,17 @@ delete env.CODEX_MANAGED_BY_PNPM; delete env.CODEX_MANAGED_BY_VITE_PLUS; env[packageManagerEnvVar] = "1"; +// Replace Node so the native binary receives signals and exits directly. +// Node exposes execve on Windows too, but calling it there is unsupported. +if (process.platform !== "win32" && typeof process.execve === "function") { + // Node ignores SIGXFSZ, and ignored signals survive execve. Installing a + // handler lets execve restore the default disposition in the native binary. + process.on("SIGXFSZ", () => {}); + process.execve(binaryPath, [binaryPath, ...process.argv.slice(2)], env); +} + +// Older Node versions and Windows need a child process. Spawn asynchronously +// so Node can forward signals while the native binary is executing. const child = spawn(binaryPath, process.argv.slice(2), { stdio: "inherit", env, diff --git a/codex-cli/tests/exit-status.test.mjs b/codex-cli/tests/exit-status.test.mjs index dd598034a1..6eb265dc71 100644 --- a/codex-cli/tests/exit-status.test.mjs +++ b/codex-cli/tests/exit-status.test.mjs @@ -81,25 +81,26 @@ test( } } assert.ok(prompted, "migration must reach its confirmation prompt"); - if (signal === "SIGHUP") { - // The wrapper must forward the signal and retain signal termination. + const processes = execFileSync("ps", ["-axo", "pid=,ppid="], { + encoding: "utf8", + }); + const children = processes + .trim() + .split("\n") + .map((line) => line.trim().split(/\s+/).map(Number)) + .filter(([, parent]) => parent === child.pid); + const usesExecve = typeof process.execve === "function"; + assert.equal( + children.length, + usesExecve ? 0 : 1, + "execve must replace the wrapper; spawn must create one native child", + ); + if (usesExecve || signal === "SIGHUP") { + // Signal the native process directly, or let the wrapper forward it. child.kill(signal); assert.deepEqual(await exited, [null, signal]); } else { // A native child can die from a signal that Node ignores by default. - const processes = execFileSync("ps", ["-axo", "pid=,ppid="], { - encoding: "utf8", - }); - const children = processes - .trim() - .split("\n") - .map((line) => line.trim().split(/\s+/).map(Number)) - .filter(([, parent]) => parent === child.pid); - assert.equal( - children.length, - 1, - "wrapper must have one native child", - ); process.kill(children[0][0], signal); assert.deepEqual(await exited, [ 128 + constants.signals[signal],