From db085b7d8ff91dfc2987c88fee0248a72e6c4b56 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 26 Aug 2026 11:43:39 +0800 Subject: [PATCH] fix(tooling): arm prompt cleanup before spawn (#129755) --- scripts/anthropic-prompt-probe.ts | 196 ++++++++++++------------ test/scripts/dev-tooling-safety.test.ts | 12 +- 2 files changed, 108 insertions(+), 100 deletions(-) diff --git a/scripts/anthropic-prompt-probe.ts b/scripts/anthropic-prompt-probe.ts index ca6e5519bdb9..23379545c3b8 100644 --- a/scripts/anthropic-prompt-probe.ts +++ b/scripts/anthropic-prompt-probe.ts @@ -21,6 +21,11 @@ import { parseStrictIntegerOption, redactForDevToolLog, } from "./lib/dev-tooling-safety.ts"; +import { + inspectManagedProcessGroup, + signalExitCode, + terminateManagedChild, +} from "./lib/managed-child-process.mts"; const TRANSPORT = process.env.OPENCLAW_PROMPT_TRANSPORT?.trim() === "direct" ? "direct" : "gateway"; const GATEWAY_PROMPT_MODE = "extra"; @@ -56,11 +61,6 @@ const GATEWAY_TIMEOUT_MS = parseStrictIntegerOption({ min: 1, raw: process.env.OPENCLAW_PROMPT_GATEWAY_TIMEOUT_MS, }); -const GATEWAY_PARENT_SIGNAL_EXIT_CODES = new Map([ - ["SIGHUP", 129], - ["SIGINT", 130], - ["SIGTERM", 143], -]); const CAPTURE_PROXY_MAX_BODY_BYTES = parseStrictIntegerOption({ fallback: 2 * 1024 * 1024, label: "OPENCLAW_PROMPT_CAPTURE_MAX_BODY_BYTES", @@ -497,6 +497,7 @@ async function runDirectPrompt( timeoutMs, }) : undefined; + const parentSignalController = createPromptProbeParentSignalController(); try { const stdout: string[] = []; @@ -524,18 +525,23 @@ async function runDirectPrompt( child.once("exit", (code, signal) => resolve({ code, signal })); }, ); - const stopDirectChild = async (signal: NodeJS.Signals = "SIGKILL") => { - signalGatewayPromptChildTree(child, signal); - await waitForGatewayPromptChildTreeExit( - child, - exitPromise.then(() => undefined), - options.shutdownWaitMs ?? 1_500, - ); + let stopPromise: Promise | undefined; + const stopDirectChild = (signal: NodeJS.Signals = "SIGKILL") => { + if (!stopPromise) { + terminateManagedChild(child, signal, { useWindowsTaskkill: false }); + stopPromise = waitForGatewayPromptChildTreeExit( + child, + options.shutdownWaitMs ?? 1_500, + ).then(() => undefined); + } + return stopPromise; }; - const removeParentSignalHandlers = installGatewayPromptParentSignalHandlers( - child, - stopDirectChild, - ); + parentSignalController.attach({ + stop: stopDirectChild, + forceKill: () => { + terminateManagedChild(child, "SIGKILL", { useWindowsTaskkill: false }); + }, + }); let timeoutTimer: ReturnType | undefined; const exit = await Promise.race([ exitPromise, @@ -550,7 +556,6 @@ async function runDirectPrompt( if (timeoutTimer) { clearTimeout(timeoutTimer); } - removeParentSignalHandlers(); }); const joinedStdout = stdout.join(""); const joinedStderr = stderr.join(""); @@ -567,6 +572,7 @@ async function runDirectPrompt( ...promptProbeTmpResult(tmpDir), }; } finally { + parentSignalController.dispose(); await proxy?.stop().catch(() => {}); await cleanupPromptProbeTmpDir(tmpDir).catch(() => {}); } @@ -582,6 +588,7 @@ async function startGatewayProcess(params: { logPath: string; }) { const logFile = await fs.open(params.logPath, "a"); + const parentSignalController = createPromptProbeParentSignalController(); const child = spawn( NODE_BIN, ["openclaw.mjs", "gateway", "--port", String(params.port), "--bind", "loopback", "--force"], @@ -625,7 +632,6 @@ async function startGatewayProcess(params: { child.stdout.on("data", trackLogWrite); child.stderr.on("data", trackLogWrite); let stopPromise: Promise | undefined; - let removeParentSignalHandlers = () => {}; const stopOnce = async (): Promise => { stopPromise ??= stopGatewayPromptChild( child, @@ -635,11 +641,16 @@ async function startGatewayProcess(params: { pendingLogWrites, logWriteErrors, ).finally(() => { - removeParentSignalHandlers(); + parentSignalController.dispose(); }); return await stopPromise; }; - removeParentSignalHandlers = installGatewayPromptParentSignalHandlers(child, stopOnce); + parentSignalController.attach({ + stop: stopOnce, + forceKill: () => { + terminateManagedChild(child, "SIGKILL", { useWindowsTaskkill: false }); + }, + }); return { async stop(): Promise { return await stopOnce(); @@ -656,25 +667,18 @@ async function stopGatewayPromptChild( logWriteErrors: readonly unknown[] = [], ): Promise { let exited = child.exitCode !== null || child.signalCode !== null; - const exitPromise = exited - ? Promise.resolve() - : new Promise((resolve) => { - child.once("exit", () => { - exited = true; - resolve(); - }); - }); if (!exited) { - signalGatewayPromptChildTree(child, "SIGINT"); + child.once("exit", () => { + exited = true; + }); } - const exitedAfterSigint = await waitForGatewayPromptChildTreeExit( - child, - exitPromise, - sigintTimeoutMs, - ); + if (!exited) { + terminateManagedChild(child, "SIGINT", { useWindowsTaskkill: false }); + } + const exitedAfterSigint = await waitForGatewayPromptChildTreeExit(child, sigintTimeoutMs); if (!exitedAfterSigint) { - signalGatewayPromptChildTree(child, "SIGKILL"); - await waitForGatewayPromptChildTreeExit(child, exitPromise, sigkillTimeoutMs); + terminateManagedChild(child, "SIGKILL", { useWindowsTaskkill: false }); + await waitForGatewayPromptChildTreeExit(child, sigkillTimeoutMs); } const failedLogWrite = (await Promise.allSettled(pendingLogWrites)).find( (result): result is PromiseRejectedResult => result.status === "rejected", @@ -687,91 +691,87 @@ async function stopGatewayPromptChild( return exited; } -function installGatewayPromptParentSignalHandlers( - child: StoppableGatewayChild, - stopGateway: () => Promise, -): () => void { - let parentSignalShutdownStarted = false; +// Arm before spawn so a parent signal is retained until child cleanup can attach. +// The first signal owns the exit code; later signals only escalate cleanup. +function createPromptProbeParentSignalController() { + let attachment: { forceKill(): void; stop(): Promise } | undefined; + let forceKillPending = false; + let receivedSignal: NodeJS.Signals | undefined; + let shutdownPromise: Promise | undefined; const handlers = new Map void>(); - const removeHandlers = () => { + const dispose = () => { for (const [signal, handler] of handlers) { process.off(signal, handler); } handlers.clear(); }; - for (const signal of GATEWAY_PARENT_SIGNAL_EXIT_CODES.keys()) { + const forceKill = () => { + try { + attachment?.forceKill(); + } catch {} + }; + const startShutdown = () => { + if (!attachment || !receivedSignal || shutdownPromise) { + return; + } + const attached = attachment; + const exitCode = signalExitCode(receivedSignal); + if (forceKillPending) { + forceKillPending = false; + forceKill(); + } + shutdownPromise = Promise.resolve() + .then(() => attached.stop()) + .catch(() => undefined) + .finally(() => { + dispose(); + process.exit(exitCode); + }); + }; + for (const signal of ["SIGHUP", "SIGINT", "SIGTERM"] satisfies NodeJS.Signals[]) { const handler = () => { - if (parentSignalShutdownStarted) { - signalGatewayPromptChildTree(child, "SIGKILL"); + if (receivedSignal) { + if (attachment) { + forceKill(); + } else { + forceKillPending = true; + } return; } - parentSignalShutdownStarted = true; - void stopGateway() - .catch(() => undefined) - .finally(() => { - removeHandlers(); - process.exit(GATEWAY_PARENT_SIGNAL_EXIT_CODES.get(signal) ?? 1); - }); + receivedSignal = signal; + startShutdown(); }; handlers.set(signal, handler); process.on(signal, handler); } - return removeHandlers; + return { + attach(nextAttachment: NonNullable) { + attachment ??= nextAttachment; + startShutdown(); + }, + dispose, + }; } async function waitForGatewayPromptChildTreeExit( child: StoppableGatewayChild, - exitPromise: Promise, timeoutMs: number, ): Promise { - let leaderExited = child.exitCode !== null || child.signalCode !== null; - const trackedExit = exitPromise.then(() => { - leaderExited = true; - }); const deadline = Date.now() + timeoutMs; + const groupOptions = { + errorPolicy: "alive-on-eperm", + inspectLeaderWhenNoGroup: true, + } as const; + const childTreeExited = () => + (child.exitCode !== null || child.signalCode !== null) && + inspectManagedProcessGroup(child, groupOptions) !== "live"; while (Date.now() < deadline) { - if (leaderExited && !gatewayPromptChildTreeIsAlive(child)) { + if (childTreeExited()) { return true; } - const waitMs = Math.min(50, Math.max(0, deadline - Date.now())); - if (leaderExited) { - await sleep(waitMs); - } else { - await Promise.race([trackedExit, sleep(waitMs)]); - } + await sleep(Math.min(25, Math.max(0, deadline - Date.now()))); } - return leaderExited && !gatewayPromptChildTreeIsAlive(child); -} - -function signalGatewayPromptChildTree( - child: StoppableGatewayChild, - signal: NodeJS.Signals, -): boolean { - if (process.platform !== "win32" && typeof child.pid === "number") { - try { - process.kill(-child.pid, signal); - return true; - } catch { - return child.kill(signal); - } - } - return child.kill(signal); -} - -function gatewayPromptChildTreeIsAlive(child: StoppableGatewayChild): boolean { - if (process.platform === "win32" || typeof child.pid !== "number") { - return false; - } - try { - process.kill(-child.pid, 0); - return true; - } catch (error) { - return !isMissingProcessError(error); - } -} - -function isMissingProcessError(error: unknown): boolean { - return typeof error === "object" && error !== null && "code" in error && error.code === "ESRCH"; + return childTreeExited(); } async function waitForGatewayReady(url: string, token: string): Promise { @@ -1006,7 +1006,7 @@ async function main() { export const testing = { cleanupPromptProbeTmpDir, - installGatewayPromptParentSignalHandlers, + createPromptProbeParentSignalController, promptProbeTmpResult, readLogTail, readRequestBody, diff --git a/test/scripts/dev-tooling-safety.test.ts b/test/scripts/dev-tooling-safety.test.ts index 05f0458c0c49..0428d3925cdd 100644 --- a/test/scripts/dev-tooling-safety.test.ts +++ b/test/scripts/dev-tooling-safety.test.ts @@ -940,7 +940,7 @@ describe("script-specific dev tooling hardening", () => { ); it.runIf(process.platform !== "win32")( - "cleans Anthropic prompt gateway descendants on parent signal", + "cleans Anthropic prompt gateway descendants when the child attaches after parent signal", async () => { const tempRoot = await fs.mkdtemp(path.join(os.tmpdir(), "openclaw-prompt-parent-signal-")); tempDirs.push(tempRoot); @@ -972,13 +972,21 @@ describe("script-specific dev tooling hardening", () => { `const { testing } = await import(${JSON.stringify( pathToFileURL(path.resolve("scripts/anthropic-prompt-probe.ts")).href, )});`, + "const signalController = testing.createPromptProbeParentSignalController();", `const child = childProcess.spawn(process.execPath, ['--input-type=module', '--eval', ${JSON.stringify(leaderScript)}], { detached: true, stdio: 'ignore' });`, "let stopPromise;", "const stopGateway = () => {", " stopPromise ??= testing.stopGatewayPromptChild(child, { close: async () => {} }, 50, 100);", " return stopPromise;", "};", - "testing.installGatewayPromptParentSignalHandlers(child, stopGateway);", + "process.on('SIGTERM', () => {", + " signalController.attach({", + " stop: stopGateway,", + " forceKill: () => {", + " try { process.kill(-child.pid, 'SIGKILL'); } catch { child.kill('SIGKILL'); }", + " },", + " });", + "});", `fs.writeFileSync(${JSON.stringify(readyPath)}, String(process.pid));`, "setInterval(() => {}, 1000);", ].join("\n"),