From 952be1d5f103ab903d85ab0d08fe90ea08730211 Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Sun, 13 Sep 2026 03:19:37 +0000 Subject: [PATCH 1/3] fix: make doctor's Chrome check and browser path predict render's resolution doctor's Chrome check and `hyperframes browser path` both called the public findBrowser() with no options, which resolves loosely: any puppeteer-cached chrome-headless-shell version, then the hyperframes-managed cache (exact CHROME_VERSION pin), then system Chrome. Render's actual resolution (ensureBrowser({ preferManagedChrome: true })) is stricter -- it only accepts the exact-pinned managed-cache build, skipping both the puppeteer-cache and system-Chrome fallbacks. So doctor/browser path could report a "cache hit" that render doesn't honor at all, immediately before an unexpected re-download. Adds an optional preferManagedChrome option to findBrowser(), mirroring ensureBrowser's existing conditional shape, and threads it through doctor's underlying checkChrome() and browser path's lookup (plus its own not-found fallback). Skipped on Linux ARM64, which has no managed chrome-headless-shell build at all -- forcing preferManagedChrome there would report "not found" even with a correctly installed system Chromium, the exact false-negative this check must avoid producing. Co-Authored-By: Miguel Angel --- packages/cli/src/browser/manager.test.ts | 77 ++++++++++++++++++++++ packages/cli/src/browser/manager.ts | 19 +++++- packages/cli/src/browser/preflight.test.ts | 45 +++++++++++++ packages/cli/src/browser/preflight.ts | 14 +++- packages/cli/src/commands/browser.ts | 6 +- 5 files changed, 155 insertions(+), 6 deletions(-) diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 987493d0fa..49c547a54d 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -843,6 +843,83 @@ describe("findBrowser — cache resolution", () => { }); }); +describe("findBrowser — preferManagedChrome", () => { + const origPlatform = process.platform; + const origArch = process.arch; + + beforeEach(() => { + vi.resetModules(); + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + Object.defineProperty(process, "arch", { value: "x64", configurable: true }); + delete process.env["HYPERFRAMES_BROWSER_PATH"]; + delete process.env["PRODUCER_HEADLESS_SHELL_PATH"]; + installChildProcessMocks(); + }); + + afterEach(() => { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + Object.defineProperty(process, "arch", { value: origArch, configurable: true }); + vi.restoreAllMocks(); + vi.doUnmock("node:fs"); + vi.doUnmock("node:os"); + vi.doUnmock("node:child_process"); + vi.doUnmock("@puppeteer/browsers"); + }); + + it("ignores a puppeteer-cache hit and resolves to the pinned hyperframes cache instead", async () => { + // Same "both populated" fixture as the unqualified test above, except the + // HF-cache entry is pinned to CHROME_VERSION so it's actually a valid + // match — proving preferManagedChrome skips the puppeteer cache entirely + // rather than merely losing a tiebreak to it. + installFsMocks({ + existing: new Set([HF_CACHE, HF_BINARY, PUPPETEER_CACHE, PUPPETEER_BINARY]), + dirs: { [PUPPETEER_CACHE]: ["linux-148.0.7778.97"] }, + }); + installPuppeteerBrowsersMock({ + installedInHfCache: [ + { browser: "chrome-headless-shell", executablePath: HF_BINARY, buildId: CHROME_VERSION }, + ], + }); + + const { findBrowser } = await import("./manager.js"); + + expect(await findBrowser()).toEqual({ executablePath: PUPPETEER_BINARY, source: "cache" }); + expect(await findBrowser({ preferManagedChrome: true })).toEqual({ + executablePath: HF_BINARY, + source: "cache", + }); + }); + + it("does not report a false cache hit against a puppeteer-cache build off the pinned version", async () => { + // The exact reported divergence: doctor's unqualified check accepted any + // puppeteer-cached version as "found", while a preferManagedChrome render + // only accepts the pinned build and would re-download here. + installFsMocks({ + existing: new Set([PUPPETEER_CACHE, PUPPETEER_BINARY]), + dirs: { [PUPPETEER_CACHE]: ["linux-148.0.7778.97"] }, + }); + installPuppeteerBrowsersMock(); + + const { findBrowser } = await import("./manager.js"); + + expect(await findBrowser()).toEqual({ executablePath: PUPPETEER_BINARY, source: "cache" }); + expect(await findBrowser({ preferManagedChrome: true })).toBeUndefined(); + }); + + it("does not fall back to system Chrome, unlike the unqualified resolution", async () => { + installFsMocks({ existing: new Set([SYSTEM_CHROME]) }); + installPuppeteerBrowsersMock(); + // The unqualified call below takes the system-Chrome path, which warns. + vi.spyOn(console, "warn").mockImplementation(() => {}); + + const { findBrowser, _resetSystemFallbackWarnForTests } = await import("./manager.js"); + _resetSystemFallbackWarnForTests(); + + expect(await findBrowser()).toEqual({ executablePath: SYSTEM_CHROME, source: "system" }); + expect(await findBrowser({ preferManagedChrome: true })).toBeUndefined(); + }); +}); + describe("isCorruptArchiveError", () => { it("matches truncated / corrupt archive extraction failures", async () => { const { isCorruptArchiveError } = await import("./manager.js"); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 1780459851..8d1ace7ae6 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -526,12 +526,22 @@ export function findSystemBrowser(): BrowserResult | undefined { /** * Find an existing browser without downloading. * Resolution: env var -> cached download -> system Chrome. + * With `preferManagedChrome`: env var -> OUR pinned cache only (puppeteer-cache + * preference and system Chrome are both skipped) — the same restriction + * `ensureBrowser` applies, minus the auto-download this function never does. + * Pass it when a "found" report has to predict what a `preferManagedChrome` + * render will actually use (`doctor`'s Chrome check, `browser path`): the + * unqualified resolution reports hits that such a render re-downloads over. */ -export async function findBrowser(): Promise { +export async function findBrowser( + options?: Pick, +): Promise { const fromEnv = findFromEnv(); if (fromEnv) return fromEnv; - const fromCache = await findFromCache(); + const fromCache = await (options?.preferManagedChrome + ? findFromHyperframesCache() + : findFromCache()); if (fromCache.result) return fromCache.result; if (fromCache.staleHyperframesCachePath) { console.warn( @@ -551,6 +561,11 @@ export async function findBrowser(): Promise { } } + // A `preferManagedChrome` render never falls back to system Chrome (see + // `ensureBrowser`), so reporting it here would only relocate the false hit + // from the puppeteer cache to system Chrome, not remove it. + if (options?.preferManagedChrome) return undefined; + const fromSystem = findSystemBrowser(); if (fromSystem) { warnSystemFallbackOnce(fromSystem.executablePath); diff --git a/packages/cli/src/browser/preflight.test.ts b/packages/cli/src/browser/preflight.test.ts index 02e0d6b7ff..28dbb938e6 100644 --- a/packages/cli/src/browser/preflight.test.ts +++ b/packages/cli/src/browser/preflight.test.ts @@ -104,6 +104,51 @@ describe("runEnvironmentChecks", () => { }); }); + it("resolves Chrome via preferManagedChrome so a reported hit predicts what render will actually use", async () => { + // Explicit false rather than relying on the test runner's actual arch — + // the ARM64-skip counterpart test below only makes sense as a contrast + // if this one is pinned to the non-ARM64 branch regardless of host. + const armSpy = vi.spyOn(manager, "isLinuxArm").mockReturnValue(false); + const spy = vi.spyOn(manager, "findBrowser").mockResolvedValue({ + executablePath: process.execPath, + source: "cache", + }); + + try { + await runEnvironmentChecks({ includeBrowser: true }); + + expect(spy).toHaveBeenCalledWith({ preferManagedChrome: true }); + } finally { + armSpy.mockRestore(); + spy.mockRestore(); + } + }); + + it("skips preferManagedChrome on Linux ARM64, which has no hyperframes-managed cache to check", async () => { + // preferManagedChrome would always report "not found" here — Chrome for + // Testing publishes no linux-arm64 build — even on a machine with a + // correctly apt-get-installed system Chromium, the exact false-negative + // this check must not produce (ensureLinuxArmBrowser's own resolution + // stays unqualified for the same reason). + const armSpy = vi.spyOn(manager, "isLinuxArm").mockReturnValue(true); + const findSpy = vi.spyOn(manager, "findBrowser").mockResolvedValue({ + executablePath: "/usr/bin/chromium-browser", + source: "system", + }); + + try { + const result = await runEnvironmentChecks({ includeBrowser: true }); + + expect(findSpy).toHaveBeenCalledWith(undefined); + expect(result.outcomes.find((outcome) => outcome.name === "Chrome")).toMatchObject({ + ok: true, + }); + } finally { + armSpy.mockRestore(); + findSpy.mockRestore(); + } + }); + it("reports Chrome as not found (no throw) when browser discovery throws on a corrupt cache", async () => { const spy = vi.spyOn(manager, "findBrowser").mockRejectedValue( Object.assign(new Error("ENOTDIR: not a directory, scandir 'chrome-headless-shell'"), { diff --git a/packages/cli/src/browser/preflight.ts b/packages/cli/src/browser/preflight.ts index fabcd97ca2..bdd6fd20ed 100644 --- a/packages/cli/src/browser/preflight.ts +++ b/packages/cli/src/browser/preflight.ts @@ -1,7 +1,7 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import { platform } from "node:os"; -import { findBrowser, type BrowserResult } from "./manager.js"; +import { findBrowser, isLinuxArm, type BrowserResult } from "./manager.js"; import { FFMPEG_PATH_ENV, FFPROBE_PATH_ENV, @@ -254,7 +254,17 @@ async function checkChrome(browserPath?: string): Promise>; try { - info = await findBrowser(); + // `preferManagedChrome` so a hit here predicts what render's + // `ensureBrowser({ preferManagedChrome: true })` will use — the unqualified + // resolution also accepts any puppeteer-cached version or system Chrome, + // neither of which render's pinned-version path honors. Skipped on Linux + // ARM64: there is no hyperframes-managed chrome-headless-shell build for + // that platform at all (Chrome for Testing doesn't publish linux-arm64), + // so `preferManagedChrome` there would always report "not found" even on + // a machine with a correctly apt-get-installed system Chromium — the + // exact false-negative this check exists to avoid, not produce. Matches + // `ensureLinuxArmBrowser`'s own unqualified resolution on that platform. + info = await findBrowser(isLinuxArm() ? undefined : { preferManagedChrome: true }); } catch { info = undefined; } diff --git a/packages/cli/src/commands/browser.ts b/packages/cli/src/commands/browser.ts index 6be367d3ae..d351e4ecbd 100644 --- a/packages/cli/src/commands/browser.ts +++ b/packages/cli/src/commands/browser.ts @@ -126,11 +126,13 @@ async function runEnsure(options?: { force?: boolean }): Promise { } async function runPath(): Promise { - const result = await findBrowser(); + // `preferManagedChrome` on both resolutions below so the printed path is the + // one `render` will actually use — same reasoning as `runEnsure` above. + const result = await findBrowser({ preferManagedChrome: true }); if (!result) { // Try a full ensure (which includes download) but write only the path try { - const ensured = await ensureBrowser(); + const ensured = await ensureBrowser({ preferManagedChrome: true }); process.stdout.write(ensured.executablePath + "\n"); } catch (err: unknown) { trackCommandFailure("browser", err); From 0cb180fa03eb07a31b63ee683e515a010a814f9e Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Tue, 15 Sep 2026 03:26:52 +0000 Subject: [PATCH 2/3] fix(cli): resolve the ARM64 browser exception once inside findBrowser MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The "Linux ARM64 has no managed chrome-headless-shell, so resolve unqualified there" rule was spelled three ways: explicitly in doctor's checkChrome, explicitly in `browser ensure`'s ARM64 branch, and only implicitly in `browser path`, which reached system Chromium on ARM64 by falling into ensureBrowser's download path and being rerouted. Move the decision into manager.ts (`resolvesManagedOnly`), shared by findBrowser and ensureBrowser, so callers pass the plain `preferManagedChrome` option and a render on ARM64 no longer takes the install lock just to find the system Chromium it could have found up front. findBrowser is documented as "find without downloading", but its stale-cache branch (manifest present, executable missing) triggered a full re-download — from doctor. It now reports that state as not found; ensureBrowser owns the re-download, and both `browser path` and doctor already route a miss there. Tests: findBrowser({ preferManagedChrome }) on Linux ARM64 still finds system Chromium; ensureBrowser does too without taking the install lock; `browser path` on ARM64 prints the system path without entering ensureBrowser; env-var override still wins under preferManagedChrome; a stale managed-cache entry makes findBrowser return undefined without calling install(). Co-Authored-By: Miguel Ángel --- packages/cli/src/browser/manager.test.ts | 93 +++++++++++++++++++++- packages/cli/src/browser/manager.ts | 65 ++++++++------- packages/cli/src/browser/preflight.test.ts | 30 ------- packages/cli/src/browser/preflight.ts | 14 +--- packages/cli/src/commands/browser.test.ts | 79 ++++++++++++++++++ 5 files changed, 204 insertions(+), 77 deletions(-) create mode 100644 packages/cli/src/commands/browser.test.ts diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 49c547a54d..180a4a2bd3 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -300,7 +300,7 @@ describe("findBrowser — cache resolution", () => { expect(result).toEqual({ executablePath: macArm64Binary, source: "cache" }); }); - it("re-downloads when the hyperframes cache manifest points at a missing binary", async () => { + it("ensureBrowser re-downloads when the hyperframes cache manifest points at a missing binary", async () => { const redownloadedBinary = join( HF_CACHE, "chrome-headless-shell", @@ -326,8 +326,8 @@ describe("findBrowser — cache resolution", () => { }); const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}); - const { findBrowser } = await import("./manager.js"); - const result = await findBrowser(); + const { ensureBrowser } = await import("./manager.js"); + const result = await ensureBrowser(); expect(result).toEqual({ executablePath: redownloadedBinary, source: "download" }); expect(warnSpy).toHaveBeenCalledWith(expect.stringContaining("Cached binary missing")); @@ -337,6 +337,32 @@ describe("findBrowser — cache resolution", () => { expect(paths.has(staleInstallDir)).toBe(false); }); + it("findBrowser reports a stale hyperframes-cache entry as not found instead of downloading", async () => { + // findBrowser is the find-only half of the API (doctor, `browser path`): + // a manifest whose executable is gone must not trigger a download from a + // diagnostic, and must not be reported as a hit either. + const staleInstallDir = join(HF_CACHE, "chrome-headless-shell", "linux-131.0.6778.85"); + installFsMocks({ existing: new Set([HF_CACHE, staleInstallDir]) }); + const install = vi.fn(async () => ({ executablePath: HF_BINARY })); + installPuppeteerBrowsersMock({ + installedInHfCache: [ + { + browser: "chrome-headless-shell", + executablePath: HF_BINARY, + path: staleInstallDir, + buildId: CHROME_VERSION, + }, + ], + installImpl: install, + }); + + const { findBrowser } = await import("./manager.js"); + + expect(await findBrowser()).toBeUndefined(); + expect(await findBrowser({ preferManagedChrome: true })).toBeUndefined(); + expect(install).not.toHaveBeenCalled(); + }); + it("ensureBrowser({force: true}) purges the whole cache before downloading, bypassing any cache/system shortcut", async () => { const staleInstallDir = join(HF_CACHE, "chrome-headless-shell", "linux-131.0.6778.85"); const downloadedBinary = join(HF_CACHE, "chrome-headless-shell", "force-downloaded"); @@ -918,6 +944,67 @@ describe("findBrowser — preferManagedChrome", () => { expect(await findBrowser()).toEqual({ executablePath: SYSTEM_CHROME, source: "system" }); expect(await findBrowser({ preferManagedChrome: true })).toBeUndefined(); }); + + it("still resolves the env var override first", async () => { + // An explicit HYPERFRAMES_BROWSER_PATH must win even when the pinned + // managed cache is populated — same precedence as ensureBrowser. + const envBinary = join(FAKE_HOME, "custom", "chrome-headless-shell"); + process.env["HYPERFRAMES_BROWSER_PATH"] = envBinary; + installFsMocks({ existing: new Set([HF_CACHE, HF_BINARY, envBinary]) }); + installPuppeteerBrowsersMock({ + installedInHfCache: [ + { browser: "chrome-headless-shell", executablePath: HF_BINARY, buildId: CHROME_VERSION }, + ], + }); + + const { findBrowser } = await import("./manager.js"); + + expect(await findBrowser({ preferManagedChrome: true })).toEqual({ + executablePath: envBinary, + source: "env", + }); + }); + + it("still finds system Chromium on Linux ARM64, where no managed build exists", async () => { + // Chrome for Testing publishes no linux-arm64 chrome-headless-shell, so a + // preferManagedChrome render reroutes to system Chromium there + // (ensureLinuxArmBrowser). A managed-only lookup would report "not found" + // on a correctly set-up machine — the option has to be a no-op on ARM64. + Object.defineProperty(process, "arch", { value: "arm64", configurable: true }); + installFsMocks({ existing: new Set([SYSTEM_CHROME]) }); + installPuppeteerBrowsersMock(); + vi.spyOn(console, "warn").mockImplementation(() => {}); + + const { findBrowser } = await import("./manager.js"); + + expect(await findBrowser({ preferManagedChrome: true })).toEqual({ + executablePath: SYSTEM_CHROME, + source: "system", + }); + }); + + it("ensureBrowser also resolves system Chromium directly on Linux ARM64, without taking the install lock", async () => { + // Same decision as findBrowser (resolvesManagedOnly): a render on ARM64 + // must not detour through the download path — and serialize on the + // install lock — just to end up at the system Chromium it could have + // found up front. + Object.defineProperty(process, "arch", { value: "arm64", configurable: true }); + const paths = installFsMocks({ existing: new Set([SYSTEM_CHROME]) }); + const install = vi.fn(async () => ({ executablePath: HF_BINARY })); + installPuppeteerBrowsersMock({ installImpl: install }); + vi.spyOn(console, "warn").mockImplementation(() => {}); + + const { ensureBrowser } = await import("./manager.js"); + + expect(await ensureBrowser({ preferManagedChrome: true })).toEqual({ + executablePath: SYSTEM_CHROME, + source: "system", + }); + expect(install).not.toHaveBeenCalled(); + // withInstallLock creates the cache root before locking; the fixture + // starts without it, so its absence proves the download path never ran. + expect(paths.has(CACHE_ROOT)).toBe(false); + }); }); describe("isCorruptArchiveError", () => { diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 8d1ace7ae6..d995f6f2e3 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -521,17 +521,34 @@ export function findSystemBrowser(): BrowserResult | undefined { return undefined; } +/** + * Whether resolution is restricted to OUR pinned cache. `preferManagedChrome` + * is a no-op on Linux ARM64: Chrome for Testing publishes no linux-arm64 + * chrome-headless-shell, so the only browser a render can use there is system + * Chromium (`ensureLinuxArmBrowser`), and a managed-only lookup would report + * "not found" on a correctly set-up machine. Shared by `findBrowser` and + * `ensureBrowser` so the two halves of the API cannot disagree. + */ +function resolvesManagedOnly(options?: Pick): boolean { + return options?.preferManagedChrome === true && !isLinuxArm(); +} + // --- Public API ------------------------------------------------------------- /** * Find an existing browser without downloading. * Resolution: env var -> cached download -> system Chrome. + * + * A stale hyperframes-cache entry (manifest present, executable missing) is + * "not found" here — the re-download belongs to `ensureBrowser`. + * * With `preferManagedChrome`: env var -> OUR pinned cache only (puppeteer-cache * preference and system Chrome are both skipped) — the same restriction - * `ensureBrowser` applies, minus the auto-download this function never does. - * Pass it when a "found" report has to predict what a `preferManagedChrome` - * render will actually use (`doctor`'s Chrome check, `browser path`): the - * unqualified resolution reports hits that such a render re-downloads over. + * `ensureBrowser` applies, minus its auto-download. Pass it when a "found" + * report has to predict what a `preferManagedChrome` render will actually use + * (`doctor`'s Chrome check, `browser path`): the unqualified resolution + * reports hits that such a render re-downloads over. No-op on Linux ARM64 + * (see `resolvesManagedOnly`). */ export async function findBrowser( options?: Pick, @@ -539,32 +556,14 @@ export async function findBrowser( const fromEnv = findFromEnv(); if (fromEnv) return fromEnv; - const fromCache = await (options?.preferManagedChrome - ? findFromHyperframesCache() - : findFromCache()); + const managedOnly = resolvesManagedOnly(options); + const fromCache = await (managedOnly ? findFromHyperframesCache() : findFromCache()); if (fromCache.result) return fromCache.result; - if (fromCache.staleHyperframesCachePath) { - console.warn( - `[browser] Cached binary missing at ${fromCache.staleHyperframesCachePath} — re-downloading...`, - ); - try { - return await withInstallLock(async () => { - if (fromCache.staleInstallPath) purgeStaleInstall(fromCache.staleInstallPath); - return downloadBrowser(); - }); - } catch (err) { - const cause = normalizeErrorMessage(err); - throw new Error( - `Cached Chrome binary was missing at ${fromCache.staleHyperframesCachePath}, and re-download failed: ${cause}\n` + - `Run \`hyperframes browser ensure --force\` to re-download.`, - ); - } - } - // A `preferManagedChrome` render never falls back to system Chrome (see + // A managed-only render never falls back to system Chrome (see // `ensureBrowser`), so reporting it here would only relocate the false hit // from the puppeteer cache to system Chrome, not remove it. - if (options?.preferManagedChrome) return undefined; + if (managedOnly) return undefined; const fromSystem = findSystemBrowser(); if (fromSystem) { @@ -626,16 +625,16 @@ async function ensureLinuxArmBrowser(options?: EnsureBrowserOptions): Promise
cached download -> system Chrome -> auto-download. * With `preferManagedChrome`: env var -> OUR pinned cache -> auto-download - * (puppeteer-cache preference and system Chrome are both skipped). + * (puppeteer-cache preference and system Chrome are both skipped). No-op on + * Linux ARM64 (see `resolvesManagedOnly`). */ export async function ensureBrowser(options?: EnsureBrowserOptions): Promise { const fromEnv = findFromEnv(); if (fromEnv) return fromEnv; + const managedOnly = resolvesManagedOnly(options); if (!options?.force) { - const fromCache = await (options?.preferManagedChrome - ? findFromHyperframesCache() - : findFromCache()); + const fromCache = await (managedOnly ? findFromHyperframesCache() : findFromCache()); if (fromCache.result) return fromCache.result; if (fromCache.staleHyperframesCachePath) { console.warn( @@ -647,7 +646,7 @@ export async function ensureBrowser(options?: EnsureBrowserOptions): Promise { }); it("resolves Chrome via preferManagedChrome so a reported hit predicts what render will actually use", async () => { - // Explicit false rather than relying on the test runner's actual arch — - // the ARM64-skip counterpart test below only makes sense as a contrast - // if this one is pinned to the non-ARM64 branch regardless of host. - const armSpy = vi.spyOn(manager, "isLinuxArm").mockReturnValue(false); const spy = vi.spyOn(manager, "findBrowser").mockResolvedValue({ executablePath: process.execPath, source: "cache", @@ -119,36 +115,10 @@ describe("runEnvironmentChecks", () => { expect(spy).toHaveBeenCalledWith({ preferManagedChrome: true }); } finally { - armSpy.mockRestore(); spy.mockRestore(); } }); - it("skips preferManagedChrome on Linux ARM64, which has no hyperframes-managed cache to check", async () => { - // preferManagedChrome would always report "not found" here — Chrome for - // Testing publishes no linux-arm64 build — even on a machine with a - // correctly apt-get-installed system Chromium, the exact false-negative - // this check must not produce (ensureLinuxArmBrowser's own resolution - // stays unqualified for the same reason). - const armSpy = vi.spyOn(manager, "isLinuxArm").mockReturnValue(true); - const findSpy = vi.spyOn(manager, "findBrowser").mockResolvedValue({ - executablePath: "/usr/bin/chromium-browser", - source: "system", - }); - - try { - const result = await runEnvironmentChecks({ includeBrowser: true }); - - expect(findSpy).toHaveBeenCalledWith(undefined); - expect(result.outcomes.find((outcome) => outcome.name === "Chrome")).toMatchObject({ - ok: true, - }); - } finally { - armSpy.mockRestore(); - findSpy.mockRestore(); - } - }); - it("reports Chrome as not found (no throw) when browser discovery throws on a corrupt cache", async () => { const spy = vi.spyOn(manager, "findBrowser").mockRejectedValue( Object.assign(new Error("ENOTDIR: not a directory, scandir 'chrome-headless-shell'"), { diff --git a/packages/cli/src/browser/preflight.ts b/packages/cli/src/browser/preflight.ts index bdd6fd20ed..9a770d881c 100644 --- a/packages/cli/src/browser/preflight.ts +++ b/packages/cli/src/browser/preflight.ts @@ -1,7 +1,7 @@ import { execFileSync } from "node:child_process"; import { existsSync } from "node:fs"; import { platform } from "node:os"; -import { findBrowser, isLinuxArm, type BrowserResult } from "./manager.js"; +import { findBrowser, type BrowserResult } from "./manager.js"; import { FFMPEG_PATH_ENV, FFPROBE_PATH_ENV, @@ -248,7 +248,7 @@ async function checkChrome(browserPath?: string): Promise { + const origPlatform = process.platform; + const origArch = process.arch; + const origEnv = { + HYPERFRAMES_BROWSER_PATH: process.env["HYPERFRAMES_BROWSER_PATH"], + PRODUCER_HEADLESS_SHELL_PATH: process.env["PRODUCER_HEADLESS_SHELL_PATH"], + }; + + beforeEach(() => { + vi.resetModules(); + delete process.env["HYPERFRAMES_BROWSER_PATH"]; + delete process.env["PRODUCER_HEADLESS_SHELL_PATH"]; + }); + + afterEach(() => { + Object.defineProperty(process, "platform", { value: origPlatform, configurable: true }); + Object.defineProperty(process, "arch", { value: origArch, configurable: true }); + for (const [key, value] of Object.entries(origEnv)) { + if (value === undefined) delete process.env[key]; + else process.env[key] = value; + } + vi.restoreAllMocks(); + vi.doUnmock("node:fs"); + }); + + it("prints the system Chromium path on Linux ARM64 without downloading", async () => { + // `browser path` resolves with preferManagedChrome so it prints what + // render uses. On Linux ARM64 there is no managed build, so that lookup + // must still surface system Chromium directly — not fall into the + // download path (ensureBrowser) to reach the same answer. + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + Object.defineProperty(process, "arch", { value: "arm64", configurable: true }); + vi.doMock("node:fs", async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + existsSync: (p: string) => { + if (p === SYSTEM_CHROME) return true; + // Whatever the host has cached must not leak into this fixture. + if (p.startsWith(HOME_CACHE_ROOT)) return false; + return real.existsSync(p); + }, + }; + }); + + const manager = await import("../browser/manager.js"); + const ensureSpy = vi + .spyOn(manager, "ensureBrowser") + .mockResolvedValue({ executablePath: "/downloaded/chrome", source: "download" }); + vi.spyOn(console, "warn").mockImplementation(() => {}); + const written: string[] = []; + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { + written.push(String(chunk)); + return true; + }); + + try { + const { default: browserCommand } = await import("./browser.js"); + await browserCommand.run?.({ + args: { subcommand: "path", force: false, _: [] }, + cmd: browserCommand, + rawArgs: ["path"], + }); + } finally { + stdoutSpy.mockRestore(); + } + + expect(written.join("")).toBe(`${SYSTEM_CHROME}\n`); + expect(ensureSpy).not.toHaveBeenCalled(); + }); +}); From 24a611ab87fff18478f3653634dbbe2416a85bb6 Mon Sep 17 00:00:00 2001 From: miga-heygen Date: Tue, 15 Sep 2026 06:15:41 +0000 Subject: [PATCH 3/3] test(cli): pin managed-only browser resolution on x64 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The `browser path` suite only covered Linux ARM64, where `preferManagedChrome` is a no-op by design, so dropping the option from either `runPath` resolution survived the tests. Add an x64 case with a puppeteer-cache binary and an empty managed cache: the command must not print the puppeteer path, and both `findBrowser` and `ensureBrowser` must be called with `{ preferManagedChrome: true }`. Pin `ensureBrowser`'s own first cache lookup the same way: on x64 with both caches populated it returns the pinned managed binary without calling `install()`, where the unqualified call returns the puppeteer-cache one. Fold the three `managedOnly ? findFromHyperframesCache() : findFromCache()` sites into a `lookupCache(managedOnly)` helper next to `resolvesManagedOnly`, and reword the preflight comment: both cache lookups now swallow their own read errors, so the remaining throw out of `findBrowser` is `@puppeteer/browsers` failing to load. Rename the preflight test that described the old corrupt-cache throw. Co-Authored-By: Miguel Ángel --- packages/cli/src/browser/manager.test.ts | 26 ++++++++ packages/cli/src/browser/manager.ts | 16 ++++- packages/cli/src/browser/preflight.test.ts | 2 +- packages/cli/src/browser/preflight.ts | 11 ++-- packages/cli/src/commands/browser.test.ts | 70 ++++++++++++++++++++++ 5 files changed, 116 insertions(+), 9 deletions(-) diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 180a4a2bd3..048431cc74 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -965,6 +965,32 @@ describe("findBrowser — preferManagedChrome", () => { }); }); + it("ensureBrowser ignores a puppeteer-cache hit and returns the pinned hyperframes cache without installing", async () => { + // ensureBrowser's own first cache lookup must make the same managed-only + // decision as findBrowser — otherwise render launches the puppeteer-cache + // build that doctor / `browser path` just said it would skip. + installFsMocks({ + existing: new Set([HF_CACHE, HF_BINARY, PUPPETEER_CACHE, PUPPETEER_BINARY]), + dirs: { [PUPPETEER_CACHE]: ["linux-148.0.7778.97"] }, + }); + const install = vi.fn(async () => ({ executablePath: HF_BINARY })); + installPuppeteerBrowsersMock({ + installedInHfCache: [ + { browser: "chrome-headless-shell", executablePath: HF_BINARY, buildId: CHROME_VERSION }, + ], + installImpl: install, + }); + + const { ensureBrowser } = await import("./manager.js"); + + expect(await ensureBrowser()).toEqual({ executablePath: PUPPETEER_BINARY, source: "cache" }); + expect(await ensureBrowser({ preferManagedChrome: true })).toEqual({ + executablePath: HF_BINARY, + source: "cache", + }); + expect(install).not.toHaveBeenCalled(); + }); + it("still finds system Chromium on Linux ARM64, where no managed build exists", async () => { // Chrome for Testing publishes no linux-arm64 chrome-headless-shell, so a // preferManagedChrome render reroutes to system Chromium there diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index d995f6f2e3..3f7a8fcd30 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -533,6 +533,16 @@ function resolvesManagedOnly(options?: Pick { + return managedOnly ? findFromHyperframesCache() : findFromCache(); +} + // --- Public API ------------------------------------------------------------- /** @@ -557,7 +567,7 @@ export async function findBrowser( if (fromEnv) return fromEnv; const managedOnly = resolvesManagedOnly(options); - const fromCache = await (managedOnly ? findFromHyperframesCache() : findFromCache()); + const fromCache = await lookupCache(managedOnly); if (fromCache.result) return fromCache.result; // A managed-only render never falls back to system Chrome (see @@ -634,7 +644,7 @@ export async function ensureBrowser(options?: EnsureBrowserOptions): Promise { } }); - it("reports Chrome as not found (no throw) when browser discovery throws on a corrupt cache", async () => { + it("reports Chrome as not found (no throw) when browser discovery throws", async () => { const spy = vi.spyOn(manager, "findBrowser").mockRejectedValue( Object.assign(new Error("ENOTDIR: not a directory, scandir 'chrome-headless-shell'"), { code: "ENOTDIR", diff --git a/packages/cli/src/browser/preflight.ts b/packages/cli/src/browser/preflight.ts index 9a770d881c..e9fc048a6b 100644 --- a/packages/cli/src/browser/preflight.ts +++ b/packages/cli/src/browser/preflight.ts @@ -247,11 +247,12 @@ async function checkChrome(browserPath?: string): Promise>; try { // `preferManagedChrome` so a hit here predicts what render's diff --git a/packages/cli/src/commands/browser.test.ts b/packages/cli/src/commands/browser.test.ts index b51f37551e..d94716f814 100644 --- a/packages/cli/src/commands/browser.test.ts +++ b/packages/cli/src/commands/browser.test.ts @@ -5,6 +5,14 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; const SYSTEM_CHROME = "/usr/bin/google-chrome"; // Both the puppeteer and the hyperframes-managed caches live under here. const HOME_CACHE_ROOT = join(homedir(), ".cache"); +const PUPPETEER_CACHE = join(HOME_CACHE_ROOT, "puppeteer", "chrome-headless-shell"); +const PUPPETEER_VERSION_DIR = "linux-148.0.7778.97"; +const PUPPETEER_BINARY = join( + PUPPETEER_CACHE, + PUPPETEER_VERSION_DIR, + "chrome-headless-shell-linux64", + "chrome-headless-shell", +); describe("hyperframes browser path", () => { const origPlatform = process.platform; @@ -76,4 +84,66 @@ describe("hyperframes browser path", () => { expect(written.join("")).toBe(`${SYSTEM_CHROME}\n`); expect(ensureSpy).not.toHaveBeenCalled(); }); + + it("does not print a puppeteer-cache binary on x64 when the pinned managed cache is empty", async () => { + // The ARM64 case above cannot tell a qualified lookup from an unqualified + // one — preferManagedChrome is a no-op there by design. On x64 the two + // diverge: the unqualified resolution would print the puppeteer-cache + // binary, which a preferManagedChrome render ignores and re-downloads + // over. Both `runPath` resolutions must carry the option so the printed + // path is the one render will actually launch. + Object.defineProperty(process, "platform", { value: "linux", configurable: true }); + Object.defineProperty(process, "arch", { value: "x64", configurable: true }); + vi.doMock("node:fs", async (importOriginal) => { + const real = await importOriginal(); + return { + ...real, + existsSync: (p: string) => { + if (p === PUPPETEER_CACHE || p === PUPPETEER_BINARY) return true; + if (p.startsWith(HOME_CACHE_ROOT)) return false; + return real.existsSync(p); + }, + readdirSync: ((p: string, ...rest: unknown[]) => + p === PUPPETEER_CACHE + ? [PUPPETEER_VERSION_DIR] + : (real.readdirSync as (...args: unknown[]) => unknown)( + p, + ...rest, + )) as typeof real.readdirSync, + }; + }); + + const manager = await import("../browser/manager.js"); + const findSpy = vi.spyOn(manager, "findBrowser"); + const ensureSpy = vi + .spyOn(manager, "ensureBrowser") + .mockResolvedValue({ executablePath: "/downloaded/chrome", source: "download" }); + const written: string[] = []; + const stdoutSpy = vi.spyOn(process.stdout, "write").mockImplementation((chunk) => { + written.push(String(chunk)); + return true; + }); + + try { + const { default: browserCommand } = await import("./browser.js"); + await browserCommand.run?.({ + args: { subcommand: "path", force: false, _: [] }, + cmd: browserCommand, + rawArgs: ["path"], + }); + } finally { + stdoutSpy.mockRestore(); + } + + // Sanity: the fixture really does hold a binary the unqualified lookup + // would have accepted, so a miss below is the option at work. + expect(await manager.findBrowser()).toEqual({ + executablePath: PUPPETEER_BINARY, + source: "cache", + }); + expect(written.join("")).toBe("/downloaded/chrome\n"); + expect(findSpy).toHaveBeenCalledWith({ preferManagedChrome: true }); + expect(ensureSpy).toHaveBeenCalledTimes(1); + expect(ensureSpy).toHaveBeenCalledWith({ preferManagedChrome: true }); + }); });