From 662daabad243727bc66b9960f6ab3810228b66fa Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Fri, 13 Nov 2026 00:56:38 +0000 Subject: [PATCH] Forward repeated signals through the npm wrapper (#61221) ## Why The npm wrapper's spawn fallback stopped forwarding signals after the first one, preventing a second signal from interrupting a child's graceful shutdown. ## What changed Remove the `child.killed` guard so the wrapper continues forwarding `SIGINT`, `SIGTERM`, and `SIGHUP` until the child exits. ## Testing Add a regression test that forces the spawn fallback and verifies a running child receives a second `SIGTERM` and its numeric exit status is preserved. GitOrigin-RevId: be96e784e2e4f419c43a5897d264f55fcdfd625f --- codex-cli/bin/codex.js | 9 +--- codex-cli/tests/exit-status.test.mjs | 63 +++++++++++++++++++++------- 2 files changed, 50 insertions(+), 22 deletions(-) diff --git a/codex-cli/bin/codex.js b/codex-cli/bin/codex.js index 132509521e..df68e8ce65 100755 --- a/codex-cli/bin/codex.js +++ b/codex-cli/bin/codex.js @@ -258,14 +258,9 @@ child.on("error", (err) => { process.exit(1); }); -// Forward common termination signals to the child so that it shuts down -// gracefully. In the handler we temporarily disable the default behavior of -// exiting immediately; once the child has been signaled we simply wait for -// its exit event which will in turn terminate the parent (see below). +// Keep forwarding signals until the child exits so a second signal can +// interrupt a graceful shutdown. const forwardSignal = (signal) => { - if (child.killed) { - return; - } try { child.kill(signal); } catch { diff --git a/codex-cli/tests/exit-status.test.mjs b/codex-cli/tests/exit-status.test.mjs index 6eb265dc71..17a74a60c4 100644 --- a/codex-cli/tests/exit-status.test.mjs +++ b/codex-cli/tests/exit-status.test.mjs @@ -15,6 +15,50 @@ import path from "node:path"; import { createInterface } from "node:readline"; import test from "node:test"; +async function installWrapper(directory, executable) { + const arch = process.arch === "arm64" ? "aarch64" : "x86_64"; + const target = `${arch}-${process.platform === "darwin" ? "apple-darwin" : "unknown-linux-musl"}`; + const bin = path.join(directory, "vendor", target, "bin"); + await mkdir(bin, { recursive: true }); + await symlink(executable, path.join(bin, "codex")); + await mkdir(path.join(directory, "bin")); + await writeFile(path.join(directory, "package.json"), '{"type":"module"}'); + const wrapper = path.join(directory, "bin", "codex.js"); + await copyFile(new URL("../bin/codex.js", import.meta.url), wrapper); + return wrapper; +} + +test( + "the npm spawn fallback forwards a second SIGTERM to a running child", + { skip: process.platform === "win32" }, + async () => { + const directory = await mkdtemp(path.join(tmpdir(), "codex-npm-signal-")); + try { + const wrapper = await installWrapper(directory, process.execPath); + const preload = path.join(directory, "fallback.cjs"); + await writeFile(preload, "process.execve = undefined;"); + const child = spawn(process.execPath, [ + "--require", + preload, + wrapper, + "-e", + `let signals = 0; + process.on("SIGTERM", () => { + if (++signals === 2) process.exit(17); + console.log("received first SIGTERM"); + }); + setTimeout(() => process.exit(1), 5000); + console.log("ready");`, + ]); + const acknowledgements = createInterface({ input: child.stdout }); + acknowledgements.on("line", () => child.kill("SIGTERM")); + assert.deepEqual(await once(child, "exit"), [17, null]); + } finally { + await rm(directory, { recursive: true, force: true }); + } + }, +); + test( "the npm wrapper preserves numeric exits and child signals", { @@ -29,23 +73,12 @@ test( let child; let killTimer; try { - const arch = process.arch === "arm64" ? "aarch64" : "x86_64"; - const target = `${arch}-${process.platform === "darwin" ? "apple-darwin" : "unknown-linux-musl"}`; - const bin = path.join(directory, "vendor", target, "bin"); - const home = path.join(directory, "home"); - await mkdir(bin, { recursive: true }); - await mkdir(home); - await mkdir(path.join(directory, "bin")); - await writeFile( - path.join(directory, "package.json"), - '{"type":"module"}', - ); - const wrapper = path.join(directory, "bin", "codex.js"); - await copyFile(new URL("../bin/codex.js", import.meta.url), wrapper); - await symlink( + const wrapper = await installWrapper( + directory, path.resolve(process.env.CODEX_TEST_BINARY), - path.join(bin, "codex"), ); + const home = path.join(directory, "home"); + await mkdir(home); const env = { ...process.env, CODEX_HOME: home, CODEX_SQLITE_HOME: home }; // A normal CLI argument error must retain its numeric exit status.