fix(browser): isolate timed-out snippets

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

@@ -174,6 +174,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.
 
 ## 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,22 +32,76 @@
 //
 // 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 may continue, so we permanently invalidate
+// the Session object it received. Abandoned code can finish local work but
+// cannot reconnect or send later CDP commands; the next tool call gets a
+// fresh Session from SessionStore.
 //
 // 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 { Effect, Schema, Semaphore } from "effect"
 import { SessionStore } from "./session-store"
 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"
+
+const executionLocks = new Map<string, { semaphore: Semaphore.Semaphore; users: number }>()
+
+const serialized = <A, E extends Error, R>(
+  sessionID: string,
+  timeout: number,
+  effect: (remaining: number) => Effect.Effect<A, E, R>,
+) =>
+  Effect.suspend(() => {
+    const existing = executionLocks.get(sessionID)
+    const entry = existing ?? { semaphore: Semaphore.makeUnsafe(1), users: 0 }
+    if (!existing) executionLocks.set(sessionID, entry)
+    entry.users++
+    const startedAt = Date.now()
+    const waitTimeout = () =>
+      Effect.fail(new Error(`browser_execute timed out after ${timeout} ms waiting for a previous call; no code was run`))
+    return Effect.uninterruptibleMask((restore) =>
+      restore(
+        entry.semaphore.take(1).pipe(
+          Effect.timeoutOrElse({
+            duration: timeout,
+            orElse: waitTimeout,
+          }),
+        ),
+      ).pipe(
+        Effect.flatMap(() => {
+          const remaining = timeout - (Date.now() - startedAt)
+          return restore(remaining <= 0 ? waitTimeout() : Effect.suspend(() => effect(remaining))).pipe(
+            Effect.ensuring(entry.semaphore.release(1)),
+          )
+        }),
+      ),
+    ).pipe(
+      Effect.ensuring(
+        Effect.sync(() => {
+          entry.users--
+          if (entry.users === 0 && executionLocks.get(sessionID) === entry) executionLocks.delete(sessionID)
+        }),
+      )
+    )
+  })
+
+const timeoutOutput = (output: string) => {
+  const bytes = Buffer.from(output, "utf8")
+  if (bytes.length <= MAX_TIMEOUT_OUTPUT_BYTES) return output
+
+  const markerBytes = Buffer.byteLength(TIMEOUT_OUTPUT_TRUNCATED)
+  let start = bytes.length - (MAX_TIMEOUT_OUTPUT_BYTES - markerBytes)
+  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 +211,21 @@ const serialize = (v: unknown): string => {
 export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string) {
   const skillsDir = yield* Effect.promise(() => Skills.resolveSkillsDir(dataDir))
 
-  const execute = (args: Parameters, ctx: ExecuteContext) =>
-    Effect.gen(function* () {
-      const session = SessionStore.get(ctx.sessionID)
+  const executeLocked = (args: Parameters, ctx: ExecuteContext, timeout: number, configuredTimeout: number) => {
+    const session = SessionStore.get(ctx.sessionID)
+    const captured = { active: true, output: "" }
+    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
@@ -191,12 +246,9 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
       // when `BCODE_SCREENSHOT_DIR` is set, also written to disk for
       // eval-judge consumption. Two consumers of one tap.
       //
-      // Concurrency note: parallel execute() calls against the same Session
-      // (rare but possible — different sessionIDs share no Session, but a
-      // single sessionID with two in-flight tool calls would) each subscribe
-      // independently and would each see all screenshots produced during
-      // their lifetime. Acceptable for v1; opencode tool calls within one
-      // assistant message are serialized anyway.
+      // Calls sharing a sessionID are serialized before resolving their
+      // Session, so a timed-out call can invalidate its object without
+      // disrupting the next queued call.
       const screenshots: CollectedScreenshot[] = []
       const dumpDir = process.env.BCODE_SCREENSHOT_DIR
       const startedAt = Date.now()
@@ -223,14 +275,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.gen(function* () {
+            captured.active = false
+            const output = timeoutOutput(captured.output)
+            const error = new Error(
+              [
+                `browser_execute timed out after ${configuredTimeout} ms; CDP session was reset`,
+                output.trim() ? `Partial console output before timeout:\n${output.trimEnd()}` : "",
+              ]
+                .filter(Boolean)
+                .join("\n\n"),
+            )
+            yield* Effect.sync(() => SessionStore.invalidate(ctx.sessionID, session, error))
+            return yield* Effect.fail(error)
+          }),
       }),
     )
+  }
+
+  const execute = (args: Parameters, ctx: ExecuteContext) => {
+    const timeout = Math.max(1, Math.min(args.timeout ?? DEFAULT_TIMEOUT_MS, MAX_TIMEOUT_MS))
+    return serialized(ctx.sessionID, timeout, (remaining) => executeLocked(args, ctx, remaining, timeout))
+  }
 
   return { parameters, execute, skillsDir }
 })