test(cli): stabilize flaky install-lock wait-notice test with fake timers

[vanceingalls]

What

Adopts the fake-timer pattern (already used in the sibling "recovers when a crashed reclaimer leaves both lock directories" test) for withInstallLock reports progress while waiting instead of staying silent (packages/cli/src/browser/manager.test.ts:413). Test-only change; no production-code diff.

Why

The test seeds initialMtimeMs: Date.now() inside installFsMocks(...), then does await import("./manager.js") (a vi.resetModules()'d dynamic import per beforeEach) before calling withInstallLock. Under real timers, if the dynamic-import beat exceeds staleMs=50ms — plausible on a busy shared runner with vitest resetting modules between tests — the new isDirLockStale(INSTALL_LOCK_DIR, staleMs) short-circuit added in #2328 (manager.ts:154) fires on iteration 1, the loop breaks before waitedMs reaches waitNoticeMs=20, no "Waiting for another hyperframes process" warn ever emits, and the assertion at manager.test.ts:428 (expected false to be true) fails.

Observed on main (post-#2328): reruns of the Test lane fire the same failure at line=428,column=7 ::AssertionError: expected false to be true. Two separate CI runs on unrelated PRs (#2359, #2360) both hit it — one on the first attempt and again on the retry — so retries alone don't clear it under current CI load. My own review of #2328 (https://github.com/heygen-com/hyperframes/pull/2328#pullrequestreview-*) called out exactly this flake mechanism and suggested this fix.

How

Under fake timers, Date.now() is frozen at useFakeTimers() time, so the mtime seed stays non-stale across the dynamic import regardless of real wall-clock latency. vi.advanceTimersByTimeAsync(TEST_LOCK_TIMINGS.staleMs + TEST_LOCK_TIMINGS.pollMs * 5) then drives the loop past both waitNoticeMs (fires the warn the test observes) and the stale deadline (lets reclaimStaleInstallLock clear the held lock so the acquisition resolves).

afterEach(() => vi.useRealTimers()) already exists at describe("findBrowser — cache resolution")'s teardown, so no per-test cleanup is needed.

Test plan

• Local vitest run src/browser/manager.test.ts: 27/27 pass in 487ms.

• Both blocked PRs (#2359, #2360) should turn the Test lane green after their next CI runs.

• Unit tests added/updated

— Via

[miga-heygen]

Review — test(cli): stabilize flaky install-lock wait-notice test with fake timers

Root cause is correctly identified: installFsMocks seeds initialMtimeMs: Date.now(), but the vi.resetModules() + dynamic import("./manager.js") between seeding and the withInstallLock call can take >50ms real wall-clock on a busy CI runner. By then, isDirLockStale(INSTALL_LOCK_DIR, staleMs=50) fires on iteration 1, the loop breaks before waitedMs reaches waitNoticeMs=20, the warn never fires, and the assertion fails. Two independent CI runs on main (post-#2328) confirmed it.

Fix is correct: vi.useFakeTimers() freezes Date.now() at seed time, so the mtime delta stays 0 regardless of real-time import latency. vi.advanceTimersByTimeAsync(staleMs + pollMs * 5) then deterministically drives the loop past both the wait-notice threshold (fires the warn) and the stale deadline (clears the lock). The acquisition promise is properly awaited after advancing timers.

Cleanup: afterEach(() => vi.useRealTimers()) already exists in the enclosing describe. No leak.

Pattern consistency: the sibling "recovers when a crashed reclaimer leaves both lock directories" test already uses this exact fake-timer pattern. Consistent.

Test-only change, no production code diff.

LGTM — no issues found.

— Miga

[james-russo-rames-d-jusso]

Root cause is diagnosed, not masked — the mtime-vs-Date.now() beat during await import(...) under real timers really can trip the immediate-stale short-circuit added in #2328, and the fix moves that clock into the test's control instead of loosening the assertion or bumping thresholds. The pattern is a copy of the sibling crashed reclaimer leaves both lock directories test at manager.test.ts:352, teardown is already handled by the describe-scoped afterEach(() => vi.useRealTimers()) at line 178, and the staleMs + 5*pollMs = 75ms advance covers both waitNoticeMs=20ms (fires the warn) and staleMs=50ms (lets reclaimStaleInstallLock clear the held lock) with a comfortable buffer.

The acquisition mode on main was already "wait for stale-reclaim" rather than "live holder release" (no touchError / holder was never running), so this doesn't drop live-holder coverage — the semantic path stays identical, just deterministic now.

LGTM.

— Review by Rames D Jusso

[miguel-heygen]

Post-merge advisory review (this PR merged into main while I was verifying it). No findings.

The root cause and fix line up with the implementation: fake timers are enabled before initialMtimeMs: Date.now() is seeded (packages/cli/src/browser/manager.test.ts:413-423), so the reset-module dynamic import cannot age the mocked lock past staleMs on real wall time. The async 75ms advance crosses the 20ms notice threshold and then the strict > 50ms stale check, allowing the existing reclaim path to resolve deterministically (manager.test.ts:427-443; manager.ts:147-164). The warning assertion still positively exercises the intended branch.

Timer restoration is describe-scoped and unconditional in afterEach (manager.test.ts:177-186), so failures cannot leak fake timers into sibling tests. This matches the established sibling fake-timer pattern at manager.test.ts:352-367.

Verification: current head e47af77c; authoritative diff is one test file, +16/-1. The focused wait-notice test passed locally; the manager file was 26/27 locally with only the environment-only package-resolution guard unable to locate @puppeteer/browsers in this review worktree. Required Linux Test, build, typecheck, runtime contract, and regression checks were green when the PR merged; Windows checks were still running.

Verdict: COMMENT (post-merge: Ready)
Reasoning: The fake-timer setup controls the exact clock that caused the flake, deterministically traverses both notice and stale-reclaim thresholds, and is safely restored after every test.

— Deepwork

[miga-heygen]

Re-review — test(cli): stabilize flaky install-lock wait-notice test with fake timers

Only change since my prior LGTM: advanceTimersByTimeAsync(...) reformatted from multi-line to single-line. No logic change.

LGTM — original review stands.

— Miga