diff --git a/CHANGELOG.md b/CHANGELOG.md index d6aeb8ef769..e2f6d7e06ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,6 +54,7 @@ Docs: https://docs.openclaw.ai - Release/CI/E2E: bound release candidate GitHub API calls so stalled network requests cannot wedge workflow and artifact polling. - Release/CI/E2E: bound Discord smoke API calls in cross-OS release checks so host-side round trips cannot hang on stalled fetches. - Release/CI/E2E: bound RPC RTT gateway readiness probes so a half-open local HTTP response cannot stall cleanup past the readiness deadline. +- Scripts/UI: stop descendant processes from wrapped non-interactive commands when `run-with-env` receives shutdown signals. - Control UI: lazy-load the usage view so the initial app bundle stays below the chunk warning threshold. - Build: keep Baileys optional image backends external so source builds do not warn about missing `jimp` or `sharp`. - Build: render independent CLI startup metadata help snapshots concurrently to cut cold build-all metadata time. diff --git a/scripts/run-with-env.mjs b/scripts/run-with-env.mjs index 15b309b3055..b78acb0c10f 100644 --- a/scripts/run-with-env.mjs +++ b/scripts/run-with-env.mjs @@ -66,19 +66,80 @@ function main(argv = process.argv.slice(2)) { } const spawnCommand = resolveSpawnCommand(parsed.command, parsed.args); + const useChildProcessGroup = process.platform !== "win32" && !process.stdin.isTTY; const child = spawn(spawnCommand.command, spawnCommand.args, { + detached: useChildProcessGroup, env: { ...process.env, ...parsed.env, }, stdio: "inherit", }); + const forceKillDelayMs = Math.max( + 1, + Number.parseInt(process.env.OPENCLAW_RUN_WITH_ENV_FORCE_KILL_MS ?? "5000", 10) || 5_000, + ); 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. Forward direct wrapper // shutdown signals that would otherwise only kill this small parent process. - const forwardedSignals = ["SIGTERM", "SIGHUP"]; + const forwardedSignals = useChildProcessGroup + ? ["SIGTERM", "SIGHUP", "SIGINT"] + : ["SIGTERM", "SIGHUP"]; + const signalChild = (signal) => { + if (useChildProcessGroup && typeof child.pid === "number") { + try { + process.kill(-child.pid, signal); + return; + } catch (error) { + if (error?.code !== "ESRCH") { + child.kill(signal); + return; + } + } + } + child.kill(signal); + }; + const childProcessGroupAlive = () => { + if (!useChildProcessGroup || typeof child.pid !== "number") { + return false; + } + try { + process.kill(-child.pid, 0); + return true; + } catch { + return false; + } + }; + const exitWithForwardedSignal = () => { + if (!forwardedSignal) { + return; + } + const finish = () => { + if (forceKillTimer) { + clearTimeout(forceKillTimer); + } + process.kill(process.pid, forwardedSignal); + }; + if (!childProcessGroupAlive()) { + finish(); + return; + } + const deadline = Date.now() + forceKillDelayMs; + const drainTimer = setInterval(() => { + if (!childProcessGroupAlive()) { + clearInterval(drainTimer); + finish(); + return; + } + if (Date.now() >= deadline) { + clearInterval(drainTimer); + signalChild("SIGKILL"); + finish(); + } + }, 50); + }; const cleanupSignalHandlers = () => { for (const signal of forwardedSignals) { @@ -90,8 +151,8 @@ function main(argv = process.argv.slice(2)) { signal, () => { forwardedSignal ??= signal; - child.kill(signal); - forceKillTimer ??= setTimeout(() => child.kill("SIGKILL"), 5_000); + signalChild(signal); + forceKillTimer ??= setTimeout(() => signalChild("SIGKILL"), forceKillDelayMs); }, ]), ); @@ -101,13 +162,13 @@ function main(argv = process.argv.slice(2)) { child.on("exit", (code, signal) => { cleanupSignalHandlers(); + if (forwardedSignal) { + exitWithForwardedSignal(); + return; + } if (forceKillTimer) { clearTimeout(forceKillTimer); } - if (forwardedSignal) { - process.kill(process.pid, forwardedSignal); - return; - } if (signal) { process.kill(process.pid, signal); return; diff --git a/test/scripts/run-with-env.test.ts b/test/scripts/run-with-env.test.ts index 9a922402a13..e3f4cc4d99a 100644 --- a/test/scripts/run-with-env.test.ts +++ b/test/scripts/run-with-env.test.ts @@ -41,6 +41,15 @@ async function waitForExit( }); } +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + describe("run-with-env", () => { it("parses leading env assignments before the command separator", () => { expect( @@ -114,13 +123,13 @@ describe("run-with-env", () => { }); }); - it.runIf(process.platform !== "win32").each(["SIGTERM", "SIGHUP"] as const)( + it.runIf(process.platform !== "win32").each(["SIGTERM", "SIGHUP", "SIGINT"] as const)( "forwards parent %s to the wrapped command", async (signal) => { const tempDir = mkdtempSync(path.join(tmpdir(), "openclaw-run-with-env-signals-")); const readyFile = path.join(tempDir, "ready"); const signaledFile = path.join(tempDir, "signaled"); - const handlerLines = ["SIGTERM", "SIGHUP"].flatMap((handledSignal) => [ + const handlerLines = ["SIGTERM", "SIGHUP", "SIGINT"].flatMap((handledSignal) => [ `process.on('${handledSignal}', () => {`, ` fs.writeFileSync(process.env.SIGNALED_FILE, '${handledSignal}');`, " setTimeout(() => process.exit(0), 25);", @@ -161,6 +170,140 @@ describe("run-with-env", () => { }, ); + it.runIf(process.platform !== "win32")( + "cleans up wrapped command descendants on wrapper shutdown", + async () => { + const tempDir = mkdtempSync(path.join(tmpdir(), "openclaw-run-with-env-descendants-")); + const readyFile = path.join(tempDir, "ready"); + const grandchildReadyFile = path.join(tempDir, "grandchild-ready"); + const grandchildPidFile = path.join(tempDir, "grandchild-pid"); + const grandchildScript = [ + "const fs = require('node:fs');", + "process.on('SIGTERM', () => {});", + "process.on('SIGHUP', () => {});", + "fs.writeFileSync(process.env.GRANDCHILD_READY_FILE, 'ready');", + "setInterval(() => {}, 1000);", + ].join("\n"); + const childScript = [ + "const { spawn } = require('node:child_process');", + "const fs = require('node:fs');", + `const grandchild = spawn(process.execPath, ['-e', ${JSON.stringify(grandchildScript)}], { stdio: 'ignore' });`, + "fs.writeFileSync(process.env.GRANDCHILD_PID_FILE, String(grandchild.pid));", + "fs.writeFileSync(process.env.READY_FILE, 'ready');", + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join("\n"); + const wrapper = spawn( + process.execPath, + [ + "scripts/run-with-env.mjs", + `READY_FILE=${readyFile}`, + `GRANDCHILD_READY_FILE=${grandchildReadyFile}`, + `GRANDCHILD_PID_FILE=${grandchildPidFile}`, + "--", + "node", + "-e", + childScript, + ], + { + cwd: process.cwd(), + env: { ...process.env, OPENCLAW_RUN_WITH_ENV_FORCE_KILL_MS: "200" }, + stdio: "ignore", + }, + ); + let grandchildPid = 0; + + try { + await waitFor(() => existsSync(readyFile), "wrapped command readiness"); + await waitFor( + () => existsSync(grandchildReadyFile), + "wrapped command descendant readiness", + ); + grandchildPid = Number(readFileSync(grandchildPidFile, "utf8")); + expect(grandchildPid).toBeGreaterThan(0); + expect(isProcessAlive(grandchildPid)).toBe(true); + + wrapper.kill("SIGTERM"); + const exit = await waitForExit(wrapper, 3_000); + expect(exit).toEqual({ code: null, signal: "SIGTERM" }); + await waitFor( + () => !isProcessAlive(grandchildPid), + "wrapped command descendant cleanup", + 5_000, + ); + } finally { + wrapper.kill("SIGKILL"); + if (grandchildPid > 0 && isProcessAlive(grandchildPid)) { + process.kill(grandchildPid, "SIGKILL"); + } + rmSync(tempDir, { force: true, recursive: true }); + } + }, + ); + + it.runIf(process.platform !== "win32")( + "lets wrapped command descendants finish during the shutdown grace period", + async () => { + const tempDir = mkdtempSync(path.join(tmpdir(), "openclaw-run-with-env-grace-")); + const readyFile = path.join(tempDir, "ready"); + const gracefulFile = path.join(tempDir, "graceful"); + const grandchildReadyFile = path.join(tempDir, "grandchild-ready"); + const grandchildScript = [ + "const fs = require('node:fs');", + "fs.writeFileSync(process.env.GRANDCHILD_READY_FILE, 'ready');", + "process.on('SIGTERM', () => {", + " setTimeout(() => {", + " fs.writeFileSync(process.env.GRACEFUL_FILE, 'done');", + " process.exit(0);", + " }, 75);", + "});", + "setInterval(() => {}, 1000);", + ].join("\n"); + const childScript = [ + "const { spawn } = require('node:child_process');", + "const fs = require('node:fs');", + `spawn(process.execPath, ['-e', ${JSON.stringify(grandchildScript)}], { stdio: 'ignore' });`, + "fs.writeFileSync(process.env.READY_FILE, 'ready');", + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join("\n"); + const wrapper = spawn( + process.execPath, + [ + "scripts/run-with-env.mjs", + `READY_FILE=${readyFile}`, + `GRACEFUL_FILE=${gracefulFile}`, + `GRANDCHILD_READY_FILE=${grandchildReadyFile}`, + "--", + "node", + "-e", + childScript, + ], + { + cwd: process.cwd(), + env: { ...process.env, OPENCLAW_RUN_WITH_ENV_FORCE_KILL_MS: "1000" }, + stdio: "ignore", + }, + ); + + try { + await waitFor(() => existsSync(readyFile), "wrapped command readiness"); + await waitFor( + () => existsSync(grandchildReadyFile), + "wrapped command descendant readiness", + ); + wrapper.kill("SIGTERM"); + + const exit = await waitForExit(wrapper, 3_000); + expect(exit).toEqual({ code: null, signal: "SIGTERM" }); + expect(readFileSync(gracefulFile, "utf8")).toBe("done"); + } 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,