diff --git a/packages/cli/src/beats/headlessAnalyzer.ts b/packages/cli/src/beats/headlessAnalyzer.ts index e532955bfd..5ae0f99166 100644 --- a/packages/cli/src/beats/headlessAnalyzer.ts +++ b/packages/cli/src/beats/headlessAnalyzer.ts @@ -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 { + 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); diff --git a/packages/cli/src/browser/launch.process.test.ts b/packages/cli/src/browser/launch.process.test.ts new file mode 100644 index 0000000000..1475445b77 --- /dev/null +++ b/packages/cli/src/browser/launch.process.test.ts @@ -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(); + +/** 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); +}); diff --git a/packages/cli/src/browser/launch.ts b/packages/cli/src/browser/launch.ts new file mode 100644 index 0000000000..f4b593b492 --- /dev/null +++ b/packages/cli/src/browser/launch.ts @@ -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(); +const pending = new Map, AbortController>(); +const CLOSE_GRACE_MS = 1_000; +let listening = false; +let shutdown: Promise | 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 { + 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 { + 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 { + 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 { + 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((resolve) => child.once("exit", () => resolve())); + let timer: NodeJS.Timeout | undefined; + const grace = new Promise((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); + } +} diff --git a/packages/cli/src/capture/captureAttempt.ts b/packages/cli/src/capture/captureAttempt.ts index 9303d642cb..e4c5f711fe 100644 --- a/packages/cli/src/capture/captureAttempt.ts +++ b/packages/cli/src/capture/captureAttempt.ts @@ -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), diff --git a/packages/cli/src/capture/captureCompositionFrame.browserFallback.test.ts b/packages/cli/src/capture/captureCompositionFrame.browserFallback.test.ts index 23a45a9090..41b611215c 100644 --- a/packages/cli/src/capture/captureCompositionFrame.browserFallback.test.ts +++ b/packages/cli/src/capture/captureCompositionFrame.browserFallback.test.ts @@ -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, }; diff --git a/packages/cli/src/capture/captureCompositionFrame.test.ts b/packages/cli/src/capture/captureCompositionFrame.test.ts index 7c3c0699cb..baad424554 100644 --- a/packages/cli/src/capture/captureCompositionFrame.test.ts +++ b/packages/cli/src/capture/captureCompositionFrame.test.ts @@ -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 }"); }); }); diff --git a/packages/cli/src/capture/captureCompositionFrame.ts b/packages/cli/src/capture/captureCompositionFrame.ts index 4b0aa39211..6c271f83a8 100644 --- a/packages/cli/src/capture/captureCompositionFrame.ts +++ b/packages/cli/src/capture/captureCompositionFrame.ts @@ -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 => { - 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( diff --git a/packages/cli/src/commands/layout.ts b/packages/cli/src/commands/layout.ts index 7a03d3b61e..f7416b7b70 100644 --- a/packages/cli/src/commands/layout.ts +++ b/packages/cli/src/commands/layout.ts @@ -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( diff --git a/packages/cli/src/commands/motionShot.test.ts b/packages/cli/src/commands/motionShot.test.ts index 9b6117e0d7..2c3c35e6ba 100644 --- a/packages/cli/src/commands/motionShot.test.ts +++ b/packages/cli/src/commands/motionShot.test.ts @@ -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 }"); diff --git a/packages/cli/src/commands/motionShot.ts b/packages/cli/src/commands/motionShot.ts index 36f96b5c93..12c3fe15b6 100644 --- a/packages/cli/src/commands/motionShot.ts +++ b/packages/cli/src/commands/motionShot.ts @@ -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( diff --git a/packages/cli/src/commands/validate.ts b/packages/cli/src/commands/validate.ts index ba7d7f70d0..cef5ccc74c 100644 --- a/packages/cli/src/commands/validate.ts +++ b/packages/cli/src/commands/validate.ts @@ -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(