fix(browser): preserve session after tool timeout - #136
MagMueller merged 9 commits into
Conversation
Sorry, something went wrong.
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="packages/bcode-browser/src/browser-execute.ts">
<violation number="1" location="packages/bcode-browser/src/browser-execute.ts:312">
P2: When a timeout recovery cannot find the preserved page target (and there is no fallback page target either), this branch throws and leaves `v4TimeoutRecovery[sessionID]` set. On every later `browser_execute` for the same session, `ensureCloudConnected` re-enters the recovery path (`v4Bootstrapped.has(sessionID) && !recoveryState` is false), skips the already-open connect, hits `getTargets`, finds no page, and throws the same error again — and because `ensureCloudConnected` runs before the snippet is executed, the agent can never run a snippet to open a new tab or reconnect manually. The sessionID is effectively permanently stuck until a page target happens to appear in that browser. Consider clearing the recovery entry on this path (e.g. `v4TimeoutRecovery.delete(sessionID)` before throwing) so a subsequent snippet can run and let the agent reconnect explicitly, or otherwise provide a way out of the recovery state.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Fix all with cubic | Re-trigger cubic
Sorry, something went wrong.
| const page = targets.find((target) => target.targetId === recoveryState?.targetId) | ||
| ?? targets.find((target) => target.type === "page" && !target.url.startsWith("chrome://")) | ||
| if (recoveryState && !page) { | ||
| throw new Error("No page target available after browser_execute timeout") |
There was a problem hiding this comment.
P2: When a timeout recovery cannot find the preserved page target (and there is no fallback page target either), this branch throws and leaves v4TimeoutRecovery[sessionID] set. On every later browser_execute for the same session, ensureCloudConnected re-enters the recovery path (v4Bootstrapped.has(sessionID) && !recoveryState is false), skips the already-open connect, hits getTargets, finds no page, and throws the same error again — and because ensureCloudConnected runs before the snippet is executed, the agent can never run a snippet to open a new tab or reconnect manually. The sessionID is effectively permanently stuck until a page target happens to appear in that browser. Consider clearing the recovery entry on this path (e.g. v4TimeoutRecovery.delete(sessionID) before throwing) so a subsequent snippet can run and let the agent reconnect explicitly, or otherwise provide a way out of the recovery state.
Prompt for AI agentsCheck if this issue is valid — if so, understand the root cause and fix it. At packages/bcode-browser/src/browser-execute.ts, line 312:
<comment>When a timeout recovery cannot find the preserved page target (and there is no fallback page target either), this branch throws and leaves `v4TimeoutRecovery[sessionID]` set. On every later `browser_execute` for the same session, `ensureCloudConnected` re-enters the recovery path (`v4Bootstrapped.has(sessionID) && !recoveryState` is false), skips the already-open connect, hits `getTargets`, finds no page, and throws the same error again — and because `ensureCloudConnected` runs before the snippet is executed, the agent can never run a snippet to open a new tab or reconnect manually. The sessionID is effectively permanently stuck until a page target happens to appear in that browser. Consider clearing the recovery entry on this path (e.g. `v4TimeoutRecovery.delete(sessionID)` before throwing) so a subsequent snippet can run and let the agent reconnect explicitly, or otherwise provide a way out of the recovery state.</comment>
<file context>
@@ -286,27 +294,33 @@ export const make = Effect.fn("BrowserExecute.make")(function* (dataDir: string)
+ const page = targets.find((target) => target.targetId === recoveryState?.targetId)
+ ?? targets.find((target) => target.type === "page" && !target.url.startsWith("chrome://"))
+ if (recoveryState && !page) {
+ throw new Error("No page target available after browser_execute timeout")
+ }
if (page) await session.use(page.targetId)
</file context>
Sorry, something went wrong.
There was a problem hiding this comment.
Superseded by the latest revision: the recovery map and target lookup were removed entirely. The next invocation continues on the existing Session without a pre-snippet recovery path.
Sorry, something went wrong.
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
Sorry, something went wrong.
Summary
Why
JavaScript promises cannot be preemptively cancelled. Retiring the complete CDP transport prevents late commands, but also destroys browser context. A per-call async execution scope blocks future browser effects from the timed-out invocation without reconnecting or re-enumerating targets.
The required browser skill changes only two narrow cases: after a timeout it says the existing session is preserved, and when a new tab opens it explains that the active attachment remains on the old page until the intended type: "page" target is selected. Other navigation and waiting guidance is unchanged.
Testing