fix(cli): every managed browser closes within a second even if Chrome never exits (#6553)

* fix(cli): every managed browser closes within a second even if Chrome never exits

Commands called browser.close() directly, which waits for the Chrome process to exit without a limit. A headless shell that acknowledged Browser.close and then stayed alive left the CLI hanging after its work was done. The managed browser's close() is now the bounded close that kills a browser that has not exited.

* test(cli): browser stand-ins carry the close() a real browser has

packages/cli/src/browser/launch.boundedClose.test.ts

@@ -0,0 +1,20 @@
+import { spawn } from "node:child_process";
+import { once } from "node:events";
+import type { Browser, LaunchOptions, PuppeteerNode } from "puppeteer-core";
+import { expect, it } from "vitest";
+import { launchManagedBrowser } from "./launch.js";
+
+it("kills a browser that acknowledged close but never exits, so every caller's close() returns", async () => {
+  const child = spawn(process.execPath, ["-e", "setTimeout(() => {}, 60000)"], { stdio: "pipe" });
+  const browser = { process: () => child, close: () => once(child, "exit") } as unknown as Browser;
+  const puppeteer = {
+    launch: async (_options: LaunchOptions) => browser,
+  } as unknown as PuppeteerNode;
+  try {
+    const launched = await launchManagedBrowser(puppeteer, { headless: true }, { diagnose: false });
+    await launched.close();
+    expect(child.signalCode).toBe("SIGKILL");
+  } finally {
+    child.kill("SIGKILL");
+  }
+});

packages/cli/src/browser/launch.noListen.test.ts

@@ -15,7 +15,7 @@ vi.mock("./gpuPolicy.js", async (actual) => ({
     const puppeteer = {
       launch: async (options: LaunchOptions) => {
         probe.options.push(options);
-        return { process: () => new EventEmitter() };
+        return { process: () => new EventEmitter(), close: async () => {} };
       },
     } as unknown as PuppeteerNode;
     await hooks.launch(puppeteer, { headless: true, args: ["--headless"] });

packages/cli/src/browser/launch.pipeEof.test.ts

@@ -15,7 +15,7 @@ it("closes the browser's pipes when it exits, so an orphaned helper cannot hold
     stdio: ["pipe", "pipe", "pipe", "pipe", "pipe"],
   });
   const puppeteer = {
-    launch: async (_options: LaunchOptions) => ({ process: () => child }),
+    launch: async (_options: LaunchOptions) => ({ process: () => child, close: async () => {} }),
   } as unknown as PuppeteerNode;
   await launchManagedBrowser(puppeteer, { headless: true }, { diagnose: false });
   await expect

packages/cli/src/browser/launch.ts

@@ -26,6 +26,7 @@ export class BrowserCleanupError extends AggregateError {}
 const browsers = new Set<Browser>();
 const closing = new WeakMap<Browser, Promise<void>>();
 const watchdogs = new WeakMap<Browser, ChromeWatchdog>();
+const gracefulClose = new WeakMap<Browser, () => Promise<void>>();
 const pending = new Map<Promise<Browser>, AbortController>();
 const diagnosing = new Set<AbortController>();
 const sessionCleanups = new Set<() => Promise<void>>();
@@ -193,6 +194,8 @@ async function launchOwnedBrowser(
     browsers.add(browser);
     const watchdog = watchChromeTree(child, launchedAt);
     if (watchdog) watchdogs.set(browser, watchdog);
+    gracefulClose.set(browser, browser.close.bind(browser));
+    browser.close = () => closeManagedBrowser(browser);
     child.once("exit", () => {
       browsers.delete(browser);
       destroyBrowserPipes(child);
@@ -273,6 +276,10 @@ async function descendantsToReap(pid: number | undefined) {
   return chromeDescendants(pid).catch(() => []);
 }
 
+function closeGracefully(browser: Browser): Promise<void> {
+  return (gracefulClose.get(browser) ?? (() => browser.close()))();
+}
+
 async function closedWithoutKill(browser: Browser): Promise<boolean> {
   const child = browser.process();
   if (!child) throw new Error("A managed browser lost its owned process.");
@@ -286,7 +293,9 @@ async function closedWithoutKill(browser: Browser): Promise<boolean> {
   });
   try {
     await Promise.race([
-      browser.close().catch((error: unknown) => console.warn("Browser close failed:", error)),
+      closeGracefully(browser).catch((error: unknown) =>
+        console.warn("Browser close failed:", error),
+      ),
       grace,
     ]);
     const clean = child.exitCode !== null || child.signalCode !== null;

packages/cli/src/browser/sandboxDenied.test.ts

@@ -37,7 +37,8 @@ describe("a browser launch a sandbox refuses", () => {
   });
 });
 
-const fakeBrowser = () => ({ process: () => new EventEmitter() }) as unknown as Browser;
+const fakeBrowser = () =>
+  ({ process: () => new EventEmitter(), close: async () => {} }) as unknown as Browser;
 
 function recordingPuppeteer(failures: (string | Error)[]) {
   const launches: LaunchOptions[] = [];

packages/cli/src/capture/launchCaptureBrowser.signal.test.ts

@@ -33,14 +33,12 @@ it.each(["SIGINT", "SIGTERM"] as const)(
       exitCode: null as number | null,
       signalCode: null,
     });
-    const browser = Object.assign(new EventEmitter(), {
-      process: () => child,
-      close: vi.fn(async () => {
-        child.exitCode = 0;
-        child.emit("exit", 0);
-        browser.emit("disconnected");
-      }),
+    const close = vi.fn(async () => {
+      child.exitCode = 0;
+      child.emit("exit", 0);
+      browser.emit("disconnected");
     });
+    const browser = Object.assign(new EventEmitter(), { process: () => child, close });
     mocks.launch.mockResolvedValue(browser);
     mocks.proxyClose.mockResolvedValue(undefined);
     const { launchCaptureBrowser } = await import("./launchCaptureBrowser.js");
@@ -62,6 +60,6 @@ it.each(["SIGINT", "SIGTERM"] as const)(
     expect(await exit).toEqual({ code: signal === "SIGINT" ? 130 : 143, profileExists: false });
     await session.close();
     await session.close();
-    expect(browser.close).toHaveBeenCalledOnce();
+    expect(close).toHaveBeenCalledOnce();
   },
 );