Merge pull request #136 from browser-use/timeout-recovery

fix(browser): preserve session after tool timeout

Author: MagMueller

packages/bcode-browser/skills/browser-execute/SKILL.md

@@ -110,6 +110,8 @@ If a target-scoped command throws `CdpError` code `-32001` (`Session with given
 
 Every explicit reconnect or browser switch retires the previous socket and clears its active target attachment. Re-list targets, call `session.use(...)`, and rediscover DOM nodes and Runtime objects before continuing.
 
+Opening a tab creates a new `page` target but does not switch the active attachment. Call `Target.getTargets` again and `session.use(targetId)` when continuing there.
+
 ## Driving a page
 Domain methods follow `session.<Domain>.<method>(params)` and return Promises. 
 The full surface (652 commands) is the Chrome DevTools Protocol.
@@ -200,7 +202,7 @@ console.log(JSON.stringify(titles))
 ## Guardrails
 - Top-level `import` statements inside the snippet body are not allowed. Use `await import(...)` instead.
 - No CPU-bound infinite loops without `await` — they ignore the timeout. Insert `await new Promise(r => setTimeout(r, 0))` to yield.
-- `browser_execute` defaults to 60s (max 600s). For longer work, set the tool's top-level `timeout`; inner CDP timeouts do not extend it. Keep batches small and log progress — timeout errors return recent logs, and a timeout resets the CDP session. Reconnect deliberately after a timeout so a run that switched browsers cannot silently return to its original browser.
+- `browser_execute` defaults to 60s; longer timeouts delay your next turn. A timeout does not close CDP, though its last command may still run. `Target.getTargets` succeeding means CDP is live; `session.connect()` is then a no-op, and reattaching the same target does not restart its renderer.
 
 ## Console
 - `console.log`, `console.error`, `console.warn`, `console.info`, `console.debug` are all captured and streamed to the user. Treat them as your stdout. Other `console.*` methods write to bcode's stderr without being captured into the tool result.

packages/bcode-browser/src/browser-execute.ts

@@ -35,20 +35,18 @@
 //
 // Cancellation: JS Promises are not preemptively cancellable. A snippet
 // without `await` yield-points (e.g. `for (let i = 0; i < 1e9; i++) {}`)
-// runs to completion before our timeout fiber observes it. When a yielding
-// snippet times out, its Promise keeps running as an orphan — so on timeout
-// we retire the exact Session object the snippet received (rejects future
-// connect/_call, closes the socket) and evict it from SessionStore. The
-// orphan can finish local work but cannot keep driving the browser, and the
-// next tool call gets a fresh Session instead of sharing a socket with it.
-// The timeout error carries the console output captured so far.
+// runs to completion before our timeout fiber observes it. A yielding snippet
+// keeps running as an orphan after timeout, so each call receives a scoped
+// Session view. The view rejects methods after its deadline while the real
+// Session and its tabs remain available to the next call.
 //
 // Level 1 per decisions.md §1c — substantial implementation lives here. The
 // Level-2 hook in packages/opencode is a thin adapter.
 
 import fs from "fs/promises"
 import path from "path"
 import { Effect, Schema } from "effect"
+import { withSessionExecution } from "./cdp/session"
 import { SessionStore } from "./session-store"
 import { Skills } from "./skills"
 
@@ -175,13 +173,13 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
   const skillsDir = yield* Effect.promise(() => Skills.resolveSkillsDir(dataDir))
 
   // Effect values are re-runnable, so per-run state lives inside the suspend
-  // thunk: each run resolves its own Session (the timeout handler retires
-  // exactly that object) and its own capture buffer (a re-run after a timeout
-  // must not inherit a retired Session or a frozen capture).
+  // thunk: each run gets its own execution scope and capture buffer. A re-run
+  // after a timeout must not inherit an inactive scope or frozen capture.
   const execute = (args: Parameters, ctx: ExecuteContext) =>
     Effect.suspend(() => {
     const session = SessionStore.get(ctx.sessionID)
     const captured = { active: true, output: "" }
+    const sessionExecution = { active: true }
     const timeout = Math.min(args.timeout ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS)
     return Effect.gen(function* () {
       yield* Effect.promise(() => fs.mkdir(ctx.workspaceDir, { recursive: true }))
@@ -248,7 +246,7 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
       })
 
       const ran = yield* Effect.tryPromise({
-        try: () => wrapped(session, snippetConsole),
+        try: () => withSessionExecution(sessionExecution, () => wrapped(session, snippetConsole)),
         catch: (err) => new Error(`browser_execute snippet threw: ${err instanceof Error ? err.stack ?? err.message : String(err)}`),
       }).pipe(Effect.ensuring(Effect.sync(() => unsubscribe())))
 
@@ -260,21 +258,16 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
         orElse: () =>
           Effect.suspend(() => {
             captured.active = false
+            sessionExecution.active = false
             const output = timeoutOutput(captured.output)
             const error = new Error(
               [
-                `browser_execute timed out after ${timeout} ms; CDP session was reset — reconnect in the next snippet`,
+                `browser_execute timed out after ${timeout} ms; the timeout did not close the CDP session`,
                 output.trim() ? `Partial console output before timeout:\n${output.trimEnd()}` : "",
               ]
                 .filter(Boolean)
                 .join("\n\n"),
             )
-            // Always retires this snippet's Session; the identity check
-            // inside only guards the store delete, so a successor Session is
-            // never evicted. A concurrent same-sessionID call would share the
-            // retired object — acceptable for v1, opencode serializes tool
-            // calls within an assistant message.
-            SessionStore.invalidate(ctx.sessionID, session, error)
             return Effect.fail(error)
           }),
       }),
@@ -304,9 +297,8 @@ async function ensureCloudConnected(sessionID: string, session: ReturnType<typeo
     await connecting
   } finally {
     // One automatic attempt per logical BrowserCode session. A later disconnect
-    // (including after timeout replacement) must be surfaced:
-    // BU_CDP_WS is the browser selected at run start, not necessarily a newer
-    // browser the agent explicitly switched to during this run.
+    // must be surfaced: BU_CDP_WS is the browser selected at run start, not
+    // necessarily a newer browser the agent explicitly switched to during this run.
     v4Bootstrapped.add(sessionID)
     if (v4Connections.get(sessionID) === connecting) v4Connections.delete(sessionID)
   }

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

@@ -6,13 +6,29 @@
  * Target.sendMessageToTarget envelopes).
  */
 
+import { AsyncLocalStorage } from 'node:async_hooks';
 import { bindDomains, type Domains, type Transport } from './generated.ts';
 
 type Pending = {
   resolve: (v: unknown) => void;
   reject: (e: unknown) => void;
 };
 
+export type SessionExecution = { active: boolean };
+
+const sessionExecution = new AsyncLocalStorage<SessionExecution>();
+
+export const withSessionExecution = <T>(
+  execution: SessionExecution,
+  run: () => T,
+): T => sessionExecution.run(execution, run);
+
+const assertExecutionActive = (): void => {
+  if (sessionExecution.getStore()?.active === false) {
+    throw new Error('browser_execute call already timed out');
+  }
+};
+
 export type ConnectOptions = {
   /** Full WS URL: ws://host:port/devtools/browser/<id>. Escape hatch. */
   wsUrl?: string;
@@ -117,6 +133,7 @@ export class Session implements Transport {
   }
 
   private openWs(wsUrl: string, timeoutMs: number): Promise<void> {
+    assertExecutionActive();
     // Re-checked here (not only in connect) because connect awaits resolver/
     // detection steps first — an invalidation landing during those must not
     // open a late socket for a retired Session.
@@ -170,21 +187,20 @@ export class Session implements Transport {
   }
 
   close(): void {
+    assertExecutionActive();
     this.ws?.close();
   }
 
   /**
    * Permanently retire this Session object.
    *
-   * `browser_execute` timeouts cannot preempt the snippet's Promise — the
-   * orphan keeps running and would otherwise share this object (and its
-   * socket) with the next tool call, interleaving two authors on one
-   * transport. Invalidation rejects all future `connect`/`_call` attempts
-   * and closes the socket (the close handler rejects in-flight calls);
-   * `SessionStore.invalidate` removes the entry so the next call gets a
-   * fresh Session.
+   * Invalidation rejects all future `connect`/`_call` attempts and closes the
+   * socket; `SessionStore.invalidate` removes the entry so a later lookup gets
+   * a fresh Session. `browser_execute` timeouts use scoped execution instead,
+   * preserving this object and its browser connection for the next call.
    */
   invalidate(error: Error): void {
+    assertExecutionActive();
     if (this.invalidatedError) return;
     this.invalidatedError = error;
     const ws = this.ws;
@@ -199,12 +215,14 @@ export class Session implements Transport {
    */
   async use(targetId: string): Promise<string> {
     const r = await this._call('Target.attachToTarget', { targetId, flatten: true }) as { sessionId: string };
+    assertExecutionActive();
     this.activeSessionId = r.sessionId;
     return r.sessionId;
   }
 
   /** Set the active sessionId directly (e.g. one you already attached). */
   setActiveSession(sessionId: string | undefined): void {
+    assertExecutionActive();
     this.activeSessionId = sessionId;
   }
 
@@ -214,9 +232,19 @@ export class Session implements Transport {
 
   /** Subscribe to all CDP events. Returns an unsubscribe fn. */
   onEvent(fn: (method: string, params: unknown, sessionId?: string) => void): () => void {
-    this.eventListeners.push(fn);
+    assertExecutionActive();
+    // WebSocket events arrive in the socket's async context, not the context
+    // where the listener was registered. Restore that registration context so
+    // callbacks created by a timed-out browser_execute call cannot keep using
+    // the persistent Session after their execution scope is deactivated.
+    const execution = sessionExecution.getStore();
+    const listener = execution
+      ? (method: string, params: unknown, sessionId?: string) =>
+          sessionExecution.run(execution, () => fn(method, params, sessionId))
+      : fn;
+    this.eventListeners.push(listener);
     return () => {
-      this.eventListeners = this.eventListeners.filter(x => x !== fn);
+      this.eventListeners = this.eventListeners.filter(x => x !== listener);
     };
   }
 
@@ -231,9 +259,15 @@ export class Session implements Transport {
    * agnostic of any one method's semantics.
    */
   onCallResult(fn: (method: string, params: unknown, result: unknown) => void): () => void {
-    this.callResultListeners.push(fn);
+    assertExecutionActive();
+    const execution = sessionExecution.getStore();
+    const listener = execution
+      ? (method: string, params: unknown, result: unknown) =>
+          sessionExecution.run(execution, () => fn(method, params, result))
+      : fn;
+    this.callResultListeners.push(listener);
     return () => {
-      this.callResultListeners = this.callResultListeners.filter(x => x !== fn);
+      this.callResultListeners = this.callResultListeners.filter(x => x !== listener);
     };
   }
 
@@ -249,6 +283,7 @@ export class Session implements Transport {
     opts: { predicate?: (params: T) => boolean; timeoutMs?: number } = {},
     ...rest: never[]
   ): Promise<T> {
+    assertExecutionActive();
     // Both legacy positional shapes fail loudly rather than silently reverting
     // to the 30s default: `(method, predicate)` lands on the first guard,
     // `(method, predicate?, timeoutMs)` on the second. Snippets are written at
@@ -288,6 +323,7 @@ export class Session implements Transport {
 
   // Transport implementation. Called by the generated domain bindings.
   _call(method: string, params: unknown = {}): Promise<unknown> {
+    assertExecutionActive();
     if (this.invalidatedError) return Promise.reject(this.invalidatedError);
     if (!this.ws || this.ws.readyState !== WebSocket.OPEN) {
       return Promise.reject(new Error('Not connected. Call session.connect(...) first.'));

packages/bcode-browser/test/browser-auto-connect.test.ts

@@ -34,6 +34,18 @@ const server = Bun.serve({
         typeof request.method !== "string"
       )
         return;
+      if (
+        request.method === "Page.navigate" &&
+        "params" in request &&
+        request.params &&
+        typeof request.params === "object" &&
+        "url" in request.params &&
+        request.params.url === "https://stuck.example"
+      ) {
+        pageCallsWithSession++;
+        setTimeout(() => ws.send(JSON.stringify({ id: request.id, result: {} })), 40);
+        return;
+      }
       const result = (() => {
         if (request.method === "Target.getTargets")
           return {
@@ -276,8 +288,10 @@ test("an explicit browser switch retires the old socket and target attachment",
   );
 });
 
-test("a timeout replacement does not auto-attach the run-start browser again", async () => {
+test("a timeout preserves the same browser and target", async () => {
   connections = 0;
+  attachedCalls = 0;
+  pageCallsWithSession = 0;
   await withEnv(
     { V4_RUN_ID: "run-timeout", BU_CDP_WS: wsUrl, BU_CDP_URL: undefined },
     () =>
@@ -287,26 +301,31 @@ test("a timeout replacement does not auto-attach the run-start browser again", a
             impl.execute(
               {
                 description: "Time out after initial V4 bootstrap",
-                code: "await new Promise(resolve => setTimeout(resolve, 100))",
+                code: `
+                  await session.Page.navigate({ url: "https://stuck.example" })
+                  try { await session.Page.navigate({ url: "https://too-late.example" }) } catch {}
+                `,
                 timeout: 10,
               },
               { sessionID, workspaceDir },
             ),
           ),
         ).rejects.toThrow("browser_execute timed out");
 
-        await expect(
-          Effect.runPromise(
-            impl.execute(
-              {
-                description: "Do not silently return to the run-start browser",
-                code: "return await session.Page.navigate({ url: 'https://sap.com' })",
-              },
-              { sessionID, workspaceDir },
-            ),
+        const recovered = await Effect.runPromise(
+          impl.execute(
+            {
+              description: "Continue on the same browser",
+              code: "return await session.Page.navigate({ url: 'https://sap.com' })",
+            },
+            { sessionID, workspaceDir },
           ),
-        ).rejects.toThrow("Not connected. Call session.connect(...) first.");
+        );
+        expect(JSON.parse(recovered.result)).toEqual({});
+        await new Promise((resolve) => setTimeout(resolve, 40));
         expect(connections).toBe(1);
+        expect(attachedCalls).toBe(1);
+        expect(pageCallsWithSession).toBe(2);
       }),
   );
 });

packages/bcode-browser/test/browser-execute.test.ts

@@ -225,9 +225,9 @@ test("console.debug is captured; uncommon methods fall through without throwing"
 })
 
 // Timeout isolation: a timed-out snippet keeps running as an orphan (JS
-// Promises are not preemptible), so the tool must retire the Session object
-// the snippet received and surface captured output in the error. No Chrome
-// required — the snippets sleep without touching the browser.
+// Promises are not preemptible), so its scoped Session view must stop working
+// while the persistent Session remains available to the next call. Browser
+// behavior is covered by browser-auto-connect; these snippets need no Chrome.
 const runTimeout = async (id: string, code: string, timeout: number, onChunk?: (o: string) => Effect.Effect<void>) => {
   const data = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-to-"))
   const ws = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-to-ws-"))
@@ -249,7 +249,7 @@ const runTimeout = async (id: string, code: string, timeout: number, onChunk?: (
   return err
 }
 
-test("timeout returns partial output and retires the session", async () => {
+test("timeout returns partial output and preserves the session", async () => {
   const id = "timeout-isolation-test"
   const before = SessionStore.get(id)
   const err = await runTimeout(
@@ -261,12 +261,10 @@ test("timeout returns partial output and retires the session", async () => {
   expect(err).toContain("timed out after 100 ms")
   expect(err).toContain("Partial console output before timeout:")
   expect(err).toContain("progress-marker")
-  // The orphan's Session is permanently dead...
+  // The next call gets the exact same persistent Session. The orphan only had
+  // a scoped view, whose post-timeout CDP behavior is covered by the V4 test.
   expect(before.isConnected()).toBe(false)
-  await expect(before.connect({ wsUrl: "ws://127.0.0.1:9/nope" })).rejects.toThrow(/timed out after 100 ms/)
-  await expect(before.domains.Runtime.evaluate({ expression: "1" })).rejects.toThrow(/timed out after 100 ms/)
-  // ...and the next tool call gets a fresh one.
-  expect(SessionStore.get(id)).not.toBe(before)
+  expect(SessionStore.get(id)).toBe(before)
   await SessionStore.evict(id)
 })
 
@@ -294,7 +292,7 @@ test("re-running the execute effect after a timeout gets fresh state", async ()
   const impl = await Effect.runPromise(BrowserExecute.make(data))
   // One Effect value, run twice. Each run must resolve its own Session and
   // capture buffer — the second run's error must carry its own partial
-  // output, not inherit the first run's frozen capture or retired Session.
+  // output, not inherit the first run's frozen capture or scoped view.
   // onChunk deliveries discriminate: a run that inherited a frozen capture
   // buffer never tees, so it produces zero chunks (the frozen buffer still
   // *contains* run 1's text, which is why asserting on the error message