diff --git a/backend/apps/agents/core/error_classify.py b/backend/apps/agents/core/error_classify.py index bf7825df..b0d8c395 100644 --- a/backend/apps/agents/core/error_classify.py +++ b/backend/apps/agents/core/error_classify.py @@ -412,6 +412,16 @@ def is_connection_lost(exc: BaseException) -> bool: return isinstance(exc, p_get_transient_exc_types()) +# 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 +# cannot promote itself into a verdict (ENG-365 learned that the hard way with "line 401,"). +P_PERMANENT_STATUS = re.compile( + r"(?:API\s+Error:\s*|HTTP\s+|status(?:\s*code)?\s*[:=]\s*|\[)\s*(?:400|422)\b", + re.IGNORECASE, +) + + @typechecked def is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> bool: # The Claude CLI's underlying ProcessError stringifies to a generic "Command failed with exit code 1 / Check stderr output for details"; the real cause (rate_limit_error / No pool capacity available / 429 / overloaded) only surfaces in the subprocess's stderr stream, which we capture via the SDK's `stderr` callback and pass in as extra_text. Classify against both so we catch capacity errors regardless of which channel carried the message. @@ -422,6 +432,12 @@ def is_transient_capacity_error(exc: BaseException, extra_text: str = "") -> boo # 335s of retries cannot fix a certificate; the user has to (ENG-218, reproduced against badssl). if is_cert_failure(exc, extra_text): return False + # Nor can any wait fix a malformed request. Ahead of every hint-driven branch below on purpose: + # 9router appends "(reset after Ns)" to EVERYTHING, and that substring appears in both the + # reset-hint rule and TRANSIENT_CAPACITY_PATTERNS, so a deterministic 400 was parking on a 900s + # ladder forever for a request that could never succeed (ENG-395, found via ENG-394). + if combined and P_PERMANENT_STATUS.search(combined): + return False # A failure that names its own recovery window ("reset after 1m 57s") heals itself, even when # it's dressed as a 401; the reset hint outranks the auth-shaped non-transient veto (caught live). if combined and re.search(r"reset\s+after\s+\d", combined, re.IGNORECASE): diff --git a/backend/apps/agents/manager/run/handle_run_error.py b/backend/apps/agents/manager/run/handle_run_error.py index 18e1978a..6502638b 100644 --- a/backend/apps/agents/manager/run/handle_run_error.py +++ b/backend/apps/agents/manager/run/handle_run_error.py @@ -201,7 +201,8 @@ async def handle_run_error(e: Exception, session: AgentSession, session_id: str, from backend.apps.agents.manager.run.reconnect_resume import clear_reconnect_wait clear_reconnect_wait(session) if p_delay is not None: - logger.info(f"Agent {session_id}: connection lost past the in-turn budget; retrying in {p_delay}s") + p_why = "connection lost" if is_connection_lost(e) else "provider unavailable" + logger.info(f"Agent {session_id}: {p_why} past the in-turn budget; retrying in {p_delay}s") await ws_manager.send_to_session(session_id, "agent:reconnect_wait", { "session_id": session_id, "retry_in_s": p_delay, diff --git a/backend/tests/test_error_classify.py b/backend/tests/test_error_classify.py index afaf8d48..3124333e 100644 --- a/backend/tests/test_error_classify.py +++ b/backend/tests/test_error_classify.py @@ -144,3 +144,28 @@ def test_cert_failure_is_never_transient(): assert is_transient_capacity_error(httpx.ConnectError(msg)) is False # A cert-free transport hiccup keeps its transient classification. assert is_transient_capacity_error(httpx.ConnectError("Connection refused")) is True + + +def test_a_malformed_request_is_never_transient_however_it_is_dressed(): + # 9router appends "(reset after Ns)" to EVERYTHING, and that substring is load-bearing in two + # separate branches, so a deterministic 400 parked on a 900s ladder forever (ENG-395/ENG-394). + from backend.apps.agents.core.error_classify import is_transient_capacity_error + b400 = ('API Error: 400 {"error":{"message":"[claude/claude-opus-5] [400]: Tool X cannot have ' + 'both defer_loading=true and cache_control set. (reset after 21s)"}}') + assert is_transient_capacity_error(RuntimeError(b400)) is False + + +def test_the_reset_hint_still_rescues_what_it_was_written_for(): + # The controls that keep the fix from being a blanket kill: a rotating token really does heal, + # and a "400" that is a line number or a token count is not a status at all (ENG-365). + from backend.apps.agents.core.error_classify import is_transient_capacity_error as t + assert t(RuntimeError("API Error: 401 unauthorized (reset after 1m 57s)")) is True + assert t(RuntimeError('API Error: 429 {"message":"rate_limit_error (reset after 21s)"}')) is True + assert t(RuntimeError("File runner.py, line 400, in execute (reset after 3s)")) is True + assert t(RuntimeError("context has 4000 tokens (reset after 3s)")) is True + + +def test_the_retry_log_does_not_claim_a_transport_fault_it_did_not_classify(): + src = open("backend/apps/agents/manager/run/handle_run_error.py").read() + assert 'p_why = "connection lost" if is_connection_lost(e) else "provider unavailable"' in src, \ + "the log sent a reader hunting a transport fault that was never classified"