mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-09 11:17:44 +02:00
[eric] agents: BrowserAgents is exempt from the quick-tool wedge, and the two name lists became one
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01U6zrBsUCNzpMBnov3rTVYV
This commit is contained in:
co-authored by
Claude Opus 5
parent
debc9eb629
commit
b2f797776f
@@ -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
|
||||
@@ -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).
|
||||
|
||||
@@ -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:
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user