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

Merged
AL
3mo ago
+247-16

Problem

browser_execute runs snippets in-process, and a timeout cannot preempt the snippet's Promise. Two consequences:

  1. Orphan interleaving. The timed-out snippet keeps running against the same Session object that SessionStore hands to the next tool call. Abandoned code and a fresh call then share one CDP socket: the orphan navigates or clicks while the new snippet is mid-scrape, the new snippet's waitFor fires on the orphan's events. It does not crash — it returns plausible-but-wrong results the model builds on.
  2. Lost progress. The timeout error was the string browser_execute timed out. A snippet that logged useful progress for 60s returned none of it.

Change

On timeout:

  • Retire the exact Session the snippet received. Session.invalidate(error) rejects all future connect/_call with the timeout error and closes the socket (the existing close handler rejects in-flight calls). SessionStore.invalidate is identity-checked, so a stale caller can never evict a successor Session a newer call is using. The next tool call gets a fresh Session; the orphan can finish local work but cannot keep driving the browser.
  • Freeze the capture buffer — post-timeout console.log and onChunk updates stop — and return the last 8 KiB of captured output in the error, cut on a UTF-8 sequence boundary, with an explicit truncation marker.
  • Document the timeout contract in the skill guardrails: 60s default / 600s max, top-level timeout is the only knob, keep batches small and log progress, reconnect after a timeout.

What this deliberately does not include

  • No abort-signal plumbing through connect/resolveWsUrl/DevToolsActivePort polling. Those windows are milliseconds wide, and socket close + identity eviction already prevent the failure that matters (cross-call interleaving).
  • No per-session locking/queueing. opencode serializes tool calls within an assistant message; concurrent same-session browser_execute calls have not been observed. If a production trace ever shows one, that is the moment to add serialization — with the trace as the test fixture.

Credit

Supersedes #118, which correctly diagnosed the orphan problem and whose identity-checked eviction shape is kept here. This PR takes the minimal core and drops the speculative machinery; rationale in the #118 close comment.

Validation

  • Three new tests (no Chrome needed): timeout error carries partial output, retired Session rejects connect/_call, store hands out a fresh Session; capture and onChunk stop after timeout; tail-capping preserves multibyte UTF-8. All three fail with the source change reverted.
  • Full packages/bcode-browser suite: 20 pass, 8 env-gated skips, 1 pre-existing failure (workspace-import cache-bust flake, fails identically on clean main).
  • bun typecheck clean.

Summary by cubic

Hardened browser_execute timeouts by retiring the exact timed‑out Session, stopping streaming, and including the last 8 KiB of console output in the error. Also documents a 60s default (600s max) and ensures re-runs get a fresh session and capture buffer.

  • Bug Fixes
    • Retire the exact Session on timeout via Session.invalidate; rejects future connect/_call and closes the socket. SessionStore.invalidate now always retires the expected object; identity check only guards the map delete so successors aren’t evicted.
    • Re-check invalidation during connect/openWs (entry, open, close) and rethrow retirement errors from detection so late sockets can’t open; in‑flight and pre‑open calls fail with the retirement error.
    • Move execute state (Session, capture buffer, timeout) inside Effect.suspend to avoid re-run leakage; freeze capture and onChunk after timeout; attach the last 8 KiB of UTF‑8‑safe console output with a truncation marker.
    • Update SKILL.md with the timeout contract and reconnect‑after‑timeout guidance.