diff --git a/backend/apps/agents/manager/delegation_tool_names.py b/backend/apps/agents/manager/delegation_tool_names.py new file mode 100644 index 00000000..da9f8a74 --- /dev/null +++ b/backend/apps/agents/manager/delegation_tool_names.py @@ -0,0 +1,27 @@ +"""The delegation tool names, in ONE place, because two lists of names that must agree will not. + +`BrowserAgents` was registered as a real tool and left out of the wedge-exemption set, so every +PARALLEL browser run was shot 25 seconds in by the quick-tool watchdog. The singular `BrowserAgent` +was exempt, so the bug was invisible to anyone testing one browser at a time, and looked like "browser +use disconnects constantly" to the one person running several (Haik, ~100% failure over weeks). + +Nothing checked the two lists against each other. Now there is only one list. +""" + +from typing import List, Set + +# Every tool that hands work to a browser, an app, or another agent. These BLOCK for as long as the +# delegated run takes, so no quick-tool deadline may apply to them. +BROWSER_DELEGATION_TOOLS: List[str] = [ + "CreateBrowserAgent", "BrowserAgent", "BrowserAgents", "AppAgent", +] + +# Everything else that legitimately blocks on a human, a model, or a whole delegated run. +OTHER_BLOCKING_TOOLS: Set[str] = { + "AskUI", "AskUserQuestion", "ShowUI", + "SpawnAgent", "InvokeAgent", "RequestHumanIntervention", + "MCPSearch", "MCPActivate", + "RunToolScript", +} + +BLOCKING_TOOLS: Set[str] = set(BROWSER_DELEGATION_TOOLS) | OTHER_BLOCKING_TOOLS diff --git a/backend/apps/agents/manager/register_builtin_mcp_servers.py b/backend/apps/agents/manager/register_builtin_mcp_servers.py index 855d3468..bd1f9cff 100644 --- a/backend/apps/agents/manager/register_builtin_mcp_servers.py +++ b/backend/apps/agents/manager/register_builtin_mcp_servers.py @@ -29,7 +29,8 @@ def register_builtin_mcp_servers( agents_dir = os.path.dirname(p_agents_pkg.__file__) # With no renderer for a webview and no human for a prompt, we shadow the map once here and let the existing deny short-circuits skip those modules; nothing below may read the un-shadowed one. builtin_perms = apply_unreachable_denies(builtin_perms) - browser_delegation_tools = ["CreateBrowserAgent", "BrowserAgent", "BrowserAgents", "AppAgent"] + from backend.apps.agents.manager.delegation_tool_names import BROWSER_DELEGATION_TOOLS + browser_delegation_tools = list(BROWSER_DELEGATION_TOOLS) # ReadAgentWork rides InvokeAgent's policy unless set on its own: a user who denied delegation # denied reading other sessions too, and inheriting is how that stays true without them having # to find a second toggle (never widen a tool surface silently). diff --git a/backend/apps/agents/manager/run/handle_run_error.py b/backend/apps/agents/manager/run/handle_run_error.py index e63d57e0..8bc32e17 100644 --- a/backend/apps/agents/manager/run/handle_run_error.py +++ b/backend/apps/agents/manager/run/handle_run_error.py @@ -3,6 +3,7 @@ the exception (long-context / capacity / free-trial / auth / unknown-model / unc emits the matching system message + WS event. Pulled out of agent_manager so the loop stays under the file ceiling; pure relocation, no self (operates on the passed run state).""" +import asyncio import logging from typing import List from typeguard import typechecked @@ -115,6 +116,39 @@ def p_report_model_error(subkind: str, session_id: str, session: AgentSession, t except Exception: logger.debug(f"submit_diagnostic {subkind} failed", exc_info=True) +async def p_try_runtime_repair(session, session_id: str) -> bool: + """Put the runtime back mid-turn. True only when it is back AND stayed back. + + Returns False for every ambiguous outcome (no package, restore failed, antivirus took it again), + because the caller's fallback is an honest card and a half-repair must not suppress it.""" + try: + from backend.apps.agents.core.bundled_cli_missing import bundled_cli_missing + from backend.apps.agents.core.cli_self_heal import repair_bundled_cli + p_gone = bundled_cli_missing() + if p_gone is None: + return False + p_result = await asyncio.to_thread(repair_bundled_cli, p_gone) + if not (p_result.repaired and not p_result.retaken): + logger.warning("runtime self-heal did not stick: %s", p_result.detail) + return False + logger.info("runtime self-heal succeeded mid-turn: %s", p_result.detail) + p_msg = Message( + role="system", + content="A core OpenSwarm component had been removed, most likely by antivirus. " + "It has been restored from your installer, so you can send that message again.", + branch_id=session.active_branch_id, + ) + absorb_repeat_card(session, p_msg) + await ws_manager.send_to_session(session_id, "agent:message", { + "session_id": session_id, + "message": p_msg.model_dump(mode="json"), + }) + return True + except Exception: + logger.exception("runtime self-heal raised; falling back to the card") + return False + + async def handle_run_error(e: Exception, session: AgentSession, session_id: str, turn: TurnState, p_stderr_buffer: List[str]) -> None: logger.exception(f"Agent {session_id} error: {e}") session.status = "error" @@ -192,6 +226,11 @@ async def handle_run_error(e: Exception, session: AgentSession, session_id: str, "message": error_msg.model_dump(mode="json"), }) p_report_model_error("cert_failure", session_id, session, turn, e, p_stderr_tail) + elif is_cli_binary_missing(e, extra_text=p_stderr_tail) and await p_try_runtime_repair(session, session_id): + # Repaired mid-turn: the file is back and stayed back, so the user gets a retry chip rather + # than a card about antivirus. Boot-time repair only covers a quarantine that happened while + # the app was closed; this is the one that happens while they are working. + pass elif is_cli_binary_missing(e, extra_text=p_stderr_tail): # The bundled CLI vanished from an installed app (Windows AV quarantine class; 22 of 25 field installs never recovered). The raw "not found at: C:\..." card is unactionable; name the likely cause and the two real fixes. # "Restore it from your antivirus quarantine" is technically right and empirically useless: diff --git a/backend/apps/agents/manager/streaming/unwedge_sidecar.py b/backend/apps/agents/manager/streaming/unwedge_sidecar.py index 2dcd86c1..2b273578 100644 --- a/backend/apps/agents/manager/streaming/unwedge_sidecar.py +++ b/backend/apps/agents/manager/streaming/unwedge_sidecar.py @@ -35,13 +35,9 @@ CORE_PREFIX = "mcp__openswarm-core__" # Every core tool that may block on a human, a model, or a whole delegated run. A timeout on these # would be a capability regression, which is worse than the bug. -P_BLOCKING_TOOLS: Set[str] = { - "AskUI", "AskUserQuestion", "ShowUI", - "CreateBrowserAgent", "BrowserAgent", "AppAgent", - "SpawnAgent", "InvokeAgent", "RequestHumanIntervention", - "MCPSearch", "MCPActivate", - "RunToolScript", -} +# Imported, never restated: this set and the registered delegation tools drifted apart once already +# and cost a user weeks of browser runs. +from backend.apps.agents.manager.delegation_tool_names import BLOCKING_TOOLS as P_BLOCKING_TOOLS @typechecked diff --git a/backend/tests/test_cli_self_heal.py b/backend/tests/test_cli_self_heal.py index 1b057c40..7e19f0bb 100644 --- a/backend/tests/test_cli_self_heal.py +++ b/backend/tests/test_cli_self_heal.py @@ -146,3 +146,42 @@ def test_the_repair_never_heals_in_silence(): assert returns, "the health endpoint stopped reporting cli_missing" for ln in returns: assert "cli_repair" in ln, f"a return path hides the repair: {ln.strip()[:80]}" + + +@pytest.mark.asyncio +async def test_a_MIDSESSION_quarantine_repairs_instead_of_carding(monkeypatch, tmp_path): + """Boot-time repair only covers a quarantine that happened while the app was closed. The one + that happens while someone is working still ended the turn with an antivirus card, which is the + exact moment Kittie was in.""" + import backend.apps.agents.manager.run.handle_run_error as hre + import backend.apps.agents.core.bundled_cli_missing as det + import backend.apps.agents.core.cli_self_heal as sh + from backend.apps.agents.core.models import AgentSession + + monkeypatch.setattr(det, "bundled_cli_missing", lambda: "/gone/claude.exe") + monkeypatch.setattr(sh, "repair_bundled_cli", + lambda dest, *a, **k: sh.RepairResult(repaired=True, detail="restored")) + sent = [] + async def p_send(sid, ev, payload): sent.append(payload) + monkeypatch.setattr(hre.ws_manager, "send_to_session", p_send) + + s = AgentSession(name="t", model="opus-5") + assert await hre.p_try_runtime_repair(s, "sess") is True + assert sent and "restored" in str(sent[-1]).lower() + assert "send that message again" in str(sent[-1]).lower(), "tell them what to do next" + + +@pytest.mark.asyncio +async def test_a_repair_that_does_not_stick_lets_the_card_stand(monkeypatch): + """The ambiguous outcomes must NOT suppress the card: a half-repair that reads as success is + worse than the card, because the user retries into the same wall with no explanation.""" + import backend.apps.agents.manager.run.handle_run_error as hre + import backend.apps.agents.core.bundled_cli_missing as det + import backend.apps.agents.core.cli_self_heal as sh + from backend.apps.agents.core.models import AgentSession + + monkeypatch.setattr(det, "bundled_cli_missing", lambda: "/gone/claude.exe") + for result in (sh.RepairResult(repaired=True, retaken=True, detail="taken again"), + sh.RepairResult(repaired=False, detail="no package")): + monkeypatch.setattr(sh, "repair_bundled_cli", lambda dest, *a, r=result, **k: r) + assert await hre.p_try_runtime_repair(AgentSession(name="t", model="opus-5"), "sess") is False diff --git a/backend/tests/test_no_delegation_tool_is_wedge_eligible.py b/backend/tests/test_no_delegation_tool_is_wedge_eligible.py new file mode 100644 index 00000000..07d49964 --- /dev/null +++ b/backend/tests/test_no_delegation_tool_is_wedge_eligible.py @@ -0,0 +1,53 @@ +"""Every tool that hands work to a browser, an app, or another agent must be exempt from the +quick-tool wedge deadline. This is the test that was missing. + +`BrowserAgents` (the PARALLEL browser tool) was registered as a real tool and left out of the +exemption set, so the 25s quick-tool watchdog shot the sidecar 25 seconds into every parallel browser +run. The singular `BrowserAgent` was exempt, so anyone testing one browser at a time saw nothing, +while the one person running several reported "browser use disconnects constantly" at an almost 100% +failure rate for weeks. + +The defect is the repo's signature shape: two lists of names that must agree, with nothing checking +them against each other (same as .gitignore vs build.files). There is one list now, and this test is +what keeps it that way.""" + +from backend.apps.agents.core.models import AgentSession +from backend.apps.agents.manager.delegation_tool_names import ( + BLOCKING_TOOLS, BROWSER_DELEGATION_TOOLS, +) +from backend.apps.agents.manager.register_builtin_mcp_servers import register_builtin_mcp_servers +from backend.apps.agents.manager.streaming.unwedge_sidecar import CORE_PREFIX, is_quick_core_tool + + +def test_every_registered_browser_tool_is_wedge_exempt(): + """Asserted against what the app REGISTERS, not against a copy of the list, so adding a tool to + the registry without exempting it fails here instead of in a user's browser run.""" + servers = {} + browser_tools, _ = register_builtin_mcp_servers(servers, AgentSession(name="t"), {}, None, None) + assert browser_tools, "the registry stopped returning browser tools" + for t in browser_tools: + assert not is_quick_core_tool(CORE_PREFIX + t), \ + f"{t} is registered but not exempt: a run using it dies at the 25s quick-tool deadline" + + +def test_the_parallel_form_specifically(_=None): + """Named on its own because it is the one that was missing, and because a plural/singular pair is + exactly the kind of near-duplicate a reader's eye slides over.""" + for t in ("BrowserAgent", "BrowserAgents"): + assert not is_quick_core_tool(CORE_PREFIX + t), f"{t} must never be treated as a quick tool" + + +def test_the_two_lists_cannot_drift_because_there_is_only_one(): + src = open("backend/apps/agents/manager/streaming/unwedge_sidecar.py", encoding="utf-8").read() + assert "delegation_tool_names import" in src, "the watchdog must import the names, not restate them" + assert '"BrowserAgent"' not in src, "a second hand-written copy is how this bug happened" + reg = open("backend/apps/agents/manager/register_builtin_mcp_servers.py", encoding="utf-8").read() + assert "BROWSER_DELEGATION_TOOLS" in reg, "the registry must read the same list" + + +def test_an_ordinary_quick_tool_is_still_watched(): + """The innocent case. Exempting everything would delete the guard: a genuinely wedged sidecar on + a millisecond tool is what the 25s deadline exists for.""" + for t in ("MemoryRead", "SettingsRead", "ListScheduledWorkflows"): + assert is_quick_core_tool(CORE_PREFIX + t), f"{t} should still be watched" + assert set(BROWSER_DELEGATION_TOOLS) <= BLOCKING_TOOLS