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
This commit is contained in:
Eric Traut
2026-11-13 01:21:09 +00:00
committed by copyberry
parent e1340f2ea8
commit 662daabad2
2 changed files with 50 additions and 22 deletions
+2 -7
View File
@@ -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 {
+48 -15
View File
@@ -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.