diff --git a/scripts/lib/managed-child-process.mjs b/scripts/lib/managed-child-process.mjs index 9efc998cee0b..43ea4b71ec8f 100644 --- a/scripts/lib/managed-child-process.mjs +++ b/scripts/lib/managed-child-process.mjs @@ -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 | 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 | 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; } } diff --git a/scripts/run-with-env.mjs b/scripts/run-with-env.mjs index b34ed2d21c47..308a1cd47a7c 100644 --- a/scripts/run-with-env.mjs +++ b/scripts/run-with-env.mjs @@ -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); }); diff --git a/test/scripts/managed-child-process.test.ts b/test/scripts/managed-child-process.test.ts index f39c26322e74..b1fb3680e465 100644 --- a/test/scripts/managed-child-process.test.ts +++ b/test/scripts/managed-child-process.test.ts @@ -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", () => { diff --git a/test/scripts/run-with-env.test.ts b/test/scripts/run-with-env.test.ts index da999319fea3..06ab7730f304 100644 --- a/test/scripts/run-with-env.test.ts +++ b/test/scripts/run-with-env.test.ts @@ -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 { + const startedAt = Date.now(); + while (!predicate()) { + if (Date.now() - startedAt > timeoutMs) { + throw new Error(`timed out waiting for ${label}`); + } + await new Promise((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"); + }); });