From dd89898133efddf83b29ead8a6f62ab12bee070e Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Mon, 22 Jun 2026 02:51:05 +0200 Subject: [PATCH] fix(scripts): clamp package candidate timers --- .../resolve-openclaw-package-candidate.mjs | 36 +++++++++++--- ...resolve-openclaw-package-candidate.test.ts | 49 +++++++++++++++++++ 2 files changed, 78 insertions(+), 7 deletions(-) diff --git a/scripts/resolve-openclaw-package-candidate.mjs b/scripts/resolve-openclaw-package-candidate.mjs index 0e372d43f42e..db765df93450 100644 --- a/scripts/resolve-openclaw-package-candidate.mjs +++ b/scripts/resolve-openclaw-package-candidate.mjs @@ -26,6 +26,7 @@ const COMMAND_STDOUT_CAPTURE_MAX_CHARS = 8 * 1024 * 1024; const COMMAND_STDERR_CAPTURE_MAX_CHARS = 128 * 1024; const COMMAND_TIMEOUT_KILL_AFTER_MS = 5_000; const COMMAND_PROCESS_TREE_EXIT_POLL_MS = 50; +const MAX_TIMER_TIMEOUT_MS = 2_147_000_000; const ACTIVE_CHILD_KILLERS = new Set(); const SIGNAL_EXIT_CODES = { SIGHUP: 129, @@ -183,8 +184,30 @@ export function resolveNpmPackageCandidatePackRunner(packageSpec, outputDir, par }); } +function numericTimerValueMs(valueMs) { + const value = Number(valueMs); + return Number.isFinite(value) ? Math.floor(value) : undefined; +} + +function resolveTimerTimeoutMs(valueMs, fallbackMs = MAX_TIMER_TIMEOUT_MS) { + const value = numericTimerValueMs(valueMs) ?? numericTimerValueMs(fallbackMs); + return Math.min(Math.max(value ?? MAX_TIMER_TIMEOUT_MS, 1), MAX_TIMER_TIMEOUT_MS); +} + +function resolveOptionalTimerTimeoutMs(valueMs) { + if (valueMs === undefined) { + return undefined; + } + return resolveTimerTimeoutMs(valueMs, 1); +} + function run(command, args, options = {}) { return new Promise((resolve, reject) => { + const resolvedTimeoutMs = resolveOptionalTimerTimeoutMs(options.timeoutMs); + const resolvedKillAfterMs = resolveTimerTimeoutMs( + options.killAfterMs, + COMMAND_TIMEOUT_KILL_AFTER_MS, + ); const useProcessGroup = process.platform !== "win32"; const spawnOptions = { cwd: options.cwd ?? ROOT_DIR, @@ -205,21 +228,20 @@ function run(command, args, options = {}) { const killChild = (signal) => signalChildProcessTree(child, signal, { useProcessGroup }); const terminateChild = () => { killChild("SIGTERM"); - const killAfterMs = options.killAfterMs ?? COMMAND_TIMEOUT_KILL_AFTER_MS; - forceKillAt = Date.now() + killAfterMs; + forceKillAt = Date.now() + resolvedKillAfterMs; killTimer = setTimeout(() => { killTimer = undefined; forceKillAt = undefined; killChild("SIGKILL"); - }, killAfterMs); + }, resolvedKillAfterMs); }; const timeout = - options.timeoutMs === undefined + resolvedTimeoutMs === undefined ? undefined : setTimeout(() => { timedOut = true; terminateChild(); - }, options.timeoutMs); + }, resolvedTimeoutMs); timeout?.unref?.(); ACTIVE_CHILD_KILLERS.add(killChild); let stdout = { text: "", truncatedChars: 0 }; @@ -257,14 +279,14 @@ function run(command, args, options = {}) { } if (timedOut) { const timeoutError = new Error( - `${command} ${args.join(" ")} timed out after ${options.timeoutMs}ms`, + `${command} ${args.join(" ")} timed out after ${resolvedTimeoutMs}ms`, ); if (killTimer) { void finishTimedOutProcessTree(child, { forceKillAt, killChild, killTimer, - killAfterMs: options.killAfterMs ?? COMMAND_TIMEOUT_KILL_AFTER_MS, + killAfterMs: resolvedKillAfterMs, useProcessGroup, }).then(() => reject(timeoutError), reject); return; diff --git a/test/scripts/resolve-openclaw-package-candidate.test.ts b/test/scripts/resolve-openclaw-package-candidate.test.ts index 82b7c7762f4c..f5da3e132740 100644 --- a/test/scripts/resolve-openclaw-package-candidate.test.ts +++ b/test/scripts/resolve-openclaw-package-candidate.test.ts @@ -5,6 +5,7 @@ import { access, mkdir, mkdtemp, readFile, rm, writeFile } from "node:fs/promise import { tmpdir } from "node:os"; import path from "node:path"; import { pathToFileURL } from "node:url"; +import { MAX_TIMER_TIMEOUT_MS } from "@openclaw/normalization-core/number-coercion"; import { afterEach, describe, expect, it, vi } from "vitest"; import { resolveWindowsTaskkillPath } from "../../scripts/lib/windows-taskkill.mjs"; import { @@ -409,6 +410,15 @@ describe("resolve-openclaw-package-candidate", () => { ).rejects.toThrow(/produced more than \d+ captured stdout chars/u); }); + it("clamps oversized package runner command timers before scheduling", async () => { + await expect( + runCommandForTest(process.execPath, ["-e", "setTimeout(() => process.exit(0), 25);"], { + killAfterMs: MAX_TIMER_TIMEOUT_MS + 1, + timeoutMs: MAX_TIMER_TIMEOUT_MS + 1, + }), + ).resolves.toBe(""); + }); + it("kills timed-out package runner process groups", async () => { if (process.platform === "win32") { return; @@ -448,6 +458,45 @@ describe("resolve-openclaw-package-candidate", () => { } }); + it("clamps oversized package runner kill grace before scheduling", async () => { + if (process.platform === "win32") { + return; + } + + const dir = await mkdtemp(path.join(tmpdir(), "openclaw-package-runner-grace-")); + tempDirs.push(dir); + const childPidPath = path.join(dir, "child.pid"); + const cleanupPath = path.join(dir, "child.cleanup"); + let childPid: number | undefined; + + try { + const childScript = [ + "const fs = require('node:fs');", + `fs.writeFileSync(${JSON.stringify(childPidPath)}, String(process.pid));`, + "process.on('SIGTERM', () => {", + ` setTimeout(() => { fs.writeFileSync(${JSON.stringify(cleanupPath)}, 'clean'); process.exit(0); }, 75);`, + "});", + "setInterval(() => {}, 1000);", + ].join("\n"); + + const timeoutAssertion = expect( + runCommandForTest(process.execPath, ["-e", childScript], { + killAfterMs: MAX_TIMER_TIMEOUT_MS + 1, + timeoutMs: 500, + }), + ).rejects.toThrow(/timed out after 500ms/u); + + await waitForFile(childPidPath, 2_000); + childPid = Number.parseInt(readFileSync(childPidPath, "utf8"), 10); + await timeoutAssertion; + expect(readFileSync(cleanupPath, "utf8")).toBe("clean"); + } finally { + if (childPid !== undefined && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + } + }); + it("rejects timed-out package runner commands when descendants exit cleanly", async () => { if (process.platform === "win32") { return;