diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 987493d0fa..048431cc74 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"); @@ -843,6 +869,170 @@ 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(); + }); + + 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("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 + // (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", () => { 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..3f7a8fcd30 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -521,35 +521,59 @@ 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(); +} + +/** + * The cache leg of resolution, keyed on `resolvesManagedOnly`: OUR pinned cache + * alone when managed-only, otherwise puppeteer's cache first (see + * `findFromCache`). One function so every lookup in `findBrowser` and + * `ensureBrowser` picks the same source. + */ +function lookupCache(managedOnly: boolean): Promise { + return managedOnly ? findFromHyperframesCache() : findFromCache(); +} + // --- 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 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(): Promise { +export async function findBrowser( + options?: Pick, +): Promise { const fromEnv = findFromEnv(); if (fromEnv) return fromEnv; - const fromCache = await findFromCache(); + const managedOnly = resolvesManagedOnly(options); + const fromCache = await lookupCache(managedOnly); 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 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 (managedOnly) return undefined; const fromSystem = findSystemBrowser(); if (fromSystem) { @@ -611,16 +635,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 lookupCache(managedOnly); if (fromCache.result) return fromCache.result; if (fromCache.staleHyperframesCachePath) { console.warn( @@ -632,7 +656,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("resolves Chrome via preferManagedChrome so a reported hit predicts what render will actually use", async () => { + const spy = vi.spyOn(manager, "findBrowser").mockResolvedValue({ + executablePath: process.execPath, + source: "cache", + }); + + try { + await runEnvironmentChecks({ includeBrowser: true }); + + expect(spy).toHaveBeenCalledWith({ preferManagedChrome: true }); + } finally { + spy.mockRestore(); + } + }); + + 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 fabcd97ca2..e9fc048a6b 100644 --- a/packages/cli/src/browser/preflight.ts +++ b/packages/cli/src/browser/preflight.ts @@ -247,14 +247,19 @@ 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. + info = await findBrowser({ preferManagedChrome: true }); } catch { info = undefined; } diff --git a/packages/cli/src/commands/browser.test.ts b/packages/cli/src/commands/browser.test.ts new file mode 100644 index 0000000000..d94716f814 --- /dev/null +++ b/packages/cli/src/commands/browser.test.ts @@ -0,0 +1,149 @@ +import { join } from "node:path"; +import { homedir } from "node:os"; +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; + 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(); + }); + + 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 }); + }); +}); 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);