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>
This commit is contained in:
@@ -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
|
||||
|
||||
@@ -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)
|
||||
|
||||
75
tests/agent/test_malformed_tool_args_no_fallback.py
Normal file
75
tests/agent/test_malformed_tool_args_no_fallback.py
Normal file
@@ -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 == []
|
||||
Reference in New Issue
Block a user