From 3caba7bc9b7c87f95a746bdab761c22a28576b20 Mon Sep 17 00:00:00 2001 From: zapabob <1920071390@campus.ouj.ac.jp> Date: Sun, 31 May 2026 22:08:21 +0900 Subject: [PATCH] fix(agent): guard malformed fallback chains --- agent/chat_completion_helpers.py | 38 ++++++++++++++-- agent/conversation_loop.py | 8 ++-- run_agent.py | 18 +++++++- tests/gateway/test_empty_model_recovery.py | 26 +++++++++++ tests/run_agent/test_provider_fallback.py | 53 ++++++++++++++++++++++ 5 files changed, 133 insertions(+), 10 deletions(-) diff --git a/agent/chat_completion_helpers.py b/agent/chat_completion_helpers.py index 0c051cd66bff1..6e746871ca007 100644 --- a/agent/chat_completion_helpers.py +++ b/agent/chat_completion_helpers.py @@ -1020,14 +1020,44 @@ def try_activate_fallback(agent, reason: "FailoverReason | None" = None) -> bool # source of the 429 so the cooldown should not be reset/extended. fallback_already_active = bool(getattr(agent, "_fallback_activated", False)) current_provider = (getattr(agent, "provider", "") or "").strip().lower() - primary_provider = ((agent._primary_runtime or {}).get("provider") or "").strip().lower() + primary_provider = ( + ((getattr(agent, "_primary_runtime", None) or {}).get("provider") or "") + .strip() + .lower() + ) if (not fallback_already_active) or (primary_provider and current_provider == primary_provider): agent._rate_limited_until = time.monotonic() + 60 - if agent._fallback_index >= len(agent._fallback_chain): + + chain = getattr(agent, "_fallback_chain", None) or [] + if not isinstance(chain, (list, tuple)): + logger.warning( + "Fallback unavailable: invalid fallback chain type %s", + type(chain).__name__, + ) + agent._fallback_chain = [] + agent._fallback_index = 0 return False - fb = agent._fallback_chain[agent._fallback_index] - agent._fallback_index += 1 + try: + index = int(getattr(agent, "_fallback_index", 0) or 0) + except (TypeError, ValueError): + index = 0 + if index < 0: + index = 0 + agent._fallback_index = index + + if index >= len(chain): + return False + + fb = chain[index] + agent._fallback_index = index + 1 + if not isinstance(fb, dict): + logger.warning( + "Fallback skip: chain entry %s is not a mapping", + type(fb).__name__, + ) + return agent._try_activate_fallback() + fb_provider = (fb.get("provider") or "").strip().lower() fb_model = (fb.get("model") or "").strip() if not fb_provider or not fb_model: diff --git a/agent/conversation_loop.py b/agent/conversation_loop.py index bb6c6229cdb70..5fc4448d38a73 100644 --- a/agent/conversation_loop.py +++ b/agent/conversation_loop.py @@ -1421,7 +1421,7 @@ def _stop_spinner(): # Eager fallback: empty/malformed responses are a common # rate-limit symptom. Switch to fallback immediately # rather than retrying with extended backoff. - if agent._fallback_index < len(agent._fallback_chain): + if agent._has_pending_fallback(): agent._buffer_status("⚠️ Empty/malformed response — switching to fallback...") if agent._try_activate_fallback(): retry_count = 0 @@ -2689,7 +2689,7 @@ def _stop_spinner(): FailoverReason.rate_limit, FailoverReason.billing, } - if is_rate_limited and agent._fallback_index < len(agent._fallback_chain): + if is_rate_limited and agent._has_pending_fallback(): # Don't eagerly fallback if credential pool rotation may # still recover. See _pool_may_recover_from_rate_limit # for the single-credential-pool and CloudCode-quota @@ -4135,7 +4135,7 @@ def _stop_spinner(): # chain. This covers the case where a model # (e.g. GLM-4.5-Air) consistently returns empty # due to context degradation or provider issues. - if _truly_empty and agent._fallback_chain: + if _truly_empty and agent._has_pending_fallback(): logger.warning( "Empty response after %d retries — " "attempting fallback (model=%s, provider=%s)", @@ -4199,7 +4199,7 @@ def _stop_spinner(): ) agent._emit_status( "❌ Model returned no content after all retries" - + (" and fallback attempts." if agent._fallback_chain else + + (" and fallback attempts." if getattr(agent, "_fallback_chain", None) else ". No fallback providers configured.") ) diff --git a/run_agent.py b/run_agent.py index 18ca748908d04..9f8c79f217b3a 100644 --- a/run_agent.py +++ b/run_agent.py @@ -3597,8 +3597,22 @@ def _has_pending_fallback(self) -> bool: ``try_activate_fallback`` (#35314, #17446). """ chain = getattr(self, "_fallback_chain", None) or [] - index = getattr(self, "_fallback_index", 0) - return index < len(chain) + if not isinstance(chain, (list, tuple)): + return False + try: + index = int(getattr(self, "_fallback_index", 0) or 0) + except (TypeError, ValueError): + index = 0 + if index < 0: + index = 0 + if index >= len(chain): + return False + return any( + isinstance(entry, dict) + and bool((entry.get("provider") or "").strip()) + and bool((entry.get("model") or "").strip()) + for entry in chain[index:] + ) # ── Per-turn primary restoration ───────────────────────────────────── diff --git a/tests/gateway/test_empty_model_recovery.py b/tests/gateway/test_empty_model_recovery.py index 2c4be44793145..eafc8e72a0cb0 100644 --- a/tests/gateway/test_empty_model_recovery.py +++ b/tests/gateway/test_empty_model_recovery.py @@ -145,3 +145,29 @@ def test_has_pending_fallback_missing_attrs(): """Bare agent with no fallback attributes set must default to False, not crash.""" agent = _bare_agent() assert agent._has_pending_fallback() is False + + +def test_has_pending_fallback_invalid_chain_type(): + """Malformed runtime fallback state should be treated as exhausted.""" + agent = _bare_agent() + agent._fallback_chain = object() + agent._fallback_index = 0 + assert agent._has_pending_fallback() is False + + +def test_has_pending_fallback_skips_malformed_entries(): + agent = _bare_agent() + agent._fallback_chain = [ + None, + {"provider": "", "model": "gpt-5"}, + {"provider": "openai", "model": "gpt-5"}, + ] + agent._fallback_index = 0 + assert agent._has_pending_fallback() is True + + +def test_has_pending_fallback_malformed_entries_only(): + agent = _bare_agent() + agent._fallback_chain = [None, {"provider": "openai"}] + agent._fallback_index = 0 + assert agent._has_pending_fallback() is False diff --git a/tests/run_agent/test_provider_fallback.py b/tests/run_agent/test_provider_fallback.py index b179cc341cc50..082b45d599c78 100644 --- a/tests/run_agent/test_provider_fallback.py +++ b/tests/run_agent/test_provider_fallback.py @@ -90,6 +90,59 @@ def test_exhausted_returns_false(self): agent = _make_agent(fallback_model=None) assert agent._try_activate_fallback() is False + def test_none_chain_returns_false_instead_of_type_error(self): + """#35848: runtime-corrupted fallback chain must not mask the primary error.""" + agent = _make_agent(fallback_model=None) + agent._fallback_chain = None + agent._fallback_index = 0 + + assert agent._try_activate_fallback() is False + assert agent._fallback_index == 0 + + def test_malformed_chain_type_returns_false_and_resets(self): + """Truthy non-list chains should not be treated as fallback entries.""" + agent = _make_agent(fallback_model=None) + agent._fallback_chain = {"provider": "openai", "model": "gpt-4o"} + agent._fallback_index = 0 + + with patch("agent.auxiliary_client.resolve_provider_client") as mock_resolve: + assert agent._try_activate_fallback() is False + + assert agent._fallback_chain == [] + assert agent._fallback_index == 0 + mock_resolve.assert_not_called() + + def test_malformed_chain_entry_skips_to_next(self): + """Partially-populated fallback chains should skip bad entries.""" + agent = _make_agent(fallback_model=None) + agent._fallback_chain = [ + None, + {"provider": "openai", "model": "gpt-4o"}, + ] + agent._fallback_index = 0 + + with patch( + "agent.auxiliary_client.resolve_provider_client", + return_value=(_mock_client(), "gpt-4o"), + ): + assert agent._try_activate_fallback() is True + assert agent.model == "gpt-4o" + assert agent._fallback_index == 2 + + def test_has_pending_fallback_handles_none_and_malformed_state(self): + agent = _make_agent(fallback_model=None) + + agent._fallback_chain = None + agent._fallback_index = 0 + assert agent._has_pending_fallback() is False + + agent._fallback_chain = {"provider": "openai", "model": "gpt-4o"} + assert agent._has_pending_fallback() is False + + agent._fallback_chain = [None, {"provider": "openai", "model": "gpt-4o"}] + agent._fallback_index = "not-an-int" + assert agent._has_pending_fallback() is True + def test_advances_index(self): fbs = [ {"provider": "openai", "model": "gpt-4o"},