mirror of
https://github.com/openswarm-ai/openswarm.git
synced 2026-09-12 20:57:42 +02:00
[eric] agents: the deferred-tool 400 (ENG-394) earns one CLI respawn instead of carding out on top of the turn's work
This commit is contained in:
@@ -439,6 +439,25 @@ def is_connection_lost(exc: BaseException) -> bool:
|
||||
return isinstance(exc, p_get_transient_exc_types())
|
||||
|
||||
|
||||
# A poisoned TOOL SCHEMA, not a poisoned request. The CLI defers our MCP tools when it connects
|
||||
# while 9router is down, and the first ToolSearch that loads one sends it carrying both
|
||||
# defer_loading and cache_control, which the API rejects. It arrives as a 400 so P_PERMANENT_STATUS
|
||||
# correctly refuses to WAIT on it, but a fresh CLI re-registers the tools and the same turn goes
|
||||
# through: same shape as a dead socket (the process holds a corpse), not as a 429 (a healthy
|
||||
# connection carrying a no). Anchored on both halves so an unrelated 400 mentioning caching can't
|
||||
# claim a respawn.
|
||||
P_STALE_TOOL_SCHEMA = re.compile(
|
||||
r"defer_loading[^\n]{0,80}cache_control|cache_control[^\n]{0,80}defer_loading",
|
||||
re.IGNORECASE,
|
||||
)
|
||||
|
||||
|
||||
@typechecked
|
||||
def is_stale_tool_schema_error(exc: BaseException, extra_text: str = "") -> bool:
|
||||
"""True for the CLI-side deferred-tool 400 that a respawn cures and waiting never will."""
|
||||
return bool(P_STALE_TOOL_SCHEMA.search(f"{exc!s}\n{extra_text}"))
|
||||
|
||||
|
||||
# A MALFORMED request: the provider will answer identically forever, so no wait helps. Deliberately
|
||||
# narrow. 401 stays out (a rotating token really does heal, which is why the reset-hint rule exists),
|
||||
# and so do 408/429. Matched only in status POSITION, so a "400" in a line number or a byte count
|
||||
|
||||
@@ -139,6 +139,7 @@ class AgentSession(BaseModel):
|
||||
needs_fresh_session: bool = False
|
||||
# A new CLI process that RESUMES the same transcript (dead transport, stale token, core sidecar never connected); unlike needs_fresh_session nothing is rebuilt, so no history is ever re-authored as text (ENG-382).
|
||||
needs_respawn: bool = False
|
||||
stale_tool_schema_retry_used: bool = False
|
||||
# Auto-continue: agent loop dispatches a hidden turn at end-of-loop using pending_continuation_prompt. Race-free vs background tasks.
|
||||
pending_continuation: bool = False
|
||||
pending_continuation_prompt: Optional[str] = None
|
||||
|
||||
@@ -172,6 +172,7 @@ class Messaging(AgentManagerProtocol):
|
||||
session.empty_finish_progress_mark = 0
|
||||
session.empty_finish_surfaced = False
|
||||
session.auth_retry_used = False
|
||||
session.stale_tool_schema_retry_used = False
|
||||
# The repeat-quit floor and the vanishing-quit rule key on this; one false positive used to arm both for the session's life (ENG-364).
|
||||
session.empty_finish_total = 0
|
||||
# The borrowed API key was for one ask, and this is a new one; back to the lane they chose.
|
||||
|
||||
@@ -15,6 +15,7 @@ from backend.apps.agents.manager.streaming.state import TurnState
|
||||
from backend.apps.agents.core.error_classify import (
|
||||
is_context_overflow_error,
|
||||
is_long_context_error,
|
||||
is_stale_tool_schema_error,
|
||||
is_transient_capacity_error,
|
||||
is_free_trial_exhausted,
|
||||
is_out_of_tokens,
|
||||
@@ -159,6 +160,19 @@ async def handle_run_error(e: Exception, session: AgentSession, session_id: str,
|
||||
except Exception:
|
||||
p_stderr_tail = ""
|
||||
# No completed-mask here anymore: current_turn_emitted stays True until a ResultMessage lands, so the old "already answered" early-return fired on every MID-TASK death (models narrate between tool calls) and converted a dead run into a fake "completed". Reaching this handler with an overflow means the valve's compact-and-retry already failed once; the user must see the card.
|
||||
# Ahead of every card-emitting branch on purpose: this 400 is the CLI holding a stale tool
|
||||
# registration, not the provider refusing. Below the generic error branch it would card out on
|
||||
# top of a turn's real work (61 tool calls, live, 2026-08-31); a respawn just continues it.
|
||||
if is_stale_tool_schema_error(e, extra_text=p_stderr_tail):
|
||||
from backend.apps.agents.manager.streaming.auth_retry import try_stale_tool_schema_self_heal
|
||||
if try_stale_tool_schema_self_heal(session):
|
||||
logger.warning(
|
||||
f"Agent {session_id}: CLI sent a deferred tool with cache_control (ENG-394); "
|
||||
f"respawning it and redoing that step"
|
||||
)
|
||||
return
|
||||
logger.warning(f"Agent {session_id}: deferred-tool 400 again after a respawn; carding it")
|
||||
|
||||
if is_context_overflow_error(e, extra_text=p_stderr_tail):
|
||||
p_tier_gate = is_long_context_error(e, extra_text=p_stderr_tail)
|
||||
friendly_msg = (
|
||||
|
||||
@@ -69,3 +69,29 @@ def try_transient_self_heal(session: AgentSession, delay_s: int = 0) -> bool:
|
||||
session.pending_continuation_prompt = TRANSIENT_RETRY_PROMPT
|
||||
session.pending_continuation_delay_s = max(0, delay_s)
|
||||
return True
|
||||
|
||||
|
||||
STALE_TOOL_SCHEMA_RETRY_PROMPT = (
|
||||
"Your tool definitions were stale on that last step and the connection has been rebuilt with "
|
||||
"fresh ones. Redo that one step, then carry on where you left off."
|
||||
)
|
||||
|
||||
|
||||
@typechecked
|
||||
def try_stale_tool_schema_self_heal(session: AgentSession) -> bool:
|
||||
"""One respawn for the deferred-tool 400 (ENG-394): the CLI re-registers its tools on a new
|
||||
process, so the same turn goes through instead of dying on top of the work it already did.
|
||||
|
||||
Its own budget, like the auth one-shot and for the same reason: this and an expiring token
|
||||
arrive by different doors moments apart, and a shared counter would let one eat the other's
|
||||
retry. One is the whole budget; a second identical 400 means respawning is not the cure and the
|
||||
user is owed the honest card rather than a loop.
|
||||
"""
|
||||
if session.stale_tool_schema_retry_used or session.pending_continuation:
|
||||
return False
|
||||
session.stale_tool_schema_retry_used = True
|
||||
session.needs_respawn = True
|
||||
session.pending_continuation = True
|
||||
session.pending_continuation_prompt = STALE_TOOL_SCHEMA_RETRY_PROMPT
|
||||
session.pending_continuation_delay_s = 0
|
||||
return True
|
||||
|
||||
@@ -91,14 +91,24 @@ def test_heal_never_stacks_on_a_pending_continuation():
|
||||
assert session.auth_retry_used is False, "a refused heal must not burn the budget"
|
||||
|
||||
|
||||
def test_a_real_user_message_reopens_the_heal_budget():
|
||||
# Wire-check both directions: the flag is set by the heal AND cleared with the other per-ask
|
||||
# budgets on a real (non-hidden) user message.
|
||||
@pytest.mark.parametrize("flag", ["auth_retry_used", "stale_tool_schema_retry_used"])
|
||||
def test_a_real_user_message_reopens_the_heal_budget(flag):
|
||||
"""Wire-check both directions: each one-shot is set by its heal AND cleared with the other
|
||||
per-ask budgets when a real (non-hidden) user message arrives.
|
||||
|
||||
Anchored on the reset block's own comment rather than on the FIRST `if not hidden:` plus a
|
||||
400-character window: that version silently started reading a different block the moment an
|
||||
unrelated `if not hidden:` was added earlier in the file, and failed for a reason that had
|
||||
nothing to do with the behaviour it guards.
|
||||
"""
|
||||
import inspect
|
||||
from backend.apps.agents.manager import Messaging
|
||||
src = inspect.getsource(Messaging)
|
||||
block = src[src.index("if not hidden:"):src.index("if not hidden:") + 400]
|
||||
assert "auth_retry_used = False" in block
|
||||
start = src.index("A human actively driving the session forgives its crash history")
|
||||
block = src[start:src.index("if not hidden:", start)] if "if not hidden:" in src[start:] else src[start:]
|
||||
assert f"{flag} = False" in block, (
|
||||
f"{flag} is never reopened, so one spent retry disarms that heal for the session's life"
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
|
||||
@@ -0,0 +1,140 @@
|
||||
"""ENG-394: the deferred-tool 400 is cured by a respawn, so it must not card out on top of the work.
|
||||
|
||||
Reproduced live on packaged 1.7.10-exp.2, 2026-08-31, deterministically: kill 9router, launch a
|
||||
session, and the CLI defers all 44 MCP tools. The first ToolSearch that loads one sends it carrying
|
||||
both defer_loading and cache_control, the API 400s, and the turn died after 61 real tool calls.
|
||||
|
||||
Control pair from that run: router UP -> 0 ToolSearch, both arms completed; router DOWN -> 1
|
||||
ToolSearch, both arms 400.
|
||||
"""
|
||||
import pytest
|
||||
|
||||
from backend.apps.agents.core.error_classify import is_stale_tool_schema_error, is_transient_capacity_error
|
||||
from backend.apps.agents.core.models import AgentSession
|
||||
from backend.apps.agents.manager.streaming.auth_retry import (
|
||||
try_auth_self_heal,
|
||||
try_stale_tool_schema_self_heal,
|
||||
)
|
||||
|
||||
REAL = (
|
||||
"The agent runtime reported this turn failed (stop_sequence). API Error: 400 "
|
||||
'{"error":{"message":"[claude/claude-sonnet-4-6] [400]: Tool '
|
||||
"'mcp__openswarm-core__CreateBrowserAgent' cannot have both defer_loading=true and cache_control "
|
||||
'set. Tools with defer_loading cannot use prompt caching. (reset after 30s)"}}'
|
||||
)
|
||||
REAL_SHOWUI = REAL.replace("CreateBrowserAgent", "ShowUI")
|
||||
|
||||
|
||||
def p_session() -> AgentSession:
|
||||
return AgentSession(id="s1", name="t", model="sonnet")
|
||||
|
||||
|
||||
def test_the_real_400_from_the_live_drill_is_recognised():
|
||||
assert is_stale_tool_schema_error(RuntimeError(REAL))
|
||||
assert is_stale_tool_schema_error(RuntimeError(REAL_SHOWUI))
|
||||
|
||||
|
||||
def test_it_is_also_found_when_the_text_arrives_on_stderr():
|
||||
assert is_stale_tool_schema_error(RuntimeError("Command failed with exit code 1"), extra_text=REAL)
|
||||
|
||||
|
||||
@pytest.mark.parametrize("innocent", [
|
||||
# The question this list answers: what legitimate case looks like the bad case?
|
||||
"API Error: 400 {\"error\":{\"message\":\"max_tokens is too large\"}}",
|
||||
"API Error: 429 rate_limit_error (reset after 21s)",
|
||||
"prompt caching is enabled for this request",
|
||||
"defer_loading is supported on this model",
|
||||
"File \"x.py\", line 400, in run # cache_control mentioned in a traceback",
|
||||
"The model provider returned an expired-credential error",
|
||||
])
|
||||
def test_ordinary_failures_never_claim_a_respawn(innocent):
|
||||
assert not is_stale_tool_schema_error(RuntimeError(innocent))
|
||||
|
||||
|
||||
def test_the_400_still_refuses_to_be_waited_on():
|
||||
"""The ENG-395 rule must survive: this is a 400, so no backoff ladder may adopt it."""
|
||||
assert not is_transient_capacity_error(RuntimeError(REAL))
|
||||
|
||||
|
||||
def test_one_respawn_is_armed_then_the_budget_is_spent():
|
||||
s = p_session()
|
||||
assert try_stale_tool_schema_self_heal(s) is True
|
||||
assert s.needs_respawn is True
|
||||
assert s.pending_continuation is True
|
||||
assert s.pending_continuation_delay_s == 0
|
||||
|
||||
s.pending_continuation = False # the dispatcher consumed it
|
||||
assert try_stale_tool_schema_self_heal(s) is False, "a second identical 400 must card, not loop"
|
||||
|
||||
|
||||
def test_it_does_not_eat_the_auth_one_shot():
|
||||
"""Separate budgets: these arrive by different doors moments apart."""
|
||||
s = p_session()
|
||||
assert try_stale_tool_schema_self_heal(s) is True
|
||||
s.pending_continuation = False
|
||||
assert try_auth_self_heal(s) is True, "the auth retry lost its budget to the schema retry"
|
||||
|
||||
|
||||
def test_it_never_stomps_a_continuation_that_is_already_armed():
|
||||
s = p_session()
|
||||
s.pending_continuation = True
|
||||
s.pending_continuation_prompt = "something else already owns this"
|
||||
assert try_stale_tool_schema_self_heal(s) is False
|
||||
assert s.pending_continuation_prompt == "something else already owns this"
|
||||
|
||||
|
||||
def test_the_veto_sits_above_every_card_emitting_branch():
|
||||
"""Ordering, not just behaviour (the recurring defect in this codebase is a guard placed inside
|
||||
one branch). Below the generic handler this would card on top of the turn's work."""
|
||||
import inspect
|
||||
from backend.apps.agents.manager.run import handle_run_error as mod
|
||||
src = inspect.getsource(mod.handle_run_error)
|
||||
mine = src.index("is_stale_tool_schema_error(e")
|
||||
for later in ("is_context_overflow_error(e", "is_transient_capacity_error(e", "is_auth_error(e"):
|
||||
assert mine < src.index(later), f"the respawn veto must precede {later}"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_handle_run_error_arms_the_respawn_and_emits_no_card(monkeypatch):
|
||||
"""The wiring test. The unit tests above all pass with the branch deleted from the handler,
|
||||
which is exactly how a fix ships broken; this one drives the real door."""
|
||||
from backend.apps.agents.manager.run import handle_run_error as mod
|
||||
from backend.apps.agents.manager.streaming.state import TurnState
|
||||
|
||||
sent: list[tuple[str, dict]] = []
|
||||
|
||||
async def p_send(session_id, event, payload):
|
||||
sent.append((event, payload))
|
||||
|
||||
monkeypatch.setattr(mod.ws_manager, "send_to_session", p_send)
|
||||
|
||||
s = p_session()
|
||||
s.dashboard_id = "d"
|
||||
turn = TurnState()
|
||||
await mod.handle_run_error(RuntimeError(REAL), s, s.id, turn, [])
|
||||
|
||||
assert s.needs_respawn is True, "the CLI must be respawned; a fresh one re-registers its tools"
|
||||
assert s.pending_continuation is True, "the turn must continue, not die on top of its work"
|
||||
cards = [m for m in s.messages if m.role == "system"]
|
||||
assert cards == [], f"a curable 400 must not card out; got {[c.content[:60] for c in cards]}"
|
||||
assert not any(e == "agent:message" for e, _ in sent), "no error message may reach the user"
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_the_second_identical_400_does_card(monkeypatch):
|
||||
"""The reverse obligation: when respawning is provably not the cure, the user is owed the card
|
||||
rather than a silent loop."""
|
||||
from backend.apps.agents.manager.run import handle_run_error as mod
|
||||
from backend.apps.agents.manager.streaming.state import TurnState
|
||||
|
||||
async def p_send(session_id, event, payload):
|
||||
return None
|
||||
|
||||
monkeypatch.setattr(mod.ws_manager, "send_to_session", p_send)
|
||||
|
||||
s = p_session()
|
||||
s.dashboard_id = "d"
|
||||
s.stale_tool_schema_retry_used = True # the one-shot is already spent
|
||||
await mod.handle_run_error(RuntimeError(REAL), s, s.id, TurnState(), [])
|
||||
|
||||
assert [m for m in s.messages if m.role == "system"], "a spent budget must produce an honest card"
|
||||
Reference in New Issue
Block a user