From a653bb0cbeaaefc1e275b2e3408c3968011d1304 Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 30 Jun 2026 18:59:45 -0700 Subject: [PATCH] refactor(moa): unify slot provider-identity on the single call_llm chokepoint (#55991) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit _slot_runtime maintained a hand-listed name-preservation set ({nous, anthropic, openai-codex, xai-oauth, bedrock}) that returned bare provider+model to avoid call_llm collapsing an explicit base_url to the generic 'custom' route. That duplicated _resolve_task_provider_model's _preserve_provider_with_base_url guard (a provider-catalog capability check) and had to be extended by hand for every provider with custom auth/signing — the exact drift that produced the anthropic (#54609) and bedrock (#54912) 429/ empty-response bugs. Removes the whitelist: _slot_runtime now forwards the resolved base_url/api_key/ api_mode for every slot, and the single chokepoint (_resolve_task_provider_model -> _preserve_provider_with_base_url) decides identity preservation. Behavior is unchanged for the five providers — their provider branches (codex Responses+Cloudflare, xai-oauth, bedrock SigV4, anthropic OAuth Bearer+anthropic-beta, nous Portal tags) re-resolve their own credentials by name and ignore a forwarded base_url/api_key, so forwarding is safe even for bedrock's placeholder 'aws-sdk' key. Verified via real-import E2E: _slot_runtime -> _resolve_task_provider_model preserves openai-codex/xai-oauth/bedrock/anthropic/nous (+openrouter control) — none collapse to custom. Tests updated to assert the pipeline invariant against the real resolver instead of the removed whitelist's bare-return shape. --- agent/moa_loop.py | 42 +++++++------------- tests/run_agent/test_moa_loop_mode.py | 57 +++++++++++++++++++-------- 2 files changed, 56 insertions(+), 43 deletions(-) diff --git a/agent/moa_loop.py b/agent/moa_loop.py index 206c7e50ec4..2e846a51c02 100644 --- a/agent/moa_loop.py +++ b/agent/moa_loop.py @@ -93,33 +93,21 @@ def _slot_runtime(slot: dict[str, str]) -> dict[str, Any]: from hermes_cli.runtime_provider import resolve_runtime_provider rt = resolve_runtime_provider(requested=provider, target_model=model) - resolved_provider = str(rt.get("provider") or provider).strip().lower() - # call_llm treats an explicit base_url as a custom endpoint. That is - # correct for ordinary OpenAI-compatible targets, but wrong for OAuth / - # provider-backed targets whose provider branch adds auth refresh, - # request metadata, or request-shape adapters. Keep those providers - # identified by name. - # ``bedrock`` belongs here too: its provider branch builds an - # AWS-SigV4-signed client (or IAM-role-signed) against the - # bedrock-runtime endpoint. resolve_runtime_provider returns that - # endpoint's base_url plus a PLACEHOLDER api_key ("aws-sdk") — there is - # no real bearer token. Forwarding base_url+api_key makes call_llm treat - # it as a plain OpenAI-compatible endpoint and POST with an unsigned - # fake bearer, which Bedrock answers with an empty/malformed - # ChatCompletion (choices=None). Keeping it identified by name routes it - # through the real signed bedrock branch. - # - # ``anthropic`` likewise: subscription OAuth setup-tokens (sk-ant-oat*) - # require Bearer auth plus the ``anthropic-beta: oauth-*`` header, which - # only the anthropic provider branch adds. Forwarding base_url+api_key - # sends the OAuth token as ``x-api-key``, which Anthropic rejects with a - # bare 429. - if resolved_provider in {"nous", "anthropic", "openai-codex", "xai-oauth", "bedrock"}: - return out - # Pass the resolved endpoint through so call_llm builds the request for - # the provider's actual API surface instead of auto-detecting. base_url - # routes call_llm to the right adapter (incl. anthropic_messages mode); - # api_key is the resolved credential for that provider. + # Forward the resolved endpoint through to call_llm unconditionally. + # call_llm's _resolve_task_provider_model() is the single chokepoint that + # decides whether an explicit base_url collapses a call to the generic + # ``custom`` route or keeps the provider's real identity: it preserves + # identity for any first-class provider (via + # _preserve_provider_with_base_url, a provider-catalog capability check), + # so provider branches that add auth refresh / request metadata / + # request-shape adapters — anthropic OAuth (Bearer + anthropic-beta), + # openai-codex Responses wrapping + Cloudflare headers, xai-oauth, + # bedrock SigV4 signing, nous Portal tags — still fire. Those branches + # re-resolve their own credentials by name and ignore a forwarded + # base_url/api_key, so forwarding is safe even for a placeholder key + # (bedrock's "aws-sdk"). We used to maintain a name-preservation set here + # too; that duplicated the chokepoint and drifted out of sync, so the + # single source of truth now lives in call_llm. if rt.get("base_url"): out["base_url"] = rt["base_url"] if rt.get("api_key"): diff --git a/tests/run_agent/test_moa_loop_mode.py b/tests/run_agent/test_moa_loop_mode.py index ff2f31b7e40..1f64256ec2b 100644 --- a/tests/run_agent/test_moa_loop_mode.py +++ b/tests/run_agent/test_moa_loop_mode.py @@ -177,11 +177,14 @@ def test_moa_slots_routed_through_resolve_runtime_provider(monkeypatch): def test_moa_codex_slot_preserves_provider_identity(monkeypatch): """Codex slots must not become custom chat-completions endpoints. - _resolve_task_provider_model treats any explicit base_url as provider=custom. - For openai-codex that bypasses the Codex auxiliary branch, losing the - Cloudflare headers and Responses adapter required for chatgpt.com/backend-api/codex. + _slot_runtime forwards the resolved base_url/api_key/api_mode; the single + chokepoint that must NOT collapse openai-codex to provider=custom is + _resolve_task_provider_model (via _preserve_provider_with_base_url). If it + collapsed, the Codex auxiliary branch — Cloudflare headers + Responses + adapter for chatgpt.com/backend-api/codex — would be bypassed. """ from agent import moa_loop + from agent.auxiliary_client import _resolve_task_provider_model def fake_resolve(*, requested, target_model=None): return { @@ -196,8 +199,20 @@ def test_moa_codex_slot_preserves_provider_identity(monkeypatch): ) rt = moa_loop._slot_runtime({"provider": "openai-codex", "model": "gpt-5.5"}) + # _slot_runtime forwards the resolved endpoint unconditionally now. + assert rt["provider"] == "openai-codex" + assert rt["model"] == "gpt-5.5" + assert rt["base_url"] == "https://chatgpt.com/backend-api/codex" - assert rt == {"provider": "openai-codex", "model": "gpt-5.5"} + # The chokepoint preserves openai-codex identity despite the explicit + # base_url (api_mode is forwarded to call_llm directly, not the resolver). + resolver_kwargs = {k: v for k, v in rt.items() if k != "api_mode"} + resolved_provider, _model, base_url, _api_key, _mode = _resolve_task_provider_model( + task="moa_reference", + **resolver_kwargs, + ) + assert resolved_provider == "openai-codex" + assert base_url == "https://chatgpt.com/backend-api/codex" @pytest.mark.parametrize("provider", ["minimax-oauth", "qwen-oauth"]) @@ -686,17 +701,17 @@ def test_moa_facade_reruns_references_on_new_turn(monkeypatch, tmp_path): def test_slot_runtime_anthropic_oauth_routes_through_provider_branch(monkeypatch): - """Native anthropic slots must NOT forward base_url/api_key. + """Native anthropic slots must keep their provider identity, not collapse to custom. anthropic OAuth setup-tokens (sk-ant-oat*) require Bearer auth + the - ``anthropic-beta: oauth-*`` header, which only the provider branch of - call_llm adds. If _slot_runtime forwarded base_url/api_key, call_llm would - treat the slot as a plain custom endpoint and send the token as x-api-key, - which Anthropic rejects with a bare 429. So a whitelisted provider - (anthropic) returns only provider/model, while a non-whitelisted provider - (openrouter) forwards the resolved base_url/api_key. + ``anthropic-beta: oauth-*`` header, which only the anthropic provider branch + of call_llm adds. _slot_runtime forwards the resolved base_url/api_key for + every provider now; the single chokepoint that must NOT collapse anthropic + to provider=custom (which would send the token as x-api-key → bare 429) is + _resolve_task_provider_model via _preserve_provider_with_base_url. """ from agent import moa_loop + from agent.auxiliary_client import _resolve_task_provider_model def fake_resolve(*, requested, target_model=None): return { @@ -709,15 +724,25 @@ def test_slot_runtime_anthropic_oauth_routes_through_provider_branch(monkeypatch "hermes_cli.runtime_provider.resolve_runtime_provider", fake_resolve ) - # Whitelisted: anthropic must skip base_url/api_key forwarding. + # _slot_runtime forwards the resolved endpoint for anthropic like any slot. anthropic_rt = moa_loop._slot_runtime( {"provider": "anthropic", "model": "claude-opus-4-8"} ) - assert anthropic_rt == {"provider": "anthropic", "model": "claude-opus-4-8"} - assert "base_url" not in anthropic_rt - assert "api_key" not in anthropic_rt + assert anthropic_rt["provider"] == "anthropic" + assert anthropic_rt["base_url"] == "https://resolved.example/v1" - # Non-whitelisted: openrouter still forwards the resolved endpoint. + # The chokepoint preserves anthropic identity despite the explicit base_url, + # so call_llm routes through the anthropic provider branch (not custom). + resolved_provider, _model, base_url, _api_key, _mode = _resolve_task_provider_model( + task="moa_reference", + provider="anthropic", + model="claude-opus-4-8", + base_url="https://resolved.example/v1", + api_key="resolved-key", + ) + assert resolved_provider == "anthropic" + + # A generic provider (openrouter) is likewise forwarded and preserved. other_rt = moa_loop._slot_runtime( {"provider": "openrouter", "model": "some-model"} )