Merge pull request #126 from browser-use/timeout-isolation

fix(browser): retire timed-out snippet sessions and return partial output

Author: Alezander9

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

@@ -197,6 +197,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 in the next snippet).
 
 ## 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

@@ -32,10 +32,13 @@
 //
 // 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. `Effect.timeoutOrElse`
-// fails the surrounding fiber but the orphan Promise keeps running until it
-// finishes. This matches the `uv run` subprocess case (SIGTERM only after
-// the Python signal handler yields). Document, don't fix.
+// 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.
 //
 // Level 1 per decisions.md §1c — substantial implementation lives here. The
 // Level-2 hook in packages/opencode is a thin adapter.
@@ -48,6 +51,18 @@ import { Skills } from "./skills"
 
 const DEFAULT_TIMEOUT_MS = 60 * 1000
 const MAX_TIMEOUT_MS = 10 * 60 * 1000
+const MAX_TIMEOUT_OUTPUT_BYTES = 8 * 1024
+const TIMEOUT_OUTPUT_TRUNCATED = "[partial console output truncated; showing final bytes]\n"
+
+// Tail-cap the captured output for the timeout error: last 8 KiB, snapped
+// forward to a UTF-8 sequence start so multibyte characters survive the cut.
+const timeoutOutput = (output: string) => {
+  const bytes = Buffer.from(output, "utf8")
+  if (bytes.length <= MAX_TIMEOUT_OUTPUT_BYTES) return output
+  let start = bytes.length - (MAX_TIMEOUT_OUTPUT_BYTES - Buffer.byteLength(TIMEOUT_OUTPUT_TRUNCATED))
+  while (start < bytes.length && (bytes[start]! & 0xc0) === 0x80) start++
+  return TIMEOUT_OUTPUT_TRUNCATED + bytes.subarray(start).toString("utf8")
+}
 
 // Field order matters: providers stream tool-call args in schema-declared
 // order, so the model commits to whichever field comes first. `code` is the
@@ -157,20 +172,27 @@ const serialize = (v: unknown): string => {
 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).
   const execute = (args: Parameters, ctx: ExecuteContext) =>
-    Effect.gen(function* () {
-      const session = SessionStore.get(ctx.sessionID)
+    Effect.suspend(() => {
+    const session = SessionStore.get(ctx.sessionID)
+    const captured = { active: true, output: "" }
+    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 }))
 
       const wrapped = yield* Effect.try({
         try: () => new AsyncFunction("session", "console", args.code),
         catch: (err) => new Error(`syntax error in browser_execute snippet: ${err}`),
       })
 
-      let output = ""
       const tee = (...a: unknown[]) => {
-        output += a.map((x) => (typeof x === "string" ? x : serialize(x))).join(" ") + "\n"
-        if (ctx.onChunk) Effect.runFork(ctx.onChunk(output))
+        if (!captured.active) return
+        captured.output += a.map((x) => (typeof x === "string" ? x : serialize(x))).join(" ") + "\n"
+        if (ctx.onChunk) Effect.runFork(ctx.onChunk(captured.output))
       }
       // Prototype-chain to the real `console` so uncommon methods (`debug`,
       // `dir`, `trace`, `table`, `group`, …) don't throw when a snippet calls
@@ -223,14 +245,34 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
         catch: (err) => new Error(`browser_execute snippet threw: ${err instanceof Error ? err.stack ?? err.message : String(err)}`),
       }).pipe(Effect.ensuring(Effect.sync(() => unsubscribe())))
 
