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],