From b5c08787776a10d68a474c309bb4559066460d37 Mon Sep 17 00:00:00 2001 From: xingzhou Date: Thu, 30 Jul 2026 04:56:21 +0800 Subject: [PATCH] fix(build): bound bundled plugin asset generators (#111366) Fail build and copy generators with ETIMEDOUT after 600000ms and terminate the complete managed process tree. Keep the timeout policy in the bundled-assets caller while preserving the shared helper for distinct callers such as #111349. Co-authored-by: Vincent Koc --- scripts/bundled-plugin-assets.mjs | 37 +++++++--- scripts/lib/managed-child-process.d.mts | 5 +- scripts/lib/managed-child-process.mjs | 34 +++++++-- test/scripts/bundled-plugin-assets.test.ts | 82 ++++++++++++++++++++++ test/scripts/managed-child-process.test.ts | 59 ++++++++++++++++ 5 files changed, 200 insertions(+), 17 deletions(-) diff --git a/scripts/bundled-plugin-assets.mjs b/scripts/bundled-plugin-assets.mjs index f1292b377a16..1a7b45535fbe 100644 --- a/scripts/bundled-plugin-assets.mjs +++ b/scripts/bundled-plugin-assets.mjs @@ -5,10 +5,13 @@ import { spawnSync } from "node:child_process"; import fs from "node:fs/promises"; import path from "node:path"; import { fileURLToPath, pathToFileURL } from "node:url"; +import { runManagedCommand } from "./lib/managed-child-process.mjs"; import { listGeneratedExtensionAssetSources } from "./lib/static-extension-assets.mjs"; const rootDir = path.resolve(path.dirname(fileURLToPath(import.meta.url)), ".."); const VALID_PHASES = new Set(["build", "copy"]); +// Each complete bundled-plugin asset generator gets the same 10-minute build ceiling. +const BUNDLED_PLUGIN_ASSET_HOOK_TIMEOUT_MS = 600_000; async function readJsonFile(filePath) { return JSON.parse(await fs.readFile(filePath, "utf8")); @@ -101,7 +104,7 @@ export async function readBundledPluginAssetHooks(options = {}) { } hooks.push({ - aliases: [...aliases].toSorted(), + aliases: [...aliases].toSorted((left, right) => left.localeCompare(right)), command, packageName: packageJson.name, phase, @@ -118,6 +121,7 @@ export async function readBundledPluginAssetHooks(options = {}) { */ export async function runBundledPluginAssetHooks(options = {}) { const phase = options.phase; + const timeoutMs = options.timeoutMs ?? BUNDLED_PLUGIN_ASSET_HOOK_TIMEOUT_MS; const hooks = await readBundledPluginAssetHooks(options); if (hooks.length === 0) { const scope = options.plugins?.length ? ` for ${options.plugins.join(", ")}` : ""; @@ -127,14 +131,29 @@ export async function runBundledPluginAssetHooks(options = {}) { for (const hook of hooks) { console.log(`[${hook.pluginId}] ${phase}: ${hook.command}`); - const result = spawnSync(hook.command, { - cwd: hook.pluginDir, - env: process.env, - shell: true, - stdio: "inherit", - }); - if (result.status !== 0) { - process.exit(result.status ?? 1); + let status; + try { + status = await runManagedCommand({ + bin: hook.command, + cwd: hook.pluginDir, + env: process.env, + shell: true, + stdio: "inherit", + timeoutMs, + }); + } catch (error) { + if (error && typeof error === "object" && "code" in error && error.code === "ETIMEDOUT") { + throw Object.assign( + new Error( + `Bundled plugin asset ${phase} hook timed out after ${timeoutMs}ms: ${hook.pluginId}`, + ), + { code: "ETIMEDOUT" }, + ); + } + throw error; + } + if (status !== 0) { + process.exit(status); } } } diff --git a/scripts/lib/managed-child-process.d.mts b/scripts/lib/managed-child-process.d.mts index 77e1b4306060..81ac94fcf725 100644 --- a/scripts/lib/managed-child-process.d.mts +++ b/scripts/lib/managed-child-process.d.mts @@ -34,6 +34,7 @@ export function terminateManagedChild( * windowsVerbatimArguments?: boolean; * platform?: NodeJS.Platform; * comSpec?: string; + * timeoutMs?: number; * onReady?: (child: import("node:child_process").ChildProcess) => void; * }} options * @returns {Promise} @@ -48,6 +49,7 @@ export function runManagedCommand({ shell, windowsVerbatimArguments, comSpec, + timeoutMs, onReady, }: { bin: string; @@ -59,6 +61,7 @@ export function runManagedCommand({ windowsVerbatimArguments?: boolean; platform?: NodeJS.Platform; comSpec?: string; + timeoutMs?: number; onReady?: (child: import("node:child_process").ChildProcess) => void; }): Promise; /** @@ -99,7 +102,7 @@ export function createManagedCommandSpawnSpec({ command: string; options: { cwd: string | undefined; - env: NodeJS.ProcessEnv | undefined; + env: import("node:child_process").SpawnOptions["env"]; stdio: import("node:child_process").StdioOptions; shell: boolean; detached: boolean; diff --git a/scripts/lib/managed-child-process.mjs b/scripts/lib/managed-child-process.mjs index 452f68d50dc4..b2b2e3d22f03 100644 --- a/scripts/lib/managed-child-process.mjs +++ b/scripts/lib/managed-child-process.mjs @@ -84,6 +84,7 @@ export function terminateManagedChild( * windowsVerbatimArguments?: boolean; * platform?: NodeJS.Platform; * comSpec?: string; + * timeoutMs?: number; * onReady?: (child: import("node:child_process").ChildProcess) => void; * }} options * @returns {Promise} @@ -98,6 +99,7 @@ export async function runManagedCommand({ shell = platform === "win32", windowsVerbatimArguments, comSpec, + timeoutMs, onReady, }) { const spawnSpec = createManagedCommandSpawnSpec({ @@ -119,6 +121,8 @@ export async function runManagedCommand({ }; addManagedChild(managedChild); onReady?.(child); + let timeoutTimer = null; + let timedOut = false; try { return await new Promise((resolve, reject) => { @@ -129,21 +133,37 @@ export async function runManagedCommand({ } if (managedChild.receivedSignal) { terminateManagedChild(child, "SIGKILL"); + resolve(signalExitCode(managedChild.receivedSignal)); + return; } - resolve( - managedChild.receivedSignal - ? signalExitCode(managedChild.receivedSignal) - : signal - ? signalExitCode(signal) - : (status ?? 1), - ); + if (timedOut) { + reject(createManagedCommandTimeoutError(timeoutMs)); + return; + } + resolve(signal ? signalExitCode(signal) : (status ?? 1)); }); + if (timeoutMs !== undefined) { + timeoutTimer = setTimeout(() => { + timedOut = true; + // Shell commands may spawn grandchildren, so timeout cleanup owns the whole tree. + terminateManagedChild(child, "SIGKILL", { platform }); + }, timeoutMs); + } }); } finally { + if (timeoutTimer) { + clearTimeout(timeoutTimer); + } removeManagedChild(managedChild); } } +function createManagedCommandTimeoutError(timeoutMs) { + return Object.assign(new Error(`Managed command timed out after ${timeoutMs}ms`), { + code: "ETIMEDOUT", + }); +} + /** * Build the spawn command, args, and options used by managed command execution. * diff --git a/test/scripts/bundled-plugin-assets.test.ts b/test/scripts/bundled-plugin-assets.test.ts index 7192244d047c..b9b0a64a3f2b 100644 --- a/test/scripts/bundled-plugin-assets.test.ts +++ b/test/scripts/bundled-plugin-assets.test.ts @@ -2,12 +2,14 @@ import { execFileSync } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; +import { setTimeout as delay } from "node:timers/promises"; import { afterEach, describe, expect, it, vi } from "vitest"; import { buildDiscordActivitySdk } from "../../scripts/build-discord-activity-sdk.mjs"; import { listStaleGeneratedPluginAssets, parseBundledPluginAssetArgs, readBundledPluginAssetHooks, + runBundledPluginAssetHooks, } from "../../scripts/bundled-plugin-assets.mjs"; import { listGeneratedExtensionAssetSources } from "../../scripts/lib/static-extension-assets.mjs"; import { @@ -161,6 +163,58 @@ describe("bundled plugin assets", () => { }); }); + it("bounds stalled asset hooks and reports the affected plugin safely", async () => { + await withPluginAssetFixture(async (rootDir) => { + const pluginDir = path.join(rootDir, "extensions", "canvas"); + const packagePath = path.join(pluginDir, "package.json"); + const packageJson = JSON.parse(fs.readFileSync(packagePath, "utf8")) as { + openclaw: { assetScripts: { build: string } }; + }; + packageJson.openclaw.assetScripts.build = "node scripts/launch-stall.mjs"; + fs.writeFileSync(packagePath, JSON.stringify(packageJson, null, 2)); + fs.mkdirSync(path.join(pluginDir, "scripts")); + const pidFile = path.join(pluginDir, "stall.pid"); + fs.writeFileSync( + path.join(pluginDir, "scripts", "launch-stall.mjs"), + [ + 'import { spawn } from "node:child_process";', + 'import { writeFileSync } from "node:fs";', + "const child = spawn(process.execPath, [", + ' "-e",', + ' "process.on(\\"SIGTERM\\", () => {}); setTimeout(() => process.exit(0), 5_000); setInterval(() => {}, 100);",', + '], { stdio: "ignore" });', + `writeFileSync(${JSON.stringify(pidFile)}, String(child.pid));`, + 'process.on("SIGTERM", () => {});', + "setInterval(() => {}, 100);", + "", + ].join("\n"), + ); + + const startedAt = Date.now(); + let thrown: unknown; + let childPid = 0; + try { + await runBundledPluginAssetHooks({ phase: "build", rootDir, timeoutMs: 500 }); + } catch (error) { + thrown = error; + } + try { + expect(Date.now() - startedAt).toBeLessThan(2_000); + childPid = Number(fs.readFileSync(pidFile, "utf8")); + await waitForProcessExit(childPid); + expect(thrown).toMatchObject({ + code: "ETIMEDOUT", + message: "Bundled plugin asset build hook timed out after 500ms: canvas", + }); + expect((thrown as Error).message).not.toContain("launch-stall.mjs"); + } finally { + if (childPid && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + } + }); + }); + it("skips cleanly when a requested plugin is absent", async () => { await withPluginAssetFixture(async (rootDir) => { await expect( @@ -217,3 +271,31 @@ describe("bundled plugin assets", () => { }); }); }); + +async function waitForProcessExit(pid: number, timeoutMs = 1_500) { + const startedAt = Date.now(); + while (isProcessAlive(pid)) { + if (Date.now() - startedAt > timeoutMs) { + throw new Error(`process ${pid} remained alive after timeout cleanup`); + } + await delay(5); + } +} + +function isProcessAlive(pid: number) { + try { + process.kill(pid, 0); + } catch { + return false; + } + if (process.platform !== "linux") { + return true; + } + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + // kill(pid, 0) also succeeds for a terminated process awaiting reaping. + return stat.charAt(stat.lastIndexOf(")") + 2) !== "Z"; + } catch { + return false; + } +} diff --git a/test/scripts/managed-child-process.test.ts b/test/scripts/managed-child-process.test.ts index 8b42f0bb33cc..c3cb0f50ebff 100644 --- a/test/scripts/managed-child-process.test.ts +++ b/test/scripts/managed-child-process.test.ts @@ -221,6 +221,56 @@ describe("managed-child-process", () => { } }); + it("times out and kills managed command descendants", async () => { + const dir = createTempDir("openclaw-managed-timeout-"); + const childPath = path.join(dir, "child.mjs"); + const childPidPath = path.join(dir, "child.pid"); + const descendantPidPath = path.join(dir, "descendant.pid"); + fs.writeFileSync( + childPath, + ` +import { spawn } from "node:child_process"; +import fs from "node:fs"; + +const descendant = spawn(process.execPath, [ + "-e", + "process.on('SIGTERM', () => {}); setTimeout(() => process.exit(0), 5_000); setInterval(() => {}, 1000);", +], { stdio: "ignore" }); +fs.writeFileSync(process.argv[2], String(process.pid)); +fs.writeFileSync(process.argv[3], String(descendant.pid)); +process.on("SIGTERM", () => {}); +setInterval(() => {}, 1_000); +`, + "utf8", + ); + + let childPid = 0; + let descendantPid = 0; + try { + await expect( + runManagedCommand({ + bin: process.execPath, + args: [childPath, childPidPath, descendantPidPath], + shell: false, + stdio: "ignore", + timeoutMs: 500, + }), + ).rejects.toMatchObject({ code: "ETIMEDOUT" }); + + childPid = Number(fs.readFileSync(childPidPath, "utf8")); + descendantPid = Number(fs.readFileSync(descendantPidPath, "utf8")); + await waitFor(() => !isProcessAlive(childPid), 1_500); + await waitFor(() => !isProcessAlive(descendantPid), 1_500); + } finally { + if (childPid && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + if (descendantPid && isProcessAlive(descendantPid)) { + process.kill(descendantPid, "SIGKILL"); + } + } + }); + posixIt( "kills managed child process group descendants when the runner is terminated", async () => { @@ -325,7 +375,16 @@ async function waitForClose(child: ReturnType) { function isProcessAlive(pid: number) { try { process.kill(pid, 0); + } catch { + return false; + } + if (process.platform !== "linux") { return true; + } + try { + const stat = fs.readFileSync(`/proc/${pid}/stat`, "utf8"); + // kill(pid, 0) also succeeds for a terminated process awaiting reaping. + return stat.charAt(stat.lastIndexOf(")") + 2) !== "Z"; } catch { return false; }