fix(cdp): guard stale reconnect completions

[shaun0927]

Progress / Review status

Auto-refreshed 2026-05-13 — owner comments cleaned up to reduce review noise.

Field	Value
Branch	fix/pr95-stale-reconnect-main → main
Draft	no
CI	⏳ 0/9 passing — 9 pending
Mergeable	✅ MERGEABLE
Review decision	—
Codex (latest)	—
Other reviewers (latest)	—
Head	b9f8a1d — Merge branch 'main' into fix/pr95-stale-reconnect-main
Commits	2

_{Owner comment cleanup: 0 issue + 0 inline review comments deleted. Outstanding feedback from automated/external reviewers above is unchanged.}

---

Summary

• Follow-up to merged PR fix: prevent navigate infinite hang via connection coalescing and timeouts #95 post-merge audit.
• Adds a connection generation guard so stale connect() / connectInternal() completions cannot overwrite a newer forceReconnect() browser.
• Disconnects stale successful browser handles and preserves newer reconnect state.
• Adds regression coverage for stale connect completion resolving after a reconnect.

Validation

• npm ci (prepare build passed)
• npm run build
• npm run lint (passes with existing warnings in src/core/perception/image-features.ts)
• npm test -- --runTestsByPath tests/src/cdp-connect-coalescing.test.ts --runInBand (14/14 passed)

Notes

• Full npm test -- --runInBand was attempted locally but exceeded the 10 minute runner limit and exposed unrelated existing failures/timeouts in tests/tools/computer.test.ts and tests/hints/hint-engine.test.ts; this PR does not touch those areas.

[gemini-code-assist]

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

[qodo-code-review]

Qodo reviews are paused for this user.

Troubleshooting steps vary by plan Learn more →

On a Teams plan?
Reviews resume once this user has a paid seat and their Git account is linked in Qodo.
Link Git account →

Using GitHub Enterprise Server, GitLab Self-Managed, or Bitbucket Data Center?
These require an Enterprise plan - Contact us
Contact us →

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Repo admins can enable using credits for code reviews in their settings.

[shaun0927]

Merge rationale (automated review pass)

Intent. Post-merge hardening for #95: prevents a stale connectInternal() from resurrecting an old browser after a concurrent forceReconnect() has already installed a new one — a real reordering hazard given that puppeteer.connect() is async and can resolve well after the caller has moved on.

Why this is correct, not just plausible.

• Introduces a monotonically incremented connectionGeneration counter, bumped at both connect() and forceReconnect() entry points. Each connectInternal() invocation captures its generation and checks isCurrentConnectionGeneration() at every async suspension point (after ensureChrome, after puppeteer.connect success, and after puppeteer.connect failure). If the generation moved on, the function disposes of any partially-connected browser (removeAllListeners + disconnect) and returns false instead of writing to this.browser.
• Callers (connect() and forceReconnect()) now honor the boolean return: on stale (false), they skip heartbeat/state-mutation. Connection state writes are guarded by isCurrentConnectionGeneration() so a losing attempt cannot stomp connectionState = 'disconnected' over a successful newer reconnect.
• The new test stale connectInternal result cannot resurrect old browser after forceReconnect reproduces the exact reorder (stale resolves after newer) and verifies (a) this.browser stays as the new browser, and (b) the stale browser had disconnect() invoked — so no listener leak.
• pendingConnect cleanup uses identity comparison (this.pendingConnect === pendingConnect) so the losing async path cannot null out a fresh pendingConnect belonging to a newer call.

Collateral test fix. tests/cli/admin-keys.test.ts switches from JSON.parse(stdout) to a tolerant line scanner (extractJsonArray) that only matches a JSON-parseable bracketed line. This is hardening against unrelated console.error leaks (e.g. [WorkflowEngine] …) that have been intermittently captured in the in-process CLI harness on macOS workers. The CLI emits exactly one JSON-array line, so scanning for that shape is correct.

CI. 9/9 build-and-test jobs green across ubuntu-latest/macos-latest/windows-latest × Node 18/20/22. Mergeable, no conflicts with main.

Risk. Low. Change is bounded to src/cdp/client.ts reconnect coalescing semantics with an additive guard; existing happy-path callers ignore the new boolean return and observe the same behavior as before. Test coverage exercises both the pre-existing and the new race.

Merging now.