mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-02 14:28:59 +02:00
[eric] agents: a delegated run gets a deadline in its own units, not the quick-tool ceiling
This commit is contained in:
@@ -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")
|
||||
|
||||
@@ -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
|
||||
Reference in New Issue
Block a user