fix(agent): forward fallback_providers[].extra_body during fallback activation (#26460) - #26483
Closed
briandevans wants to merge 2 commits into
Closed
Conversation
…ctivation (NousResearch#26460) `fallback_providers` entries can carry request-scoped metadata such as OpenRouter `extra_body.provider.{order, allow_fallbacks}` for per-route provider pinning, but `_try_activate_fallback()` only consumed `provider`, `model`, `base_url`, `api_key`, and `key_env` / `api_key_env`. Any `extra_body` block on the fallback entry was silently dropped, so users could not express "main route untouched, fallback route pinned to a specific OpenRouter provider order" without using the global `provider_routing` knob — which then leaked into every other OpenRouter request in the session. Fix: when activating a fallback entry, merge `fb["extra_body"]` into `self.request_overrides["extra_body"]`. The chat_completions transport already forwards `request_overrides["extra_body"]` into the outbound request body, so no transport-side change is needed. To keep this strictly scoped to the active fallback route, snapshot `request_overrides` in `_primary_runtime` at init / `switch_model` and restore it in `_restore_primary_runtime()` — the next turn's primary restoration drops the fallback's `extra_body` automatically. Invariants protected: - Fallback `extra_body` is forwarded into the request on the very next call (no transport-side rebuild needed). - Activating a fallback never mutates the agent-level `provider_routing` knobs (`providers_allowed`, `providers_ignored`, `providers_order`, `provider_sort`). - Pre-existing user `request_overrides["extra_body"]` is preserved on merge; fallback wins on key collision. - Non-dict `extra_body` is ignored, not crashed. - `_restore_primary_runtime()` resets `request_overrides` from the snapshot, so the override never leaks past the active fallback turn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Forwards a fallback chain entry's extra_body (e.g. OpenRouter provider.order) into request_overrides when activating that fallback, so per-fallback routing reaches the transport without mutating global provider routing knobs. Adds snapshot/restore for request_overrides so the override is cleared when the primary route is restored.
Changes:
- Snapshot
request_overridesinto_primary_runtimeat init and inswitch_model. - Merge fallback entry
extra_bodyintorequest_overridesin_try_activate_fallback. - Restore
request_overridesfrom snapshot in_restore_primary_runtime.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| run_agent.py | Snapshot/restore request_overrides and merge fallback-entry extra_body on activation. |
| tests/run_agent/test_provider_fallback.py | New test class covering forwarding, isolation, merge, restoration, and snapshot semantics. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+8866
to
+8871
| existing_eb = base_overrides.get("extra_body") | ||
| merged_eb = ( | ||
| {**existing_eb, **fb_extra_body} | ||
| if isinstance(existing_eb, dict) | ||
| else dict(fb_extra_body) | ||
| ) |
Comment on lines
+411
to
+418
| def test_invalid_extra_body_type_is_ignored(self): | ||
| """Defensive: a non-dict extra_body in the chain entry must not | ||
| crash activation or mutate request_overrides.""" | ||
| fbs = [{ | ||
| "provider": "openrouter", | ||
| "model": "z-ai/glm-5.1", | ||
| "extra_body": "not-a-dict", # invalid shape | ||
| }] |
Comment on lines
+2488
to
+2492
| "request_overrides": ( | ||
| dict(getattr(self, "request_overrides", None) or {}) | ||
| if isinstance(getattr(self, "request_overrides", None), dict) | ||
| else {} | ||
| ), |
Comment on lines
+9001
to
+9003
| # extra_body.provider routing). Snapshot may be absent on | ||
| # older sessions saved before #26460. | ||
| self.request_overrides = dict(rt.get("request_overrides", {})) |
alt-glitch
added
type/bug
Addresses four findings from the Copilot review on NousResearch#26483: 1. **Deep-merge nested extra_body keys** — `{**existing, **fb_extra_body}` was a shallow merge, so a fallback entry's `extra_body["provider"] = {"order": [...], "allow_fallbacks": False}` clobbered any pre-existing nested keys (e.g. OpenRouter `provider.require_parameters`, `provider.data_collection`). Added a `_deep_merge_extra_body` helper that merges one level deep — fallback wins on leaf collisions, primary wins on absent leaves. Documented semantics in the call-site comment. 2. **Test coverage for `extra_body: {}`** — the `and fb_extra_body` truthiness check correctly treats an explicit empty dict the same as an absent key, but that path was untested. Added `test_empty_extra_body_dict_does_not_inject_key` to lock in the behavior so a future cleanup of the truthiness guard doesn't silently inject an empty `extra_body` override. 3. **Deduplicate snapshot expression** — the 4-line `dict(getattr(...) or {}) if isinstance(...) else {}` pattern was verbatim in `__init__` and `switch_model`, plus a similar guarded copy in `_try_activate_fallback`. Extracted into a `_snapshot_request_overrides` staticmethod on `AIAgent` and adopted at all three sites. 4. **Older-session restore back-compat** — when restoring from a snapshot that lacks `request_overrides` (sessions persisted before NousResearch#26460), the old code unconditionally overwrote `self.request_overrides` with `{}`, silently dropping any overrides the user set after the snapshot was captured. Now guarded by `if "request_overrides" in rt:`, so older snapshots leave the current value untouched. Added `test_restore_from_older_snapshot_preserves_current_overrides` to cover this. Plus a deep-merge regression test (`test_fallback_extra_body_deep_merges_nested_provider_dict`) that exercises the original Copilot scenario: primary `extra_body.provider.require_parameters=True` survives activation of a fallback entry that adds `provider.order` / `allow_fallbacks`. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
@copilot All four findings addressed in commit d0214c3:
Plus a deep-merge regression test ( |
Contributor
Author
|
CI audit — all 7 test failures are pre-existing baselines on clean
All seven reproduce on clean |
Contributor
Author
|
Closing to keep the queue clean — branch is several hundred commits behind |
This was referenced Jul 27, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fallback_providersentries can carryextra_body(e.g. OpenRouterprovider.{order, allow_fallbacks}for per-route provider pinning) but the activation path silently dropped it. Forward it intorequest_overridesso the existing chat_completions transport merge picks it up on the very next request.request_overridesin_primary_runtime(init +switch_model) and restore it in_restore_primary_runtime()so the override is strictly scoped to the active fallback route — no leak into unrelated requests, no leak past the next turn.Fixes #26460.
The bug
_try_activate_fallback()inrun_agent.pyreadsprovider,model,base_url,api_key, andkey_env/api_key_envoff the chosen fallback entry. Anything else on that entry — including a request-scopedextra_bodyblock — is dropped on the floor. The reporter's case (validated against live OpenRouter):The fallback activates and Hermes routes through OpenRouter, but the
provider.order/allow_fallbacksbody block never lands in the request — so OpenRouter free-routes to whatever upstream is cheapest. The only existing way to pin providers (the agent-levelprovider_routingconfig) is too broad: it injects the sameproviderblock into every OpenRouter request in the session, distorting routing for unrelated models picked up later via/model.The fallback-suffix workaround (
z-ai/glm-5.1:baidu/fp8) is accepted by OpenRouter but the issue's live testing shows it does not actually pin the serving provider.The fix
agent/transports/chat_completions.pyalready mergesrequest_overrides["extra_body"]into the outboundextra_body(lines 387-389 / 496-505). The fix is to wire the fallback entry'sextra_bodyintorequest_overridesat activation time:_try_activate_fallback): after the fallback swap succeeds, mergefb.get("extra_body")intoself.request_overrides["extra_body"]. Pre-existing userextra_bodykeys are preserved; the fallback wins on key collision. Non-dictextra_bodyis ignored._primary_runtimeat init +switch_model): capturerequest_overridesso a later restoration knows the baseline._restore_primary_runtime): resetrequest_overridesfrom the snapshot so the fallback override is dropped the moment the primary route comes back.The agent-level
provider_routingknobs (providers_allowed,providers_ignored,providers_order,provider_sort) are intentionally not touched — they remain global, the fallback override is request-scoped.Test plan
tests/run_agent/test_provider_fallback.py::TestFallbackEntryExtraBodyForwarded):test_fallback_extra_body_forwarded_to_request_overrides— happy path: provider order + allow_fallbacks lands inrequest_overrides["extra_body"].test_global_provider_routing_unchanged— invariant: agent-levelprovider_routingknobs identical before/after activation.test_fallback_without_extra_body_does_not_inject_key— entry withoutextra_bodydoes not add the key.test_fallback_extra_body_merges_with_existing_extra_body— pre-existing userextra_bodypreserved; fallback wins on collision.test_invalid_extra_body_type_is_ignored— defensive: non-dictextra_bodydoes not crash activation.test_restore_primary_runtime_clears_fallback_extra_body— invariant: after restore, override is gone.test_primary_runtime_snapshot_includes_request_overrides— invariant: snapshot has the restoration anchor.tests/run_agent/test_provider_fallback.py(29),test_primary_runtime_restore.py,test_fallback_model.py,test_compressor_fallback_update.py,test_switch_model_fallback_prune.py,test_anthropic_third_party_oauth_guard.py— 104 passed._try_activate_fallbackmakestest_fallback_extra_body_forwarded_to_request_overridesfail (request_overrideshas noextra_bodykey after activation); reapplying it passes.Contract protected
extra_bodyis forwarded on activation.extra_body.provider.orderproduced noproviderblock in outbound request.extra_bodydoes not inject the key (test).provider_routingknobs.extra_bodywas dropped — the new behavior must continue to leave them alone.extra_bodysurvives the merge.request_overrides["extra_body"]["some_field"]— must not be clobbered by fallback merge.test_fallback_extra_body_merges_with_existing_extra_body._restore_primary_runtime()clears the fallback'sextra_body.request_overridessnapshot, the fallback's override would persist into the primary turn.test_restore_primary_runtime_clears_fallback_extra_body.extra_bodydoes not crash activation.extra_body: not-a-dict.test_invalid_extra_body_type_is_ignored.Related
_try_activate_fallback/_restore_primary_runtime/_primary_runtimemachinery as fix(agent): clear stale config context_length on model switch #22387 (config_context_length clear), [Bug]: Local claude-cli custom provider timeout is reported as Empty response and fallback loops #22548 (skip-self dedup), [Feature]: 遇到http 529错误的时候，应该尝试切换成fallback的模型 #11314 (pool-rotation gating). All adjacent tests still green.