Skip to content

fix(browser): recover stale CDP sessions - #112

Closed
MagMueller wants to merge 5 commits into
mainfrom
session-recovery
Closed

fix(browser): recover stale CDP sessions#112
MagMueller wants to merge 5 commits into
mainfrom
session-recovery

Conversation

@MagMueller

@MagMueller MagMueller commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Problem

A flattened CDP target session can disappear while the browser-level WebSocket and page target remain usable. Reopening the root socket does not repair that target session, and an obsolete socket can still reject calls owned by a replacement socket.

Recovery also must not replay an arbitrary command on a different page. A rejected click, navigation, or evaluation is safe to retry only after reattaching the exact original target.

Change

  • Treat no-argument connect() as ensure-connected, preserving a healthy socket and active target.
  • Scope pending calls and close handling to the WebSocket that owns them.
  • On Chrome error -32001, serialize recovery, list targets at browser level, and reattach the exact original page.
  • Restore only domains that were enabled on the stale session.
  • Retry the rejected command once only after exact-target reattachment.
  • If the original target closed or is unknown, return a clear error without creating another page or replaying the command.
  • Never replay after an ambiguous socket drop.

Reproduction

The tests use a local synthetic CDP WebSocket:

  1. Attach page-1 and return session-1.
  2. Reject a page command with CDP -32001: Session with given id not found.
  3. Keep page-1 in Target.getTargets.
  4. Verify one reattachment to page-1, domain restoration, and one successful retry.

A second case removes page-1 before recovery and verifies that a side-effecting command is sent only once, no blank page is created, and no other target receives the command.

Validation

  • bun test in packages/bcode-browser: 21 pass, 8 environment-gated skips
  • bun typecheck in packages/bcode-browser
  • Pre-push filtered typecheck: 13/13 packages pass
  • git diff --check

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 2 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/bcode-browser/src/cdp/session.ts Outdated

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed

Reply with feedback, questions, or to request a fix.

Fix all with cubic | Re-trigger cubic

Comment thread packages/bcode-browser/src/cdp/session.ts Outdated
Comment thread packages/bcode-browser/src/cdp/session.ts Outdated
@Alezander9

Copy link
Copy Markdown
Member

Thanks Magnus — the diagnosis here is correct and it uncovered a real production problem. But we traced it one layer further and the root cause is on our side in cloud, not in session.ts.

Where the -32001 comes from

CDP sessionIds are owned by the WebSocket connection that created them. When a socket closes, Chrome detaches every session that socket created, and a different socket has never heard of that sessionId — even to the same browser and the same tab.

No-arg session.connect() unconditionally opens a new WebSocket (session.ts:81-113) and does not clear activeSessionId. So any snippet that reconnects inherits a sessionId belonging to the previous socket, and every page-level command after that fails with CDP -32001: Session with given id not found.

And we were telling the agent to do exactly that. cloud/backend/sandboxes/v4-worker/app.py:1193-1194 hands every UI run this:

// Attach if not already attached
await session.connect()

That comment is false. The Session object — socket and activeSessionId — persists across browser_execute calls via SessionStore (session-store.ts:22-28), so the correct pattern is connect once, use() once, then just drive. The defensive reconnect at the top of every snippet is what manufactures the stale-session storm this PR recovers from.

Same as #111#114: the prompt taught a pattern the API doesn't support.

Why we don't want the code-side fix either

Two reasons beyond "the prompt caused it".

-32001 is the right error. Note that connect() deliberately does not clear activeSessionId. If it did, page commands would go out with no sessionId and be silently interpreted as browser-level commands — a much worse, much quieter failure. Instead the agent gets a loud, precise, learnable error meaning "you are on a new socket, your attachment is gone." We'd rather teach that than hide it.

Making no-arg connect() return early when connected introduces a worse bug. isConnected() is readyState === WebSocket.OPEN (session.ts:139-141). A connection that dies without a FIN — VM paused, NAT timeout, proxy tunnel dropped — stays OPEN indefinitely. That is precisely the state in which an agent tries to reconnect. An early return would mean: agent correctly diagnoses a dead socket, calls connect(), and we silently do nothing and return success. The agent did the right thing and the tool lied to it. connect() always connecting is both the least surprising behavior and the one that matches every other CDP client.

On the recovery machinery specifically

Independent of the above, we're wary of the enabledDomains replay even if -32001 persisted. Replaying X.enable restores event delivery, but a real page session also carries Page.addScriptToEvaluateOnNewDocument, Emulation.* overrides, Fetch/Network interception patterns, and session-scoped auth. The agent would get a session that reports healthy and behaves subtly differently — and blindly replaying Fetch.enable re-arms request interception on a page whose state has moved on. A partially-restored session that looks complete is worse than a clear error.

The retry-once reasoning is sound, though — -32001 means Chrome rejected before executing, so replay is safe. That distinction was the right call and we'd want it if we ever do build this.

Two real bugs you surfaced that we are tracking separately

Both are internal correctness, both need two concurrent sockets to manifest, and neither changes agent-facing semantics. Recording them so they aren't lost:

  1. this.ws = ws is assigned synchronously before the socket opens (session.ts:135). A failed connect({ wsUrl: B }) clobbers a working connection to A.
  2. The close handler rejects all pending calls, not just the ones owned by that socket (session.ts:130-134), so socket B's failure rejects socket A's in-flight work.

We'll fix these on evidence rather than pre-emptively, since the prompt fix makes two-socket states rare.

What's happening instead

A cloud-side prompt fix: correct the false comment at app.py:1193-1194, and — the gap your PR made us notice — teach the reattach step after a legitimate reconnect. app.py:1352-1353 tells the agent to reconnect on a dropped socket but never says to re-use() the tab afterward, so even a correct recovery lands in -32001. Same for the browser-swap recipe at app.py:1373-1374.

We're also merging #115 as-is. Its -32001 paragraph — especially "Repeated session.connect() calls do not replace a stale target session" — is exactly the teaching that closes this loop, and it belongs in the skill.

Closing this one. If -32001 survives the prompt fix in production, please reopen with the counts and we'll revisit the recovery path with data.

@Alezander9

Copy link
Copy Markdown
Member

Cloud-side fix is up: browser-use/cloud#5223.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants