fix(scripts): forward run-with-env termination

This commit is contained in:
Vincent Koc
2026-06-01 22:24:33 +02:00
parent 50b69e16dc
commit c9d35c7172
4 changed files with 168 additions and 8 deletions

View File

@@ -1,4 +1,5 @@
import { spawn } from "node:child_process";
import { constants as osConstants } from "node:os";
import { buildCmdExeCommandLine } from "../windows-cmd-helpers.mjs";
const FORWARDED_SIGNALS = ["SIGINT", "SIGTERM", "SIGHUP"];
@@ -93,11 +94,17 @@ export async function runManagedCommand({
try {
return await new Promise((resolve, reject) => {
child.once("error", reject);
child.once("close", (status) => {
child.once("close", (status, signal) => {
if (managedChild.forceKillTimer) {
clearTimeout(managedChild.forceKillTimer);
}
resolve(managedChild.receivedSignal ? signalExitCode(managedChild.receivedSignal) : (status ?? 1));
resolve(
managedChild.receivedSignal
? signalExitCode(managedChild.receivedSignal)
: signal
? signalExitCode(signal)
: (status ?? 1),
);
});
});
} finally {
@@ -108,8 +115,8 @@ export async function runManagedCommand({
/**
* @param {{
* child: import("node:child_process").ChildProcess;
* forceKillTimer: NodeJS.Timeout | null;
* receivedSignal: NodeJS.Signals | null;
* forceKillTimer: ReturnType<typeof setTimeout> | null;
* receivedSignal: string | null;
* }} managedChild
*/
function addManagedChild(managedChild) {
@@ -120,8 +127,8 @@ function addManagedChild(managedChild) {
/**
* @param {{
* child: import("node:child_process").ChildProcess;
* forceKillTimer: NodeJS.Timeout | null;
* receivedSignal: NodeJS.Signals | null;
* forceKillTimer: ReturnType<typeof setTimeout> | null;
* receivedSignal: string | null;
* }} managedChild
*/
function removeManagedChild(managedChild) {
@@ -256,7 +263,7 @@ function signalNumberFor(signal) {
case "SIGTERM":
return 15;
default:
return 0;
return osConstants.signals?.[signal] ?? 0;
}
}

View File

@@ -73,8 +73,41 @@ function main(argv = process.argv.slice(2)) {
},
stdio: "inherit",
});
let forwardedSignal = null;
let forceKillTimer = null;
// Keep the child in the foreground process group so TTY signals such as
// Ctrl-C, Ctrl-Z, and window resizes stay native. SIGTERM is the direct
// wrapper-kill path CI and launchers use, so forward only that case.
const forwardedSignals = ["SIGTERM"];
const cleanupSignalHandlers = () => {
for (const signal of forwardedSignals) {
process.off(signal, signalHandlers.get(signal));
}
};
const signalHandlers = new Map(
forwardedSignals.map((signal) => [
signal,
() => {
forwardedSignal ??= signal;
child.kill(signal);
forceKillTimer ??= setTimeout(() => child.kill("SIGKILL"), 5_000);
},
]),
);
for (const [signal, handler] of signalHandlers) {
process.on(signal, handler);
}
child.on("exit", (code, signal) => {
cleanupSignalHandlers();
if (forceKillTimer) {
clearTimeout(forceKillTimer);
}
if (forwardedSignal) {
process.kill(process.pid, forwardedSignal);
return;
}
if (signal) {
process.kill(process.pid, signal);
return;
@@ -83,6 +116,10 @@ function main(argv = process.argv.slice(2)) {
});
child.on("error", (error) => {
cleanupSignalHandlers();
if (forceKillTimer) {
clearTimeout(forceKillTimer);
}
console.error(error);
process.exit(1);
});

View File

@@ -26,6 +26,7 @@ describe("managed-child-process", () => {
expect(signalExitCode("SIGHUP")).toBe(129);
expect(signalExitCode("SIGINT")).toBe(130);
expect(signalExitCode("SIGTERM")).toBe(143);
expect(signalExitCode("SIGKILL")).toBe(137);
});
it("wraps Windows shell argv through cmd.exe without Node shell mode", () => {

View File

@@ -1,4 +1,7 @@
import { spawnSync } from "node:child_process";
import { spawn, spawnSync, type ChildProcess } from "node:child_process";
import { existsSync, mkdtempSync, readFileSync, rmSync } from "node:fs";
import { tmpdir } from "node:os";
import path from "node:path";
import { describe, expect, it } from "vitest";
import {
isRunWithEnvHelpRequest,
@@ -6,6 +9,38 @@ import {
resolveSpawnCommand,
} from "../../scripts/run-with-env.mjs";
async function waitFor(predicate: () => boolean, label: string, timeoutMs = 3_000): Promise<void> {
const startedAt = Date.now();
while (!predicate()) {
if (Date.now() - startedAt > timeoutMs) {
throw new Error(`timed out waiting for ${label}`);
}
await new Promise<void>((resolve) => {
setTimeout(resolve, 25);
});
}
}
async function waitForExit(
child: ChildProcess,
timeoutMs = 3_000,
): Promise<{ code: number | null; signal: NodeJS.Signals | null }> {
return await new Promise((resolve, reject) => {
const timer = setTimeout(() => {
child.kill("SIGKILL");
reject(new Error("timed out waiting for child exit"));
}, timeoutMs);
child.once("exit", (code, signal) => {
clearTimeout(timer);
resolve({ code, signal });
});
child.once("error", (error) => {
clearTimeout(timer);
reject(error);
});
});
}
describe("run-with-env", () => {
it("parses leading env assignments before the command separator", () => {
expect(
@@ -78,4 +113,84 @@ describe("run-with-env", () => {
args: ["scripts/run-vitest.mjs"],
});
});
it.runIf(process.platform !== "win32")(
"forwards parent termination to the wrapped command",
async () => {
const tempDir = mkdtempSync(path.join(tmpdir(), "openclaw-run-with-env-signals-"));
const readyFile = path.join(tempDir, "ready");
const signaledFile = path.join(tempDir, "signaled");
const childScript = [
"const fs = require('node:fs');",
"process.on('SIGTERM', () => {",
" fs.writeFileSync(process.env.SIGNALED_FILE, 'SIGTERM');",
" setTimeout(() => process.exit(0), 25);",
"});",
"fs.writeFileSync(process.env.READY_FILE, 'ready');",
"setInterval(() => {}, 1000);",
].join("\n");
const wrapper = spawn(
process.execPath,
[
"scripts/run-with-env.mjs",
`READY_FILE=${readyFile}`,
`SIGNALED_FILE=${signaledFile}`,
"--",
"node",
"-e",
childScript,
],
{ cwd: process.cwd(), stdio: "ignore" },
);
try {
await waitFor(() => existsSync(readyFile), "wrapped command readiness");
wrapper.kill("SIGTERM");
const exit = await waitForExit(wrapper);
expect(exit).toEqual({ code: null, signal: "SIGTERM" });
expect(readFileSync(signaledFile, "utf8")).toBe("SIGTERM");
} finally {
wrapper.kill("SIGKILL");
rmSync(tempDir, { force: true, recursive: true });
}
},
);
it.runIf(process.platform !== "win32")("preserves wrapped command signal exits", () => {
const result = spawnSync(
process.execPath,
[
"scripts/run-with-env.mjs",
"OPENCLAW_RUN_WITH_ENV_SIGNAL_TEST=1",
"--",
"node",
"-e",
"process.kill(process.pid, 'SIGTERM')",
],
{ cwd: process.cwd(), encoding: "utf8" },
);
expect(result.status).toBeNull();
expect(result.signal).toBe("SIGTERM");
});
it.runIf(process.platform !== "win32")("preserves wrapped command force-kill exits", () => {
const result = spawnSync(
process.execPath,
[
"scripts/run-with-env.mjs",
"OPENCLAW_RUN_WITH_ENV_SIGNAL_TEST=1",
"--",
"node",
"-e",
"process.kill(process.pid, 'SIGKILL')",
],
{ cwd: process.cwd(), encoding: "utf8" },
);
expect(result.status).toBeNull();
expect(result.signal).toBe("SIGKILL");
});
});