fix(cli): select host-compatible cached browser

[miguel-heygen]

What

Select cached Chrome Headless Shell executables only from the current host platform and architecture.

Why

A cache containing executables for multiple hosts could return the first existing entry, allowing a foreign binary to reach browser launch instead of falling through to a compatible cached or system browser.

How

• Map Darwin arm64/x64, Linux arm64/x64, and Windows arm64/ia32/x64 to one Chrome for Testing cache layout before probing.
• Apply the same selection rule in the CLI cache manager and engine resolver.
• Filter HyperFrames-managed cache records by the platform reported by @puppeteer/browsers.

Scope: this changes automatic cache discovery only. Explicit config/environment browser overrides remain caller-controlled, executable headers are not inspected, and cloud browser provisioning is unchanged.

Test plan

• Unit tests added/updated
• Manual testing performed (browser path selected the Linux/x64 cache on a Linux/x64 host)
• Documentation updated (not applicable)
• Engine and CLI typechecks
• Engine and CLI builds
• Formatting, lint, and changed-file audit

[james-russo-rames-d-jusso]

Reviewed at bd6f79d.

Right fix for the reported class of bug. findFromPuppeteerCache / findCachedHeadlessShell used to accept the first-existing dir across all four Chrome-for-Testing platforms per version, so a mac-arm64 host with a stray linux64 binary in its Puppeteer cache would launch the linux binary. Now both scan the ONE directory that matches ${process.platform}/${process.arch} and fall through to system/download if that entry is absent. findFromHyperframesCache gets the parallel treatment via b.platform === detectBrowserPlatform(). Test coverage on the four common host combos (darwin/arm64, darwin/x64, linux/x64, win32/x64) in both the CLI test and engine test — verified each selects only its own directory even when all four are present. LGTM overall — a few things worth surfacing:

Concerns

linux/arm64 and win32/arm64 map to their x64 directories. Both map entries in CACHED_HEADLESS_SHELL_EXECUTABLES point arm64 hosts at their x64 counterparts (chrome-headless-shell-linux64 and chrome-headless-shell-win64). This reflects Chrome-for-Testing's actual shipping matrix — CFT publishes linux64, mac-arm64, mac-x64, win32, win64; there's no native linux-arm64 or win-arm64 build, so @puppeteer/browsers on those hosts downloads the x64 one and expects it to run under emulation. So the map matches current reality — but it also means a linux/arm64 host with an x86_64 binary cached will pass the "host-compatible" filter and reach page.goto(), which is exactly the class of failure the PR closes for darwin/arm64. Two low-cost mitigations to consider:

• Add a comment on the two entries explaining WHY they intentionally share dirs with x64 (CFT ships no native arm64 build for Linux/Windows). Otherwise the next maintainer will read the map as symmetric and either "fix" the arm64→x64 mapping or trust it as validation of arch-compatible fallback.
• Alternatively, if you want to treat linux/arm64 and win32/arm64 as "no compatible cache" and force fallthrough to system/download, undefined those two entries and cachedHeadlessShellExecutable() returns undefined → resolution skips the puppeteer cache. That's a stricter posture that would surface arch-mismatch failures earlier (before launch). Explicit trade-off worth deciding, not a bug.

Byte-duplicated map + resolver in two packages. CACHED_HEADLESS_SHELL_EXECUTABLES and cachedHeadlessShellExecutable() are line-for-line identical between packages/cli/src/browser/manager.ts and packages/engine/src/services/browserManager.ts. The pre-existing comment in the file ("Same shape as resolveHeadlessShellPath in engine/browserManager.ts — keep them aligned") is the current documentation for this coupling. If CFT ever adds a new host directory (e.g. linux-arm64 when it ships) or renames one, both files need identical edits, and CI has no test that catches drift between them. A shared module (packages/shared or wherever the workspace convention lives) would eliminate the risk; scope-appropriate to defer if there's a package-boundary reason the two can't share. Nit as-is; blocker only if the two ever DO drift.

Nits

