mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-08-17 18:25:42 +02:00
[eric] agents: a delegated run whose result is lost recovers instead of hanging the parent forever
This commit is contained in:
@@ -23,7 +23,7 @@ from backend.apps.agents.core.ws_manager import ws_manager
|
||||
from backend.apps.settings.settings import load_settings
|
||||
from backend.apps.agents.manager.permissions import path_gate
|
||||
from backend.apps.agents.manager.permissions.decision import effective_policy
|
||||
from backend.apps.agents.manager.streaming.unwedge_sidecar import arm_wedge_watchdog
|
||||
from backend.apps.agents.manager.streaming.unwedge_sidecar import arm_delegation_watchdog, arm_wedge_watchdog
|
||||
from backend.apps.agents.manager.prompt.tool_catalog import gated_mcp_server_names
|
||||
from backend.apps.agents.manager.prompt.prompt_context import (
|
||||
TOOLSEARCH_LOOP_THRESHOLD,
|
||||
@@ -198,4 +198,5 @@ async def pre_tool_hook(ctx: HookContext, input_data: dict, tool_use_id: Optiona
|
||||
ctx.tool_start_times[tool_use_id] = time.time()
|
||||
# A frozen sidecar hangs a quick core tool forever; the watchdog turns that into the self-healing death (ENG-303).
|
||||
arm_wedge_watchdog(ctx, tool_use_id, tool_name)
|
||||
arm_delegation_watchdog(ctx, tool_use_id, tool_name)
|
||||
return {}
|
||||
|
||||
@@ -148,3 +148,71 @@ def arm_wedge_watchdog(ctx: object, tool_use_id: str, tool_name: str) -> None:
|
||||
arm_retry(getattr(ctx, "session", None))
|
||||
|
||||
loop.call_later(WEDGE_SECONDS, p_check)
|
||||
|
||||
|
||||
# Delegation tools legitimately run for minutes, so they are exempt from the 25s deadline above.
|
||||
# The exemption assumed the child's result always comes home; measured 2026-08-15 on a packaged
|
||||
# build, a CreateBrowserAgent child COMPLETED (backend returned its HTTP 200, sidecar went back to
|
||||
# readline) while the parent hung on the outstanding tool call for 20+ minutes: the result died on
|
||||
# the sidecar->CLI stdio hop and nothing above it has a deadline. When every child is terminal and
|
||||
# stays terminal across two consecutive checks, the wait is provably pointless; recover the same
|
||||
# way the quick class does.
|
||||
P_DELEGATION_TOOLS: Set[str] = {"CreateBrowserAgent", "BrowserAgent", "BrowserAgents", "AppAgent"}
|
||||
DELEGATION_CHECK_SECONDS = 75.0
|
||||
|
||||
|
||||
@typechecked
|
||||
def is_delegation_core_tool(tool_name: str) -> bool:
|
||||
return tool_name.startswith(P_CORE_PREFIX) and tool_name[len(P_CORE_PREFIX):] in P_DELEGATION_TOOLS
|
||||
|
||||
|
||||
@typechecked
|
||||
def delegation_children_settled(session_id: str) -> bool:
|
||||
"""True when this session HAS delegated children and every one of them is terminal. No children
|
||||
yet is NOT settled: a run queued behind the admission cap can wait minutes legitimately."""
|
||||
from backend.apps.agents.agent_manager import agent_manager
|
||||
kids = [s for s in agent_manager.sessions.values()
|
||||
if getattr(s, "parent_session_id", None) == session_id and getattr(s, "mode", "") == "browser-agent"]
|
||||
if not kids:
|
||||
return False
|
||||
return all(getattr(s, "status", "") in ("completed", "error", "failed", "stopped") for s in kids)
|
||||
|
||||
|
||||
@typechecked
|
||||
def arm_delegation_watchdog(ctx: object, tool_use_id: str, tool_name: str) -> None:
|
||||
"""Recurring, slow-cadence sibling of arm_wedge_watchdog for delegation tools. Fires the same
|
||||
unwedge+retry only after TWO consecutive checks (>=75s apart) see every child terminal while
|
||||
the tool call is still outstanding, so a child that is merely slow can never trip it."""
|
||||
if not is_delegation_core_tool(tool_name):
|
||||
return
|
||||
try:
|
||||
loop = asyncio.get_running_loop()
|
||||
except RuntimeError:
|
||||
return
|
||||
|
||||
started = time.time()
|
||||
settled_streak = {"n": 0}
|
||||
|
||||
def p_check() -> None:
|
||||
times = getattr(ctx, "tool_start_times", None)
|
||||
if not isinstance(times, dict) or tool_use_id not in times:
|
||||
return
|
||||
session_id = getattr(ctx, "session_id", "")
|
||||
if not session_id:
|
||||
return
|
||||
try:
|
||||
settled = delegation_children_settled(session_id)
|
||||
except Exception:
|
||||
settled = False
|
||||
settled_streak["n"] = settled_streak["n"] + 1 if settled else 0
|
||||
if settled_streak["n"] >= 2:
|
||||
logger.warning(
|
||||
"delegation result lost: %s outstanding %.0fs on session %s with every child terminal; recovering",
|
||||
tool_name, time.time() - started, session_id[:8],
|
||||
)
|
||||
loop.run_in_executor(None, unwedge, session_id, tool_name, time.time() - started)
|
||||
arm_retry(getattr(ctx, "session", None))
|
||||
return
|
||||
loop.call_later(DELEGATION_CHECK_SECONDS, p_check)
|
||||
|
||||
loop.call_later(DELEGATION_CHECK_SECONDS, p_check)
|
||||
|
||||
@@ -0,0 +1,92 @@
|
||||
"""A delegated run whose result is lost cannot hang the parent forever (live specimen 2026-08-15).
|
||||
|
||||
CreateBrowserAgent is exempt from the 25s unwedge because a browser run legitimately takes minutes.
|
||||
The exemption assumed results always come home; a packaged-build stress run produced the miss: the
|
||||
child COMPLETED (backend logged the run summary and answered the sidecar's HTTP call with 200, the
|
||||
sidecar returned to readline), and the parent sat 'running' on the outstanding tool call for 20+
|
||||
minutes. These pin the backstop: two consecutive all-children-terminal observations while the call
|
||||
is still outstanding -> the same unwedge+retry recovery the quick class already has.
|
||||
"""
|
||||
import asyncio
|
||||
from typing import Dict
|
||||
|
||||
import pytest
|
||||
|
||||
from backend.apps.agents.core.models import AgentSession
|
||||
from backend.apps.agents.manager.streaming import unwedge_sidecar as u
|
||||
|
||||
|
||||
def test_delegation_classifier_is_exact():
|
||||
assert u.is_delegation_core_tool("mcp__openswarm-core__CreateBrowserAgent")
|
||||
assert u.is_delegation_core_tool("mcp__openswarm-core__AppAgent")
|
||||
assert not u.is_delegation_core_tool("mcp__openswarm-core__MemoryWrite"), "quick class stays on the 25s path"
|
||||
assert not u.is_delegation_core_tool("mcp__openswarm-core__AskUI"), "a human answering is not a lost result"
|
||||
assert not u.is_delegation_core_tool("CreateBrowserAgent"), "only core-prefixed names"
|
||||
|
||||
|
||||
def test_settled_requires_children_and_all_terminal(monkeypatch):
|
||||
from backend.apps.agents import agent_manager as am
|
||||
|
||||
parent = AgentSession(id="par-1", name="p", model="sonnet")
|
||||
running_child = AgentSession(id="kid-1", name="k1", model="sonnet", mode="browser-agent", parent_session_id="par-1")
|
||||
running_child.status = "running"
|
||||
done_child = AgentSession(id="kid-2", name="k2", model="sonnet", mode="browser-agent", parent_session_id="par-1")
|
||||
done_child.status = "completed"
|
||||
|
||||
monkeypatch.setattr(am.agent_manager, "sessions", {"par-1": parent}, raising=False)
|
||||
assert u.delegation_children_settled("par-1") is False, "no children = maybe queued behind admission, NOT settled"
|
||||
|
||||
monkeypatch.setattr(am.agent_manager, "sessions", {"par-1": parent, "kid-1": running_child, "kid-2": done_child}, raising=False)
|
||||
assert u.delegation_children_settled("par-1") is False, "one live child means the wait is legitimate"
|
||||
|
||||
running_child.status = "completed"
|
||||
assert u.delegation_children_settled("par-1") is True
|
||||
|
||||
|
||||
class P_Ctx:
|
||||
def __init__(self, session: AgentSession, times: Dict[str, float]):
|
||||
self.session = session
|
||||
self.session_id = session.id
|
||||
self.tool_start_times = times
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_two_settled_checks_fire_the_recovery(monkeypatch):
|
||||
fired = {}
|
||||
monkeypatch.setattr(u, "unwedge", lambda sid, tool, age: fired.setdefault("unwedge", (sid, tool)))
|
||||
monkeypatch.setattr(u, "arm_retry", lambda s: fired.setdefault("retry", s.id if s else None))
|
||||
monkeypatch.setattr(u, "delegation_children_settled", lambda sid: True)
|
||||
monkeypatch.setattr(u, "DELEGATION_CHECK_SECONDS", 0.05)
|
||||
|
||||
sess = AgentSession(id="par-2", name="p", model="sonnet")
|
||||
ctx = P_Ctx(sess, {"tu-1": 0.0})
|
||||
u.arm_delegation_watchdog(ctx, "tu-1", "mcp__openswarm-core__CreateBrowserAgent")
|
||||
await asyncio.sleep(0.3)
|
||||
assert fired.get("unwedge", (None, None))[0] == "par-2", "two settled checks must recover the parent"
|
||||
assert fired.get("retry") == "par-2", "the retry is what redoes the lost step"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_a_finished_call_disarms_and_a_live_child_resets_the_streak(monkeypatch):
|
||||
fired = {}
|
||||
monkeypatch.setattr(u, "unwedge", lambda *a: fired.setdefault("unwedge", a))
|
||||
monkeypatch.setattr(u, "arm_retry", lambda s: fired.setdefault("retry", True))
|
||||
monkeypatch.setattr(u, "DELEGATION_CHECK_SECONDS", 0.05)
|
||||
|
||||
# Finished call: the post hook popped the id, so the first check returns silently.
|
||||
sess = AgentSession(id="par-3", name="p", model="sonnet")
|
||||
u.arm_delegation_watchdog(P_Ctx(sess, {}), "tu-gone", "mcp__openswarm-core__CreateBrowserAgent")
|
||||
|
||||
# Oscillating child (settles once, then a new child appears): the streak must reset, never fire.
|
||||
seq = iter([True, False, True, False, True, False])
|
||||
monkeypatch.setattr(u, "delegation_children_settled", lambda sid: next(seq, False))
|
||||
u.arm_delegation_watchdog(P_Ctx(sess, {"tu-2": 0.0}), "tu-2", "mcp__openswarm-core__CreateBrowserAgent")
|
||||
await asyncio.sleep(0.4)
|
||||
assert "unwedge" not in fired, "a merely-slow delegation must never be recovered out from under itself"
|
||||
|
||||
|
||||
def test_quick_tools_never_arm_the_delegation_watchdog():
|
||||
# Belt and suspenders: arming for a quick tool would double-recover with the 25s path.
|
||||
sess = AgentSession(id="par-4", name="p", model="sonnet")
|
||||
u.arm_delegation_watchdog(P_Ctx(sess, {"tu-3": 0.0}), "tu-3", "mcp__openswarm-core__MemoryRead")
|
||||
# No loop assertions needed: is_delegation_core_tool returned False, nothing scheduled.
|
||||
Reference in New Issue
Block a user