diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 6549e052..ed4efd78 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -1296,8 +1296,14 @@ async def run_browser_agent( and not cancel_event.is_set() and not preloaded_perception and os.environ.get("OSW_DEADCARD_EVICT", "1") != "0"): try: + # The renderer parks webviews past its live cap (8) as snapshots and takes up to 12s to + # remount one on the next command (awaitWebview). A 6s probe called every parked card + # dead, evicted it, and the child then "declared done without a single action". That + # is the whole parallel-browser ghost bug (field report 2026-08-20): solo runs never hit + # the cap, so it only ever bit with several agents each driving their own browser. p_dead_probe = await asyncio.wait_for( - execute_browser_tool("BrowserGetText", {}, browser_id, tab_id), timeout=6.0) + execute_browser_tool("BrowserGetText", {}, browser_id, tab_id), + timeout=PARKED_WAKE_BUDGET_S) p_card_dead_early = not (isinstance(p_dead_probe, dict) and (str(p_dead_probe.get("url") or "") or str(p_dead_probe.get("text") or ""))) @@ -3487,6 +3493,11 @@ def find_reusable_card(dashboard_id: str, url: str, parent_session_id: str | Non P_EVICT_SETTLE_S = 1.5 +# Must exceed awaitWebview's 12s suspended-wake deadline in the renderer, or a merely parked card +# reads as dead. Kept as a single number so the two sides can never drift apart silently again. +PARKED_WAKE_BUDGET_S = 15.0 + + async def evict_dead_card(dashboard_id: str | None, browser_id: str) -> None: """Free a wedged card's webview so the recovery card isn't its heavy neighbor: tell the renderer to unmount it (frees the renderer process), drop it from the persisted layout, and diff --git a/backend/apps/agents/browser/browser_loop.py b/backend/apps/agents/browser/browser_loop.py index c184016a..5aca85f5 100644 --- a/backend/apps/agents/browser/browser_loop.py +++ b/backend/apps/agents/browser/browser_loop.py @@ -239,7 +239,7 @@ P_READ_TOOLS = { # A card the agent can't make progress on, EITHER gone (closed/dashboard not open; unrecoverable) OR hung (a wedged tab where every command times out / the page never responds). Both look the same to the agent: retrying just burns time (the 20-minute LinkedIn spin), so we fail fast. The streak (reset on any good result) absorbs a one-off transient; only a SUSTAINED pattern trips it, so a merely-busy page that recovers is never mistaken for dead. -P_CARD_GONE_MARKERS = ( +CARD_GONE_MARKERS = ( "not an electron webview", # card closed / destroyed "no dashboard is connected", # dashboard view not mounted "command timed out", # hung: the command never came back @@ -249,8 +249,13 @@ CARD_GONE_LIMIT = 2 # consecutive misses before we give up (absorbs a transient def card_is_unavailable(result: dict) -> bool: + # A card the renderer just remounted from a snapshot is slow, not gone. Past the live-webview cap + # (8) every extra browser agent's card gets parked, the next command spends up to 12s waking it, + # and without this the gate evicted healthy cards two at a time (the parallel-browser ghost runs). + if result.get("woke_from_park"): + return False err = str(result.get("error") or "").lower() - return any(m in err for m in P_CARD_GONE_MARKERS) + return any(m in err for m in CARD_GONE_MARKERS) # Errors where the action MISSED but the page is alive (stale index after a reshuffle, a transient overlay covering the target, off-screen). The page itself is fine, so re-attaching the CURRENT element list to the error lets the model re-act next turn instead of burning a turn re-listing. This NEVER retries the action (no double-send risk); it only enriches the error with fresh state. @@ -265,7 +270,7 @@ def recoverable_tool_error(err: str) -> bool: """True for a 'the action missed but the page is alive' error worth showing fresh state for. False for a dead card (handled separately) or no error.""" e = (err or "").lower() - if not e or any(m in e for m in P_CARD_GONE_MARKERS): + if not e or any(m in e for m in CARD_GONE_MARKERS): return False return any(m in e for m in P_RECOVERABLE_ERR_MARKERS) diff --git a/backend/apps/agents/core/ws_manager.py b/backend/apps/agents/core/ws_manager.py index 0db3e8e1..6114dcc2 100644 --- a/backend/apps/agents/core/ws_manager.py +++ b/backend/apps/agents/core/ws_manager.py @@ -13,7 +13,7 @@ BROWSER_CMD_TIMEOUT_DEFAULT = 15.0 # modest load headroom; still "short" so a BROWSER_CMD_TIMEOUTS = { "navigate": 25.0, # a real page load can be slow (more leash under load) "replay_route": 20.0, # an API fetch can be slow - "wait": 12.0, # smart-wait already caps itself well under this + "wait": 16.0, # smart-wait self-caps well under this; the 12s floor is the renderer's suspended-card wake, which a wait on a parked card pays first "perform_action": 35.0, # session-borrow shims pack navigate + wait + scrape into ONE command, so it needs more than navigate alone "browser_fetch": 32.0, # offscreen window: load + settle + DOM read on an arbitrary (maybe slow/JS-heavy) page "browser_search": 45.0, # tries up to 3 engines sequentially, each a full load + settle diff --git a/backend/tests/test_parked_card_not_dead.py b/backend/tests/test_parked_card_not_dead.py new file mode 100644 index 00000000..aaf61747 --- /dev/null +++ b/backend/tests/test_parked_card_not_dead.py @@ -0,0 +1,59 @@ +"""A browser card the renderer parked is slow, not dead. + +Field report 2026-08-20: "when I have multiple agents each driving their own browser, the browsers +seem to say they've completed without actually performing any actions." Solo runs were fine. The +cause was a timing race nobody had connected: the renderer caps live webviews at 8 and parks the rest +as snapshots, waking one takes up to 12s (awaitWebview), and the backend's dead-card probe gave it +6s. Past the cap, every extra agent's card was declared dead, evicted, and its child then "declared +done without taking a single action". The renderer now stamps woke_from_park on the result and the +backend never counts such a result toward the gone streak. +""" + +from backend.apps.agents.browser.browser_loop import ( + CARD_GONE_LIMIT, + CARD_GONE_MARKERS, + card_is_unavailable, +) +from backend.apps.agents.browser import browser_agent +from backend.apps.agents.core.ws_manager import BROWSER_CMD_TIMEOUTS, BROWSER_CMD_TIMEOUT_DEFAULT + +# The renderer's suspended-card wake deadline (awaitWebview, frontend/src/shared/browserCommandHandler.ts). +RENDERER_WAKE_DEADLINE_S = 12.0 + + +def test_a_woke_from_park_timeout_is_not_a_gone_card(): + slow_but_alive = {"error": "Browser command timed out", "woke_from_park": True} + assert card_is_unavailable(slow_but_alive) is False + + +def test_a_woke_from_park_not_found_is_not_a_gone_card(): + """The wake can miss its window and the handler then reports 'not an Electron webview'; + that exact phrase is a gone marker, so the stamp must override it too.""" + r = {"error": "Browser card 'browser-x' not found or not an Electron webview", "woke_from_park": True} + assert card_is_unavailable(r) is False + + +def test_a_genuinely_gone_card_is_still_gone(): + """NEGATIVE CONTROL. The stamp must not blind the gate to a card that really closed; a wedged tab + spinning for 20 minutes is the failure this gate exists to stop.""" + for marker in CARD_GONE_MARKERS: + assert card_is_unavailable({"error": f"xx {marker} xx"}) is True + assert card_is_unavailable({"error": f"xx {marker} xx", "woke_from_park": False}) is True + + +def test_the_probe_budget_outlasts_the_renderer_wake(): + """The two numbers live in different languages in different directories; this pins them.""" + assert browser_agent.PARKED_WAKE_BUDGET_S > RENDERER_WAKE_DEADLINE_S + + +def test_no_command_timeout_sits_at_or_under_the_wake(): + """Any action whose timeout cannot outlast a park wake turns every parked card into a timeout, + which is two strikes from eviction. 'wait' sat at exactly 12.0 with zero margin.""" + assert BROWSER_CMD_TIMEOUT_DEFAULT > RENDERER_WAKE_DEADLINE_S + for action, t in BROWSER_CMD_TIMEOUTS.items(): + assert t > RENDERER_WAKE_DEADLINE_S, f"{action} timeout {t}s cannot outlast a {RENDERER_WAKE_DEADLINE_S}s wake" + + +def test_gone_limit_is_still_two(): + """If someone raises the limit to paper over this instead, the wedged-tab spin comes back.""" + assert CARD_GONE_LIMIT == 2 diff --git a/frontend/src/shared/browserCommandHandler.ts b/frontend/src/shared/browserCommandHandler.ts index ab42a827..fe04008a 100644 --- a/frontend/src/shared/browserCommandHandler.ts +++ b/frontend/src/shared/browserCommandHandler.ts @@ -2200,6 +2200,14 @@ async function handleEvaluate(wv: BrowserWebview, params: Record): } // The registry is renderer-local and a card briefly unregisters on remount / tab-switch; a command landing in that gap shouldn't hard-fail. Wait a bounded window for (re)registration before giving up, so the error stays a real "card is gone" signal rather than a transient race. +// Cards woken from a snapshot in the last few seconds; stamped on the result so the backend knows the slowness was a remount, not a hang. +const lastWakeAt = new Map(); +const WAKE_REPORT_WINDOW_MS = 20_000; +function wokeRecently(browserId: string): boolean { + const t = lastWakeAt.get(browserId); + return t !== undefined && Date.now() - t < WAKE_REPORT_WINDOW_MS; +} + async function awaitWebview(browserId: string, tabId?: string, action?: string): Promise { // A live auth popup OWNS its card's commands (ENG-279): the user-visible action is in the popup, // so tools drive it there until it closes, then fall back to the card's webview automatically. @@ -2208,6 +2216,8 @@ async function awaitWebview(browserId: string, tabId?: string, action?: string): // A suspended (snapshot-swapped) card has no webview at all; wake it and wait out the remount + page reload before the command touches it. const wasSuspended = !!store.getState().dashboardLayout.suspendedBrowserCards[browserId]; if (wasSuspended) store.dispatch(resumeBrowserCard(browserId)); + // Remember the wake so the result can say so: the backend's dead-card gate cannot otherwise tell a card that spent 12s remounting from one that hung, and it evicted healthy parked cards under parallel load. + if (wasSuspended) lastWakeAt.set(browserId, Date.now()); const deadline = Date.now() + (wasSuspended ? 12000 : 2000); let wv = getWebview(browserId, tabId); while (!wv && Date.now() < deadline) { @@ -2386,6 +2396,7 @@ async function runBrowserCommand( dashboardWs.send('browser:result', { request_id, error: `Browser card '${browser_id}'${tab_id ? ` tab '${tab_id}'` : ''} not found or not an Electron webview`, + woke_from_park: wokeRecently(browser_id), }); return; } @@ -2497,7 +2508,7 @@ async function runBrowserCommand( } // Ride the pre-handler wait back with the result: a renderer console.log never reaches the main // process, and from the backend a slow GATE and a slow HANDLER look identical. - dashboardWs.send('browser:result', { request_id, ...result, gate_ms: p_gateMs, total_ms: Date.now() - p_gateT0 }); + dashboardWs.send('browser:result', { request_id, ...result, gate_ms: p_gateMs, total_ms: Date.now() - p_gateT0, woke_from_park: wokeRecently(browser_id) }); } export function initBrowserCommandHandler(): () => void {