fix(cli): closing the managed Chrome also ends the helper processes it leaves behind (#6542)

* fix(cli): closing the managed Chrome also ends the helper processes it leaves behind

The close waited a second for Chrome to exit and then killed only Chrome's own process.
Under load that second is often not enough, and the helpers (the GPU and renderer
processes, crashpad_handler) were reparented to init and kept running after a finished
run. The close now records the processes under Chrome before it starts, and ends any of
them still running (under the same command) once Chrome is gone.

* test(cli): the helper-reaping test polls for the helper's exit instead of waiting 200 ms

* fix(cli): the Chrome process listing is bounded and no longer runs at import

The ps call that records Chrome's helpers ran with no time limit before the
bounded browser close, so a stalled ps made the close unbounded. It now has a
2 s limit, and a timeout records no helpers.

execFile is resolved when ps runs, not when the module loads, so tests that
mock node:child_process without execFile import it again.

packages/cli/src/browser/chromeTree.mocked.test.ts

@@ -0,0 +1,30 @@
+import { describe, expect, it, vi } from "vitest";
+
+const execFile = vi.hoisted(() => vi.fn());
+vi.mock("node:child_process", () => ({ execFile }));
+
+import { chromeDescendants, reapChromeTree } from "./chromeTree.js";
+
+const psFails = (error: Error & { killed?: boolean }) =>
+  execFile.mockImplementation((...args: unknown[]) => {
+    (args.at(-1) as (e: Error) => void)(error);
+  });
+
+describe("chromeTree ps call", () => {
+  it("is bounded, and a ps killed at the timeout records no descendants", async () => {
+    psFails(Object.assign(new Error("ps timed out"), { killed: true }));
+    await expect(chromeDescendants(1)).resolves.toEqual([]);
+    expect(execFile.mock.calls[0]?.[2]).toMatchObject({ timeout: expect.any(Number) });
+  });
+
+  it("does not hide other ps failures", async () => {
+    psFails(new Error("spawn ps ENOENT"));
+    await expect(chromeDescendants(1)).rejects.toThrow("ENOENT");
+  });
+
+  it("reaps nothing without running ps when no member was recorded", async () => {
+    execFile.mockClear();
+    await reapChromeTree([]);
+    expect(execFile).not.toHaveBeenCalled();
+  });
+});

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

@@ -0,0 +1,45 @@
+import { spawn } from "node:child_process";
+import { expect, it } from "vitest";
+import type { Browser } from "puppeteer-core";
+import { waitForTestCondition } from "../utils/processTestUtils.js";
+import { closeManagedBrowser } from "./launch.js";
+
+const alive = (pid: number) => {
+  try {
+    process.kill(pid, 0);
+    return true;
+  } catch {
+    return false;
+  }
+};
+
+it.runIf(process.platform !== "win32")(
+  "closing a browser also ends the helper processes it leaves behind",
+  async () => {
+    const main = spawn(
+      process.execPath,
+      [
+        "-e",
+        `const h=require("node:child_process").spawn("sleep",["60"],{detached:true,stdio:"ignore"});h.unref();console.log(h.pid);setInterval(()=>{},1e6)`,
+      ],
+      { stdio: ["ignore", "pipe", "ignore"] },
+    );
+    const helper = Number(
+      await new Promise<string>((resolve) => main.stdout!.once("data", (d) => resolve(String(d)))),
+    );
+    expect(alive(helper)).toBe(true);
+    const browser = {
+      process: () => main,
+      close: async () => {
+        main.kill("SIGKILL");
+      },
+    } as unknown as Browser;
+    try {
+      await closeManagedBrowser(browser);
+      await waitForTestCondition(() => !alive(helper), 5_000);
+    } finally {
+      if (alive(helper)) process.kill(helper, "SIGKILL");
+    }
+  },
+  30_000,
+);

packages/cli/src/browser/chromeTree.ts

@@ -0,0 +1,57 @@
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+
+const PS_TIMEOUT_MS = 2_000;
+
+export interface TreeMember {
+  pid: number;
+  command: string;
+}
+
+async function processTable(): Promise<Array<TreeMember & { ppid: number }>> {
+  let stdout: string;
+  try {
+    ({ stdout } = await promisify(execFile)("ps", ["-axo", "pid=,ppid=,command="], {
+      maxBuffer: 16 * 1024 * 1024,
+      timeout: PS_TIMEOUT_MS,
+    }));
+  } catch (error) {
+    // A stalled ps is killed at the timeout: nothing recorded, so the close stays bounded.
+    if ((error as { killed?: boolean }).killed) return [];
+    throw error;
+  }
+  return stdout.split("\n").flatMap((line) => {
+    const row = /^\s*(\d+)\s+(\d+)\s+(.*)$/.exec(line);
+    return row ? [{ pid: Number(row[1]), ppid: Number(row[2]), command: row[3]! }] : [];
+  });
+}
+
+/** Every process under `rootPid` now, so they can be ended after the root is gone and they are reparented to init. */
+export async function chromeDescendants(rootPid: number): Promise<TreeMember[]> {
+  const table = await processTable();
+  const found: TreeMember[] = [];
+  const queue = [rootPid];
+  while (queue.length > 0) {
+    const parent = queue.shift()!;
+    for (const row of table) {
+      if (row.ppid !== parent) continue;
+      found.push({ pid: row.pid, command: row.command });
+      queue.push(row.pid);
+    }
+  }
+  return found;
+}
+
+/** Ends the members still running under the command they had, so a reused pid is left alone. */
+export async function reapChromeTree(members: readonly TreeMember[]): Promise<void> {
+  if (members.length === 0) return;
+  const commands = new Map((await processTable()).map((row) => [row.pid, row.command]));
+  for (const { pid, command } of members) {
+    if (commands.get(pid) !== command) continue;
+    try {
+      process.kill(pid, "SIGKILL");
+    } catch {
+      /* already gone */
+    }
+  }
+}

packages/cli/src/browser/launch.ts

@@ -9,6 +9,7 @@ import {
 import { normalizeErrorMessage } from "../utils/errorMessage.js";
 import { requestCliExit } from "../utils/commandResult.js";
 import { processIsAlive } from "../utils/processTree.js";
+import { chromeDescendants, reapChromeTree } from "./chromeTree.js";
 import { watchChromeTree, type ChromeWatchdog } from "./windowsChromeWatchdog.js";
 import {
   inAgentSandbox,
@@ -259,10 +260,16 @@ async function closeBrowser(browser: Browser): Promise<void> {
   }
 }
 
+async function descendantsToReap(pid: number | undefined) {
+  if (pid === undefined || process.platform === "win32") return [];
+  return chromeDescendants(pid).catch(() => []);
+}
+
 async function closedWithoutKill(browser: Browser): Promise<boolean> {
   const child = browser.process();
   if (!child) throw new Error("A managed browser lost its owned process.");
   if (child.exitCode !== null || child.signalCode !== null) return false;
+  const tree = await descendantsToReap(child.pid);
   const exited = new Promise<void>((resolve) => child.once("exit", () => resolve()));
   let timer: NodeJS.Timeout | undefined;
   let poll: NodeJS.Timeout | undefined;
@@ -289,6 +296,7 @@ async function closedWithoutKill(browser: Browser): Promise<boolean> {
       }, EXIT_POLL_MS);
     });
     await Promise.race([exited, goneBeforeExitEvent, exitLimit]);
+    await reapChromeTree(tree).catch(() => undefined);
     return clean;
   } finally {
     clearTimeout(timer);