fix(browser): recover answers after recoverable CDP disconnect - #327
Conversation
Probe DevTools liveness on client disconnect and attempt a one-shot auto-reattach on connection-lost so completed Pro answers are harvested when Chrome and the target remain alive. Fixes steipete#326
|
Codex review: needs maintainer review before merge. Reviewed July 18, 2026, 3:34 PM ET / 19:34 UTC. Summary Reproducibility: yes. at source level: the prior path treated every CDP client disconnect as a closed Chrome window, while the PR supplies focused tests and real Chrome/Chromium forced-disconnect scripts for the recoverable-target scenario. I did not execute the live browser harness during this read-only review. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Security Review detailsBest possible solution: Merge the bounded recovery path after the required checks complete, retaining confirmed-target gating for automatic reattachment and the existing running-session guidance fallback when recovery cannot finish. Do we have a high-confidence way to reproduce the issue? Yes, at source level: the prior path treated every CDP client disconnect as a closed Chrome window, while the PR supplies focused tests and real Chrome/Chromium forced-disconnect scripts for the recoverable-target scenario. I did not execute the live browser harness during this read-only review. Is this the best way to solve the issue? Yes. Distinguishing endpoint and target liveness, then attempting one bounded reattach only for a confirmed live target, is narrower and safer than treating every disconnect as either terminal or indefinitely retryable. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning high; reviewed against 8e3456e7f2dc. Label changesLabel changes:
Label justifications:
Evidence reviewedWhat I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (11 earlier review cycles; latest 8 shown)
|
Demonstrate recoverable client disconnect vs closed-Chrome fallback against a live DevTools endpoint for PR merge readiness.
|
Posted merge-readiness proof for both disconnect outcomes against real Chrome CDP.
Updated the PR body with the redacted transcript. @clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Keep the user-visible note in the PR body instead of editing CHANGELOG.md.
Add an end-to-end harness that drops Oracle's CDP sockets mid-run while Chrome stays alive, then requires auto-reattach to complete the session. Also fix inline-cookie expiration normalization so Unix-second expiries from sweet-cookie are not treated as milliseconds.
|
@clawsweeper re-review Full browser E2E proof is now on the branch:
Live run forced a mid-flight CDP socket drop while Chrome stayed alive; Oracle kept the session reattachable, auto-reattach harvested the answer, and marked the session Redacted excerpt is in the PR body. Also fixed inline-cookie Unix-second expiration mangling that was breaking ChatGPT cookie reuse during the E2E setup. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
writeFileSync umask/mode-on-create alone left reusable session cookies readable after overwrite of a permissive destination. Always chmod 0600 and add focused permission regression coverage.
|
@clawsweeper re-review Addressed the P1 cookie-export permission finding:
Head: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Open/truncate and fchmod 0600 on the destination before writing secret bytes so an existing permissive file cannot expose cookies mid-write.
|
@clawsweeper re-review Addressed the P1 descriptor-before-write finding on cookie export:
Head: |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Add Ubuntu/Docker classification proof plus CI Chromium smoke, skip auto-reattach on non-recoverable disconnects, and bound one-shot recovery.
|
@clawsweeper re-review Linux/Ubuntu classification parity + fail-closed recovery:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Add container-opt-in Chrome flags and an Xvfb/gdb harness that forces a live ChatGPT disconnect on Debian Chromium and completes via auto-reattach.
|
@clawsweeper re-review Authenticated Linux forced-disconnect E2E now passes:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Treat target-list probe errors as non-recoverable so auto-reattach only runs after a confirmed live target, reducing false-positive resumes.
|
@clawsweeper re-review Fail-closed liveness tightening for the remaining availability residual:
|
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
Summary
Fixes recoverable CDP client disconnects that previously stranded completed browser answers as permanently
running.When the target-level DevTools WebSocket drops mid-run, Oracle used to treat every disconnect as "Chrome window closed," keep the session running, and skip the auto-reattach path that already exists for
assistant-timeout. In #326 the browser and page target were still alive, ChatGPT finished the answer, and Oracle never harvested it.Approach
/json/version+ optional target list) after a CDP client disconnect.recoverableDisconnectinstead of a closed-window message.connection-lostwithrecoverableDisconnect: true, attempt a one-shotresumeBrowserSessionauto-reattach by default; keep the existing "session stays running + guidance" fallback when resume fails.autoReattachIntervalMsis configured and the disconnect is recoverable, retain multi-attempt polling (hard-capped).Runtime proof (merge readiness)
Platform parity table
cdp-disconnect-proof.mjs)PROOF_OKcdp-disconnect-proof.mjs)node:24-bookworm+ Chromium)PROOF_OKubuntu-latest+ Chromium (cdp-disconnect-proof-linux)E2E_PROOF_OK→completedgdbfd close)E2E_PROOF_OK→completedIssue #326 was reported on Ubuntu. Classification + liveness probing is platform-neutral HTTP against DevTools; Linux Docker and the new Ubuntu CI job cover that residual.
1) Classification proof (no ChatGPT login)
Script:
node scripts/cdp-disconnect-proof.mjsLinux helper:
bash scripts/run-cdp-disconnect-proof-linux.shRedacted macOS transcript (2026-07-17):
Redacted Linux Docker transcript (2026-07-17, Debian bookworm + Chromium):
2) Full browser E2E (forced disconnect → auto-reattach → completed)
Script:
scripts/oracle-e2e-cdp-disconnect-proof.mjsFlow used:
scripts/export-chatgpt-cookies.mjs(session=true).--browser-inline-cookies-file+--browser-model-strategy current.promptSubmitted, forcibly close Oracle's established CDP sockets while Chrome stays up (macOS:lldb; Linux:gdb).completed.Redacted transcript excerpt (2026-07-17, macOS):
Redacted Linux Docker transcript excerpt (2026-07-17, Debian bookworm + Chromium + Xvfb):
Also includes a small fix: inline-cookie expiration normalization no longer treats Unix-second expiries (~1.7e9) as milliseconds (which was zeroing ChatGPT session cookies to 1970 and causing false login failures).
3) Cookie export permissions
scripts/export-chatgpt-cookies.mjsnow writes throughwriteOwnerOnlyFile, which always enforces mode0600after write (including overwrite of an existing permissive destination). Focused coverage:tests/scripts/write-owner-only-file.test.ts.Local verification:
Availability / fail-closed notes
maxAttempts: 1) unlessautoReattachIntervalMsis explicitly configured.runningwith reattach guidance (manual recovery still possible).merge-risk: availabilityremains inherent to disconnect recovery; mitigated by classification + bounded retries + Linux classification parity.Test plan
pnpm exec vitest run tests/browser/cdpLiveness.test.ts tests/cli/sessionRunner.test.ts(includes one-shot bound + non-recoverable skip)pnpm exec vitest run tests/browser/cookies.test.ts(Unix-second expiry case)pnpm exec vitest run tests/scripts/write-owner-only-file.test.tspnpm run typecheck/ buildnode scripts/cdp-disconnect-proof.mjs→PROOF_OKbash scripts/run-cdp-disconnect-proof-linux.sh→PROOF_OKORACLE_BROWSER_COOKIES_FILE=… node scripts/oracle-e2e-cdp-disconnect-proof.mjs→E2E_PROOF_OK(macOS)bash scripts/run-oracle-e2e-cdp-disconnect-linux.sh→E2E_PROOF_OK(Linux Docker + Xvfb + gdb)cdp-disconnect-proof-linuxadded forubuntu-latestFixes #326