fix(model-picker): stop routing OpenAI selection to OpenRouter (#37175)
The /model picker emitted a standalone slug=openai row (gated on
OPENAI_API_KEY). Selecting it ran resolve_provider_full("openai"),
which resolved the legacy providers.py alias openai->openrouter BEFORE
checking the user's own providers.openai config — silently switching
users onto OpenRouter (HTTP 401 when they have no OR key).
- model_switch.list_authenticated_providers: skip vendor names that are
aliases to an aggregator (isolates openai->openrouter; copilot/kimi/etc.
are real providers and unaffected). Kills the phantom picker row.
- providers.resolve_provider_full: user-config providers.<name> now wins
over the built-in alias table, so providers.openai (api.openai.com)
beats the alias.
- model_switch PATH A: user-config providers resolve credentials via
their own endpoint instead of the name-based runtime resolver that
doesn't know user-config slugs; plus a fail-loud guard for explicit
unauthed-aggregator hops.
Verified E2E with the reporter's config (no OR key): selecting OpenAI +
gpt-4o-mini now resolves to api.openai.com instead of openrouter.ai.
This commit is contained in:
@ -700,6 +700,48 @@ def switch_model(
|
||||
|
||||
target_provider = pdef.id
|
||||
|
||||
# Guard against silent aggregator hops. A vendor name like bare
|
||||
# "openai" is an alias that resolves to an aggregator ("openrouter").
|
||||
# If the user explicitly asked for that vendor but the aggregator it
|
||||
# routes to has no credentials, do NOT silently switch them onto an
|
||||
# unauthed endpoint (the classic HTTP 401 "Missing Authentication
|
||||
# header"). Point them at the real direct provider instead.
|
||||
from hermes_cli.models import _AGGREGATOR_PROVIDERS as _AGG_PROVIDERS
|
||||
from hermes_cli.providers import ALIASES as _PROVIDER_ALIAS_TABLE
|
||||
_explicit_norm = explicit_provider.strip().lower()
|
||||
_alias_target = _PROVIDER_ALIAS_TABLE.get(_explicit_norm)
|
||||
if (
|
||||
_alias_target
|
||||
and _alias_target == target_provider
|
||||
and target_provider != _explicit_norm
|
||||
and target_provider in _AGG_PROVIDERS
|
||||
):
|
||||
_authed = get_authenticated_provider_slugs(
|
||||
current_provider=current_provider,
|
||||
user_providers=user_providers,
|
||||
custom_providers=custom_providers,
|
||||
)
|
||||
if target_provider not in _authed:
|
||||
_suggestions = [
|
||||
s for s in _authed
|
||||
if s.startswith(_explicit_norm) and s != _explicit_norm
|
||||
]
|
||||
_hint = (
|
||||
f" Did you mean: {', '.join(_suggestions)}?"
|
||||
if _suggestions else ""
|
||||
)
|
||||
return ModelSwitchResult(
|
||||
success=False,
|
||||
target_provider=target_provider,
|
||||
provider_label=pdef.name,
|
||||
is_global=is_global,
|
||||
error_message=(
|
||||
f"Provider '{_explicit_norm}' is an alias that routes "
|
||||
f"through {get_label(target_provider)}, which "
|
||||
f"has no credentials configured.{_hint}"
|
||||
),
|
||||
)
|
||||
|
||||
# If no model specified, try auto-detect from endpoint
|
||||
if not new_model:
|
||||
if pdef.base_url:
|
||||
@ -854,25 +896,62 @@ def switch_model(
|
||||
api_mode = ""
|
||||
|
||||
if provider_changed or explicit_provider:
|
||||
try:
|
||||
runtime = resolve_runtime_provider(
|
||||
requested=target_provider,
|
||||
target_model=new_model,
|
||||
)
|
||||
api_key = runtime.get("api_key", "")
|
||||
base_url = runtime.get("base_url", "")
|
||||
api_mode = runtime.get("api_mode", "")
|
||||
except Exception as e:
|
||||
return ModelSwitchResult(
|
||||
success=False,
|
||||
target_provider=target_provider,
|
||||
provider_label=provider_label,
|
||||
is_global=is_global,
|
||||
error_message=(
|
||||
f"Could not resolve credentials for provider "
|
||||
f"'{provider_label}': {e}"
|
||||
),
|
||||
)
|
||||
import os
|
||||
# User-config providers (providers.<name> in config.yaml) carry their
|
||||
# own base_url + transport + key reference. resolve_runtime_provider()
|
||||
# resolves by provider NAME and doesn't know user-config slugs (e.g. a
|
||||
# block named "openai"), so it would re-resolve from scratch and fail
|
||||
# or hop to an aggregator. Use the pdef's endpoint directly instead.
|
||||
_user_pdef = None
|
||||
if explicit_provider and user_providers:
|
||||
from hermes_cli.providers import resolve_user_provider as _ruser
|
||||
_user_pdef = _ruser(explicit_provider.strip().lower(), user_providers)
|
||||
if _user_pdef is None:
|
||||
_user_pdef = _ruser(target_provider, user_providers)
|
||||
if _user_pdef is not None and _user_pdef.base_url:
|
||||
_ucfg = (user_providers or {}).get(explicit_provider.strip().lower()) \
|
||||
or (user_providers or {}).get(target_provider) or {}
|
||||
_ukey = str(_ucfg.get("api_key", "") or "").strip()
|
||||
if _ukey.startswith("${") and _ukey.endswith("}"):
|
||||
_ukey = os.environ.get(_ukey[2:-1], "").strip()
|
||||
if not _ukey:
|
||||
_kenv = str(_ucfg.get("key_env", "") or "").strip()
|
||||
if _kenv:
|
||||
_ukey = os.environ.get(_kenv, "").strip()
|
||||
try:
|
||||
runtime = resolve_runtime_provider(
|
||||
requested=target_provider,
|
||||
explicit_api_key=_ukey or None,
|
||||
explicit_base_url=_user_pdef.base_url,
|
||||
target_model=new_model,
|
||||
)
|
||||
api_key = runtime.get("api_key", "") or _ukey
|
||||
base_url = runtime.get("base_url", "") or _user_pdef.base_url
|
||||
api_mode = runtime.get("api_mode", "")
|
||||
except Exception:
|
||||
api_key = _ukey
|
||||
base_url = _user_pdef.base_url
|
||||
api_mode = ""
|
||||
else:
|
||||
try:
|
||||
runtime = resolve_runtime_provider(
|
||||
requested=target_provider,
|
||||
target_model=new_model,
|
||||
)
|
||||
api_key = runtime.get("api_key", "")
|
||||
base_url = runtime.get("base_url", "")
|
||||
api_mode = runtime.get("api_mode", "")
|
||||
except Exception as e:
|
||||
return ModelSwitchResult(
|
||||
success=False,
|
||||
target_provider=target_provider,
|
||||
provider_label=provider_label,
|
||||
is_global=is_global,
|
||||
error_message=(
|
||||
f"Could not resolve credentials for provider "
|
||||
f"'{provider_label}': {e}"
|
||||
),
|
||||
)
|
||||
else:
|
||||
try:
|
||||
runtime = resolve_runtime_provider(
|
||||
@ -1195,7 +1274,24 @@ def list_authenticated_providers(
|
||||
curated["lmstudio"] = live
|
||||
|
||||
# --- 1. Check Hermes-mapped providers ---
|
||||
from hermes_cli.models import _AGGREGATOR_PROVIDERS as _AGG_PROVIDERS
|
||||
from hermes_cli.providers import ALIASES as _PROVIDER_ALIAS_TABLE
|
||||
for hermes_id, mdev_id in PROVIDER_TO_MODELS_DEV.items():
|
||||
# Skip vendor names that are merely aliases routing through an
|
||||
# aggregator (e.g. bare "openai" → "openrouter"). These are NOT
|
||||
# directly-routable providers: emitting them as their own picker
|
||||
# row produces a phantom entry that, when selected, resolves via
|
||||
# resolve_provider_full() to the aggregator (OpenRouter) — silently
|
||||
# switching a user off their real provider onto an endpoint they
|
||||
# may have no key for (HTTP 401). The user's real provider (e.g.
|
||||
# openai-api, or a providers.openai config row) covers this vendor.
|
||||
_alias_target = _PROVIDER_ALIAS_TABLE.get(hermes_id)
|
||||
if (
|
||||
_alias_target
|
||||
and _alias_target != hermes_id
|
||||
and _alias_target in _AGG_PROVIDERS
|
||||
):
|
||||
continue
|
||||
# Skip aliases that map to the same models.dev provider (e.g.
|
||||
# kimi-coding and kimi-coding-cn both → kimi-for-coding).
|
||||
# The first one with valid credentials wins (#10526).
|
||||
|
||||
@ -677,6 +677,20 @@ def resolve_provider_full(
|
||||
ProviderDef if found, else None.
|
||||
"""
|
||||
canonical = normalize_provider(name)
|
||||
raw = name.strip().lower()
|
||||
|
||||
# 0. User-defined config providers win over the built-in alias table.
|
||||
# A user who declares ``providers.<name>`` in config.yaml has stated
|
||||
# explicit intent for that name — it must not be hijacked by a legacy
|
||||
# vendor alias (e.g. bare "openai" → "openrouter"). Resolve the raw
|
||||
# name against user config FIRST so a configured ``providers.openai``
|
||||
# (pointing at api.openai.com) beats the alias that would otherwise
|
||||
# silently route to OpenRouter. Only the raw (pre-alias) name is tried
|
||||
# here; canonical/alias resolution still happens below.
|
||||
if user_providers:
|
||||
user_pdef = resolve_user_provider(raw, user_providers)
|
||||
if user_pdef is not None:
|
||||
return user_pdef
|
||||
|
||||
# 1. Built-in (models.dev + overlays)
|
||||
pdef = get_provider(canonical)
|
||||
@ -690,7 +704,7 @@ def resolve_provider_full(
|
||||
if user_pdef is not None:
|
||||
return user_pdef
|
||||
# Try original name (in case alias didn't match)
|
||||
user_pdef = resolve_user_provider(name.strip().lower(), user_providers)
|
||||
user_pdef = resolve_user_provider(raw, user_providers)
|
||||
if user_pdef is not None:
|
||||
return user_pdef
|
||||
|
||||
|
||||
@ -254,8 +254,13 @@ def test_openai_native_curated_catalog_is_non_empty():
|
||||
assert len(_PROVIDER_MODELS["openai"]) >= 4
|
||||
|
||||
|
||||
def test_list_authenticated_providers_openai_built_in_nonzero_total(monkeypatch):
|
||||
"""Built-in openai row must not report total_models=0 when creds exist."""
|
||||
def test_list_authenticated_providers_openai_alias_not_emitted_as_phantom(monkeypatch):
|
||||
"""Bare 'openai' is an alias to the OpenRouter aggregator, NOT a directly-
|
||||
routable provider. It must NOT be emitted as its own picker row: selecting
|
||||
such a row resolves via resolve_provider_full() to OpenRouter, silently
|
||||
switching the user onto an endpoint they may have no key for (HTTP 401).
|
||||
Real OpenAI access comes via 'openai-api' (direct) or a providers.openai
|
||||
config entry — both of which carry api.openai.com. See model-picker bug."""
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-test")
|
||||
monkeypatch.setattr(
|
||||
"agent.models_dev.fetch_models_dev",
|
||||
@ -271,8 +276,63 @@ def test_list_authenticated_providers_openai_built_in_nonzero_total(monkeypatch)
|
||||
max_models=50,
|
||||
)
|
||||
row = next((p for p in providers if p.get("slug") == "openai"), None)
|
||||
assert row is not None
|
||||
assert row["total_models"] > 0
|
||||
assert row is None, (
|
||||
"bare 'openai' alias must not appear as a standalone picker row — "
|
||||
"it routes through OpenRouter and traps users without an OR key"
|
||||
)
|
||||
|
||||
|
||||
def test_resolve_provider_full_user_config_openai_beats_alias():
|
||||
"""A providers.openai config entry must win over the built-in
|
||||
'openai' → 'openrouter' alias. Regression for the model-picker bug
|
||||
where users with provider=openai-api + a providers.openai config block
|
||||
had their OpenAI selection silently routed to OpenRouter (HTTP 401)."""
|
||||
from hermes_cli.providers import resolve_provider_full
|
||||
|
||||
user_providers = {
|
||||
"openai": {
|
||||
"name": "OpenAI-API",
|
||||
"api": "https://api.openai.com/v1",
|
||||
"transport": "codex_responses",
|
||||
"models": {"gpt-5.4-nano": {}},
|
||||
}
|
||||
}
|
||||
pdef = resolve_provider_full("openai", user_providers, [])
|
||||
assert pdef is not None
|
||||
# Must resolve to the user's direct endpoint, NOT the OpenRouter aggregator.
|
||||
assert pdef.id == "openai"
|
||||
assert pdef.source == "user-config"
|
||||
assert pdef.base_url == "https://api.openai.com/v1"
|
||||
assert "openrouter" not in pdef.base_url
|
||||
|
||||
|
||||
def test_switch_model_user_config_openai_does_not_hop_to_openrouter(monkeypatch):
|
||||
"""End-to-end: selecting a providers.openai config row in the picker must
|
||||
resolve to api.openai.com, never silently switch to OpenRouter."""
|
||||
monkeypatch.setenv("CUSTOM_OPENAI_API_KEY", "sk-resolved")
|
||||
user_providers = {
|
||||
"openai": {
|
||||
"name": "OpenAI-API",
|
||||
"api": "https://api.openai.com/v1",
|
||||
"api_key": "${CUSTOM_OPENAI_API_KEY}",
|
||||
"transport": "codex_responses",
|
||||
"models": {"gpt-5.4-nano": {}, "gpt-4o-mini": {}},
|
||||
}
|
||||
}
|
||||
result = switch_model(
|
||||
raw_input="gpt-4o-mini",
|
||||
current_provider="openai-api",
|
||||
current_model="gpt-5.4-nano",
|
||||
current_base_url="https://api.openai.com/v1",
|
||||
current_api_key="sk-test",
|
||||
explicit_provider="openai",
|
||||
user_providers=user_providers,
|
||||
custom_providers=[],
|
||||
)
|
||||
assert result.success, result.error_message
|
||||
assert result.target_provider != "openrouter"
|
||||
assert "openrouter" not in (result.base_url or "")
|
||||
assert result.base_url == "https://api.openai.com/v1"
|
||||
|
||||
|
||||
def test_list_authenticated_providers_user_openai_official_url_fallback(monkeypatch):
|
||||
|
||||
Reference in New Issue
Block a user