diff --git a/backend/apps/agents/manager/streaming/unwedge_sidecar.py b/backend/apps/agents/manager/streaming/unwedge_sidecar.py index 2b273578..86332d73 100644 --- a/backend/apps/agents/manager/streaming/unwedge_sidecar.py +++ b/backend/apps/agents/manager/streaming/unwedge_sidecar.py @@ -29,6 +29,15 @@ WEDGE_SECONDS = 25.0 LATE_WEDGE_SECONDS = 120.0 # Still heartbeating at the late deadline means a genuinely long call; only this long dies regardless, so a hung per-call thread cannot hang the session forever (ENG-368). HARD_WEDGE_SECONDS = 300.0 +# A delegated run is a whole other agent doing real work, so five minutes is a deadline for the +# WRONG unit. The exemption above already says these tools may block for as long as the delegated run +# takes, but it only governed the 25s arming: `wedge_verdict` never received the tool name, so the +# ceiling killed them anyway. Measured in the field 2026-09-01: browser runs killed at 450s and 600s +# outstanding on a healthy, heartbeating sidecar, each one ending the turn mute in front of the user. +# The stale-heartbeat rule is untouched and still kills a genuinely dead sidecar in seconds, which is +# the check that actually protects the session; this ceiling is only the backstop for "alive but the +# call never returns", and for a delegated run that has to be measured in the delegated run's units. +DELEGATED_HARD_WEDGE_SECONDS = 1800.0 HEARTBEAT_FRESH_S = 12.0 CORE_PREFIX = "mcp__openswarm-core__" @@ -88,10 +97,28 @@ def heartbeat_age(session_id: str) -> float: @typechecked -def wedge_verdict(outstanding_s: float, hb_age: float) -> str: +def hard_ceiling_for(tool_name: str) -> float: + """The ceiling in the unit the WORK is measured in: a quick tool answers in milliseconds, a + delegated run takes as long as the browser/app/agent it handed off to. + + Only a DECLARED delegation tool earns the longer one. An unknown name, an empty one, or a + non-core tool keeps the strict 300s, so a caller that forgets to pass the name gets today's + behaviour rather than a silent thirty-minute reprieve. The declared signal is the one shared + list in delegation_tool_names, which exists because two lists of names that must agree will not. + """ + if tool_name.startswith(CORE_PREFIX) and tool_name[len(CORE_PREFIX):] in P_BLOCKING_TOOLS: + return DELEGATED_HARD_WEDGE_SECONDS + return HARD_WEDGE_SECONDS + + +@typechecked +def wedge_verdict(outstanding_s: float, hb_age: float, tool_name: str = "") -> str: """kill | extend. A stale heartbeat is a wedged PROCESS: kill at whichever deadline sees it. A fresh - one is a slow call: keep extending until the hard ceiling (a hung thread must not hang the session).""" - if outstanding_s >= HARD_WEDGE_SECONDS: + one is a slow call: keep extending until the hard ceiling (a hung thread must not hang the session). + + The ceiling is per-tool, not one global constant: the old single 300s killed healthy delegated + runs, which is a deadline on the wrong unit of work.""" + if outstanding_s >= hard_ceiling_for(tool_name): return "kill" if hb_age > HEARTBEAT_FRESH_S: return "kill" @@ -170,9 +197,9 @@ def arm_wedge_watchdog(ctx: object, tool_use_id: str, tool_name: str) -> None: if not session_id: return outstanding = time.time() - started - verdict = wedge_verdict(outstanding, heartbeat_age(session_id)) + verdict = wedge_verdict(outstanding, heartbeat_age(session_id), tool_name) if verdict == "extend": - p_next = LATE_WEDGE_SECONDS if outstanding < LATE_WEDGE_SECONDS else HARD_WEDGE_SECONDS + p_next = LATE_WEDGE_SECONDS if outstanding < LATE_WEDGE_SECONDS else hard_ceiling_for(tool_name) logger.info( f"Agent {session_id}: core tool {tool_name} outstanding {outstanding:.0f}s but the " f"sidecar heartbeat is fresh (alive, slow); re-checking at {p_next:.0f}s") diff --git a/backend/tests/test_delegated_hard_ceiling.py b/backend/tests/test_delegated_hard_ceiling.py new file mode 100644 index 00000000..581b3452 --- /dev/null +++ b/backend/tests/test_delegated_hard_ceiling.py @@ -0,0 +1,78 @@ +"""A delegated run must not be killed by a deadline meant for a millisecond tool. + +Field, 2026-09-01: browser runs on a HEALTHY, heartbeating sidecar were killed at 450s and 600s +outstanding, each ending the turn mute in front of the user. The exemption for delegated tools was +real but only governed the 25s arming; `wedge_verdict` never received the tool name, so the single +300s ceiling killed them anyway. That is a deadline on the wrong unit of work: a quick tool answers +in milliseconds, a delegated run takes as long as the browser it handed off to. + +The protection that actually matters is untouched: a stale heartbeat is a wedged PROCESS and still +dies in seconds, at any age. This ceiling is only the backstop for "alive but never returns". +""" +import pytest + +from backend.apps.agents.manager.streaming.unwedge_sidecar import ( + DELEGATED_HARD_WEDGE_SECONDS, HARD_WEDGE_SECONDS, HEARTBEAT_FRESH_S, + hard_ceiling_for, wedge_verdict, +) + +BROWSER = "mcp__openswarm-core__BrowserAgent" +CREATE = "mcp__openswarm-core__CreateBrowserAgent" +QUICK = "mcp__openswarm-core__MemoryWrite" + + +@pytest.mark.parametrize("outstanding", [450.0, 600.0]) +def test_the_two_kills_from_the_field_now_survive(outstanding): + assert wedge_verdict(outstanding, 1.0, BROWSER) == "extend" + + +def test_a_quick_tool_keeps_the_old_ceiling_exactly(): + assert hard_ceiling_for(QUICK) == HARD_WEDGE_SECONDS + assert wedge_verdict(HARD_WEDGE_SECONDS, 1.0, QUICK) == "kill" + + +def test_delegated_tools_get_the_longer_one(): + for t in (BROWSER, CREATE, "mcp__openswarm-core__AppAgent", "mcp__openswarm-core__BrowserAgents"): + assert hard_ceiling_for(t) == DELEGATED_HARD_WEDGE_SECONDS, t + + +def test_a_dead_sidecar_still_dies_in_seconds_even_for_a_delegated_tool(): + """The half that keeps this safe: a stale heartbeat means the PROCESS is gone, and no exemption + may protect that. Without this the change would trade a false kill for a real hang.""" + assert wedge_verdict(30.0, HEARTBEAT_FRESH_S + 1, BROWSER) == "kill" + assert wedge_verdict(1.0, 999.0, BROWSER) == "kill" + + +def test_a_delegated_run_is_still_bounded_eventually(): + """Not unbounded: a hung call must not hang the session forever, it just gets measured in the + delegated run's own units.""" + assert wedge_verdict(DELEGATED_HARD_WEDGE_SECONDS, 1.0, BROWSER) == "kill" + assert DELEGATED_HARD_WEDGE_SECONDS > HARD_WEDGE_SECONDS + + +def test_only_a_DECLARED_delegation_tool_earns_the_longer_ceiling(): + """Fail safe toward today's behaviour: an unknown name, an empty one, or a non-core tool keeps + the strict 300s, so a caller that forgets to pass the name cannot silently buy 30 minutes. + + The protection against the BrowserAgents-class bug (a real delegation tool left out of the + exemption and killed at the ceiling) is NOT a lenient default, it is the single shared list.""" + assert hard_ceiling_for("mcp__openswarm-core__SomethingAddedLater") == HARD_WEDGE_SECONDS + assert hard_ceiling_for("") == HARD_WEDGE_SECONDS + assert hard_ceiling_for("Bash") == HARD_WEDGE_SECONDS + + +def test_the_exemption_list_is_the_one_shared_list_not_a_second_copy(): + """Two lists of names that must agree will not; that drift is what cost weeks of browser runs.""" + from backend.apps.agents.manager.streaming import unwedge_sidecar as mod + from backend.apps.agents.manager.delegation_tool_names import BLOCKING_TOOLS + assert mod.P_BLOCKING_TOOLS is BLOCKING_TOOLS, "it must be imported, never restated" + for t in ("BrowserAgent", "BrowserAgents", "CreateBrowserAgent", "AppAgent"): + assert t in BLOCKING_TOOLS + + +def test_the_call_site_passes_the_tool_name(): + """The whole bug was that it did not. A test on wedge_verdict alone would still pass.""" + import inspect + from backend.apps.agents.manager.streaming import unwedge_sidecar as mod + src = inspect.getsource(mod) + assert "wedge_verdict(outstanding, heartbeat_age(session_id), tool_name)" in src