fix(cli): purge stale/partial browser installs instead of wedging retries

[miguel-heygen]

Root cause

Two independent reports of the same failure: a chrome-headless-shell zip extraction gets interrupted (Windows AV lock, sleep/wake, ctrl-C) and leaves only the alphabetically-early files (ABOUT/LICENSE) in the target directory, no executable. Every subsequent browser ensure (or implicit re-download from findBrowser/ensureBrowser) sees the directory already exists and hands it straight to @puppeteer/browsers' install(), which throws "folder exists but the executable is missing" without re-extracting — permanently wedging the machine until someone manually deletes the directory. --force didn't help because it was a phantom flag: browser.ts never declared it as a real CLI option, so it silently did nothing (mentioned only in an error-message string).

findFromCache() already detects this exact case (dir exists, exe missing) and returns it as staleHyperframesCachePath, but findBrowser()/ensureBrowser() fed that straight into a re-download without ever deleting the stale directory first, so install() hit the same "exists" branch every time.

Fix

• findFromCache() also returns staleInstallPath (InstalledBrowser's .path — the actual install-folder root, not the missing executablePath) for the stale case.
• Both findBrowser() and ensureBrowser() now purge that directory (rmSync, inside the existing withInstallLock mutex from fix(cli): lock chrome-headless-shell install against concurrent extraction races #1866 so a purge can't race a concurrent installer) before retrying, so install() actually re-extracts instead of erroring.
• Wired up a real --force flag on hyperframes browser ensure: it purges the whole HF-managed cache (reusing the already-tested clearBrowser()) and skips every cache/system shortcut, so it always gets a fresh download regardless of what's currently on disk — matching what the existing (previously false) help text already claimed it did.

Not fixed here

Neither report's machine had a usable auto-detected system Chrome fallback on Windows — SYSTEM_CHROME_PATHS only lists macOS/Linux paths, so findFromSystem() can never succeed on win32. Both reporters worked around this manually via HYPERFRAMES_BROWSER_PATH, which still works fine; adding real Windows system-Chrome detection is a distinct, larger change.

Test plan

• Extended manager.test.ts's existing stale-cache-redownload test to include a populated stale install directory and assert it's gone before the mocked install() is called (previously only asserted the redownload happened, not that the fix's purge step ran).
• Added a new test for ensureBrowser({force: true}) purging the cache and bypassing a healthy cache/system-Chrome shortcut.
• Fixed the shared fs mock's rmSync to actually simulate recursive deletion (drop nested tracked paths too), which the new tests need and the old ones never exercised.
• Full CLI suite: bun run --cwd packages/cli test — 97 files / 1222 tests passing.
• Full workspace bun run build — clean.
• bunx oxlint / bunx oxfmt --check on changed files — clean.

[james-russo-rames-d-jusso]

Reviewed at 7690c6f.

🟢 Correct fix for a real production wedge — the staleInstallPath addition to CacheLookupResult + purgeStaleInstall() before every retry closes the exact loop where @puppeteer/browsers install() throws "folder exists but exe missing." The --force flag is finally wired up as a real citext CLI option (previously phantom-flag in error-message strings only). Test coverage adds both a stale-dir-purge assertion and a full --force flow with healthy cache + system Chrome both present (verifying force ignores both shortcuts).

Verification notes:

• Purge under withInstallLock in findBrowser / ensureBrowser non-force path — correct, concurrent installer can't race a purge (uses the #1866 mutex).
• clearBrowser() in the --force path removes CACHE_DIR entirely; withInstallLock then mkdirSyncs it back before lock acquisition. Clean.
• Post-lock re-check now also purges any stale install it finds (if (afterLock.staleInstallPath) purgeStaleInstall(...)) — handles the "concurrent invocation partially extracted while we waited" case.

Concerns:

• 🟠 clearBrowser() in the --force path is called BEFORE withInstallLock acquisition — two concurrent --force invocations would each wipe CACHE_DIR independently, and one could delete a directory the other is mid-extracting into. The reclaim-lock protects the lockfile itself but not the cache contents pre-lock. Consider moving clearBrowser() INSIDE the withInstallLock block for --force (paying one extra mkdir-then-rmdir, negligible), or documenting that --force is not concurrency-safe. Not a common workflow but worth acknowledging explicitly.
• 🟡 Windows-specific: rmSync on a directory whose executable is held open by a concurrent Chrome process throws EBUSY. Not new (existing clearBrowser has the same behavior) but --force invites users to try it when things are broken — the error surface should probably suggest "close all Chrome processes and retry" if rmSync throws. Deferrable.

Nits:

• 🟡 The ARM64 Linux path early-returns without honoring --force (system-Chrome/apt-install flow, no download cache to purge). Comment at line ~35 acknowledges this. Consider a one-line info log when --force is passed on ARM64 so users don't think it silently ran.

No cross-repo consumers of these internal browser-manager functions. Test coverage is exemplary — both the mock rmSync recursive-tree fix and the two new tests are the right shape.

— Rames D Jusso

[vanceingalls]

Reviewed at 7690c6f.

🟢 R1 verdict — LGTM, co-signing Rames with one additional runtime-interop note

Independently verified: staleInstallPath addition + purgeStaleInstall before every retry closes the @puppeteer/browsers install() "folder exists but exe missing" loop; --force is now a real citty flag wired through EnsureBrowserOptions. Rames' concerns (concurrent --force cache race, Windows EBUSY, ARM64 silent no-op) all reproduce independently — I'll co-sign rather than repeat.

One additional finding

findBrowser's stale path lacks the afterLock re-check — 🟡

packages/cli/src/browser/manager.ts:394-397

ensureBrowser's stale-cache path enters withInstallLock and then does the standard "concurrent invocation may have finished while we waited" re-check via findFromCache inside the lock (lines 502-506). But findBrowser's stale-cache path just purges + downloads unconditionally — no afterLock re-check:

return await withInstallLock(async () => {
  if (fromCache.staleInstallPath) purgeStaleInstall(fromCache.staleInstallPath);
  return downloadBrowser();
});

So two concurrent hyperframes render invocations (both going through findBrowser) that both see the same stale install:

1. Both call withInstallLock → serialized.
2. First takes lock, purges, downloads, releases.
3. Second takes lock, purges (deletes the freshly-downloaded install), re-downloads.

Costs bandwidth on a rare race; doesn't corrupt state (the second re-download still succeeds). Not a P0 — the render use case is rare-to-parallelize and the wedge case this PR fixes is far more painful. Worth a follow-up to make findBrowser's stale path symmetric with ensureBrowser's.

Concurrent --force (Rames' 🟠 concern)

Independently reproduce Rames' concern: clearBrowser() is called BEFORE withInstallLock acquisition on the force path, so two concurrent --force invocations can each wipe CACHE_DIR mid-write from the other. Not a common workflow (interactive browser ensure --force is a one-shot), but the fix is trivial — move clearBrowser() inside the lock block for --force. Deferrable, agree with Rames' stance.

Fine to merge from a runtime-interop perspective.

---

Review by Via (runtime-interop lens) — co-signing Rames

[vanceingalls]

Addendum — 🟠 revising from 🟢 LGTM

Rames' concurrent review flagged something my original runtime-interop pass under-graded. Verified independently at head 7690c6fd:

clearBrowser() in the --force path runs outside withInstallLock.

packages/cli/src/browser/manager.ts:494:

  } else {
    // `--force` means "always get a fresh managed download" — purging the
    // whole HF-managed cache up front (reusing clearBrowser's already-tested
    // recursive rmSync)...
    clearBrowser();
  }

  return withInstallLock(async () => {          // <-- line 497

Two concurrent hyperframes browser ensure --force calls both hit clearBrowser() before either enters withInstallLock. Sequence: Process A clearBrowser() → A enters lock → A begins download. Meanwhile Process B clearBrowser() runs while A is mid-download → B's rmSync(CACHE_DIR) destroys A's partial install. A's download completes but its output is gone; A returns a BrowserResult pointing at a path that no longer exists.

The lock-dir relocation elsewhere in this PR (moving .chrome.install.lock out of CACHE_DIR so clearBrowser() can't destroy in-flight locks) is a correct half of the concurrency story — it fixes the lock-eviction bug for the normal install path. But the --force-branch clearBrowser() at line 494 sits outside the lock and can still trample another --force caller's install.

Fix shape — clearBrowser() needs to move inside the withInstallLock callback, so a single --force caller purges + reinstalls atomically:

return withInstallLock(async () => {
  if (options?.force) clearBrowser();
  // ...existing lock body
});

(There's a secondary consideration: withInstallLock itself creates CACHE_ROOT_DIR via mkdirSync, so the ordering works — the lock dir survives because it's now outside CACHE_DIR.)

My original review's finding on findBrowser's stale path lacking the afterLock re-check stands as a separate softer nit (redundant purge under a stale-hit race). The --force-path finding is the load-bearing one and I missed grading it as blocker-class runtime-interop; overall verdict revised to 🟠 endorsing Rames' fix shape.

---

Addendum by Via (runtime-interop lens)

[james-russo-rames-d-jusso]

Reviewed at 6d54ab1.

🟢 R2 verdict — R1 hold resolved via structural PREEMPT, LGTM

Delta from R1 (7690c6fd): commit 6d54ab14 fix(cli): serialize force browser cache purge addresses my R1 🟠 hold by restructuring lock placement rather than just narrowing the race window. Genuine PREEMPT, not a probability reduction.

R1 blocker verification — 🟢 resolved

R1 finding was: clearBrowser() in the --force path ran OUTSIDE withInstallLock, so two concurrent --force invocations could race — one process's clearBrowser() (recursive rmSync on CACHE_DIR) could wipe another process's in-flight .install.lock sitting inside that same directory, breaking the mutex.

Resolved at packages/cli/src/browser/manager.ts:23-24, 42-46, 92-93, 486-501:

1. Lock dirs moved outside CACHE_DIR. New CACHE_ROOT_DIR = ~/.cache/hyperframes (line 23), with INSTALL_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.lock") and INSTALL_RECLAIM_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.reclaim.lock") (lines 42-46). Since clearBrowser() recursively removes CACHE_DIR = ~/.cache/hyperframes/chrome/ (a subdirectory), it can no longer reach the sibling lock files. Structural elimination of the race, not window-narrowing.
2. clearBrowser() moved inside withInstallLock. The --force branch at lines 486-501 in ensureBrowser() now calls clearBrowser() inside the withInstallLock callback (line 493), so even the intra-force ordering is serialized.
3. withInstallLock mkdir target updated. Line 92-93: if (!existsSync(CACHE_ROOT_DIR)) mkdirSync(CACHE_ROOT_DIR, { recursive: true }) — the parent that must exist for the atomic mkdirSync mutex is now the root, not the (about-to-be-wiped) chrome subdir. Well-sequenced.
4. New regression test (manager.test.ts:238-268, serializes concurrent force downloads so one purge cannot delete another installer's lock) fires two ensureBrowser({force: true}) in parallel and asserts maxActiveInstalls === 1 and the lock is cleaned up. Directly pins the invariant my R1 flagged.

Also noticed one bonus fix the diff sneaks in: the non-force after-lock path (line 500) now purges staleInstallPath if findFromCache re-detects a stale install between the initial check and the lock acquisition — closes a smaller window I hadn't explicitly called out at R1. Nice.

R1 nits — still open, remain 🟡 defer

• Windows EBUSY when Chrome is currently running. No change. rmSync({recursive: true, force: true}) on a directory whose executable is running as another process still throws EBUSY on Windows, and both reporters were on Windows. Defer to a follow-up if it recurs — probably rare in the reported scenario (users reproduce with Chrome not running because they're waiting on hyperframes render to open it).
• ARM64 Linux --force no-op. No change. The CLI at browser.ts:32-33 short-circuits runEnsure for ARM64 Linux (system Chromium only, no download cache to force-purge) before reading options.force, and the help text at line 40-41 was updated to note --force is a no-op here in the leading comment — but the end user running hyperframes browser ensure --force on ARM64 still sees a normal "system Chromium found" success message with no explicit "you asked for force but this platform doesn't support it" hint. Minor UX polish, not a correctness issue.

Peer awareness

Via's Addendum — 🟠 revising from 🟢 LGTM at 2026-07-04T20:29:18Z on 7690c6fd independently escalated to the same R1 finding I did (co-signing on the concurrent-force race). Both R1 verdicts land in the same place; this R2 resolves both.

AI-trailer sweep

Clean. Both commits (7690c6fd, 6d54ab14) are human-authored prose, no Co-Authored-By: Claude / [skip ci] / generation markers.

— Rames D Jusso

[vanceingalls]

🟢 R2 verdict — resolved

The concurrent-force race Rames and I flagged at R1 is closed. Two structural moves at head 6d54ab14:

1. clearBrowser() moved INSIDE the withInstallLock callback and gated on options?.force.
2. Lock dirs relocated to a new CACHE_ROOT_DIR (~/.cache/hyperframes/) as siblings of CACHE_DIR (~/.cache/hyperframes/chrome/), so a --force purge of the Chrome cache cannot delete another installer's in-flight lock even if the lock code were called before the force-purge.

Belt + suspenders on the same race — either move alone would close it; both together also survive future refactors that might reorder purge vs lock.

Finding-by-finding

1. Concurrent --force race destroys mid-download install — ✅ resolved

packages/cli/src/browser/manager.ts:489-505 and :43-44

// 1) Purge is now inside the lock (was outside at R1):
return withInstallLock(async () => {
  if (options?.force) {
    // `--force` means "always get a fresh managed download" — purging the
    // whole HF-managed cache after acquiring the install lock keeps two
    // concurrent force retries from deleting each other's in-flight lock or
    // partially extracted install.
    clearBrowser();
  }
  // ...
});

// 2) Lock dirs relocated outside CACHE_DIR:
const CACHE_ROOT_DIR = join(homedir(), ".cache", "hyperframes");
const CACHE_DIR = join(homedir(), ".cache", "hyperframes", "chrome");
const INSTALL_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.lock");
const INSTALL_RECLAIM_LOCK_DIR = join(CACHE_ROOT_DIR, ".chrome.install.reclaim.lock");

withInstallLock now ensures CACHE_ROOT_DIR (not CACHE_DIR) exists before acquiring the lock — correct pairing with the relocation.

Bonus: new regression test serializes concurrent force downloads so one purge cannot delete another installer's lock at manager.test.ts:239-266 asserts maxActiveInstalls === 1 under two concurrent ensureBrowser({ force: true }) calls, and asserts paths.has(HF_LOCK) mid-install (the purge would remove the lock at the old path but not the new sibling path). Good coverage that pins BOTH structural moves.

---

R2 by Via (runtime-interop lens)