fix(models): never auto-switch to a provider the user has no credentials for
/model <name> on provider A, where the name is only known to provider B
(static catalog or OpenRouter), switched the session to B even when B had
no key: an immediate 401 for most vendors, and for OpenRouter — whose
runtime resolves with an EMPTY key instead of raising — a silent switch
onto a metered aggregator. The dashboard's flat Model field had two more
copies of the same guess ("vendor/model on a native provider" → openrouter).
detect_provider_for_model() now walks its ladder as candidates and skips any
target without credentials (env/.env key, auth-store login, or a usable
credential pool entry). Exceptions: the user NAMED the provider (/model nous)
or there is no current provider yet ("auto") — then the guess is handed back
so the credential step fails loudly instead of silently ignoring input. A
vendor/ prefix naming a provider declared in `providers:` is a selection, not
a guess, and always routes. The dashboard fallbacks apply the same gate.
Tests that pinned "switch to OpenRouter/vendor with no key" now grant the
credential they assumed; two new invariants cover the gate.
This commit is contained in:
@@ -930,8 +930,16 @@ def _resolve_provider_prefix(model_name: str) -> Optional[tuple[str, str]]:
|
||||
|
||||
def detect_provider_for_model(
|
||||
model_name: str, current_provider: str) -> Optional[tuple[str, str]]:
|
||||
"""Auto-detect the best provider for a model name: static catalogs (bare provider name → its
|
||||
default; direct catalog match), then the OpenRouter catalog, then a configured ``vendor/`` prefix."""
|
||||
"""Auto-detect the best provider for a model name: the current provider's live catalog, static
|
||||
catalogs (bare provider name → its default; direct catalog match), then the OpenRouter catalog,
|
||||
then a configured ``vendor/`` prefix.
|
||||
|
||||
Never hands back a provider the user holds no credentials for: an unauthenticated guess is
|
||||
skipped and the ladder continues (``None`` = stay on the current provider). Exceptions: the user
|
||||
NAMED the provider (``/model nous``), or there is no current provider yet (``auto``) — then the
|
||||
first guess is returned so the credential step fails loudly instead of silently ignoring input."""
|
||||
from hermes_cli.models_detect import current_provider_catalog_match, provider_has_credentials
|
||||
|
||||
name = (model_name or "").strip()
|
||||
if not name:
|
||||
return None
|
||||
@@ -939,28 +947,47 @@ def detect_provider_for_model(
|
||||
# The current provider's LIVE catalog outranks every static guess: a model it already serves
|
||||
# (Codex early-access ids, Portal-only slugs, Ollama Cloud models absent from _PROVIDER_MODELS)
|
||||
# must never re-route the session to another vendor or to metered OpenRouter.
|
||||
from hermes_cli.models_detect import current_provider_catalog_match
|
||||
|
||||
served = current_provider_catalog_match(name, current_provider)
|
||||
if served is not None:
|
||||
return (current_provider, served) if served != name else None
|
||||
|
||||
no_selection = (current_provider or "").strip().lower() in {"", "auto"}
|
||||
for candidate in _detection_candidates(name, current_provider):
|
||||
if candidate is None:
|
||||
return None # the current catalog owns this name
|
||||
if no_selection or candidate[0] == current_provider or provider_has_credentials(candidate[0]):
|
||||
return candidate
|
||||
if _PROVIDER_ALIASES.get(name.lower(), name.lower()) == candidate[0]:
|
||||
return candidate # explicitly named provider: let the credential step report it
|
||||
logger.debug("Skipping auto-switch of '%s' to %s: no credentials configured", name, candidate[0])
|
||||
# A ``vendor/model`` prefix naming a provider the user DECLARED in ``providers:`` is a selection,
|
||||
# not a guess — hand it back even before its key is wired up.
|
||||
return _resolve_provider_prefix(name)
|
||||
|
||||
|
||||
def _detection_candidates(name: str, current_provider: str):
|
||||
"""Yield ``(provider, model)`` guesses in ladder order; ``None`` means the current provider's own
|
||||
catalog owns the name (stop, stay)."""
|
||||
static_match = detect_static_provider_for_model(name, current_provider)
|
||||
if static_match:
|
||||
return static_match
|
||||
yield static_match
|
||||
if _model_in_provider_catalog(name.lower(), _provider_keys(current_provider)):
|
||||
return None
|
||||
yield None
|
||||
return
|
||||
|
||||
# OpenRouter catalog (exact slug, then bare model part).
|
||||
or_slug = _find_openrouter_slug(name)
|
||||
if or_slug:
|
||||
if current_provider != "openrouter" or or_slug != name:
|
||||
return ("openrouter", or_slug)
|
||||
return None # already on openrouter with matching name
|
||||
if current_provider == "openrouter" and or_slug == name:
|
||||
yield None # already on openrouter with matching name
|
||||
return
|
||||
yield ("openrouter", or_slug)
|
||||
|
||||
# Explicit ``vendor/model`` prefix naming a configured provider — AFTER the OpenRouter lookup so
|
||||
# aggregator-native slugs (``deepseek/deepseek-chat``) keep their routing.
|
||||
return _resolve_provider_prefix(name)
|
||||
prefixed = _resolve_provider_prefix(name)
|
||||
if prefixed:
|
||||
yield prefixed
|
||||
|
||||
|
||||
def _find_openrouter_slug(model_name: str) -> Optional[str]:
|
||||
|
||||
@@ -1,10 +1,15 @@
|
||||
"""Live-catalog guard for ``detect_provider_for_model``.
|
||||
"""Live-catalog and credential guards for ``detect_provider_for_model``.
|
||||
|
||||
Split out of ``hermes_cli.models``. The detection ladder there consults static catalogs, then the
|
||||
OpenRouter catalog. Providers whose static list lags their live catalog (Codex accounts with
|
||||
early-access models, Nous Portal, Ollama Cloud) have no static entry to stop the ladder, so a bare
|
||||
name the CURRENT provider already serves fell through to OpenRouter and the session was silently
|
||||
rebuilt on a metered aggregator (#97487, WolframRvnwlf's $100 Astra incident).
|
||||
OpenRouter catalog, and its answer used to be applied blindly. Two guards close the class of
|
||||
"put the user on a provider they never selected":
|
||||
|
||||
* the CURRENT provider's live catalog outranks every static guess (Codex early-access ids, Nous
|
||||
Portal slugs, Ollama Cloud models absent from ``_PROVIDER_MODELS`` — #97487, the $100 Astra
|
||||
incident);
|
||||
* an auto-detected TARGET must be a provider the user has credentials for. Guessing a vendor the
|
||||
user never signed into either 401s or, for OpenRouter (whose runtime resolves with an empty key
|
||||
instead of raising), silently bills a metered aggregator.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
@@ -37,3 +42,29 @@ def current_provider_catalog_match(model_name: str, current_provider: str) -> Op
|
||||
return None
|
||||
return next((mid for mid in catalog if mid.lower() == wanted), None) or next(
|
||||
(mid for mid in catalog if "/" in mid and mid.split("/", 1)[1].lower() == wanted), None)
|
||||
|
||||
|
||||
def provider_has_credentials(provider: str) -> bool:
|
||||
"""Whether *provider* can be switched to without the user typing a key: env/.env key, auth
|
||||
store login, or a usable credential-pool entry. ``custom``/``custom:*`` targets only come out
|
||||
of the ladder when the user declared them in config, so they count as authenticated."""
|
||||
from hermes_cli.auth import get_auth_status, has_usable_secret
|
||||
from hermes_cli.config import get_env_value_prefer_dotenv
|
||||
|
||||
pid = (provider or "").strip().lower()
|
||||
if not pid:
|
||||
return False
|
||||
if pid == "custom" or pid.startswith("custom:"):
|
||||
return True
|
||||
try:
|
||||
if pid == "openrouter" and has_usable_secret(get_env_value_prefer_dotenv("OPENROUTER_API_KEY")):
|
||||
return True
|
||||
status = get_auth_status(pid) or {}
|
||||
if status.get("logged_in") or status.get("configured"):
|
||||
return True
|
||||
from agent.credential_pool import load_pool
|
||||
|
||||
pool = load_pool(pid)
|
||||
return bool(pool.has_credentials() and pool.has_available())
|
||||
except Exception:
|
||||
return False
|
||||
|
||||
@@ -426,7 +426,12 @@ def _normalize_main_model_assignment(provider: str, model: str) -> tuple[str, st
|
||||
canonical = normalize_provider(cur_provider)
|
||||
prov_in = cur_provider
|
||||
else:
|
||||
canonical = prov_in = "openrouter"
|
||||
from hermes_cli.models_detect import provider_has_credentials
|
||||
|
||||
# Only guess OpenRouter when the user actually holds a key for it; otherwise keep the
|
||||
# pair as sent rather than persisting a provider they never selected.
|
||||
if provider_has_credentials("openrouter"):
|
||||
canonical = prov_in = "openrouter"
|
||||
|
||||
if canonical in _KNOWN_PROVIDER_NAMES and not canonical.startswith("custom"):
|
||||
try:
|
||||
@@ -770,11 +775,15 @@ def _infer_provider_on_model_change(model_val: str, prev_provider: str) -> tuple
|
||||
|
||||
if "/" in name:
|
||||
try:
|
||||
from hermes_cli.models_detect import provider_has_credentials
|
||||
|
||||
cur_is_aggregator = normalize_provider(prev_provider) in _AGGREGATOR_PROVIDERS
|
||||
# A vendor slug on a native provider is a guess at an aggregator; never guess one the
|
||||
# user has no key for — that silently writes a metered provider into config.yaml.
|
||||
if not cur_is_aggregator and provider_has_credentials("openrouter"):
|
||||
return "openrouter", name
|
||||
except Exception:
|
||||
cur_is_aggregator = False
|
||||
if not cur_is_aggregator:
|
||||
return "openrouter", name
|
||||
pass
|
||||
return "", name
|
||||
|
||||
|
||||
|
||||
@@ -34,7 +34,15 @@ def test_custom_provider_name_canonicalizes_to_durable_slug():
|
||||
|
||||
|
||||
def test_unknown_vendor_still_uses_aggregator_fallback():
|
||||
assert _normalize({}, "unconfigured-vendor") == (
|
||||
"openrouter",
|
||||
"vendor/model-a",
|
||||
)
|
||||
with patch("hermes_cli.models_detect.provider_has_credentials", lambda p: p == "openrouter"):
|
||||
assert _normalize({}, "unconfigured-vendor") == (
|
||||
"openrouter",
|
||||
"vendor/model-a",
|
||||
)
|
||||
|
||||
|
||||
def test_unknown_vendor_without_openrouter_key_is_not_reassigned():
|
||||
"""No key for the guessed aggregator → keep the pair as sent instead of persisting a provider
|
||||
the user never selected."""
|
||||
with patch("hermes_cli.models_detect.provider_has_credentials", lambda p: False):
|
||||
assert _normalize({}, "unconfigured-vendor") == ("unconfigured-vendor", "vendor/model-a")
|
||||
@@ -63,7 +63,10 @@ class TestVendorPrefixRouting:
|
||||
assert models.detect_provider_for_model("notaprovider/foo-model", "anthropic") is None
|
||||
|
||||
def test_openrouter_slug_still_wins_over_prefix_routing(self, monkeypatch):
|
||||
"""Aggregator-native slugs keep their existing OpenRouter routing."""
|
||||
"""Aggregator-native slugs keep their existing OpenRouter routing (when OpenRouter is
|
||||
authenticated — an unkeyed aggregator is never auto-selected)."""
|
||||
from hermes_cli import models_detect
|
||||
monkeypatch.setattr(models_detect, "provider_has_credentials", lambda p: p == "openrouter")
|
||||
monkeypatch.setattr(
|
||||
models, "_find_openrouter_slug", lambda _name: "deepseek/deepseek-chat"
|
||||
)
|
||||
@@ -72,6 +75,8 @@ class TestVendorPrefixRouting:
|
||||
assert detected == ("openrouter", "deepseek/deepseek-chat")
|
||||
|
||||
def test_bare_model_detection_unchanged(self, monkeypatch):
|
||||
from hermes_cli import models_detect
|
||||
monkeypatch.setattr(models_detect, "provider_has_credentials", lambda p: p == "deepseek")
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda _name: None)
|
||||
detected = models.detect_provider_for_model("deepseek-chat", "anthropic")
|
||||
assert detected == ("deepseek", "deepseek-chat")
|
||||
|
||||
@@ -144,7 +144,7 @@ class TestDetectProviderForModel:
|
||||
with patch(
|
||||
"hermes_cli.models.fetch_openrouter_models",
|
||||
side_effect=AssertionError("network lookup should not run"),
|
||||
):
|
||||
), patch("hermes_cli.models_detect.provider_has_credentials", return_value=True):
|
||||
result = detect_provider_for_model("sonnet", "auto")
|
||||
assert result is not None
|
||||
assert result[0] == "anthropic"
|
||||
|
||||
51
tests/hermes_cli/test_models_detect_credential_gate.py
Normal file
51
tests/hermes_cli/test_models_detect_credential_gate.py
Normal file
@@ -0,0 +1,51 @@
|
||||
"""Auto-detection must never hand the user a provider they hold no credentials for.
|
||||
|
||||
Regression for the "accidental provider" class: ``/model <name>`` on provider A, where the name is
|
||||
only known to provider B (static catalog or OpenRouter), used to switch the session to B even when
|
||||
B had no key — an immediate 401 for most vendors, and for OpenRouter (whose runtime resolves with an
|
||||
empty key instead of raising) a silent switch onto a metered aggregator.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from hermes_cli import models
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def no_live_catalog(monkeypatch):
|
||||
monkeypatch.setattr(models, "cached_provider_model_ids", lambda provider, **_: [])
|
||||
monkeypatch.setattr(models, "_find_openrouter_slug", lambda name: f"vendor/{name}")
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def authed(monkeypatch):
|
||||
"""Pin which providers count as authenticated; everything else has no credentials."""
|
||||
from hermes_cli import models_detect
|
||||
|
||||
granted: set[str] = set()
|
||||
monkeypatch.setattr(models_detect, "provider_has_credentials", lambda p: p in granted)
|
||||
return granted
|
||||
|
||||
|
||||
class TestNoCredentialsNoSwitch:
|
||||
def test_openrouter_only_model_stays_when_no_openrouter_key(self, no_live_catalog, authed):
|
||||
assert models.detect_provider_for_model("some-model-only-openrouter-has", "deepseek") is None
|
||||
|
||||
def test_openrouter_remap_allowed_with_key(self, no_live_catalog, authed):
|
||||
authed.add("openrouter")
|
||||
assert models.detect_provider_for_model("some-model-only-openrouter-has", "deepseek") == (
|
||||
"openrouter", "vendor/some-model-only-openrouter-has")
|
||||
|
||||
def test_static_vendor_match_requires_that_vendors_credentials(self, no_live_catalog, authed, monkeypatch):
|
||||
monkeypatch.setattr(models, "detect_static_provider_for_model", lambda n, c: ("anthropic", n))
|
||||
assert models.detect_provider_for_model("claude-something", "deepseek") is None
|
||||
authed.add("anthropic")
|
||||
assert models.detect_provider_for_model("claude-something", "deepseek") == ("anthropic", "claude-something")
|
||||
|
||||
def test_explicitly_named_provider_is_not_gated(self, no_live_catalog, authed, monkeypatch):
|
||||
"""``/model nous`` names the provider: hand it back so the credential step can prompt/fail
|
||||
loudly instead of silently ignoring the request."""
|
||||
monkeypatch.setattr(models, "detect_static_provider_for_model", lambda n, c: ("nous", "hermes-4-405b"))
|
||||
assert models.detect_provider_for_model("nous", "deepseek") == ("nous", "hermes-4-405b")
|
||||
@@ -35,6 +35,9 @@ class TestCurrentProviderCatalogWins:
|
||||
live_catalog["nous"] = ["zai/glm-5.3-flash"]
|
||||
assert models.detect_provider_for_model("glm-5.3-flash", "nous") == ("nous", "zai/glm-5.3-flash")
|
||||
|
||||
def test_unserved_model_still_walks_the_ladder(self, live_catalog):
|
||||
def test_unserved_model_still_walks_the_ladder(self, live_catalog, monkeypatch):
|
||||
from hermes_cli import models_detect
|
||||
|
||||
monkeypatch.setattr(models_detect, "provider_has_credentials", lambda p: p == "openrouter")
|
||||
live_catalog["nous"] = ["hermes-4-405b"]
|
||||
assert models.detect_provider_for_model("no-such-model", "nous") == ("openrouter", "vendor/no-such-model")
|
||||
|
||||
@@ -3242,6 +3242,7 @@ class TestDenormalizeProviderSwitch:
|
||||
"""ollama-local + a vendor/model slug → switch to openrouter and drop
|
||||
the stale local base_url (the issue's exact repro)."""
|
||||
from hermes_cli.web_server_config import _denormalize_config_from_web
|
||||
from unittest.mock import patch as _patch
|
||||
from hermes_cli.config import save_config
|
||||
|
||||
save_config({
|
||||
@@ -3253,7 +3254,8 @@ class TestDenormalizeProviderSwitch:
|
||||
}
|
||||
})
|
||||
|
||||
result = _denormalize_config_from_web({"model": "google/gemini-2.5-flash"})
|
||||
with _patch("hermes_cli.models_detect.provider_has_credentials", lambda p: p == "openrouter"):
|
||||
result = _denormalize_config_from_web({"model": "google/gemini-2.5-flash"})
|
||||
model = result["model"]
|
||||
assert model["provider"] == "openrouter"
|
||||
assert model["default"] == "google/gemini-2.5-flash"
|
||||
@@ -3265,14 +3267,16 @@ class TestDenormalizeProviderSwitch:
|
||||
"""An explicit context-length override must persist alongside a
|
||||
provider switch."""
|
||||
from hermes_cli.web_server_config import _denormalize_config_from_web
|
||||
from unittest.mock import patch as _patch
|
||||
from hermes_cli.config import save_config
|
||||
|
||||
save_config({"model": {"default": "llama3.2", "provider": "ollama-local"}})
|
||||
|
||||
result = _denormalize_config_from_web({
|
||||
"model": "google/gemini-2.5-flash",
|
||||
"model_context_length": 128000,
|
||||
})
|
||||
with _patch("hermes_cli.models_detect.provider_has_credentials", lambda p: p == "openrouter"):
|
||||
result = _denormalize_config_from_web({
|
||||
"model": "google/gemini-2.5-flash",
|
||||
"model_context_length": 128000,
|
||||
})
|
||||
model = result["model"]
|
||||
assert model["provider"] == "openrouter"
|
||||
assert model["context_length"] == 128000
|
||||
|
||||
Reference in New Issue
Block a user