fix(cli): close every browser on SIGTERM and SIGINT via one launch owner (#4910)

* fix(cli): close every browser on SIGTERM and SIGINT through one launch owner

* fix(cli): own SIGTERM before the GPU probe browser launches

* fix(cli): keep tests on the managed GPU mode and drop the engine signal flag

* test(cli): give the browser fallback fake a process handle

* test(cli): signal the GPU probe case only once the signal owner is installed

* test(cli): mark the launching case only after the signal owner is installed
This commit is contained in:
Miguel Ángel
2026-10-03 15:33:58 +00:00
committed by GitHub
parent ce08f2042e
commit 8c81efb7c2
12 changed files with 241 additions and 32 deletions
+2 -1
View File
@@ -1,3 +1,4 @@
import { launchManagedBrowser } from "../browser/launch.js";
// Run the shared beat detection (@hyperframes/core/beats) in a headless Chrome
// so results match the Studio exactly — same Web Audio decode + same
// bpm-detective. Used by the `beats` CLI command to write the beat file before
@@ -138,7 +139,7 @@ export async function analyzeBeatsHeadless(audioBytes: Buffer): Promise<Headless
const { ensureBrowser } = await import("../browser/manager.js");
const puppeteer = await import("puppeteer-core");
const browser = await ensureBrowser();
const chrome: Browser = await puppeteer.default.launch({
const chrome: Browser = await launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath: browser.executablePath,
args: ["--no-sandbox", "--disable-dev-shm-usage", "--autoplay-policy=no-user-gesture-required"],
@@ -0,0 +1,32 @@
// fallow-ignore-file unused-file
import { writeFileSync } from "node:fs";
import { registerRootExitRequester } from "../utils/commandResult.js";
import { ensureBrowser } from "./manager.js";
import { launchManagedBrowser, resolveManagedGpuMode } from "./launch.js";
const [profileDir, readyPath] = process.argv.slice(2);
if (!profileDir || !readyPath) throw new Error("Missing fixture arguments");
// Stand-in for cli.ts: records the requested exit code, then ends the process.
registerRootExitRequester((exitCode) => {
writeFileSync(`${readyPath}.exit`, String(exitCode));
process.kill(process.pid, "SIGKILL");
});
const { executablePath } = await ensureBrowser();
const puppeteer = await import("puppeteer-core");
if (process.argv[4] === "probe") {
// The signal owner is installed synchronously by the call, so "probing" is only written once a SIGTERM is handled.
const probe = resolveManagedGpuMode("auto", executablePath);
writeFileSync(readyPath, "probing");
await probe;
}
const launch = launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath,
args: ["--no-sandbox", `--user-data-dir=${profileDir}`],
});
writeFileSync(readyPath, "launching");
const browser = await launch;
writeFileSync(readyPath, "ready");
await browser.pages();
setInterval(() => undefined, 1_000);
@@ -0,0 +1,86 @@
import { spawn, type ChildProcess } from "node:child_process";
import { existsSync, mkdtempSync, readFileSync, readdirSync } from "node:fs";
import { tmpdir } from "node:os";
import { dirname, join } from "node:path";
import { fileURLToPath } from "node:url";
import { afterEach, describe, expect, it } from "vitest";
import { waitForTestCondition } from "../utils/processTestUtils.js";
const fixturePath = join(dirname(fileURLToPath(import.meta.url)), "launch.fixture.ts");
const children = new Set<ChildProcess>();
/** Pids of every process whose command line carries this profile dir (Chromium and its helpers). */
function browserPids(profileDir: string): number[] {
return readdirSync("/proc")
.filter((name) => /^\d+$/.test(name))
.filter((name) => {
try {
return readFileSync(`/proc/${name}/cmdline`, "utf8").includes(profileDir);
} catch {
return false;
}
})
.map(Number);
}
function startFixture(profileDir: string, readyPath: string, mode = "launch"): ChildProcess {
const child = spawn(
process.execPath,
["--import", "tsx", fixturePath, profileDir, readyPath, mode],
{ stdio: "ignore" },
);
children.add(child);
return child;
}
const exited = (child: ChildProcess) =>
new Promise<{ code: number | null; signal: string | null }>((resolve) =>
child.once("exit", (code, signal) => resolve({ code, signal })),
);
afterEach(() => {
for (const child of children) child.kill("SIGKILL");
children.clear();
});
describe.skipIf(process.platform !== "linux")("managed browser shutdown", () => {
it.each([
["SIGTERM", "ready", 143],
["SIGINT", "ready", 130],
["SIGTERM", "launching", 143],
] as const)(
"%s while %s leaves no browser and asks the CLI to exit",
async (signal, phase, code) => {
const dir = mkdtempSync(join(tmpdir(), "hf-launch-"));
const profileDir = join(dir, "profile");
const readyPath = join(dir, "ready");
const child = startFixture(profileDir, readyPath);
const done = exited(child);
await waitForTestCondition(
() => existsSync(readyPath) && readFileSync(readyPath, "utf8") === phase,
30_000,
);
if (phase === "ready")
await waitForTestCondition(() => browserPids(profileDir).length > 0, 30_000);
child.kill(signal);
await done;
expect(readFileSync(`${readyPath}.exit`, "utf8")).toBe(String(code));
await waitForTestCondition(() => browserPids(profileDir).length === 0, 5_000);
},
60_000,
);
it("SIGTERM during the GPU probe asks the CLI to exit instead of being swallowed", async () => {
const dir = mkdtempSync(join(tmpdir(), "hf-launch-"));
const readyPath = join(dir, "ready");
const child = startFixture(join(dir, "profile"), readyPath, "probe");
const done = exited(child);
await waitForTestCondition(
() => existsSync(readyPath) && readFileSync(readyPath, "utf8") === "probing",
30_000,
);
child.kill("SIGTERM");
await done;
expect(readFileSync(`${readyPath}.exit`, "utf8")).toBe("143");
}, 60_000);
});
+98
View File
@@ -0,0 +1,98 @@
import type { Browser, LaunchOptions, PuppeteerNode } from "puppeteer-core";
import {
resolveCaptureBrowserGpuMode,
type BrowserGpuMode,
type ResolvedBrowserGpuMode,
} from "./gpuPolicy.js";
import { requestCliExit } from "../utils/commandResult.js";
const browsers = new Set<Browser>();
const pending = new Map<Promise<Browser>, AbortController>();
const CLOSE_GRACE_MS = 1_000;
let listening = false;
let shutdown: Promise<void> | undefined;
/** CLI signals close browser admission and native processes before the root exits. */
function stopForSignal(signal: "SIGINT" | "SIGTERM"): void {
if (shutdown) return;
shutdown = closeManagedBrowsers();
void shutdown.then(
() => requestCliExit(signal === "SIGINT" ? 130 : 143),
(error: unknown) => {
console.error("Browser shutdown failed:", error);
requestCliExit(1);
},
);
}
function ownSignals(): void {
if (listening) return;
listening = true;
process.on("SIGINT", () => stopForSignal("SIGINT"));
process.on("SIGTERM", () => stopForSignal("SIGTERM"));
}
/** The GPU probe launches its own Chrome, so the signal owner must exist before it runs. */
export async function resolveManagedGpuMode(
requestedMode: BrowserGpuMode,
chromePath?: string,
): Promise<ResolvedBrowserGpuMode> {
ownSignals();
return resolveCaptureBrowserGpuMode(requestedMode, chromePath);
}
/** One owner for every CLI browser, including a launch cancelled before its connection is ready. */
export async function launchManagedBrowser(
puppeteer: PuppeteerNode,
options: LaunchOptions,
): Promise<Browser> {
if (shutdown) throw new Error("The CLI is stopping; no browser can start.");
ownSignals();
const abort = new AbortController();
const signal = options.signal ? AbortSignal.any([options.signal, abort.signal]) : abort.signal;
const launch = puppeteer.launch({
...options,
signal,
handleSIGINT: false,
handleSIGTERM: false,
});
pending.set(launch, abort);
try {
const browser = await launch;
const child = browser.process();
if (!child) throw new Error("A launched browser has no owned process.");
browsers.add(browser);
child.once("exit", () => browsers.delete(browser));
return browser;
} finally {
pending.delete(launch);
}
}
/** Resolved launches keep their signal live; only pending launches are aborted during shutdown. */
async function closeManagedBrowsers(): Promise<void> {
for (const abort of pending.values()) abort.abort();
await Promise.allSettled(pending.keys());
await Promise.all([...browsers].map(closeBrowser));
}
async function closeBrowser(browser: Browser): Promise<void> {
const child = browser.process();
if (!child) throw new Error("A managed browser lost its owned process.");
if (child.exitCode !== null || child.signalCode !== null) return;
const exited = new Promise<void>((resolve) => child.once("exit", () => resolve()));
let timer: NodeJS.Timeout | undefined;
const grace = new Promise<void>((resolve) => {
timer = setTimeout(resolve, CLOSE_GRACE_MS);
});
try {
await Promise.race([
browser.close().catch((error: unknown) => console.warn("Browser close failed:", error)),
grace,
]);
if (child.exitCode === null && child.signalCode === null) child.kill("SIGKILL");
await exited;
} finally {
clearTimeout(timer);
}
}
+2 -1
View File
@@ -1,3 +1,4 @@
import { launchManagedBrowser } from "../browser/launch.js";
import { LottieDiscovery } from "./lottieDiscovery.js";
import { createCaptureDownloadBudget } from "./readBoundedResponse.js";
/**
@@ -111,7 +112,7 @@ export async function captureWebsiteAttempt(
const { ensureBrowser } = await import("../browser/manager.js");
const browser = await ensureBrowser();
const puppeteer = await import("puppeteer-core");
const chromeBrowser = await puppeteer.default.launch({
const chromeBrowser = await launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath: browser.executablePath,
protocolTimeout: captureProtocolTimeoutMs(timeout, budgetMs),
@@ -1,3 +1,4 @@
import { EventEmitter } from "node:events";
import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
const mocks = vi.hoisted(() => ({
@@ -54,6 +55,7 @@ function fakeBrowser() {
browser: {
newPage: vi.fn(async () => page),
close: vi.fn(async () => undefined),
process: vi.fn(() => new EventEmitter()),
},
page,
};
@@ -236,9 +236,9 @@ describe("screenshot Chrome arguments", () => {
);
const layoutSource = readFileSync(new URL("../commands/layout.ts", import.meta.url), "utf8");
expect(captureSource).toContain("resolveCaptureBrowserGpuMode(");
expect(captureSource).toContain("resolveManagedGpuMode(");
expect(captureSource).toContain("{ browserGpuMode: resolvedGpuMode }");
expect(layoutSource).toContain("resolveCaptureBrowserGpuMode(");
expect(layoutSource).toContain("resolveManagedGpuMode(");
expect(layoutSource).toContain("{ browserGpuMode: resolvedGpuMode }");
});
});
@@ -1,10 +1,10 @@
import { launchManagedBrowser, resolveManagedGpuMode } from "../browser/launch.js";
import { spawn } from "node:child_process";
import type { Browser, Page } from "puppeteer-core";
import { c } from "../ui/colors.js";
import {
assertWebGpuAdapterAvailable,
compositionRequiresWebGpu,
resolveCaptureBrowserGpuMode,
resolveLocalBrowserGpuMode,
type BrowserGpuMode,
} from "../browser/gpuPolicy.js";
@@ -179,8 +179,8 @@ export async function openSettledCompositionPage(
const requestedGpuMode = options.browserGpuMode ?? resolveCliChromeGpuMode();
const requiresWebGpu = compositionRequiresWebGpu(html);
const launch = async (executablePath: string): Promise<Browser> => {
const resolvedGpuMode = await resolveCaptureBrowserGpuMode(requestedGpuMode, executablePath);
return puppeteer.default.launch({
const resolvedGpuMode = await resolveManagedGpuMode(requestedGpuMode, executablePath);
return launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath,
args: buildChromeArgs(
+5 -11
View File
@@ -1,3 +1,4 @@
import { launchManagedBrowser, resolveManagedGpuMode } from "../browser/launch.js";
import { failCommand, setCommandExitCode } from "../utils/commandResult.js";
import { defineCommand } from "citty";
import { existsSync, readFileSync } from "node:fs";
@@ -203,12 +204,8 @@ async function runLayoutAudit(
const { ensureBrowser } = await import("../browser/manager.js");
const puppeteer = await import("puppeteer-core");
const { buildChromeArgs } = await import("@hyperframes/engine");
const {
assertWebGpuAdapterAvailable,
compositionRequiresWebGpu,
resolveCaptureBrowserGpuMode,
resolveLocalBrowserGpuMode,
} = await import("../browser/gpuPolicy.js");
const { assertWebGpuAdapterAvailable, compositionRequiresWebGpu, resolveLocalBrowserGpuMode } =
await import("../browser/gpuPolicy.js");
const html = await bundleProjectHtml(projectDir);
const server = await serveStaticProjectHtml(
projectDir,
@@ -220,12 +217,9 @@ async function runLayoutAudit(
try {
const browser = await ensureBrowser();
const requestedGpuMode = resolveLocalBrowserGpuMode();
const resolvedGpuMode = await resolveCaptureBrowserGpuMode(
requestedGpuMode,
browser.executablePath,
);
const resolvedGpuMode = await resolveManagedGpuMode(requestedGpuMode, browser.executablePath);
const requiresWebGpu = compositionRequiresWebGpu(html);
chromeBrowser = await puppeteer.default.launch({
chromeBrowser = await launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath: browser.executablePath,
args: buildChromeArgs(
+1 -1
View File
@@ -82,7 +82,7 @@ describe("motion-shot adapter seeking", () => {
it("launches through the shared GPU policy and WebGPU requirement guard", () => {
const source = readFileSync(motionShotSourcePath, "utf8");
expect(source).toContain("resolveCaptureBrowserGpuMode");
expect(source).toContain("resolveManagedGpuMode");
expect(source).toContain("compositionRequiresWebGpu(html)");
expect(source).toContain("assertWebGpuAdapterAvailable(page, requiresWebGpu)");
expect(source).toContain("{ browserGpuMode: resolvedGpuMode }");
+3 -3
View File
@@ -1,3 +1,4 @@
import { launchManagedBrowser, resolveManagedGpuMode } from "../browser/launch.js";
// Onion-skin motion screenshot: seek the LIVE timeline at N equal-time steps and
// project the REAL element at each step, so an agent can SELF-VERIFY motion (the
// rendered result — every channel: position, rotation, scale, opacity, colour),
@@ -17,7 +18,6 @@ import { resolveCompositionViewportFromHtml } from "../utils/compositionViewport
import {
assertWebGpuAdapterAvailable,
compositionRequiresWebGpu,
resolveCaptureBrowserGpuMode,
resolveLocalBrowserGpuMode,
} from "../browser/gpuPolicy.js";
import {
@@ -400,9 +400,9 @@ async function openCompositionPage(
const { buildChromeArgs } = await import("@hyperframes/engine");
const size = resolveCompositionViewportFromHtml(html);
const requestedGpuMode = resolveLocalBrowserGpuMode();
const resolvedGpuMode = await resolveCaptureBrowserGpuMode(requestedGpuMode, executablePath);
const resolvedGpuMode = await resolveManagedGpuMode(requestedGpuMode, executablePath);
const requiresWebGpu = compositionRequiresWebGpu(html);
const browser = await puppeteer.default.launch({
const browser = await launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath,
args: buildChromeArgs(
+5 -10
View File
@@ -1,3 +1,4 @@
import { launchManagedBrowser, resolveManagedGpuMode } from "../browser/launch.js";
// The media-metadata wait exists twice on purpose: once Node-side and once
// inside a page.evaluate() body, which is serialized into the browser and
// cannot import the Node helper. Line-level markers don't survive the clone
@@ -432,17 +433,11 @@ async function validateInBrowser(
const puppeteer = await import("puppeteer-core");
const { buildChromeArgs, analyzeClipMediaFit } = await import("@hyperframes/engine");
const requestedGpuMode = resolveCliChromeGpuMode();
const {
assertWebGpuAdapterAvailable,
compositionRequiresWebGpu,
resolveCaptureBrowserGpuMode,
} = await import("../browser/gpuPolicy.js");
const resolvedGpuMode = await resolveCaptureBrowserGpuMode(
requestedGpuMode,
browser.executablePath,
);
const { assertWebGpuAdapterAvailable, compositionRequiresWebGpu } =
await import("../browser/gpuPolicy.js");
const resolvedGpuMode = await resolveManagedGpuMode(requestedGpuMode, browser.executablePath);
const requiresWebGpu = compositionRequiresWebGpu(html);
const chromeBrowser = await puppeteer.default.launch({
const chromeBrowser = await launchManagedBrowser(puppeteer.default, {
headless: true,
executablePath: browser.executablePath,
args: buildChromeArgs(