From df1d20b7659df7858e1c8324e84cb88fa8124af0 Mon Sep 17 00:00:00 2001 From: Vance Ingalls Date: Wed, 8 Jul 2026 16:10:43 -0700 Subject: [PATCH] fix(cli): check buildId when resolving the managed Chrome cache MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the highest-severity max-effort code-review finding on the now- merged #2082 (the drawElement Chrome-version-pin fix): findFromHyperframesCache matched a cached Chrome by browser type only, never comparing its buildId against CHROME_VERSION. Any machine that already rendered with an older hyperframes version has an old build (this pin has moved 131 -> 151 -> 152 across releases) sitting in ~/.cache/hyperframes/chrome, which satisfied the lookup and silently defeated the whole point of #2082's version bump for exactly the population it was meant to fix — drawElement's new capability probe would then permanently and silently fall back to screenshot capture instead of ever fetching a build that implements canvas.drawElementImage. Verified directly (not just via review): seeded ~/.cache/hyperframes/chrome with the old 131 build, confirmed a real render previously kept using it forever; with this fix it's correctly ignored and 152 is downloaded. New regression test locks in the buildId mismatch case. --- packages/cli/src/browser/manager.test.ts | 38 ++++++++++++++++++++++-- packages/cli/src/browser/manager.ts | 9 +++++- 2 files changed, 43 insertions(+), 4 deletions(-) diff --git a/packages/cli/src/browser/manager.test.ts b/packages/cli/src/browser/manager.test.ts index 3e60d6bb7..e09bdc98f 100644 --- a/packages/cli/src/browser/manager.test.ts +++ b/packages/cli/src/browser/manager.test.ts @@ -23,6 +23,7 @@ */ import { join, sep } from "node:path"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { CHROME_VERSION } from "./manager.js"; // Use `path.join` so the fake paths line up with whatever separator Node's // real `path.join` produces in `manager.ts` on the host running the test @@ -107,7 +108,12 @@ function installFsMocks({ existing, dirs }: FsMockOptions) { function installPuppeteerBrowsersMock( opts: { - installedInHfCache?: Array<{ browser: string; executablePath: string; path?: string }>; + installedInHfCache?: Array<{ + browser: string; + executablePath: string; + path?: string; + buildId?: string; + }>; installedInHfCacheError?: Error; installResult?: { executablePath: string }; installImpl?: () => Promise<{ executablePath: string }>; @@ -167,7 +173,9 @@ describe("findBrowser — cache resolution", () => { // last-resort fallback. installFsMocks({ existing: new Set([HF_CACHE, HF_BINARY]) }); installPuppeteerBrowsersMock({ - installedInHfCache: [{ browser: "chrome-headless-shell", executablePath: HF_BINARY }], + installedInHfCache: [ + { browser: "chrome-headless-shell", executablePath: HF_BINARY, buildId: CHROME_VERSION }, + ], }); const { findBrowser } = await import("./manager.js"); @@ -176,6 +184,25 @@ describe("findBrowser — cache resolution", () => { expect(result).toEqual({ executablePath: HF_BINARY, source: "cache" }); }); + it("does not resolve to a hyperframes-cache build from an older CHROME_VERSION pin", async () => { + // A build downloaded by a prior hyperframes version (this pin has moved + // 131 -> 151 -> 152 across releases) must not satisfy resolution, or an + // upgrade silently keeps running a stale build forever instead of ever + // fetching the version the new release actually needs (HF#2060 review). + installFsMocks({ existing: new Set([HF_CACHE, HF_BINARY, SYSTEM_CHROME]) }); + installPuppeteerBrowsersMock({ + installedInHfCache: [ + { browser: "chrome-headless-shell", executablePath: HF_BINARY, buildId: "131.0.6778.85" }, + ], + }); + + const { findBrowser } = await import("./manager.js"); + const result = await findBrowser(); + + expect(result?.executablePath).not.toBe(HF_BINARY); + expect(result).toEqual({ executablePath: SYSTEM_CHROME, source: "system" }); + }); + it("re-downloads when the hyperframes cache manifest points at a missing binary", async () => { const redownloadedBinary = join( HF_CACHE, @@ -191,7 +218,12 @@ describe("findBrowser — cache resolution", () => { const paths = installFsMocks({ existing: new Set([HF_CACHE, staleInstallDir]) }); installPuppeteerBrowsersMock({ installedInHfCache: [ - { browser: "chrome-headless-shell", executablePath: HF_BINARY, path: staleInstallDir }, + { + browser: "chrome-headless-shell", + executablePath: HF_BINARY, + path: staleInstallDir, + buildId: CHROME_VERSION, + }, ], installResult: { executablePath: redownloadedBinary }, }); diff --git a/packages/cli/src/browser/manager.ts b/packages/cli/src/browser/manager.ts index 5acf32556..7899ee861 100644 --- a/packages/cli/src/browser/manager.ts +++ b/packages/cli/src/browser/manager.ts @@ -228,7 +228,14 @@ async function findFromHyperframesCache(): Promise { ); installed = []; } - const match = installed.find((b) => b.browser === Browser.CHROMEHEADLESSSHELL); + // Match on buildId too, not just browser type — an install left over from + // an older hyperframes version (this pin has moved 131 → 151 → 152 across + // releases) must NOT satisfy resolution, or an upgrade silently keeps + // running whatever build happened to be cached instead of ever fetching + // the version this release actually needs (HF#2060 review). + const match = installed.find( + (b) => b.browser === Browser.CHROMEHEADLESSSHELL && b.buildId === CHROME_VERSION, + ); if (match && existsSync(match.executablePath)) { return { result: { executablePath: match.executablePath, source: "cache" } }; }