diff --git a/agent/auxiliary_client.py b/agent/auxiliary_client.py index 9605f4e6c..3e776eeba 100644 --- a/agent/auxiliary_client.py +++ b/agent/auxiliary_client.py @@ -5993,8 +5993,17 @@ def call_llm( # When the provider returns a 429 rate-limit (not billing), fall # back to an alternative provider instead of exhausting retries # against the same rate-limited endpoint. + # + # ── Auth error fallback (#21165) ───────────────────────────── + # When the resolved provider returns 401 and neither the Nous + # refresh path nor explicit provider credential refresh applies, + # fall back to an alternative provider instead of dropping the + # auxiliary task on the floor (silent compression failure / + # message loss). Auth is NOT a capacity error: it only bypasses + # the explicit-provider gate when the user is in auto mode. should_fallback = ( - _is_payment_error(first_err) + _is_auth_error(first_err) + or _is_payment_error(first_err) or _is_connection_error(first_err) or _is_rate_limit_error(first_err) or _is_model_incompatible_error(first_err) @@ -6024,7 +6033,9 @@ def call_llm( or _is_invalid_aux_response_error(first_err) ) if should_fallback and (is_auto or is_capacity_error): - if _is_payment_error(first_err): + if _is_auth_error(first_err): + reason = "auth error" + elif _is_payment_error(first_err): reason = "payment error" # Resolve the actual provider label (resolved_provider may be # "auto"; the client's base_url tells us which backend got the @@ -6473,8 +6484,13 @@ async def async_call_llm( raise # ── Payment / connection / rate-limit fallback (mirrors sync call_llm) ── + # Auth error fallback (#21165): a 401 that survived the refresh path + # falls back in auto mode just like the sync call_llm() path. Auth is + # NOT a capacity error, so on an explicit provider it still respects + # the user's choice (handled by the is_auto/is_capacity_error gate). should_fallback = ( - _is_payment_error(first_err) + _is_auth_error(first_err) + or _is_payment_error(first_err) or _is_connection_error(first_err) or _is_rate_limit_error(first_err) or _is_model_incompatible_error(first_err) @@ -6496,7 +6512,9 @@ async def async_call_llm( or _is_invalid_aux_response_error(first_err) ) if should_fallback and (is_auto or is_capacity_error): - if _is_payment_error(first_err): + if _is_auth_error(first_err): + reason = "auth error" + elif _is_payment_error(first_err): reason = "payment error" _mark_provider_unhealthy( _recoverable_pool_provider(resolved_provider, client) or resolved_provider diff --git a/tests/agent/test_auxiliary_client.py b/tests/agent/test_auxiliary_client.py index 7637b06d9..2100b5dcd 100644 --- a/tests/agent/test_auxiliary_client.py +++ b/tests/agent/test_auxiliary_client.py @@ -1842,6 +1842,62 @@ class TestCallLlmPaymentFallback: # Fallback client should have been used assert fallback_client.chat.completions.create.called + def test_401_auth_error_triggers_fallback_in_auto_mode(self, monkeypatch): + """401 auth errors should trigger fallback in auto mode (#21165). + + When refresh is unavailable/fails and the user is on the auto chain, + a 401 must fall back instead of silently dropping the aux task + (which caused compression message loss). + """ + monkeypatch.setenv("OPENROUTER_API_KEY", "or-key") + + primary_client = MagicMock() + primary_client.base_url = "https://api.minimax.chat/v1" + primary_client.chat.completions.create.side_effect = _AuxAuth401("expired key") + + fallback_client = MagicMock() + fallback_client.chat.completions.create.return_value = _DummyResponse("fallback auth response") + + with patch("agent.auxiliary_client._get_cached_client", + return_value=(primary_client, "minimax/minimax-m2.7")), \ + patch("agent.auxiliary_client._resolve_task_provider_model", + return_value=("auto", "minimax/minimax-m2.7", None, None, None)), \ + patch("agent.auxiliary_client._try_payment_fallback", + return_value=(fallback_client, "fallback-model", "openrouter")) as mock_fb: + result = call_llm( + task="compression", + messages=[{"role": "user", "content": "hello"}], + ) + + assert result.choices[0].message.content == "fallback auth response" + assert fallback_client.chat.completions.create.called + # Labelled as an auth error, not mis-tagged as a connection error. + assert mock_fb.call_args.kwargs.get("reason") == "auth error" + + def test_401_auth_error_no_fallback_with_explicit_provider(self, monkeypatch): + """401 on an explicitly-configured provider must NOT silently switch. + + Auth is not a capacity error: the explicit-provider gate means a 401 + respects the user's choice and raises instead of falling back. This + guards the deliberate design at the should_fallback/is_capacity gate. + """ + primary_client = MagicMock() + primary_client.base_url = "https://api.minimax.chat/v1" + primary_client.chat.completions.create.side_effect = _AuxAuth401("expired key") + + with patch("agent.auxiliary_client._get_cached_client", + return_value=(primary_client, "minimax/minimax-m2.7")), \ + patch("agent.auxiliary_client._resolve_task_provider_model", + return_value=("minimax", "minimax/minimax-m2.7", None, None, None)), \ + patch("agent.auxiliary_client._refresh_provider_credentials", return_value=False), \ + patch("agent.auxiliary_client._try_payment_fallback") as mock_fb: + with pytest.raises(_AuxAuth401): + call_llm( + task="compression", + messages=[{"role": "user", "content": "hello"}], + ) + mock_fb.assert_not_called() + class TestAuxiliaryFallbackLayering: """Explicit-provider users get layered fallback: configured_chain → main agent → warn."""