From 8ea6b5d5b2c39854fa34eb70e83e891b80bdf449 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 2 Jun 2026 12:15:48 +0200 Subject: [PATCH] fix(scripts): clean package-boundary prep process groups --- CHANGELOG.md | 1 + ...e-extension-package-boundary-artifacts.mjs | 105 +++++++++- ...tension-package-boundary-artifacts.test.ts | 184 ++++++++++++++++++ 3 files changed, 284 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b810ba7fdde..f62c94dab472 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,7 @@ Docs: https://docs.openclaw.ai - Release/CI/E2E: fail early when Crabbox sparse-sync full checkouts do not have enough local disk, with guidance for moving the sync root. - Build: render independent CLI startup metadata help snapshots concurrently to cut cold build-all metadata time. +- Plugins: stop timed-out package-boundary prep steps by process group so descendant TypeScript/helper processes do not survive local check cleanup. - Control UI: serve static assets asynchronously after safe-open checks so large UI files do not block Gateway request handling. - Scripts/UI: forward direct wrapper SIGHUP shutdown to child processes so terminal hangups do not leave wrapped dev commands running. - Gateway: return the post-expiration pending-work revision from node drains so reconnecting nodes do not observe stale queue revisions after expired items are pruned. diff --git a/scripts/prepare-extension-package-boundary-artifacts.mjs b/scripts/prepare-extension-package-boundary-artifacts.mjs index 7cecc9551fef..8e5345fece94 100644 --- a/scripts/prepare-extension-package-boundary-artifacts.mjs +++ b/scripts/prepare-extension-package-boundary-artifacts.mjs @@ -12,6 +12,18 @@ const ROOT_SHIMS_MAX_OLD_SPACE_SIZE = process.env.OPENCLAW_ROOT_SHIMS_MAX_OLD_SPACE_SIZE?.trim() || "8192"; const ROOT_SHIMS_NODE_OPTIONS = `${process.env.NODE_OPTIONS ?? ""} --max-old-space-size=${ROOT_SHIMS_MAX_OLD_SPACE_SIZE}`.trim(); +const NODE_STEP_ABORT_KILL_GRACE_MS = 1_000; +const NODE_STEP_PARENT_SIGNALS = ["SIGHUP", "SIGINT", "SIGTERM"]; +const NODE_STEP_PARENT_SIGNAL_EXIT_CODES = new Map([ + ["SIGHUP", 129], + ["SIGINT", 130], + ["SIGTERM", 143], +]); +const ACTIVE_NODE_STEP_KILLERS = new Set(); +let nodeStepParentSignalForwardersInstalled = false; +let exitingAfterParentSignal = false; +let parentSignalExitCode = 1; +let parentSignalExitTimer; function listPackageDtsOutputsFromExports({ packageDir, outputPrefix }) { const packageJson = JSON.parse( @@ -356,31 +368,100 @@ function abortSiblingSteps(abortController) { } } +function signalNodeStep(child, signal) { + if (process.platform !== "win32" && typeof child.pid === "number") { + try { + process.kill(-child.pid, signal); + return; + } catch { + // The child process group can already be gone by the time cleanup runs. + } + } + child.kill(signal); +} + +function signalActiveNodeSteps(signal) { + for (const killNodeStep of ACTIVE_NODE_STEP_KILLERS) { + killNodeStep(signal); + } +} + +function installNodeStepParentSignalForwarders() { + if (nodeStepParentSignalForwardersInstalled) { + return; + } + nodeStepParentSignalForwardersInstalled = true; + for (const signal of NODE_STEP_PARENT_SIGNALS) { + process.on(signal, () => { + const exitCode = NODE_STEP_PARENT_SIGNAL_EXIT_CODES.get(signal) ?? 1; + if (exitingAfterParentSignal) { + signalActiveNodeSteps("SIGKILL"); + process.exit(exitCode); + } + exitingAfterParentSignal = true; + parentSignalExitCode = exitCode; + signalActiveNodeSteps(signal); + parentSignalExitTimer ??= setTimeout( + () => process.exit(parentSignalExitCode), + NODE_STEP_ABORT_KILL_GRACE_MS, + ); + }); + } + process.on("exit", () => { + signalActiveNodeSteps("SIGKILL"); + }); +} + export function runNodeStep(label, args, timeoutMs, params = {}) { const abortController = params.abortController; const spawnImpl = params.spawnImpl ?? spawn; + installNodeStepParentSignalForwarders(); return new Promise((resolvePromise, rejectPromise) => { const child = spawnImpl(process.execPath, args, { cwd: repoRoot, + detached: process.platform !== "win32", env: params.env ? { ...process.env, ...params.env } : process.env, - signal: abortController?.signal, stdio: ["ignore", "pipe", "pipe"], }); let settled = false; + let canceled = false; + let killTimer; const stdoutWriter = createPrefixedOutputWriter(label, process.stdout); const stderrWriter = createPrefixedOutputWriter(label, process.stderr); + const killNodeStep = (signal) => signalNodeStep(child, signal); + ACTIVE_NODE_STEP_KILLERS.add(killNodeStep); + const abortStep = () => { + if (settled || canceled) { + return; + } + canceled = true; + killNodeStep("SIGTERM"); + killTimer = setTimeout(() => { + killTimer = undefined; + killNodeStep("SIGKILL"); + }, NODE_STEP_ABORT_KILL_GRACE_MS); + killTimer.unref?.(); + }; + function cleanup() { + clearTimeout(timer); + clearTimeout(killTimer); + ACTIVE_NODE_STEP_KILLERS.delete(killNodeStep); + abortController?.signal.removeEventListener("abort", abortStep); + } const timer = setTimeout(() => { if (settled) { return; } settled = true; - child.kill("SIGKILL"); + killNodeStep("SIGKILL"); + cleanup(); stdoutWriter.flush(); stderrWriter.flush(); abortSiblingSteps(abortController); rejectPromise(new Error(`${label} timed out after ${timeoutMs}ms`)); }, timeoutMs); + abortController?.signal.addEventListener("abort", abortStep, { once: true }); child.stdout.setEncoding("utf8"); child.stderr.setEncoding("utf8"); @@ -394,14 +475,15 @@ export function runNodeStep(label, args, timeoutMs, params = {}) { if (settled) { return; } - clearTimeout(timer); settled = true; stdoutWriter.flush(); stderrWriter.flush(); - if (error.name === "AbortError" && abortController?.signal.aborted) { - rejectPromise(new Error(`${label} canceled after sibling failure`)); + if (exitingAfterParentSignal) { + killNodeStep("SIGKILL"); + cleanup(); return; } + cleanup(); abortSiblingSteps(abortController); rejectPromise(new Error(`${label} failed to start: ${error.message}`)); }); @@ -409,10 +491,21 @@ export function runNodeStep(label, args, timeoutMs, params = {}) { if (settled) { return; } - clearTimeout(timer); settled = true; stdoutWriter.flush(); stderrWriter.flush(); + if (exitingAfterParentSignal) { + killNodeStep("SIGKILL"); + cleanup(); + return; + } + if (canceled) { + killNodeStep("SIGKILL"); + cleanup(); + rejectPromise(new Error(`${label} canceled after sibling failure`)); + return; + } + cleanup(); if (code === 0) { resolvePromise(); return; diff --git a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts index fdbf162be88f..1cb0b23ebaf1 100644 --- a/test/scripts/prepare-extension-package-boundary-artifacts.test.ts +++ b/test/scripts/prepare-extension-package-boundary-artifacts.test.ts @@ -1,7 +1,10 @@ import { EventEmitter } from "node:events"; +import { spawn } from "node:child_process"; import fs from "node:fs"; import os from "node:os"; import path from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; +import { pathToFileURL } from "node:url"; import { afterEach, describe, expect, it } from "vitest"; import { createPrefixedOutputWriter, @@ -30,6 +33,53 @@ afterEach(() => { tempRoots.clear(); }); +async function waitForFile(filePath: string, timeoutMs: number) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (fs.existsSync(filePath)) { + return; + } + await delay(25); + } + throw new Error(`Timed out waiting for ${filePath}`); +} + +function isProcessAlive(pid: number) { + try { + process.kill(pid, 0); + return true; + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ESRCH") { + return false; + } + throw error; + } +} + +async function waitForDead(pid: number, timeoutMs: number) { + const deadline = Date.now() + timeoutMs; + while (Date.now() < deadline) { + if (!isProcessAlive(pid)) { + return; + } + await delay(25); + } + throw new Error(`Process ${pid} was still alive after ${timeoutMs}ms`); +} + +async function waitForProcessExit( + child: ReturnType, + timeoutMs: number, +): Promise<{ code: number | null; signal: NodeJS.Signals | null }> { + const exit = new Promise<{ code: number | null; signal: NodeJS.Signals | null }>((resolve) => { + child.once("exit", (code, signal) => resolve({ code, signal })); + }); + const timeout = delay(timeoutMs).then(() => { + throw new Error(`Process ${child.pid ?? "unknown"} did not exit after ${timeoutMs}ms`); + }); + return Promise.race([exit, timeout]); +} + describe("prepare-extension-package-boundary-artifacts", () => { it("prefixes each completed line and flushes the trailing partial line", () => { let output = ""; @@ -69,6 +119,55 @@ describe("prepare-extension-package-boundary-artifacts", () => { expect(Date.now() - startedAt).toBeLessThan(abortBudgetMs); }, 45_000); + it.runIf(process.platform !== "win32")( + "force-kills aborted sibling step process groups", + async () => { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-boundary-abort-group-")); + tempRoots.add(rootDir); + const descendantPidPath = path.join(rootDir, "descendant.pid"); + let descendantPid = 0; + const descendantScript = [ + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(process.pid));`, + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join("\n"); + const parentScript = [ + "const { spawn } = require('node:child_process');", + `spawn(process.execPath, ["--eval", ${JSON.stringify(descendantScript)}], { stdio: "ignore" });`, + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join("\n"); + + try { + const command = runNodeStepsInParallel([ + { + label: "delayed-fail", + args: ["--eval", "setTimeout(() => process.exit(2), 150)"], + timeoutMs: 5_000, + }, + { + label: "abort-group-prep", + args: ["--eval", parentScript], + timeoutMs: 60_000, + }, + ]); + const expectedFailure = expect(command).rejects.toThrow( + "delayed-fail failed with exit code 2", + ); + await waitForFile(descendantPidPath, 1_000); + descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + + await expectedFailure; + await waitForDead(descendantPid, 2_000); + } finally { + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }, + ); + it("hard-kills timed out prep steps", async () => { const signals: Array = []; const child = new EventEmitter() as EventEmitter & { @@ -96,6 +195,91 @@ describe("prepare-extension-package-boundary-artifacts", () => { expect(signals).toEqual(["SIGKILL"]); }); + it.runIf(process.platform !== "win32")("kills timed-out prep step process groups", async () => { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-boundary-timeout-group-")); + tempRoots.add(rootDir); + const descendantPidPath = path.join(rootDir, "descendant.pid"); + let descendantPid = 0; + const descendantScript = [ + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(process.pid));`, + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join("\n"); + const parentScript = [ + "const { spawn } = require('node:child_process');", + `spawn(process.execPath, ["--eval", ${JSON.stringify(descendantScript)}], { stdio: "ignore" });`, + "setInterval(() => {}, 1000);", + ].join("\n"); + + try { + const command = runNodeStep("hung-group-prep", ["--eval", parentScript], 750); + const expectedFailure = expect(command).rejects.toThrow( + "hung-group-prep timed out after 750ms", + ); + await waitForFile(descendantPidPath, 500); + descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + + await expectedFailure; + await waitForDead(descendantPid, 2_000); + } finally { + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }); + + it.runIf(process.platform !== "win32")( + "forwards wrapper termination to detached prep step groups", + async () => { + const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-boundary-signal-group-")); + tempRoots.add(rootDir); + const descendantPidPath = path.join(rootDir, "descendant.pid"); + let descendantPid = 0; + let runnerPid = 0; + const moduleHref = pathToFileURL( + path.resolve("scripts/prepare-extension-package-boundary-artifacts.mjs"), + ).href; + const descendantScript = [ + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(descendantPidPath)}, String(process.pid));`, + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join("\n"); + const parentScript = [ + "const { spawn } = require('node:child_process');", + `spawn(process.execPath, ["--eval", ${JSON.stringify(descendantScript)}], { stdio: "ignore" });`, + "process.on('SIGTERM', () => {});", + "setInterval(() => {}, 1000);", + ].join("\n"); + const runnerScript = [ + `import { runNodeStep } from ${JSON.stringify(moduleHref)};`, + `await runNodeStep("signal-group-prep", ["--eval", ${JSON.stringify(parentScript)}], 60_000);`, + ].join("\n"); + const runner = spawn(process.execPath, ["--input-type=module", "--eval", runnerScript], { + stdio: "ignore", + }); + runnerPid = runner.pid ?? 0; + + try { + await waitForFile(descendantPidPath, 2_000); + descendantPid = Number.parseInt(fs.readFileSync(descendantPidPath, "utf8"), 10); + const runnerExit = waitForProcessExit(runner, 2_000); + runner.kill("SIGTERM"); + + expect(await runnerExit).toEqual({ code: 143, signal: null }); + await waitForDead(descendantPid, 2_000); + } finally { + if (runnerPid && isProcessAlive(runnerPid)) { + process.kill(runnerPid, "SIGKILL"); + } + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }, + ); + it("runs boundary prep steps serially for local checks", async () => { const rootDir = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-boundary-serial-")); tempRoots.add(rootDir);