From 324ee47937b33feaf8c8cd5fb01b20c2805530e8 Mon Sep 17 00:00:00 2001 From: ciregenz Date: Thu, 30 Jul 2026 20:44:22 -0700 Subject: [PATCH] [eric] browser: a command stops waiting when the window closes under it --- backend/apps/agents/core/ws_manager.py | 9 +++ backend/tests/test_browser_command_timeout.py | 58 +++++++++++++++++++ 2 files changed, 67 insertions(+) diff --git a/backend/apps/agents/core/ws_manager.py b/backend/apps/agents/core/ws_manager.py index 041732ab..5e66d58e 100644 --- a/backend/apps/agents/core/ws_manager.py +++ b/backend/apps/agents/core/ws_manager.py @@ -337,6 +337,15 @@ class ConnectionManager: ) if done: return future.result() + # The window went away WHILE we were waiting. The entry check above only guards the + # START of a command, so re-broadcasting into a closed app just burned the rest of + # the leash: measured, a run in flight when the window closed took 240.9s, while one + # that started after it was already gone failed honestly in 11.6s. Same reconnect + # grace and same error as the entry check, so a real socket blip still rides through + # and a genuinely-closed window trips the agent's card-gone streak instead. + if not self.global_connections and not await await_reconnect( + lambda: bool(self.global_connections)): + return {"error": "No dashboard is connected. Open the dashboard to use browser tools."} finally: self.browser_futures.pop(request_id, None) diff --git a/backend/tests/test_browser_command_timeout.py b/backend/tests/test_browser_command_timeout.py index 1a04b84d..f1b7feae 100644 --- a/backend/tests/test_browser_command_timeout.py +++ b/backend/tests/test_browser_command_timeout.py @@ -78,6 +78,64 @@ async def test_lost_first_delivery_heals_via_rebroadcast(monkeypatch): assert len(sends) >= 2, "command must be re-broadcast until a client answers" +# --- the window closing MID-command ------------------------------------------------------------ +# The entry check only guards the START of a command. Measured live: a run in flight when the +# window closed took 240.9s, while one that started after it was already gone failed honestly in +# 11.6s. Same code, outcome decided purely by timing. + +@pytest.mark.asyncio +async def test_a_window_closing_mid_command_fails_fast_not_at_the_bound(monkeypatch): + monkeypatch.setattr(wsm, "BROWSER_CMD_TIMEOUT_DEFAULT", 30.0) # a long leash, as navigate has + monkeypatch.setattr(wsm, "BROWSER_CMD_REBROADCAST_S", 0.1) + monkeypatch.setattr(wsm, "P_WS_RECONNECT_WAIT_S", 0.3) + m = p_mgr() + + async def p_close_window_soon(): + await asyncio.sleep(0.05) + m.global_connections = [] # red-button close: socket gone, future never resolves + + asyncio.create_task(p_close_window_soon()) + t0 = time.monotonic() + res = await m.send_browser_command("rid5", "get_text", "b1", {}) + elapsed = time.monotonic() - t0 + # the agent's card-gone streak keys on this exact wording, so it must match the entry check's + assert res == {"error": "No dashboard is connected. Open the dashboard to use browser tools."} + assert elapsed < 3.0, f"a closed window must not cost the full 30s leash, took {elapsed:.2f}s" + + +@pytest.mark.asyncio +async def test_the_fast_fail_wording_is_the_one_the_agent_watches_for(): + # If these ever drift apart, a dead browser silently stops tripping the abort and starts spinning. + from backend.apps.agents.browser.browser_loop import card_is_unavailable + m = wsm.ConnectionManager() + m.global_connections = [] + res = await m.send_browser_command("rid6", "get_text", "b1", {}) + assert card_is_unavailable(res), "the not-connected error must read as a gone card" + + +@pytest.mark.asyncio +async def test_a_brief_socket_blip_still_rides_through(monkeypatch): + # A CPU-starved renderer drops its WS for a beat and reconnects. That must NOT kill a live run. + monkeypatch.setattr(wsm, "BROWSER_CMD_TIMEOUT_DEFAULT", 5.0) + monkeypatch.setattr(wsm, "BROWSER_CMD_REBROADCAST_S", 0.1) + monkeypatch.setattr(wsm, "P_WS_RECONNECT_WAIT_S", 2.0) + m = p_mgr() + sock = m.global_connections[0] + + async def p_blip_then_return(): + await asyncio.sleep(0.05) + m.global_connections = [] # blip + await asyncio.sleep(0.2) + m.global_connections = [sock] # frontend reconnects + await asyncio.sleep(0.15) + rid = next(iter(m.browser_futures)) + m.resolve_browser_command(rid, {"text": "ok"}) + + asyncio.create_task(p_blip_then_return()) + res = await m.send_browser_command("rid7", "get_text", "b1", {}) + assert res == {"text": "ok"}, "a reconnect inside the grace must not fail the command" + + @pytest.mark.asyncio async def test_a_resolved_command_returns_immediately(monkeypatch): # a healthy command returns the moment the renderer resolves it, not at the bound