-      return { output, result: serialize(ran), screenshots } satisfies ExecuteResult
+      return { output: captured.output, result: serialize(ran), screenshots } satisfies ExecuteResult
     }).pipe(
       Effect.scoped,
       Effect.timeoutOrElse({
-        duration: Math.min(args.timeout ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS),
-        orElse: () => Effect.fail(new Error("browser_execute timed out")),
+        duration: timeout,
+        orElse: () =>
+          Effect.suspend(() => {
+            captured.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`,
+                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)
+          }),
       }),
     )
+  })
 
   return { parameters, execute, skillsDir }
 })

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

@@ -43,6 +43,7 @@ export type DetectedBrowser = {
 
 export class Session implements Transport {
   private ws?: WebSocket;
+  private invalidatedError?: Error;
   private nextId = 1;
   private pending = new Map<number, Pending>();
   private activeSessionId: string | undefined;
@@ -79,6 +80,7 @@ export class Session implements Transport {
    * and we connect directly to the supplied endpoint.
    */
   async connect(opts: ConnectOptions = {}): Promise<void> {
+    if (this.invalidatedError) throw this.invalidatedError;
     const timeoutMs = opts.timeoutMs ?? 5_000;
     if (opts.wsUrl || opts.profileDir) {
       const wsUrl = await resolveWsUrl(opts, timeoutMs);
@@ -103,6 +105,7 @@ export class Session implements Transport {
         await this.openWs(b.wsUrl, timeoutMs);
         return;
       } catch (e) {
+        if (this.invalidatedError) throw this.invalidatedError;
         const msg = e instanceof Error ? e.message : String(e);
         errors.push(`  ${b.name} @ ${b.wsUrl}: ${msg}`);
       }
@@ -113,6 +116,10 @@ export class Session implements Transport {
   }
 
   private openWs(wsUrl: string, timeoutMs: number): Promise<void> {
+    // 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.
+    if (this.invalidatedError) return Promise.reject(this.invalidatedError);
     return new Promise<void>((res, rej) => {
       const ws = new WebSocket(wsUrl);
       let done = false;
@@ -124,26 +131,46 @@ export class Session implements Transport {
         else res();
       };
       const timer = setTimeout(() => finish(new Error(`timed out after ${timeoutMs}ms`)), timeoutMs);
-      ws.addEventListener('open', () => finish());
+      ws.addEventListener('open', () => finish(this.invalidatedError));
       ws.addEventListener('error', (e) => finish(new Error(`WS error: ${(e as any)?.message ?? 'connect failed (likely 403, permission not granted, or port closed)'}`)));
       ws.addEventListener('message', (e) => this.onMessage(String(e.data)));
       ws.addEventListener('close', () => {
-        for (const [, p] of this.pending) p.reject(new Error('CDP socket closed'));
+        for (const [, p] of this.pending) p.reject(this.invalidatedError ?? new Error('CDP socket closed'));
         this.pending.clear();
-        finish(new Error('WS closed before open (likely 403 or port closed)'));
+        finish(this.invalidatedError ?? new Error('WS closed before open (likely 403 or port closed)'));
       });
       this.ws = ws;
     });
   }
 
   isConnected(): boolean {
-    return this.ws?.readyState === WebSocket.OPEN;
+    return !this.invalidatedError && this.ws?.readyState === WebSocket.OPEN;
   }
 
   close(): void {
     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.
+   */
+  invalidate(error: Error): void {
+    if (this.invalidatedError) return;
+    this.invalidatedError = error;
+    const ws = this.ws;
+    this.ws = undefined;
+    this.activeSessionId = undefined;
+    try { ws?.close(); } catch { /* ignore */ }
+  }
+
   /**
    * Pick a target and make subsequent calls auto-route to it.
    * Uses Target.attachToTarget with flatten:true (single-WS, sessionId-on-message).
@@ -239,6 +266,7 @@ export class Session implements Transport {
 
   // Transport implementation. Called by the generated domain bindings.
   _call(method: string, params: unknown = {}): Promise<unknown> {
+    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/src/session-store.ts

@@ -27,6 +27,15 @@ export const get = (sessionID: string): Session => {
   return fresh
 }
 
+// Retire a specific Session object after a browser_execute timeout. The
+// expected Session is always invalidated; the identity check only guards the
+// map delete, so a stale caller can never evict a successor Session that a
+// newer call is already using.
+export const invalidate = (sessionID: string, expected: Session, error: Error): void => {
+  if (sessions.get(sessionID) === expected) sessions.delete(sessionID)
+  expected.invalidate(error)
+}
+
 export const evict = async (sessionID: string): Promise<void> => {
   const entry = sessions.get(sessionID)
   if (!entry) return

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

@@ -224,6 +224,130 @@ test("console.debug is captured; uncommon methods fall through without throwing"
   await Promise.all([data, ws].map((d) => fs.rm(d, { recursive: true, force: true })))
 })
 
+// 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.
+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-"))
+  const err = await Effect.runPromise(
+    Effect.scoped(
+      Effect.gen(function* () {
+        const impl = yield* BrowserExecute.make(data)
+        return yield* impl.execute(
+          { description: "timeout test", code, timeout },
+          { sessionID: id, workspaceDir: ws, onChunk },
+        )
+      }),
+    ),
+  ).then(
+    () => { throw new Error("expected timeout") },
+    (e: unknown) => String(e),
+  )
+  await Promise.all([data, ws].map((d) => fs.rm(d, { recursive: true, force: true })))
+  return err
+}
+
+test("timeout returns partial output and retires the session", async () => {
+  const id = "timeout-isolation-test"
+  const before = SessionStore.get(id)
+  const err = await runTimeout(
+    id,
+    `console.log("progress-marker");
+     await new Promise((r) => setTimeout(r, 60_000));`,
+    100,
+  )
+  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...
+  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)
+  await SessionStore.evict(id)
+})
+
+test("console capture and onChunk stop after timeout", async () => {
+  const chunks: string[] = []
+  const err = await runTimeout(
+    "timeout-capture-test",
+    `console.log("early");
+     await new Promise((r) => setTimeout(r, 250));
+     console.log("late");`,
+    100,
+    (o) => Effect.sync(() => { chunks.push(o) }),
+  )
+  expect(err).toContain("early")
+  // Let the orphan's late log fire, then confirm it was not captured.
+  await new Promise((r) => setTimeout(r, 400))
+  expect(chunks.some((c) => c.includes("early"))).toBe(true)
+  expect(chunks.some((c) => c.includes("late"))).toBe(false)
+  await SessionStore.evict("timeout-capture-test")
+})
+
+test("re-running the execute effect after a timeout gets fresh state", async () => {
+  const data = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-rerun-"))
+  const ws = await fs.mkdtemp(path.join(os.tmpdir(), "bcode-rerun-ws-"))
+  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.
+  // 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
+  // alone cannot catch this).
+  const chunks: string[] = []
+  const eff = impl.execute(
+    {
+      description: "rerun test",
+      code: `console.log("progress-marker");
+             await new Promise((r) => setTimeout(r, 60_000));`,
+      timeout: 100,
+    },
+    { sessionID: "rerun-test", workspaceDir: ws, onChunk: (o) => Effect.sync(() => { chunks.push(o) }) },
+  )
+  const run = () => Effect.runPromise(eff).then(() => "resolved", (e: unknown) => String(e))
+  const first = await run()
+  const afterFirst = chunks.length
+  const second = await run()
+  expect(first).toContain("progress-marker")
+  expect(second).toContain("progress-marker")
+  expect(afterFirst).toBeGreaterThan(0)
+  expect(chunks.length).toBeGreaterThan(afterFirst)
+  await SessionStore.evict("rerun-test")
+  await Promise.all([data, ws].map((d) => fs.rm(d, { recursive: true, force: true })))
+})
+
+test("invalidate retires the expected Session even after replacement", async () => {
+  const id = "invalidate-replaced-test"
+  const s1 = SessionStore.get(id)
+  await SessionStore.evict(id)
+  const s2 = SessionStore.get(id)
+  SessionStore.invalidate(id, s1, new Error("retired stale session"))
+  // The successor entry is untouched, but the stale object is still dead.
+  expect(SessionStore.get(id)).toBe(s2)
+  await expect(s1.connect({ wsUrl: "ws://127.0.0.1:9/nope" })).rejects.toThrow(/retired stale session/)
+  await SessionStore.evict(id)
+})
+
+test("timeout output is tail-capped to valid UTF-8", async () => {
+  // ~25 KiB of multibyte lines, all logged before the sleep.
+  const err = await runTimeout(
+    "timeout-truncate-test",
+    `for (let i = 0; i < 300; i++) console.log("é".repeat(40) + "-line-" + i);
+     await new Promise((r) => setTimeout(r, 60_000));`,
+    100,
+  )
+  expect(err).toContain("[partial console output truncated; showing final bytes]")
+  expect(err).toContain("-line-299")
+  expect(err).not.toContain("-line-0\n")
+  expect(err).not.toContain("\uFFFD")
+  await SessionStore.evict("timeout-truncate-test")
+})
+
 // Concurrency safety: two overlapping execute() calls (different sessionIDs)
 // must each capture their own console output without leaking into each other
 // or into the real global console. No Chrome required — the snippets never

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

@@ -77,3 +77,30 @@ test("waitFor throws on a positional timeout rather than silently using the 30s
   await expect(session.waitFor("Test.never", { timeoutMs: 50 })).rejects.toThrow(/Timeout waiting for/)
   expect(Date.now() - started).toBeLessThan(1_000)
 })
+
+// Retirement guarantee under in-flight connects: an invalidation landing
+// while connect() is between awaits must not leave a usable or open socket.
+test("invalidate before the socket exists rejects the in-flight connect", async () => {
+  const s = new Session()
+  // connect() awaits resolveWsUrl before openWs, so invalidate() runs while
+  // no socket exists yet — openWs must refuse to create one afterwards.
+  const connecting = s.connect({ wsUrl: `ws://127.0.0.1:${server.port}/`, timeoutMs: 1_000 })
+  s.invalidate(new Error("retired by test"))
+  await expect(connecting).rejects.toThrow("retired by test")
+  expect(s.isConnected()).toBe(false)
+})
+
+test("invalidate while the socket is connecting closes it and rejects", async () => {
+  const s = new Session()
+  const connecting = s.connect({ wsUrl: `ws://127.0.0.1:${server.port}/`, timeoutMs: 1_000 })
+  // Yield one macrotask so openWs has created the WebSocket, then retire.
+  await Bun.sleep(0)
+  s.invalidate(new Error("retired by test"))
+  // Depending on whether the open event won the race, connect either rejects
+  // or resolved just before retirement — in both cases the Session must end
+  // dead with no usable transport.
+  await connecting.catch(() => {})
+  expect(s.isConnected()).toBe(false)
+  await expect(s._call("Runtime.evaluate", { expression: "1" })).rejects.toThrow("retired by test")
+  await expect(s.connect({ wsUrl: `ws://127.0.0.1:${server.port}/` })).rejects.toThrow("retired by test")
+})