diff --git a/scripts/resolve-openclaw-package-candidate.mjs b/scripts/resolve-openclaw-package-candidate.mjs index ce588b53ba9..3cb490b8df4 100644 --- a/scripts/resolve-openclaw-package-candidate.mjs +++ b/scripts/resolve-openclaw-package-candidate.mjs @@ -24,6 +24,7 @@ export const ARTIFACT_TARBALL_SCAN_MAX_ENTRIES = 10_000; 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 ACTIVE_CHILD_KILLERS = new Set(); const SIGNAL_EXIT_CODES = { SIGHUP: 129, @@ -195,7 +196,7 @@ function run(command, args, options = {}) { }); let timedOut = false; let killTimer; - let timeoutReject; + let forceKillAt; const killChild = (signal) => { if (useProcessGroup && child.pid) { try { @@ -209,11 +210,13 @@ function run(command, args, options = {}) { }; const terminateChild = () => { killChild("SIGTERM"); + const killAfterMs = options.killAfterMs ?? COMMAND_TIMEOUT_KILL_AFTER_MS; + forceKillAt = Date.now() + killAfterMs; killTimer = setTimeout(() => { killTimer = undefined; + forceKillAt = undefined; killChild("SIGKILL"); - timeoutReject?.(); - }, options.killAfterMs ?? COMMAND_TIMEOUT_KILL_AFTER_MS); + }, killAfterMs); }; const timeout = options.timeoutMs === undefined @@ -244,6 +247,7 @@ function run(command, args, options = {}) { } if (killTimer && !timedOut) { clearTimeout(killTimer); + forceKillAt = undefined; } ACTIVE_CHILD_KILLERS.delete(killChild); if ( @@ -261,7 +265,13 @@ function run(command, args, options = {}) { `${command} ${args.join(" ")} timed out after ${options.timeoutMs}ms`, ); if (killTimer) { - timeoutReject = () => reject(timeoutError); + void finishTimedOutProcessTree(child, { + forceKillAt, + killChild, + killTimer, + killAfterMs: options.killAfterMs ?? COMMAND_TIMEOUT_KILL_AFTER_MS, + useProcessGroup, + }).then(() => reject(timeoutError), reject); return; } reject(timeoutError); @@ -286,6 +296,54 @@ function run(command, args, options = {}) { }); } +async function finishTimedOutProcessTree( + child, + { forceKillAt, killAfterMs, killChild, killTimer, useProcessGroup }, +) { + const graceRemainingMs = + forceKillAt === undefined ? killAfterMs : Math.max(0, forceKillAt - Date.now()); + if (graceRemainingMs > 0) { + await waitForProcessTreeExit(child, graceRemainingMs, useProcessGroup); + } + clearTimeout(killTimer); + if (processTreeIsAlive(child, useProcessGroup)) { + killChild("SIGKILL"); + await waitForProcessTreeExit(child, killAfterMs, useProcessGroup); + } +} + +function childHasExited(child) { + return child.exitCode !== null || child.signalCode !== null; +} + +function processTreeIsAlive(child, useProcessGroup) { + if (!child || typeof child.pid !== "number") { + return false; + } + if (!useProcessGroup) { + return !childHasExited(child); + } + try { + process.kill(-child.pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } +} + +async function waitForProcessTreeExit(child, timeoutMs, useProcessGroup) { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!processTreeIsAlive(child, useProcessGroup)) { + return true; + } + await new Promise((resolvePoll) => { + setTimeout(resolvePoll, COMMAND_PROCESS_TREE_EXIT_POLL_MS); + }); + } + return !processTreeIsAlive(child, useProcessGroup); +} + function appendBoundedCommandOutput(buffer, chunk, maxChars) { const nextText = buffer.text + String(chunk); if (nextText.length <= maxChars) { diff --git a/test/scripts/resolve-openclaw-package-candidate.test.ts b/test/scripts/resolve-openclaw-package-candidate.test.ts index 17353c98b73..4dc74ad27e5 100644 --- a/test/scripts/resolve-openclaw-package-candidate.test.ts +++ b/test/scripts/resolve-openclaw-package-candidate.test.ts @@ -278,9 +278,7 @@ describe("resolve-openclaw-package-candidate", () => { ).resolves.toBe(path.join(dir, "openclaw-current.tgz")); await expect(missing(path.join(dir, "openclaw-9999.1.1.tgz"))).resolves.toBe(true); await expect(readFile(path.join(dir, "openclaw-C:evil.tgz"), "utf8")).resolves.toBe("unsafe"); - await expect(readFile(path.join(dir, "openclaw-current.tgz"), "utf8")).resolves.toBe( - "current", - ); + await expect(readFile(path.join(dir, "openclaw-current.tgz"), "utf8")).resolves.toBe("current"); }); it("bounds captured command stderr tails on failures", async () => { @@ -352,6 +350,63 @@ describe("resolve-openclaw-package-candidate", () => { } }); + it("rejects timed-out package runner commands when descendants exit cleanly", async () => { + if (process.platform === "win32") { + return; + } + + const dir = await mkdtemp(path.join(tmpdir(), "openclaw-package-runner-timeout-clean-")); + tempDirs.push(dir); + const childPidPath = path.join(dir, "child.pid"); + const readyPath = path.join(dir, "child.ready"); + const cleanupPath = path.join(dir, "child.cleanup"); + + const childScript = [ + "const fs = require('node:fs');", + "process.on('SIGTERM', () => {", + " fs.writeFileSync(process.env.OPENCLAW_TEST_CHILD_CLEANUP, 'clean');", + " setTimeout(() => process.exit(0), 75);", + "});", + "fs.writeFileSync(process.env.OPENCLAW_TEST_CHILD_READY, 'ready');", + "setInterval(() => {}, 1000);", + ].join(""); + const parentScript = [ + "const { spawn } = require('node:child_process');", + "const fs = require('node:fs');", + `const child = spawn(process.execPath, ['-e', ${JSON.stringify(childScript)}], {`, + " stdio: 'ignore',", + " env: {", + " ...process.env,", + " OPENCLAW_TEST_CHILD_CLEANUP: process.env.OPENCLAW_TEST_CHILD_CLEANUP,", + " OPENCLAW_TEST_CHILD_READY: process.env.OPENCLAW_TEST_CHILD_READY,", + " },", + "});", + "fs.writeFileSync(process.env.OPENCLAW_TEST_CHILD_PID, String(child.pid));", + "process.on('SIGTERM', () => process.exit(0));", + "setInterval(() => {}, 1000);", + ].join(""); + + const startedAt = Date.now(); + const timeoutAssertion = expect( + runCommandForTest(process.execPath, ["-e", parentScript], { + env: { + ...process.env, + OPENCLAW_TEST_CHILD_CLEANUP: cleanupPath, + OPENCLAW_TEST_CHILD_PID: childPidPath, + OPENCLAW_TEST_CHILD_READY: readyPath, + }, + killAfterMs: 1000, + timeoutMs: 1000, + }), + ).rejects.toThrow(/timed out after 1000ms/u); + + await waitForFile(readyPath, 2_000); + await timeoutAssertion; + + expect(readFileSync(cleanupPath, "utf8")).toBe("clean"); + expect(Date.now() - startedAt).toBeLessThan(1_700); + }); + it("forwards external termination to package runner process groups", async () => { if (process.platform === "win32") { return;