From 7019da8c7bb18f9f85536f86a977dd89cd4c0984 Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Wed, 17 Jun 2026 20:26:38 +0200 Subject: [PATCH] fix(scripts): wait for extension boundary process groups --- .../check-extension-package-tsc-boundary.mjs | 71 ++++++++---- ...eck-extension-package-tsc-boundary.test.ts | 102 +++++++++++++++++- 2 files changed, 153 insertions(+), 20 deletions(-) diff --git a/scripts/check-extension-package-tsc-boundary.mjs b/scripts/check-extension-package-tsc-boundary.mjs index 8928098d7ddb..168fb00eb91c 100644 --- a/scripts/check-extension-package-tsc-boundary.mjs +++ b/scripts/check-extension-package-tsc-boundary.mjs @@ -35,6 +35,8 @@ const prepareBoundaryArtifactsBin = resolve( const extensionPackageBoundaryBaseConfig = "../tsconfig.package-boundary.base.json"; const FAILURE_OUTPUT_TAIL_LINES = 40; const STEP_OUTPUT_MAX_CHARS = 256 * 1024; +const STEP_PROCESS_GROUP_EXIT_POLL_MS = 25; +const STEP_POST_FORCE_KILL_WAIT_MS = 1_000; const SLOW_COMPILE_SUMMARY_LIMIT = 10; const COMPILE_INPUT_EXTENSIONS = new Set([".ts", ".tsx", ".mts", ".cts", ".js", ".mjs", ".json"]); const ROOTDIR_BOUNDARY_CANARY_IMPORT_PATH = @@ -420,6 +422,34 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) { child.kill(signal); } }; + const processGroupAlive = () => { + if (platform === "win32" || typeof child.pid !== "number") { + return false; + } + try { + killProcess(-child.pid, 0); + return true; + } catch (error) { + return error?.code === "EPERM"; + } + }; + const waitForProcessGroupExit = async (ms) => { + const deadlineAt = Date.now() + ms; + while (Date.now() < deadlineAt) { + if (!processGroupAlive()) { + return true; + } + await new Promise((resolvePoll) => { + setTimeout(resolvePoll, STEP_PROCESS_GROUP_EXIT_POLL_MS); + }); + } + return !processGroupAlive(); + }; + const waitAfterForceKill = async () => { + if (processGroupAlive()) { + await waitForProcessGroupExit(STEP_POST_FORCE_KILL_WAIT_MS); + } + }; const abortSignal = abortController?.signal; const abortListener = () => { signalChild("SIGTERM"); @@ -443,30 +473,33 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) { settled = true; cleanup(); signalChild("SIGKILL"); - const stdoutText = formatCapturedStepOutput(stdout); - const stderrText = formatCapturedStepOutput(stderr); - const error = attachStepFailureMetadata( - new Error( - formatStepFailure(label, { + void (async () => { + await waitAfterForceKill(); + const stdoutText = formatCapturedStepOutput(stdout); + const stderrText = formatCapturedStepOutput(stderr); + const error = attachStepFailureMetadata( + new Error( + formatStepFailure(label, { + stdout: stdoutText, + stderr: stderrText, + kind: "timeout", + elapsedMs: Date.now() - startedAt, + note: `${label} timed out after ${timeoutMs}ms`, + }), + ), + label, + { stdout: stdoutText, stderr: stderrText, kind: "timeout", elapsedMs: Date.now() - startedAt, note: `${label} timed out after ${timeoutMs}ms`, - }), - ), - label, - { - stdout: stdoutText, - stderr: stderrText, - kind: "timeout", - elapsedMs: Date.now() - startedAt, - note: `${label} timed out after ${timeoutMs}ms`, - }, - ); - onFailure?.(error); - abortSiblingSteps(abortController); - rejectPromise(toLintErrorObject(error, "Step timed out")); + }, + ); + onFailure?.(error); + abortSiblingSteps(abortController); + rejectPromise(toLintErrorObject(error, "Step timed out")); + })(); }, timeoutMs); child.stdout.setEncoding("utf8"); diff --git a/test/scripts/check-extension-package-tsc-boundary.test.ts b/test/scripts/check-extension-package-tsc-boundary.test.ts index bae24a7dcfc7..ee2a0c275150 100644 --- a/test/scripts/check-extension-package-tsc-boundary.test.ts +++ b/test/scripts/check-extension-package-tsc-boundary.test.ts @@ -1,4 +1,5 @@ // Check Extension Package Tsc Boundary tests cover check extension package tsc boundary script behavior. +import { spawn } from "node:child_process"; import { EventEmitter } from "node:events"; import fs from "node:fs"; import os from "node:os"; @@ -46,6 +47,43 @@ function createMockPipe() { return pipe; } +function isProcessAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch { + return false; + } +} + +async function sleep(ms: number): Promise { + await new Promise((resolve) => { + setTimeout(resolve, ms); + }); +} + +async function waitForFile(filePath: string, timeoutMs: number): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (fs.existsSync(filePath)) { + return; + } + await sleep(25); + } + throw new Error(`timeout waiting for ${filePath}`); +} + +async function waitForDead(pid: number, timeoutMs: number): Promise { + const deadlineAt = Date.now() + timeoutMs; + while (Date.now() < deadlineAt) { + if (!isProcessAlive(pid)) { + return; + } + await sleep(25); + } + throw new Error(`process still alive: ${pid}`); +} + afterEach(() => { for (const rootDir of tempRoots) { fs.rmSync(rootDir, { force: true, recursive: true }); @@ -423,6 +461,7 @@ describe("check-extension-package-tsc-boundary", () => { it("hard-kills timed out async node steps", async () => { const processSignals: Array<[number, NodeJS.Signals | number | undefined]> = []; + let processGroupAlive = true; const child = new EventEmitter() as EventEmitter & { kill: (signal?: NodeJS.Signals | number) => boolean; pid: number; @@ -445,6 +484,13 @@ describe("check-extension-package-tsc-boundary", () => { return child; }, killProcess(pid: number, signal?: NodeJS.Signals | number) { + if (signal === "SIGKILL") { + processGroupAlive = false; + } + if (signal === 0 && !processGroupAlive) { + processSignals.push([pid, signal]); + throw Object.assign(new Error("gone"), { code: "ESRCH" }); + } processSignals.push([pid, signal]); return true; }, @@ -457,7 +503,10 @@ describe("check-extension-package-tsc-boundary", () => { (error: unknown) => error, ); - expect(processSignals).toEqual([[-1234, "SIGKILL"]]); + expect(processSignals).toEqual([ + [-1234, "SIGKILL"], + [-1234, 0], + ]); expect(failure).toBeInstanceOf(Error); if (!(failure instanceof Error)) { throw new Error("expected timeout failure to reject with an Error"); @@ -466,6 +515,57 @@ describe("check-extension-package-tsc-boundary", () => { expect((failure as { kind?: unknown }).kind).toBe("timeout"); }); + it.skipIf(process.platform === "win32")( + "waits for timed-out async node step process groups", + async () => { + const root = fs.mkdtempSync(path.join(os.tmpdir(), "openclaw-extension-tsc-timeout-")); + tempRoots.add(root); + const childPidPath = path.join(root, "child.pid"); + let childPid = 0; + const childScript = [ + "process.on('SIGTERM', () => {});", + "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' });`, + `fs.writeFileSync(${JSON.stringify(childPidPath)}, String(child.pid));`, + "setInterval(() => {}, 1000);", + ].join(""); + + try { + const failurePromise = runNodeStepAsync( + "hung-step-group", + ["--eval", parentScript], + 100, + { + spawnImpl(command: string, args: string[], options: unknown) { + return spawn(command, args, options as Parameters[2]); + }, + }, + ).then( + () => { + throw new Error("expected hung-step-group to time out"); + }, + (error: unknown) => error, + ); + + await waitForFile(childPidPath, 2_000); + childPid = Number.parseInt(fs.readFileSync(childPidPath, "utf8"), 10); + expect(isProcessAlive(childPid)).toBe(true); + + const failure = await failurePromise; + expect(failure).toBeInstanceOf(Error); + await waitForDead(childPid, 2_000); + } finally { + if (childPid && isProcessAlive(childPid)) { + process.kill(childPid, "SIGKILL"); + } + } + }, + ); + it("aborts concurrent sibling steps after the first failure", async () => { const startedAt = Date.now(); const slowStepTimeoutMs = 60_000;