diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 60d4d7976..0d1f6fa44 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -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. 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). diff --git a/hermes_cli/providers.py b/hermes_cli/providers.py index a19a4584f..f81790aa2 100644 --- a/hermes_cli/providers.py +++ b/hermes_cli/providers.py @@ -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.`` 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 diff --git a/tests/hermes_cli/test_user_providers_model_switch.py b/tests/hermes_cli/test_user_providers_model_switch.py index ec694a39f..dbf495641 100644 --- a/tests/hermes_cli/test_user_providers_model_switch.py +++ b/tests/hermes_cli/test_user_providers_model_switch.py @@ -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):