fix(browser): isolate timed-out snippets - #118
Conversation
There was a problem hiding this comment.
All reported issues were addressed across 4 files
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
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
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 4 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Fix all with cubic | Re-trigger cubic
|
Closing in favor of #126, which keeps this PR's core and drops the rest. The diagnosis here is right and important: a timed-out snippet's Promise cannot be preempted, it keeps running against the same Session object the next tool call receives, and the two interleave on one socket. That, plus returning captured output in the timeout error, is exactly what #126 ships — including this PR's identity-checked Why not merge this as-is:
The bar for reopening the Tier-2/3 machinery is a real orphan trace from production showing the minimal version leaking cross-call effects. Happy to review that PR when the trace exists. |
Problem
browser_executeruns snippets in-process. Timing out the Effect fiber does not preempt the JavaScript Promise, so abandoned code could reconnect and send CDP commands after the caller received a timeout. Same-session tool calls may also execute concurrently.Change
SessionStore.browser_executeper session ID so a timeout cannot invalidate a healthy concurrent call; queued calls resolve the fresh Session after acquiring the lock.onChunkupdates after timeout.Reproduction
Synthetic CDP tests cover a timed-out snippet that sleeps and later attempts
Runtime.evaluate. The server receives no late command. A concurrently queued call then receives a fresh Session, reconnects, and executes successfully.A separate queue-boundary test puts a 10ms call behind a 50ms same-session call. The queued call expires without running its snippet or invalidating the active Session; the first and a later call both complete normally. Additional tests cover pending waiters, connecting sockets, prompt cancellation of profile polling, UTF-8 byte capping, and stopped post-timeout capture.
Validation
browser-execute.test.ts: 8 pass, 6 environment-gated skipspackages/bcode-browsersuite: 27 pass, 8 environment-gated skipsgit diff --checkpassedThis PR is stacked on #112 (
session-recovery).