From dfd4aa4a94c726c8dab9efcb6dfb2c375e986dd2 Mon Sep 17 00:00:00 2001 From: teknium1 <127238744+teknium1@users.noreply.github.com> Date: Sat, 12 Sep 2026 07:06:12 -0700 Subject: [PATCH] fix(agent): malformed tool-call-argument 400s no longer walk the fallback chain When a proxy (Ollama, OpenRouter) rejects the MODEL's own unparseable tool-call JSON with `400 invalid tool call arguments`, the classifier returned the generic format_error verdict (`should_fallback=True`) and the non-retryable client-error path cascaded through every fallback provider: 4-5 sequential calls, 20-60s per occurrence, ending on a model that produced the same broken JSON (#12770). - error_classifier: explicit `_MALFORMED_TOOL_ARGS_PATTERNS` checked before the request-validation and overflow heuristics, returning format_error with `retryable=False, should_fallback=False`. - turn_api_error: the client-error settlement honours `should_fallback`; the verdicts that legitimately reach that branch (policy block, TLS chain, MoA shape/preset errors) now state `should_fallback=True` explicitly, so the gate changes behaviour only for the new verdict. Local validation errors keep their historical fallback. Fixes #12770. Pattern list and gating approach from #16022 by @cuyua9 (stale base); tests trimmed to two invariants. Co-authored-by: cuyua9 <2114364329@qq.com> --- agent/error_classifier.py | 25 ++++++- agent/turn_api_error.py | 11 ++- .../test_malformed_tool_args_no_fallback.py | 75 +++++++++++++++++++ 3 files changed, 104 insertions(+), 7 deletions(-) create mode 100644 tests/agent/test_malformed_tool_args_no_fallback.py diff --git a/agent/error_classifier.py b/agent/error_classifier.py index 09488986dd..bd62005c4c 100644 --- a/agent/error_classifier.py +++ b/agent/error_classifier.py @@ -242,6 +242,16 @@ _INVALID_MESSAGE_BODY_PATTERNS = ( "messages: at least one message is required", _NO_USER_QUERY_SIGNAL, ) +# Proxy-side rejection of the model's own tool-call JSON (Ollama "invalid tool call arguments", +# OpenRouter-wrapped "function_call arguments"). Checked before the generic 400 validation and +# overflow heuristics: on a large session the bare message would otherwise read as overflow. +_MALFORMED_TOOL_ARGS_PATTERNS = ( + "invalid tool call arguments", "invalid tool_call arguments", "invalid tool_calls arguments", + "invalid function call arguments", "invalid function_call arguments", + "tool call arguments are invalid", "tool_call arguments are invalid", + "function call arguments are invalid", "function_call arguments are invalid", +) + # Malformed request, identical on every retry. Some gateways (codex.nekos.me) # return these as 5xx, so the 5xx path also checks them. _REQUEST_VALIDATION_PATTERNS = ( @@ -374,14 +384,19 @@ _V_AUTH_FALLBACK = _v(_R.auth, **_ABORT_FALLBACK) _V_MODEL_NOT_FOUND = _v(_R.model_not_found, **_ABORT_FALLBACK) _V_CONTENT_BLOCKED = _v(_R.content_policy_blocked, **_ABORT_FALLBACK) _V_FORMAT_ERROR = _v(_R.format_error, **_ABORT_FALLBACK) -_V_POLICY_BLOCKED = _v(_R.provider_policy_blocked, retryable=False) -_V_SSL_CERT = _v(_R.ssl_cert_verification, retryable=False) +# A different provider (direct instead of the aggregator; another host's TLS chain) can fix these. +_V_POLICY_BLOCKED = _v(_R.provider_policy_blocked, **_ABORT_FALLBACK) +_V_SSL_CERT = _v(_R.ssl_cert_verification, **_ABORT_FALLBACK) _V_CONTEXT_OVERFLOW = _v(_R.context_overflow, should_compress=True) _V_PAYLOAD_TOO_LARGE = _v(_R.payload_too_large, should_compress=True) _V_OVERLOADED, _V_SERVER_ERROR, _V_TIMEOUT, _V_UNKNOWN = map(_v, (_R.overloaded, _R.server_error, _R.timeout, _R.unknown)) _V_IMAGE_TOO_LARGE, _V_IMAGE_CORRUPT = _v(_R.image_too_large), _v(_R.image_corrupt) _V_MULTIMODAL, _V_INVALID_ENCRYPTED = _v(_R.multimodal_tool_content_unsupported), _v(_R.invalid_encrypted_content) _V_REASONING_MANDATORY = _v(_R.reasoning_mandatory, should_compress=False, should_fallback=False) +# The MODEL emitted unparseable tool-call JSON and the proxy (Ollama, OpenRouter) rejected it: no +# other provider can fix that output, so falling back only replays the same broken turn 4-5 times +# (20-60s per occurrence, #12770). Abort this call; the loop's argument repair handles the retry. +_V_MALFORMED_TOOL_ARGS = _v(_R.format_error, retryable=False, should_fallback=False) # A reasoning-mandatory route answering ``reasoning: {enabled: false}`` (Nous Portal + OpenRouter wording). _REASONING_MANDATORY_PATTERN = "reasoning is mandatory" @@ -594,10 +609,10 @@ def _moa_special_cases(c: _Ctx) -> Optional[Verdict]: # Local MoA streaming adapter-shape bugs are not a provider outage; falling # back would silently replace the MoA route with a single model (#55933). if c.provider_slug == "moa" and any(s in str(c.error) for s in _MOA_ADAPTER_SHAPE_BUGS): - return _v(_R.format_error, retryable=False) + return _v(_R.format_error, **_ABORT_FALLBACK) # Persisted MoA preset name that was renamed/deleted — deterministic config error. from agent.errors import MoAPresetNotFoundError - return _v(_R.model_not_found, retryable=False) if isinstance(c.error, MoAPresetNotFoundError) else None + return _v(_R.model_not_found, **_ABORT_FALLBACK) if isinstance(c.error, MoAPresetNotFoundError) else None def _by_error_code(c: _Ctx) -> Optional[Verdict]: @@ -773,6 +788,8 @@ def _classify_400(c: _Ctx) -> Verdict: # prompt_cache_retention ~20% of the time): transient, retry identical request. if _is_server_injected_param_rejection(msg, c.provider_slug): return _V_SERVER_ERROR + if any(p in msg for p in _MALFORMED_TOOL_ARGS_PATTERNS): + return _V_MALFORMED_TOOL_ARGS # Before overflow: GPT-5's "Unsupported parameter: 'max_tokens'" contains it. if any(p in msg for p in _400_VALIDATION_PATTERNS) or code in _400_VALIDATION_CODES: return _V_FORMAT_ERROR diff --git a/agent/turn_api_error.py b/agent/turn_api_error.py index 305570c8d4..df539e8dce 100644 --- a/agent/turn_api_error.py +++ b/agent/turn_api_error.py @@ -272,8 +272,9 @@ def settle_unrecovered_error( # ``FailoverReason.billing`` (402) is deliberately NOT excluded: pool rotation and # eager fallback already gave up, so retrying only burns paid requests on a depleted # balance. Mirrors 401/403. + is_local_validation_error = _is_local_validation_error(api_error) is_client_error = ( - _is_local_validation_error(api_error) + is_local_validation_error or ( not classified.retryable and not classified.should_compress @@ -304,12 +305,16 @@ def settle_unrecovered_error( ) retry_count = 0 return _verdict("continue") + # ``should_fallback=False`` marks a deterministic failure no other provider can fix (the + # model's own malformed tool-call JSON, #12770): skip the cascade. Local validation errors + # carry no classifier verdict and keep their historical fallback. + fallback_allowed = classified.should_fallback or is_local_validation_error # Announce the fallback only when a chain exists, else "trying fallback..." lies # before a silent abort. - if agent._has_pending_fallback(): + if fallback_allowed and agent._has_pending_fallback(): _label = _NONRETRYABLE_LABELS.get(classified.reason, f"Non-retryable error (HTTP {status_code})") agent._buffer_status(f"⚠️ {_label} — trying fallback...") - if agent._try_activate_fallback(): + if fallback_allowed and agent._try_activate_fallback(): # Direct ``return _verdict("break")`` is load-bearing: the restart handler # re-runs the pre-API preflight against the fallback's context window. active_system_prompt = _arm_fallback_restart(agent, api_messages, active_system_prompt, _retry) diff --git a/tests/agent/test_malformed_tool_args_no_fallback.py b/tests/agent/test_malformed_tool_args_no_fallback.py new file mode 100644 index 0000000000..4cd7b054a8 --- /dev/null +++ b/tests/agent/test_malformed_tool_args_no_fallback.py @@ -0,0 +1,75 @@ +"""A 400 for the model's own malformed tool-call JSON must not walk the fallback chain (#12770).""" +from types import SimpleNamespace +from unittest.mock import patch + +import pytest + +from agent.error_classifier import FailoverReason, classify_api_error +from agent.turn_api_error import settle_unrecovered_error + + +class _Err(Exception): + status_code = 400 + response = None + + def __init__(self, message, body=None): + super().__init__(message) + self.body = body or {"error": {"message": message, "type": "invalid_request_error"}} + + +@pytest.mark.parametrize("wording", ["invalid tool call arguments", "Invalid function_call arguments"]) +def test_malformed_tool_args_400_is_terminal_without_fallback(wording): + """Large session included: the bare-ish message must not read as context overflow either.""" + verdict = classify_api_error(_Err(f"Error code: 400 - {wording}"), provider="ollama", + approx_tokens=90_000, context_length=128_000, num_messages=120) + assert verdict.reason is FailoverReason.format_error + assert (verdict.retryable, verdict.should_compress, verdict.should_fallback) == (False, False, False) + + # Unrelated request-shape 400s keep their fallback (another provider may accept the request). + assert classify_api_error(_Err("Unsupported parameter: 'max_tokens'")).should_fallback is True + + +class _Agent: + """Only the fallback seam is real; every other helper the terminal path touches is a no-op.""" + log_prefix = "" + verbose = False + provider = "ollama" + _fallback_chain = [object()] + _fallback_index = 0 + _credential_pool = None + + def __init__(self): + self.activated = [] + + def _has_pending_fallback(self): + return True + + def _try_activate_fallback(self, **kwargs): + self.activated.append(True) + return True + + def _summarize_api_error(self, error): + return str(error) + + def __getattr__(self, name): + return lambda *args, **kwargs: None + + +def test_client_error_settlement_skips_fallback_when_classifier_says_so(): + agent = _Agent() + retry = SimpleNamespace(copilot_stale_cred_retry_attempted=False, primary_recovery_attempted=False) + err = _Err("invalid tool call arguments") + classified = classify_api_error(err, provider="ollama") + + with patch("agent.conversation_loop._is_copilot_provider", lambda a: False): + verdict = settle_unrecovered_error( + agent, api_error=err, classified=classified, _retry=retry, status_code=400, error_msg=str(err), + is_context_length_error=False, is_rate_limited=False, _is_zai_coding_overload=False, + _provider="ollama", _base="http://127.0.0.1:11434/v1", _model="glm", messages=[], api_messages=[], + api_kwargs={}, active_system_prompt="", conversation_history=None, approx_tokens=10, + retry_count=0, max_retries=3, compression_attempts=0, api_call_count=1, + ) + + assert verdict.action == "return" + assert verdict.result["failure_reason"] == FailoverReason.format_error.value + assert agent.activated == []