fix(cli): check and snapshot fail fast on a missing browser path and say while Chrome downloads (#6479)

* fix(cli): check and snapshot fail fast on a missing browser path and say while Chrome downloads

* fix(cli): a non-interactive Chrome download gives up after one stall, naming the env var and doctor

packages/cli/src/browser/downloadStall.fixture.ts

@@ -1,12 +1,13 @@
 // fallow-ignore-file unused-file
 import { ensureBrowser } from "./manager.js";
 
-const [baseUrl] = process.argv.slice(2);
+const [baseUrl, attempts] = process.argv.slice(2);
 try {
   await ensureBrowser({
     preferManagedChrome: true,
     downloadStallMs: 200,
     downloadBaseUrl: baseUrl,
+    ...(attempts ? { downloadAttempts: Number(attempts) } : {}),
   });
 } catch (error) {
   process.stderr.write(`${error instanceof Error ? error.message : String(error)}\n`);

packages/cli/src/browser/downloadStall.process.test.ts

@@ -13,7 +13,7 @@ afterEach(() => {
   for (const cleanup of cleanups.splice(0)) cleanup();
 });
 
-async function runFixture(onConnection: (socket: Socket) => void) {
+async function runFixture(onConnection: (socket: Socket) => void, attempts?: number) {
   const sockets: Socket[] = [];
   const server = createServer((socket) => {
     sockets.push(socket);
@@ -26,7 +26,13 @@ async function runFixture(onConnection: (socket: Socket) => void) {
   const home = mkdtempSync(join(tmpdir(), "hf-download-stall-"));
   const child = spawn(
     process.execPath,
-    ["--import", "tsx", fixturePath, `http://127.0.0.1:${port}`],
+    [
+      "--import",
+      "tsx",
+      fixturePath,
+      `http://127.0.0.1:${port}`,
+      ...(attempts ? [String(attempts)] : []),
+    ],
     {
       env: { ...process.env, HOME: home, USERPROFILE: home, HYPERFRAMES_BROWSER_PATH: "" },
       stdio: ["ignore", "ignore", "pipe"],
@@ -51,10 +57,20 @@ async function runFixture(onConnection: (socket: Socket) => void) {
 }
 
 it("exits with code 1 when the download host accepts the connection and never answers", async () => {
-  const run = await runFixture(() => {});
+  const run = await runFixture(() => {}, 3);
 
   expect(run.code).toBe(1);
   expect(run.stderr).toContain(`The Chrome download from 127.0.0.1:${run.port} stalled`);
   expect(run.stderr).toContain("Retrying (2/2)");
   expect(existsSync(join(run.home, ".cache", "hyperframes", ".chrome.install.lock"))).toBe(false);
 }, 120_000);
+
+it("gives up after the first stall when not on a terminal, naming the env var and doctor", async () => {
+  const run = await runFixture(() => {});
+
+  expect(run.code).toBe(1);
+  expect(run.stderr).toContain(`The Chrome download from 127.0.0.1:${run.port} stalled`);
+  expect(run.stderr).not.toContain("Retrying");
+  expect(run.stderr).toContain("HYPERFRAMES_BROWSER_PATH");
+  expect(run.stderr).toContain("hyperframes doctor");
+}, 120_000);

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

@@ -1216,6 +1216,67 @@ describe("findBrowser — cache resolution", () => {
     expect(warnSpy).not.toHaveBeenCalled();
   });
 
+  it("ensureBrowser fails fast naming a configured browser path that does not exist", async () => {
+    installFsMocks({ existing: new Set(["/usr/bin/chromium"]) });
+    installPuppeteerBrowsersMock();
+    process.env["HYPERFRAMES_BROWSER_PATH"] = "/opt/typo/chrome";
+
+    const { ensureBrowser } = await import("./manager.js");
+
+    await expect(ensureBrowser()).rejects.toThrow(
+      "HYPERFRAMES_BROWSER_PATH is set to /opt/typo/chrome, which does not exist",
+    );
+  });
+
+  it("retryOnStall gives up after one 75 s stall when attempts is 1, and retries to ~195 s when 3", async () => {
+    vi.useFakeTimers();
+    const warn = vi.spyOn(console, "warn").mockImplementation(() => {});
+    try {
+      const { retryOnStall } = await import("./manager.js");
+      const { BrowserDownloadStalledError } = await import("./installInChild.js");
+      const stalled = () =>
+        new Promise<never>((_, reject) =>
+          setTimeout(() => reject(new BrowserDownloadStalledError("stalled")), 75_000),
+        );
+      const once = vi.fn(stalled);
+      const first = expect(retryOnStall(once, () => {}, 1)).rejects.toThrow("stalled");
+      await vi.advanceTimersByTimeAsync(75_000);
+      await first;
+      expect(once).toHaveBeenCalledTimes(1);
+
+      const thrice = vi.fn(stalled);
+      const all = expect(retryOnStall(thrice, () => {}, 3)).rejects.toThrow("stalled");
+      await vi.advanceTimersByTimeAsync(3 * 75_000);
+      await all;
+      expect(thrice).toHaveBeenCalledTimes(3);
+    } finally {
+      warn.mockRestore();
+      vi.useRealTimers();
+    }
+  });
+
+  it("withBrowserWaitNotice speaks every 15 s while the task runs and stops when it ends", async () => {
+    vi.useFakeTimers();
+    try {
+      const notify = vi.fn();
+      let finish = () => {};
+      const task = new Promise<string>((resolve) => (finish = () => resolve("done")));
+      const { withBrowserWaitNotice } = await import("./manager.js");
+      const result = withBrowserWaitNotice(task, notify, 15_000);
+      await vi.advanceTimersByTimeAsync(14_000);
+      expect(notify).not.toHaveBeenCalled();
+      await vi.advanceTimersByTimeAsync(31_000);
+      expect(notify).toHaveBeenCalledTimes(3);
+      expect(notify.mock.calls[0]?.[0]).toContain("HYPERFRAMES_BROWSER_PATH");
+      finish();
+      await expect(result).resolves.toBe("done");
+      await vi.advanceTimersByTimeAsync(60_000);
+      expect(notify).toHaveBeenCalledTimes(3);
+    } finally {
+      vi.useRealTimers();
+    }
+  });
+
   it("prefers HYPERFRAMES_BROWSER_PATH over PRODUCER_HEADLESS_SHELL_PATH when both are set", async () => {
     // Tiebreak matches `render.ts` — the CLI-native name canonicalizes; the
     // engine name is a compatibility alias. If both are set the caller almost

packages/cli/src/browser/manager.ts

@@ -229,6 +229,8 @@ export interface BrowserResult {
 export interface EnsureBrowserOptions {
   onProgress?: (downloadedBytes: number, totalBytes: number) => void;
   downloadStallMs?: number;
+  // Stalled-download attempts; default 3 on a terminal, 1 elsewhere (see ensureBrowser).
+  downloadAttempts?: number;
   downloadBaseUrl?: string;
   // Re-download the managed build even when cached; only its version dir is replaced.
   force?: boolean;
@@ -542,15 +544,52 @@ async function ensureLinuxArmBrowser(options?: EnsureBrowserOptions): Promise<Br
  * With `preferManagedChrome`: env var -> OUR pinned cache -> auto-download
  * (puppeteer-cache preference and system Chrome are both skipped).
  */
+const BROWSER_WAIT_NOTICE_MS = 15_000;
+
+/** Runs `task`, saying so every 15 s while it is still going, so a first-use download is never silent. */
+export async function withBrowserWaitNotice<T>(
+  task: Promise<T>,
+  notify: (message: string) => void = console.warn,
+  everyMs = BROWSER_WAIT_NOTICE_MS,
+): Promise<T> {
+  const timer = setInterval(
+    () =>
+      notify(
+        "[hyperframes] Still getting Chrome ready; the first use downloads it. " +
+          "Set HYPERFRAMES_BROWSER_PATH to use an installed Chrome instead.",
+      ),
+    everyMs,
+  );
+  timer.unref();
+  try {
+    return await task;
+  } finally {
+    clearInterval(timer);
+  }
+}
+
 export async function ensureBrowser(options?: EnsureBrowserOptions): Promise<BrowserResult> {
-  if (options?.signal) return ensureBrowserInOwnedProcess(options);
-  return ensureBrowserInCurrentProcess(options);
+  // Tools that time out at 120 s lose to three stalled attempts (~4 min); one stall then a clear error wins.
+  const resolved = {
+    ...options,
+    downloadAttempts: options?.downloadAttempts ?? (process.stdout.isTTY ? DOWNLOAD_ATTEMPTS : 1),
+  };
+  const task = resolved.signal
+    ? ensureBrowserInOwnedProcess(resolved)
+    : ensureBrowserInCurrentProcess(resolved);
+  return resolved.onProgress ? task : withBrowserWaitNotice(task);
 }
 
 // fallow-ignore-next-line complexity
 async function ensureBrowserInCurrentProcess(
   options?: EnsureBrowserOptions,
 ): Promise<BrowserResult> {
+  const configured = browserPathFromEnv();
+  if (configured && !existsSync(configured.path)) {
+    throw new Error(
+      `${configured.name} is set to ${configured.path}, which does not exist. Fix the path or unset it.`,
+    );
+  }
   const fromEnv = findFromEnv();
   if (fromEnv) return fromEnv;
 
@@ -600,6 +639,7 @@ async function ensureBrowserInOwnedProcess(options: EnsureBrowserOptions): Promi
       {
         force: options.force,
         preferManagedChrome: options.preferManagedChrome,
+        downloadAttempts: options.downloadAttempts,
       },
       {
         signal,
@@ -689,7 +729,7 @@ function wrapDownloadFailureWithBrowserPathHint(
     `Failed to install chrome-headless-shell ${managedChromeVersion()}: ${original}${unzipErrors}\n\n` +
     `Point hyperframes at an already-installed Chrome/Chromium instead:\n\n` +
     `  export HYPERFRAMES_BROWSER_PATH="${example}"\n\n` +
-    `Then re-run your command. Any Chrome build works for the screenshot ` +
+    `Run \`hyperframes doctor\` to see which Chrome was found. Then re-run your command. Any Chrome build works for the screenshot ` +
     `capture path; install a real chrome-headless-shell later if you need the ` +
     `perf-optimized BeginFrame path. Alternatively, run inside the hyperframes ` +
     `Docker image which ships a compatible headless-shell.`;
@@ -699,13 +739,17 @@ function wrapDownloadFailureWithBrowserPathHint(
 const DOWNLOAD_STALL_MS = 60_000;
 const DOWNLOAD_ATTEMPTS = 3;
 
-async function retryOnStall<T>(run: () => Promise<T>, clear: () => void): Promise<T> {
+export async function retryOnStall<T>(
+  run: () => Promise<T>,
+  clear: () => void,
+  attempts = DOWNLOAD_ATTEMPTS,
+): Promise<T> {
   for (let attempt = 1; ; attempt++) {
     try {
       return await run();
     } catch (err) {
-      if (!(err instanceof BrowserDownloadStalledError) || attempt >= DOWNLOAD_ATTEMPTS) throw err;
-      console.warn(`[hyperframes] ${err.message} Retrying (${attempt}/${DOWNLOAD_ATTEMPTS - 1}).`);
+      if (!(err instanceof BrowserDownloadStalledError) || attempt >= attempts) throw err;
+      console.warn(`[hyperframes] ${err.message} Retrying (${attempt}/${attempts - 1}).`);
       clear();
     }
   }
@@ -804,6 +848,7 @@ async function downloadBrowser(options?: EnsureBrowserOptions): Promise<BrowserR
           ),
         ),
       clearStaging,
+      options?.downloadAttempts,
     );
     assertExecutableUnpacked(stagingDir, staged.executablePath);
     unzipLog.lines = [];