Skip to content

fix(browser): isolate timed-out snippets#118

Open
MagMueller wants to merge 3 commits into
session-recoveryfrom
timeout-guard
Open

fix(browser): isolate timed-out snippets#118
MagMueller wants to merge 3 commits into
session-recoveryfrom
timeout-guard

Conversation

@MagMueller

@MagMueller MagMueller commented Jul 24, 2026

Copy link
Copy Markdown

Problem

browser_execute runs snippets in-process. JavaScript Promises cannot be preempted, so timing out the surrounding tool does not stop a yielding snippet. Its abandoned Promise can wake later, reconnect, and send CDP commands after the caller has already received a timeout.

Closing the socket alone is insufficient because the same Session object can call connect() again.

Change

  • Permanently invalidate the exact Session object used by a timed-out snippet.
  • Remove that object from SessionStore so the next tool call receives a fresh Session.
  • Reject pending calls, close the socket, clear listeners and target state, and reject every later transport operation on the abandoned object.
  • Compare the stored Session by identity so a late timeout cannot invalidate a newer replacement Session.
  • Keep the limitation explicit: CPU-bound JavaScript that never yields still cannot be interrupted in-process.

Reproduction

The regression test uses a local synthetic CDP WebSocket:

  1. Connect a Session.
  2. Run a snippet with a 10 ms tool timeout.
  3. Let the snippet sleep 50 ms, then attempt Runtime.evaluate.
  4. Verify the tool times out, the server receives zero late commands, the abandoned Session cannot reconnect, and the store returns a fresh Session for the next call.

Validation

  • bun test test/browser-execute.test.ts: 6 pass, 6 environment-gated skips
  • Regression test repeated 10 times: 10/10 pass
  • bun typecheck in packages/bcode-browser
  • Pre-push filtered typecheck: 13/13 packages pass
  • Full package suite: 21 pass, 8 environment-gated skips, 1 pre-existing unrelated edit + cache-bust workspace-import failure on Bun 1.3.14

Summary by cubic

Prevents timed-out browser_execute snippets from reconnecting or sending late CDP commands by permanently invalidating their Session. Also stops any in-flight or future connection attempts after invalidation; the next call gets a fresh Session.

  • Bug Fixes
    • Added Session.invalidate(error) to retire a Session: reject pending/future transport ops (connect, send, waitFor), close active and still-connecting sockets, clear listeners/state, gate isConnected, and guard connect/openWs across wsUrl/profile resolution and browser detection.
    • Added SessionStore.invalidate(sessionID, expected, error) to remove the exact timed-out Session by identity; SessionStore.get now returns a new Session after timeouts.
    • browser_execute now invalidates the active Session on timeout and returns a clearer error: "browser_execute timed out; CDP session was reset".
    • Tests cover: no late commands, no reconnection, closing sockets still connecting, rejecting pending/future event waiters, and blocking connections that would complete after profile resolution.
    • Limitation: CPU-bound JS that never yields still cannot be interrupted.

Written for commit f7bd611. Summary will update on new commits.

Review in cubic

@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.

1 issue found across 4 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/bcode-browser/src/browser-execute.ts">

<violation number="1" location="packages/bcode-browser/src/browser-execute.ts:235">
P2: Timed-out snippets awaiting `session.waitFor()` remain pending until their own timeout (30s by default), rather than failing when the tool expires. `invalidate()` clears their listener without rejecting the promise; track and reject these waiters (and reject new `waitFor` calls after invalidation) with the reset error.</violation>
</file>

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

Fix all with cubic | Re-trigger cubic

orElse: () =>
Effect.gen(function* () {
const error = new Error("browser_execute timed out; CDP session was reset")
yield* Effect.sync(() => SessionStore.invalidate(ctx.sessionID, session, error))

@cubic-dev-ai cubic-dev-ai Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: Timed-out snippets awaiting session.waitFor() remain pending until their own timeout (30s by default), rather than failing when the tool expires. invalidate() clears their listener without rejecting the promise; track and reject these waiters (and reject new waitFor calls after invalidation) with the reset error.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/bcode-browser/src/browser-execute.ts, line 235:

<comment>Timed-out snippets awaiting `session.waitFor()` remain pending until their own timeout (30s by default), rather than failing when the tool expires. `invalidate()` clears their listener without rejecting the promise; track and reject these waiters (and reject new `waitFor` calls after invalidation) with the reset error.</comment>

<file context>
@@ -228,9 +229,15 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
+        orElse: () =>
+          Effect.gen(function* () {
+            const error = new Error("browser_execute timed out; CDP session was reset")
+            yield* Effect.sync(() => SessionStore.invalidate(ctx.sessionID, session, error))
+            return yield* Effect.fail(error)
+          }),
</file context>
Fix with cubic

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

@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
Comment thread packages/bcode-browser/src/cdp/session.ts

@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.

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="packages/bcode-browser/src/cdp/session.ts">

<violation number="1" location="packages/bcode-browser/src/cdp/session.ts:89">
P2: A timed-out snippet using `{ profileDir }` can leave its `connect()` Promise polling for the entire profile-resolution timeout because invalidation is checked only after `resolveWsUrl()` returns. Making profile resolution cancellation-aware, or racing it with an invalidation signal, would release the abandoned operation immediately while retaining the post-resolution guard against a late socket.</violation>
</file>

Tip: Review your code locally with the cubic CLI to iterate faster.

Fix all with cubic | Re-trigger cubic

* and we connect directly to the supplied endpoint.
*/
async connect(opts: ConnectOptions = {}): Promise<void> {
this.throwIfInvalidated();

@cubic-dev-ai cubic-dev-ai Bot Jul 24, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2: A timed-out snippet using { profileDir } can leave its connect() Promise polling for the entire profile-resolution timeout because invalidation is checked only after resolveWsUrl() returns. Making profile resolution cancellation-aware, or racing it with an invalidation signal, would release the abandoned operation immediately while retaining the post-resolution guard against a late socket.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At packages/bcode-browser/src/cdp/session.ts, line 89:

<comment>A timed-out snippet using `{ profileDir }` can leave its `connect()` Promise polling for the entire profile-resolution timeout because invalidation is checked only after `resolveWsUrl()` returns. Making profile resolution cancellation-aware, or racing it with an invalidation signal, would release the abandoned operation immediately while retaining the post-resolution guard against a late socket.</comment>

<file context>
@@ -86,7 +86,7 @@ export class Session implements Transport {
    */
   async connect(opts: ConnectOptions = {}): Promise<void> {
-    if (this.invalidatedError) throw this.invalidatedError;
+    this.throwIfInvalidated();
 
     // No-argument connect is an ensure-connected operation. Reopening the
</file context>
Fix with cubic

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.

1 participant