From 2770aa5f4cf627070635c5875da594150806c39a Mon Sep 17 00:00:00 2001 From: Vincent Koc Date: Tue, 2 Jun 2026 00:29:16 +0200 Subject: [PATCH] fix(scripts): clean boundary step process groups --- .../check-extension-package-tsc-boundary.mjs | 48 +++++++++++++++++-- ...eck-extension-package-tsc-boundary.test.ts | 16 ++++--- 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/scripts/check-extension-package-tsc-boundary.mjs b/scripts/check-extension-package-tsc-boundary.mjs index 47faf30c91aa..8cbf411a026f 100644 --- a/scripts/check-extension-package-tsc-boundary.mjs +++ b/scripts/check-extension-package-tsc-boundary.mjs @@ -13,6 +13,11 @@ import { import { createRequire } from "node:module"; import os from "node:os"; import path, { dirname, join, resolve } from "node:path"; +import { + forwardSignalToVitestProcessGroup, + installVitestProcessGroupCleanup, + shouldUseDetachedVitestProcessGroup, +} from "./vitest-process-group.mjs"; const require = createRequire(import.meta.url); const repoRoot = resolve(import.meta.dirname, ".."); @@ -357,12 +362,15 @@ function abortSiblingSteps(abortController) { export function runNodeStepAsync(label, args, timeoutMs, params = {}) { const abortController = params.abortController; + const killProcess = params.killProcess ?? process.kill.bind(process); const onFailure = params.onFailure; + const platform = params.platform ?? process.platform; const spawnImpl = params.spawnImpl ?? spawn; const startedAt = Date.now(); return new Promise((resolvePromise, rejectPromise) => { const child = spawnImpl(process.execPath, args, { cwd: repoRoot, + detached: shouldUseDetachedVitestProcessGroup(platform), env: process.env, signal: abortController?.signal, stdio: ["ignore", "pipe", "pipe"], @@ -371,12 +379,42 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) { let stdout = createStepOutputCapture(); let stderr = createStepOutputCapture(); let settled = false; + let forwardedSignal = null; + const signalChild = (signal) => { + if ( + !forwardSignalToVitestProcessGroup({ + child, + kill: killProcess, + platform, + signal, + }) + ) { + child.kill(signal); + } + }; + const abortSignal = abortController?.signal; + const abortListener = () => { + signalChild("SIGTERM"); + }; + abortSignal?.addEventListener("abort", abortListener, { once: true }); + const teardownProcessCleanup = installVitestProcessGroupCleanup({ + child, + onSignal: (signal) => { + forwardedSignal ??= signal; + }, + }); + const cleanup = () => { + clearTimeout(timer); + abortSignal?.removeEventListener("abort", abortListener); + teardownProcessCleanup(); + }; const timer = setTimeout(() => { if (settled) { return; } settled = true; - child.kill("SIGKILL"); + cleanup(); + signalChild("SIGKILL"); const stdoutText = formatCapturedStepOutput(stdout); const stderrText = formatCapturedStepOutput(stderr); const error = attachStepFailureMetadata( @@ -415,7 +453,7 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) { if (settled) { return; } - clearTimeout(timer); + cleanup(); settled = true; if (error.name === "AbortError" && abortController?.signal.aborted) { rejectPromise( @@ -459,8 +497,12 @@ export function runNodeStepAsync(label, args, timeoutMs, params = {}) { if (settled) { return; } - clearTimeout(timer); + cleanup(); settled = true; + if (forwardedSignal) { + process.kill(process.pid, forwardedSignal); + return; + } if (code === 0) { resolvePromise({ stdout: formatCapturedStepOutput(stdout), diff --git a/test/scripts/check-extension-package-tsc-boundary.test.ts b/test/scripts/check-extension-package-tsc-boundary.test.ts index fe3c9d45c166..9e09c14d19d4 100644 --- a/test/scripts/check-extension-package-tsc-boundary.test.ts +++ b/test/scripts/check-extension-package-tsc-boundary.test.ts @@ -409,18 +409,17 @@ describe("check-extension-package-tsc-boundary", () => { }, 30_000); it("hard-kills timed out async node steps", async () => { - const signals: Array = []; + const processSignals: Array<[number, NodeJS.Signals | number | undefined]> = []; const child = new EventEmitter() as EventEmitter & { kill: (signal?: NodeJS.Signals | number) => boolean; + pid: number; stderr: ReturnType; stdout: ReturnType; }; + child.pid = 1234; child.stdout = createMockPipe(); child.stderr = createMockPipe(); - child.kill = (signal) => { - signals.push(signal); - return true; - }; + child.kill = () => true; const failure = await runNodeStepAsync( "hung-plugin", @@ -432,6 +431,11 @@ describe("check-extension-package-tsc-boundary", () => { expect(args).toEqual(["--eval", "setTimeout(() => {}, 60_000)"]); return child; }, + killProcess(pid: number, signal?: NodeJS.Signals | number) { + processSignals.push([pid, signal]); + return true; + }, + platform: "darwin", }, ).then( () => { @@ -440,7 +444,7 @@ describe("check-extension-package-tsc-boundary", () => { (error: unknown) => error, ); - expect(signals).toEqual(["SIGKILL"]); + expect(processSignals).toEqual([[-1234, "SIGKILL"]]); expect(failure).toBeInstanceOf(Error); if (!(failure instanceof Error)) { throw new Error("expected timeout failure to reject with an Error");