mirror of
https://github.com/NousResearch/hermes-agent.git
synced 2026-07-08 13:12:08 +00:00
refactor(moa): unify slot provider-identity on the single call_llm chokepoint (#55991)
_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.
This commit is contained in:
parent
0198713c33
commit
a653bb0cbe
2 changed files with 56 additions and 43 deletions
|
|
@ -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"):
|
||||
|
|
|
|||
|
|
@ -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"}
|
||||
)
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue