fix(browser): reuse the process DevTools connection - #487
Conversation
|
🦞👀 Pull request received. I will update this pull request when review starts. ClawSweeper review completeClawSweeper finished reviewing this revision. The review result is being finalized. |
|
Live proof for head The standalone compiled-transport proof independently counted one handshake across three discovery/attach/delayed-evaluation/dispose cycles and natural child-process exit. Real CLI service-routing fixtures also cover repeated requests using one transport. Validation includes 51 focused tests and the 2,458-test composed suite; local and branch autoreview are clean through P2. All four CI jobs passed at final head For transparency, the maintenance session used two Chrome attaches total: one preliminary warning run before correcting its handshake counter, then the single final instrumented run above. There were no per-test reconnects. Release notes are in #486. |
|
Codex review: needs maintainer review before merge. Reviewed September 11, 2026, 11:42 PM ET / September 12, 2026, 03:42 UTC (Revision 4). ClawSweeper reviewWhat this changesOracle reuses one browser debugging connection per endpoint, cleans up page sessions independently, and adds transport checks plus reliable Windows proof cleanup. Merge readiness✅ Ready for maintainer review This PR remains useful: the reviewed main branch and v0.20.1 still close browser connections between operations. No blocking defect was found; the supplied instrumented browser proof supports connection reuse across service requests. Priority: P2 Review scores
Verification
How this fits togetherOracle’s browser engine sends prompts and captures answers through Chrome’s debugging connection. Target discovery, page automation, and recovery share that connection within one CLI or service process. flowchart TD
A[CLI or service request] --> B[Browser endpoint]
B --> C{Connection available?}
C -->|Yes| D[Shared browser connection]
C -->|No| E[Chrome approval and connection]
E --> D
D --> F[Independent page sessions]
F --> G[Automation and answer capture]
G --> H[Detach session and retain connection]
Before mergeNone. Agent review detailsSecurityNone. Review metrics
Technical reviewBest possible solution: Keep connection ownership at process scope while preserving independently disposable page sessions, existing authorization checks, and normal idle CLI exit. Do we have a high-confidence way to reproduce the issue? Yes, from source: main creates and closes separate browser WebSockets for discovery and attachment, explaining repeated per-connection approvals. This review did not execute a browser reproduction. Is this the best way to solve the issue? Yes. Sharing the browser transport while retaining separate page sessions directly repairs the ownership mismatch without adding configuration or replacing existing authentication and tab-selection rules. AGENTS.md: found and applied where relevant. Codex review notes: model internal, reasoning medium; reviewed against d1b2e38e2b59. LabelsLabel justifications:
EvidenceWhat I checked:
Likely related people:
Rating scale
Overall follows the weaker of proof and patch quality. Workflow
HistoryReview history (3 earlier review cycles) |
clawsweeper
Bot
added
P2
1 participant
Chrome approves each browser WebSocket separately. Oracle previously opened and closed that socket for target discovery and each page operation, so one run or successive service requests could repeatedly show Allow remote debugging?.
Retain one browser connection per endpoint for the process lifetime, share pending approvals, and give each page an independently disposable session. Cancellation and approval deadlines release only their own lease; failed handshakes and real disconnects permit reconnection. Page listeners are removed on disposal, command callbacks remain session-bound, failed attachment cleans up only its newly created tab, and idle sockets allow a normal CLI exit.
Fixes #484.
Validation: 51 focused connection/lifecycle tests, a 2,410-test full-suite baseline followed by the focused callback regression, and the composed batch's 2,458-test suite. The compiled transport proof counted one real WebSocket handshake across three discovery/attach/evaluate/dispose cycles and natural child-process exit; repeated real CLI clients also reused one service socket against a synthetic DevTools endpoint. CI now runs the standalone transport proof after an explicit build on every platform.
The instrumented signed-in proof uses one long-lived built service: five model-warning scenarios, a Pro answer, reattach, and harvest have completed with exactly one handshake/upgrade. The same service also delivered the generated PNG with its dimensions and file ID intact. The probe refuses any second handshake. Separate CLI processes still require separate approval; use one long-running service to share approval across client commands. Release notes are consolidated in #486.
Local and committed-branch autoreviews are clean through P2. All four CI jobs passed on final head
465843e5be5b96d3a0b8d6fa32776cf10e651f00: https://github.com/steipete/oracle/actions/runs/34669035154. Ready for the orchestrator’s squash-merge decision. The Windows run exposed an existing MCP proof cleanup race (EBUSY after signalling a detached worker). The follow-up waits for worker exit before directory removal and adds bounded filesystem retries without dropping any lifecycle assertion.