From 02fee0983eb409a0bc89e34c3c87d12a145f196b Mon Sep 17 00:00:00 2001 From: ciregenz Date: Tue, 7 Jul 2026 23:04:09 -0700 Subject: [PATCH] [eric] browser: evict only AGENT-SPAWNED cards (a wedged USER card must never be deleted out from under the user; unverifiable ownership = no delete, reuse-skip is the remedy); caught in the downstream-risk review, 3 tests --- backend/apps/agents/browser/browser_agent.py | 13 ++++++++++- backend/tests/test_deadcard_evict.py | 24 ++++++++++++++++---- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/backend/apps/agents/browser/browser_agent.py b/backend/apps/agents/browser/browser_agent.py index 8ce5d3b7..143e94d7 100644 --- a/backend/apps/agents/browser/browser_agent.py +++ b/backend/apps/agents/browser/browser_agent.py @@ -2500,8 +2500,19 @@ async def p_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 WAIT for the teardown to land before the caller spawns the - recovery card. Fail-open, never raises into the abort path.""" + recovery card. Fail-open, never raises into the abort path. + ONLY agent-spawned cards are ever evicted: a user's own card (they selected it + for the agent) must never be deleted out from under them, wedged or not; for + those the DEAD_CARDS reuse-skip is the whole remedy.""" ACTIVE_AGENT_CARDS.discard(browser_id) + try: + from backend.apps.dashboards.dashboards import load as p_dash_load + p_card = p_dash_load(dashboard_id).layout.browser_cards.get(browser_id) if dashboard_id else None + if p_card is None or not getattr(p_card, "spawned_by", None): + logger.info(f"[browser-agent] {browser_id} is not an agent-spawned card; skipping evict (reuse-skip only)") + return + except Exception: + return try: await ws_manager.broadcast_global("dashboard:browser_card_evict", { "dashboard_id": dashboard_id or "", "browser_id": browser_id}) diff --git a/backend/tests/test_deadcard_evict.py b/backend/tests/test_deadcard_evict.py index 005fc0c9..c48af909 100644 --- a/backend/tests/test_deadcard_evict.py +++ b/backend/tests/test_deadcard_evict.py @@ -34,7 +34,7 @@ def p_patch(monkeypatch, cards): def test_evict_broadcasts_unmount_and_removes_from_layout(monkeypatch): - broadcasts, saved, dash = p_patch(monkeypatch, {"browser-dead": object(), "browser-keep": object()}) + broadcasts, saved, dash = p_patch(monkeypatch, {"browser-dead": FakeCard("sess-1"), "browser-keep": FakeCard("sess-1")}) ba.ACTIVE_AGENT_CARDS.add("browser-dead") asyncio.run(ba.p_evict_dead_card("dash-1", "browser-dead")) # the renderer is told to unmount exactly the dead card @@ -46,9 +46,23 @@ def test_evict_broadcasts_unmount_and_removes_from_layout(monkeypatch): assert "browser-dead" not in ba.ACTIVE_AGENT_CARDS -def test_evict_is_fail_open_without_a_dashboard(monkeypatch): +def test_evict_without_a_dashboard_deletes_nothing(monkeypatch): + # No dashboard = ownership unverifiable = fail SAFE: never unmount or delete + # what might be the user's card; the reuse-skip alone handles it. broadcasts, saved, _ = p_patch(monkeypatch, {}) - # no dashboard id: still tells the renderer to unmount (best-effort), never raises asyncio.run(ba.p_evict_dead_card("", "browser-x")) - assert broadcasts and broadcasts[0][0] == "dashboard:browser_card_evict" - assert not saved # nothing to persist without a dashboard + assert not broadcasts and not saved + + +class FakeCard: + def __init__(self, spawned_by=None): + self.spawned_by = spawned_by + + +def test_user_card_is_never_evicted(monkeypatch): + """A wedged USER card (no spawned_by) must never be deleted out from under the + user; reuse-skip is the whole remedy. Only agent-spawned cards evict.""" + broadcasts, saved, dash = p_patch(monkeypatch, {"browser-user": FakeCard(None)}) + asyncio.run(ba.p_evict_dead_card("dash-1", "browser-user")) + assert not broadcasts and not saved + assert "browser-user" in dash.layout.browser_cards