-
Notifications
You must be signed in to change notification settings - Fork 0
fix(agent): forward fallback_providers[].extra_body into the active request path (#26460) #9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,229 @@ | ||
| """Tests that fallback_providers[].extra_body is honoured during fallback. | ||
|
|
||
| Regression tests for issue #26460: OpenRouter-specific routing metadata | ||
| (provider.order, allow_fallbacks, etc.) configured under a fallback entry's | ||
| extra_body was silently dropped when the fallback was activated, because | ||
| _prefs assembly only read from agent-level attributes, not the active | ||
| fallback config. | ||
| """ | ||
|
|
||
| from unittest.mock import MagicMock, patch | ||
|
|
||
| from run_agent import AIAgent | ||
|
|
||
|
|
||
| def _make_agent_with_fallback(fallback_providers): | ||
| with ( | ||
| patch("run_agent.get_tool_definitions", return_value=[]), | ||
| patch("run_agent.check_toolset_requirements", return_value={}), | ||
| patch("run_agent.OpenAI"), | ||
| ): | ||
| agent = AIAgent( | ||
| api_key="primary-key", | ||
| base_url="https://openrouter.ai/api/v1", | ||
| quiet_mode=True, | ||
| skip_context_files=True, | ||
| skip_memory=True, | ||
| fallback_model=fallback_providers, | ||
| ) | ||
| agent.client = MagicMock() | ||
| agent.client.base_url = "https://openrouter.ai/api/v1" | ||
| return agent | ||
|
|
||
|
|
||
| def _mock_fb_client(base_url="https://openrouter.ai/api/v1", api_key="fb-key"): | ||
| m = MagicMock() | ||
| m.base_url = base_url | ||
| m.api_key = api_key | ||
| return m | ||
|
|
||
|
|
||
| # ── extra_body stored on activation ────────────────────────────────────── | ||
|
|
||
|
|
||
| class TestFallbackExtraBodyStorage: | ||
| def test_extra_body_stored_on_activation(self): | ||
| """_fallback_extra_body must be set to the entry's extra_body dict.""" | ||
| extra_body = { | ||
| "provider": { | ||
| "order": ["baidu/fp8", "gmicloud/fp8"], | ||
| "allow_fallbacks": False, | ||
| } | ||
| } | ||
| fb_entry = { | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": extra_body, | ||
| } | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| fb_client = _mock_fb_client() | ||
|
|
||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| activated = agent._try_activate_fallback() | ||
|
|
||
| assert activated | ||
| assert agent._fallback_extra_body == extra_body | ||
|
|
||
| def test_no_extra_body_stores_none(self): | ||
| """Entry without extra_body must set _fallback_extra_body to None.""" | ||
| fb_entry = {"provider": "openrouter", "model": "z-ai/glm-5.1"} | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| fb_client = _mock_fb_client() | ||
|
|
||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| activated = agent._try_activate_fallback() | ||
|
|
||
| assert activated | ||
| assert agent._fallback_extra_body is None | ||
|
|
||
| def test_empty_extra_body_stores_none(self): | ||
| """An empty dict extra_body should not pollute _prefs — stored as None.""" | ||
| fb_entry = { | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": {}, | ||
| } | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| fb_client = _mock_fb_client() | ||
|
|
||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| agent._try_activate_fallback() | ||
|
|
||
| assert agent._fallback_extra_body is None | ||
|
|
||
|
|
||
| # ── extra_body forwarded into provider_preferences ────────────────────── | ||
|
|
||
|
|
||
| class TestFallbackExtraBodyForwarding: | ||
| def _activate_with_extra_body(self, extra_body): | ||
| fb_entry = { | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": extra_body, | ||
| } | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| fb_client = _mock_fb_client() | ||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| agent._try_activate_fallback() | ||
| return agent | ||
|
|
||
| def test_provider_order_forwarded_to_prefs(self): | ||
| """provider.order from extra_body must appear in _prefs after activation.""" | ||
| from agent.chat_completion_helpers import _build_api_kwargs_for_openai | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🟢 Unused import in fallback extra_body test (suggestion) In 💡 Suggestion: Remove the unused import: delete line 125 📋 Prompt for AI AgentsIn tests/run_agent/test_26460_fallback_extra_body.py on line 125, delete the line |
||
|
|
||
| extra_body = { | ||
| "provider": { | ||
| "order": ["baidu/fp8", "gmicloud/fp8"], | ||
| "allow_fallbacks": False, | ||
| } | ||
| } | ||
| agent = self._activate_with_extra_body(extra_body) | ||
|
|
||
| # Read _prefs the same way the build path does | ||
| _prefs = {} | ||
| from agent.chat_completion_helpers import _validated_openrouter_provider_sort | ||
| if agent.providers_allowed: | ||
| _prefs["only"] = agent.providers_allowed | ||
| if agent.providers_ignored: | ||
| _prefs["ignore"] = agent.providers_ignored | ||
| if agent.providers_order: | ||
| _prefs["order"] = agent.providers_order | ||
|
|
||
| _fb_extra_body = getattr(agent, "_fallback_extra_body", None) or {} | ||
| _fb_provider_prefs = _fb_extra_body.get("provider") if isinstance(_fb_extra_body, dict) else None | ||
| if _fb_provider_prefs and isinstance(_fb_provider_prefs, dict): | ||
| _prefs.update(_fb_provider_prefs) | ||
|
|
||
| assert _prefs.get("order") == ["baidu/fp8", "gmicloud/fp8"] | ||
| assert _prefs.get("allow_fallbacks") is False | ||
|
|
||
| def test_fallback_prefs_override_global_order(self): | ||
| """Fallback-local provider.order takes precedence over global providers_order.""" | ||
| fb_entry = { | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": { | ||
| "provider": {"order": ["fallback-gpu/fp8"]} | ||
| }, | ||
| } | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| # Simulate a global providers_order set from primary config | ||
| agent.providers_order = ["primary-gpu/bf16"] | ||
| fb_client = _mock_fb_client() | ||
|
|
||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| agent._try_activate_fallback() | ||
|
|
||
| _prefs = {"order": agent.providers_order} | ||
| _fb_extra_body = getattr(agent, "_fallback_extra_body", None) or {} | ||
| _fb_pp = _fb_extra_body.get("provider") if isinstance(_fb_extra_body, dict) else None | ||
| if _fb_pp and isinstance(_fb_pp, dict): | ||
| _prefs.update(_fb_pp) | ||
|
|
||
| # Fallback-local order should win | ||
| assert _prefs["order"] == ["fallback-gpu/fp8"] | ||
|
|
||
|
|
||
| # ── extra_body cleared on restore ──────────────────────────────────────── | ||
|
|
||
|
|
||
| class TestFallbackExtraBodyClearing: | ||
| def test_extra_body_cleared_on_restore(self): | ||
| """_fallback_extra_body must be None after restore_primary_runtime.""" | ||
| from agent.agent_runtime_helpers import restore_primary_runtime | ||
|
|
||
| fb_entry = { | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": {"provider": {"order": ["baidu/fp8"]}}, | ||
| } | ||
| agent = _make_agent_with_fallback([fb_entry]) | ||
| fb_client = _mock_fb_client() | ||
|
|
||
| with patch( | ||
| "agent.chat_completion_helpers.resolve_provider_client", | ||
| return_value=(fb_client, "z-ai/glm-5.1"), | ||
| ): | ||
| agent._try_activate_fallback() | ||
|
|
||
| assert agent._fallback_extra_body is not None | ||
|
|
||
| # Simulate restore — set up minimal _primary_runtime | ||
| agent._primary_runtime = { | ||
| "model": "primary-model", | ||
| "provider": "openrouter", | ||
| "api_key": "primary-key", | ||
| "base_url": "https://openrouter.ai/api/v1", | ||
| "api_mode": "chat_completions", | ||
| } | ||
|
|
||
| with patch("agent.agent_runtime_helpers.resolve_provider_client", | ||
| return_value=(MagicMock(base_url="https://openrouter.ai/api/v1", | ||
| api_key="primary-key"), "primary-model")): | ||
| try: | ||
| restore_primary_runtime(agent) | ||
| except Exception: | ||
| pass # restore may fail in minimal test env; we only need side-effects | ||
|
|
||
| assert getattr(agent, "_fallback_extra_body", None) is None | ||
|
|
||
| def test_extra_body_none_before_any_activation(self): | ||
| """_fallback_extra_body should be absent or None on a fresh agent.""" | ||
| agent = _make_agent_with_fallback([]) | ||
| assert getattr(agent, "_fallback_extra_body", None) is None | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟠 Primary request_overrides.extra_body.provider silently overwrites fallback routing in transport layer (bug)
When a primary provider is configured via
custom_providerswithextra_body.providerrouting (e.g. OpenRouterprovider.order/allow_fallbacks),agent.request_overrides["extra_body"]carries that routing into the fallback. The transport layer's profile path (agent/transports/chat_completions.py:543-570) assemblesextra_bodyfrom the fallback'sprovider_preferences(which correctly includes_fallback_extra_body.providervia the PR's new merge atchat_completion_helpers.py:749-752) — but then appliesrequest_overridesafterwards at line 564-570, doing a shallowextra_body.update(v)that overwrites the fallback's routing with the primary's. Sincetry_activate_fallback(chat_completion_helpers.py:1277-1291) never clears or scopes downagent.request_overrides, the primary's routing silently persists and defeats the fallback's carefully configured provider directives.💡 Suggestion: During fallback activation in
try_activate_fallback()(chat_completion_helpers.py:~1291), clear or scope downagent.request_overridesto removeextra_body— or at minimum drop theproviderkey fromrequest_overrides['extra_body']— so the primary's provider routing does not overwrite the fallback's. Alternatively, applyrequest_overridesbeforeprovider_preferencesin the transport so fallback-specific routing always takes precedence.📋 Prompt for AI Agents
In agent/chat_completion_helpers.py, after line 1291 where
_fallback_extra_bodyis set, add code to neutralize the primary'srequest_overrides.extra_bodyso it doesn't overwrite fallback routing. The cleanest approach: ifagent.request_overridescontainsextra_body, either clearrequest_overrides['extra_body']entirely or pop theproviderkey from it. This ensures the transport layer in agent/transports/chat_completions.py:568 doesn't silently overwrite the fallback's provider_preferences routing with the primary's stale routing directives.