• Missing test coverage for the cross-arch fallback cases. The it.each covers darwin/arm64, darwin/x64, linux/x64, win32/x64 — but not linux/arm64, win32/arm64, or win32/ia32. Those are the ones with the "intentional shared-dir" mapping and the least obvious behavior. Adding at least the linux/arm64 case (assert it selects chrome-headless-shell-linux64 given both linux64 and mac-arm64 in the cache) would pin the current-intended-behavior into the tests so a future refactor can't quietly regress it.
• Object.defineProperty(process, ...) pollution across tests. The new tests set process.platform and process.arch per-case but don't reset them in an afterEach. Within a .each block that iterates through all four host combos, each subsequent test picks up the last one set. If a later test (elsewhere in the file, or in a re-ordered suite) reads process.platform without setting it, it'd see the value from the tail of this block. Consider a beforeEach/afterEach that restores the original values, or an outer describe scope so the mutation is bounded.

What I didn't verify

• Whether @puppeteer/browsers's current detectBrowserPlatform() returns anything the map doesn't cover (e.g. win-arm64 in a future version). Today's mapping matches the current shipping matrix; if Puppeteer adds arm64 platforms downstream and starts returning distinct strings, findFromHyperframesCache would filter by that string, but findFromPuppeteerCache would still look in the win64/linux64 directory per the map. That's a future-drift concern, not a blocker today.

— Review by Rames D Jusso

[vanceingalls]

Verdict: APPROVE — grade A — rubric CORRECT

Root-cause fit: Two-layer defense matches the migrated/shared-cache attack. HF-managed cache filters getInstalledBrowsers by b.platform === detectBrowserPlatform() (metadata-honest against puppeteer's own manifest). Puppeteer-cache scanner replaces the four-way probe with a single (platform,arch) → directory pick, so probe-order can no longer let a foreign directory win. Rosetta path is intentionally correct: process.arch==='x64' under Rosetta selects mac-x64, which is what Rosetta launches. Windows ARM correctly selects win64 (x64 Chrome under WoW64) — matches CfT's shipped artifacts.

Claims verified (each with file:line at HEAD):

• Maps Darwin arm64/x64, Linux arm64/x64, Windows arm64/ia32/x64 → CfT dir: packages/cli/src/browser/manager.ts:391-401 and packages/engine/src/services/browserManager.ts:136-146.
• Same rule applied in both CLI cache manager and engine resolver: manager.ts:412-436 + browserManager.ts:157-171.
• HF cache filters by puppeteer-reported platform: manager.ts:307-313 (b.platform === hostPlatform).
• Scope disclaimer honored — env overrides untouched: manager.ts:270-277; header inspection absent by design; cloud provisioning untouched.
• Unit tests added for CLI + engine: manager.test.ts:232-266 (foreign-platform HF entry ignored) and manager.test.ts:537-588 / browserManager.test.ts:404-471 (host-compatible selection across 4 triples).
• File tally 228/-35 matches; mergeable=MERGEABLE.

Adversarial findings:

• P2 (defensive gap) — manager.ts:397 / browserManager.ts:141: linux/arm64 maps to chrome-headless-shell-linux64 (an x86_64 binary). If a puppeteer cache is migrated from a linux/x64 host onto linux/arm64, findFromPuppeteerCache returns the x86 exe and callers ENOEXEC — the same shared-cache class this PR fixes. CfT ships no linux-arm64 build; safer to return undefined here so ensureLinuxArmBrowser (manager.ts:527) apt-installs chromium instead. Non-blocking; note in follow-up.
• nit — test matrix omits linux/arm64, win32/arm64, win32/ia32 cases from the it.each triples (manager.test.ts:537, browserManager.test.ts:404). Would exercise the very rows added to the map.

CI state: all required green — CLI smoke, Test, Typecheck, Lint, Format, Producer unit + integration, Tests on windows-latest, CLI: npx shim (macos/ubuntu/windows), GCP BeginFrame image contract, Fallow audit.

Suggested next step: Merge as-is; open a small follow-up to (a) drop the linux/arm64 row (or map to undefined) so arm64 hosts fall through to ensureLinuxArmBrowser, and (b) extend the parameterized test matrix to the three uncovered triples.

— Review by Via