fix(auxiliary): use active runtime in final main-agent fallback - #73284
fix(auxiliary): use active runtime in final main-agent fallback#73284Willhong wants to merge 2 commits into
Conversation
teknium1
left a comment
There was a problem hiding this comment.
Thanks for tracing the sync and async safety-net gap. The core premise is confirmed on current main: agent/auxiliary_client.py:4459-4460 still re-reads static main configuration, while the callers at agent/auxiliary_client.py:8563-8565 and 9159-9161 already hold a normalized runtime snapshot.
Problems
- The proposed configured named-custom path does not actually retain its active endpoint or credential. The added branch keeps
custom:<name>when it finds a configured entry, but current resolution readscustom_baseandcustom_keyfrom that entry atagent/auxiliary_client.py:5525-5529; the explicit runtime values passed by the PR are not consumed there. The config-less test does not cover this configured case.
Suggested changes
- Add a configured named-custom regression where the entry differs from
main_runtime, and make named-custom resolution honor the active runtime endpoint/key while preserving its API-mode handling.
Automated hermes-sweeper review.
| resolution_kwargs = { | ||
| "explicit_base_url": runtime.get("base_url") or None, | ||
| "explicit_api_key": runtime.get("api_key") or None, | ||
| "api_mode": runtime.get("api_mode") or None, |
There was a problem hiding this comment.
When a named custom entry exists, this leaves main_provider as custom:<name>, but resolve_provider_client() takes that branch's endpoint and key from custom_entry (agent/auxiliary_client.py:5525-5529) and ignores these runtime explicit values. Please cover a configured named-provider runtime whose endpoint/key differ from config, and preserve the active values without losing the named entry's API-mode behavior.
|
Thanks — the named-custom gap was real. Pushed What was wrong: when a Fix: the named-custom arm now lets an explicit runtime endpoint/credential override the entry's persisted values, while the entry keeps owning Regressions added —
Verified 1 and 2 fail on the previous commit (both resolve to Test run: |
|
Independent verification: Reproduced the original bug on released v0.20.0, then re-ran the identical end-to-end reproduction against this branch. Setup: On v0.20.0: On this branch (
This is the configured named-custom regression requested in the review above (runtime endpoint/key differ from the static config entry). Verified working here against a live gateway-shaped reproduction. |
…stom fallback Review follow-up: the safety net kept the full custom:<name> provider when a config entry existed, but resolve_provider_client's named-custom arm reads base_url/api_key from that entry, so the explicit runtime values passed by the fallback were dropped. A main conversation that had failed over to a different host or rotated credential retried the stale configured route. The named-custom arm now lets an explicit runtime endpoint/credential override the entry's persisted values while the entry keeps owning api_mode, model default, and header handling. A callable api_key (Azure Entra bearer provider) is passed through unchanged. Adds three real-config regressions under a temp HERMES_HOME: runtime endpoint/key win over a differing entry, api_mode=anthropic_messages still routes through AnthropicAuxiliaryClient at the un-rewritten live URL, and a runtime snapshot without endpoint/key leaves the entry intact.
e50d366 to
24e1028
Compare
|
Rebased onto current The branch had gone
Orthogonal changes, so both are kept: the scoped-secret read resolves the entry's own Rebase rather than a merge commit, so both original commits stay intact on top of Test run on the rebased head — The single failure is The review feedback from @teknium1 was addressed in |
What does this PR do?
The final auxiliary
main-agentsafety net currently re-reads the statically configured main provider/model after an auxiliary route fails. If the conversation has switched or failed over to a different runtime, the safety net can retry the stale provider instead of the provider already serving the session.This PR threads the normalized session-local
main_runtimesnapshot through the final synchronous and asynchronous safety-net calls. A complete active runtime takes priority; static main configuration remains the fallback when no runtime snapshot is available.For
openai-codex, the resolver now preserves the token and base URL selected by the active runtime instead of selecting a new route from the credential pool or auth store.This closes a narrow propagation gap between the layered safety net introduced in #27625 and the session-scoped runtime work merged in #63521.
Related Issue
Fixes #73283
Type of Change
Changes Made
How to Test
main-agentfallback resolves provider B and reuses its active authentication context and endpoint, rather than re-reading provider A from static config.Validation run on current
main:Baseline note: a broader adjacent bundle reports four order-dependent provider-health failures. The same four failures reproduce on a clean current-main worktree, and all four pass in isolation, so they are not introduced by this PR.
Checklist
Code
fix(scope):,feat(scope):, etc.)pytest tests/ -qand all tests passDocumentation & Housekeeping
docs/, docstrings) — or N/Acli-config.yaml.exampleif I added/changed config keys — or N/ACONTRIBUTING.mdorAGENTS.mdif I changed architecture or workflows — or N/AScreenshots / Logs