diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index 3be2df52..49695d79 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -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 diff --git a/backend/apps/agents/core/models.py b/backend/apps/agents/core/models.py index 612b21bb..e9bce0e0 100644 --- a/backend/apps/agents/core/models.py +++ b/backend/apps/agents/core/models.py @@ -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 diff --git a/backend/apps/agents/manager/Messaging.py b/backend/apps/agents/manager/Messaging.py index 27754d0c..73e7d4a8 100644 --- a/backend/apps/agents/manager/Messaging.py +++ b/backend/apps/agents/manager/Messaging.py @@ -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. diff --git a/backend/apps/agents/manager/run/handle_run_error.py b/backend/apps/agents/manager/run/handle_run_error.py index 8bc32e17..9e895cb7 100644 --- a/backend/apps/agents/manager/run/handle_run_error.py +++ b/backend/apps/agents/manager/run/handle_run_error.py @@ -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 = ( diff --git a/backend/apps/agents/manager/streaming/auth_retry.py b/backend/apps/agents/manager/streaming/auth_retry.py index 2563697d..84ea0b74 100644 --- a/backend/apps/agents/manager/streaming/auth_retry.py +++ b/backend/apps/agents/manager/streaming/auth_retry.py @@ -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 diff --git a/backend/tests/test_assistant_message.py b/backend/tests/test_assistant_message.py index f502c524..e689219a 100644 --- a/backend/tests/test_assistant_message.py +++ b/backend/tests/test_assistant_message.py @@ -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 diff --git a/backend/tests/test_stale_tool_schema_respawn.py b/backend/tests/test_stale_tool_schema_respawn.py new file mode 100644 index 00000000..c64a645e --- /dev/null +++ b/backend/tests/test_stale_tool_schema_respawn.py @@ -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"