From 33797073bb00df357728cd829b7e4dba8b66ddfc Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Tue, 1 Sep 2026 11:27:13 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20harden=20startup=20route=20salvage=20?= =?UTF-8?q?=E2=80=94=20aggregator-slug=20guard,=20alias=20credential=20own?= =?UTF-8?q?ership,=20oneshot=20dedup?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Follow-ups on top of #87210 (@liuhao1024) and #87246 (@JoaoMarcos44): - resolve_startup_model_route: aggregator-native slugs stay on the current routing aggregator (bare vendor slugs resolve WITHIN the aggregator first); URL-bearing aliases resolve via direct_alias_runtime_request so a foreign provider label never carries the vendor token to the alias host (#28660); route carries the alias's own api_key. - cli.py: pass current_provider; explicit --api-key wins over alias key. - Drop #87246's oneshot double-handling (main's oneshot alias+detection path already covers it once #87210's detection fix is in) and the PR-body SVG. - Rewrote/extended startup-route tests for the hardened semantics. --- cli.py | 13 ++- docs/assets/model-routing-87189.svg | 61 ------------- hermes_cli/model_switch.py | 45 +++++++++- hermes_cli/oneshot.py | 24 ------ .../test_startup_model_routing_87189.py | 86 ++++++++++++++++++- 5 files changed, 141 insertions(+), 88 deletions(-) delete mode 100644 docs/assets/model-routing-87189.svg diff --git a/cli.py b/cli.py index 059c838508..f60b2303e3 100644 --- a/cli.py +++ b/cli.py @@ -5366,12 +5366,20 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): self.model = model or _config_model or _DEFAULT_CONFIG_MODEL _startup_provider_override = "" _startup_base_url_override = "" + _startup_api_key_override = "" if self.model: from hermes_cli.model_switch import resolve_startup_model_route _startup_route = resolve_startup_model_route( self.model, explicit_provider=provider or "", + current_provider=( + provider + or _nested_provider + or CLI_CONFIG["model"].get("provider") + or os.getenv("HERMES_INFERENCE_PROVIDER") + or "" + ), user_providers=CLI_CONFIG.get("providers"), custom_providers=CLI_CONFIG.get("custom_providers"), ) @@ -5379,6 +5387,7 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): self.model = _startup_route.model _startup_provider_override = _startup_route.provider _startup_base_url_override = _startup_route.base_url + _startup_api_key_override = _startup_route.api_key # A ``moa:`` model string selects the MoA virtual provider in # one shot (parity with interactive ``/moa`` and the model picker). Do # this before provider resolution so ``-Q -m moa:`` routes @@ -5415,7 +5424,9 @@ class HermesCLI(CLIAgentSetupMixin, CLICommandsMixin, CLIBillingMixin): not _config_model or _config_model == _DEFAULT_CONFIG_MODEL ) - self._explicit_api_key = api_key + # An explicit --api-key wins; otherwise a URL-bearing startup alias + # carries its own credential for the alias host (#28660). + self._explicit_api_key = api_key or _startup_api_key_override or None self._explicit_base_url = base_url # Provider selection is resolved lazily at use-time via _ensure_runtime_credentials(). diff --git a/docs/assets/model-routing-87189.svg b/docs/assets/model-routing-87189.svg deleted file mode 100644 index defb89853c..0000000000 --- a/docs/assets/model-routing-87189.svg +++ /dev/null @@ -1,61 +0,0 @@ - - Hermes model startup routing fix - A diagram showing aliases and provider/model inputs converging at startup routing before runtime provider resolution. - - - - - - - - - - - - Issue #87189 · startup model routing - Resolve the requested route before the configured default provider can take ownership. - - - INPUT - --model nous/deepseek-v4-pro - or a configured model alias - - - FIXED CHOKE POINT - resolve_startup_model_route - alias → model + provider + base_url - prefix → configured provider only - - - RUNTIME - provider = nous - model = deepseek-v4-pro - - - - - - BEFORE - model = nous/deepseek-v4-pro - provider = anthropic - - - WRONG FALLBACK - configured default wins - requested provider never participates - route reaches the wrong endpoint - - - FAILURE - api.anthropic.com - HTTP 404: model not found - - - - - - - Regression coverage: CLI startup · oneshot · aliases · configured provider/model prefixes · aggregator namespace preservation - diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 37521867b1..e8bd679c8f 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -768,12 +768,14 @@ class StartupModelRoute(NamedTuple): model: str provider: str = "" base_url: str = "" + api_key: str = "" def resolve_startup_model_route( raw_model: str, *, explicit_provider: str = "", + current_provider: str = "", user_providers: Optional[dict] = None, custom_providers: Optional[list] = None, ) -> Optional[StartupModelRoute]: @@ -785,6 +787,13 @@ def resolve_startup_model_route( provider to an explicitly requested model. Provider/model strings are consumed only for providers present in user configuration; aggregator namespaces remain untouched. + + ``current_provider`` is the provider the session would otherwise use + (config ``model.provider`` / ``--provider``). When it is a routing + aggregator and the raw string is an aggregator-native slug + (``anthropic/claude-opus-4.6`` on OpenRouter), the input stays on the + aggregator — bare vendor slugs resolve WITHIN the aggregator first and a + ``providers:`` block for the same vendor must not steal the route. """ raw = str(raw_model or "").strip() if not raw: @@ -793,10 +802,26 @@ def resolve_startup_model_route( _ensure_direct_aliases() direct = DIRECT_ALIASES.get(raw.lower()) if direct is not None: + if explicit_provider: + # An explicit --provider wins over the alias's own label; the + # alias contributes model/base_url only. + return StartupModelRoute( + model=direct.model, + provider=explicit_provider, + base_url=direct.base_url, + ) + # Resolve through the SAME owner the interactive /model and oneshot + # paths use: a URL-bearing alias must resolve its credential for the + # alias HOST, never for its provider label — a label like + # ``anthropic`` on a foreign URL would otherwise reach that + # provider's explicit-runtime branch and put the live vendor token + # on the foreign wire (#28660). + alias_provider, alias_key = direct_alias_runtime_request(direct) return StartupModelRoute( model=direct.model, - provider=(explicit_provider or direct.provider), + provider=alias_provider, base_url=direct.base_url, + api_key=alias_key or "", ) if explicit_provider or "/" not in raw: @@ -805,6 +830,24 @@ def resolve_startup_model_route( if not prefix or not model: return None + # Aggregator-native slugs stay on the aggregator. A user on OpenRouter + # whose config also has a ``providers.anthropic`` block must NOT have + # ``anthropic/claude-opus-4.6`` silently rerouted to native Anthropic. + if current_provider: + try: + from hermes_cli.providers import ( + is_routing_aggregator as _is_routing_agg, + normalize_provider as _norm_prov, + ) + + if _is_routing_agg(_norm_prov(current_provider)): + from hermes_cli.models import _find_openrouter_slug + + if _find_openrouter_slug(raw): + return None + except Exception: + pass + configured = { str(name).strip().lower() for name in (user_providers or {}) diff --git a/hermes_cli/oneshot.py b/hermes_cli/oneshot.py index 481ec03573..e2778d67d7 100644 --- a/hermes_cli/oneshot.py +++ b/hermes_cli/oneshot.py @@ -406,20 +406,6 @@ def _run_agent( # path and the configured provider is already correct). explicit_model = (model or "").strip() or env_model if explicit_model: - from hermes_cli.model_switch import resolve_startup_model_route - - startup_route = resolve_startup_model_route( - explicit_model, - explicit_provider=provider or "", - user_providers=cfg.get("providers"), - custom_providers=cfg.get("custom_providers"), - ) - if startup_route is not None: - effective_model = startup_route.model - if effective_provider is None: - effective_provider = startup_route.provider or None - if startup_route.base_url: - explicit_base_url_from_alias = startup_route.base_url.rstrip("/") # First check DIRECT_ALIASES populated from config.yaml `model_aliases:`. # These map a user-defined alias to (model, provider, base_url) for # endpoints not in any catalog (local servers, custom proxies, etc.). @@ -461,16 +447,6 @@ def _run_agent( if detected: effective_provider, effective_model = detected - # The startup resolver owns explicit provider/model and alias - # selections. Do not let the legacy catalog fallback overwrite - # that route later in this compatibility path. - if startup_route is not None: - effective_model = startup_route.model - if effective_provider is None or not (provider or "").strip(): - effective_provider = startup_route.provider or None - if startup_route.base_url: - explicit_base_url_from_alias = startup_route.base_url.rstrip("/") - runtime = resolve_runtime_provider( requested=effective_provider, target_model=effective_model or None, diff --git a/tests/hermes_cli/test_startup_model_routing_87189.py b/tests/hermes_cli/test_startup_model_routing_87189.py index 34f2faf038..ffa3b902a9 100644 --- a/tests/hermes_cli/test_startup_model_routing_87189.py +++ b/tests/hermes_cli/test_startup_model_routing_87189.py @@ -30,6 +30,36 @@ def test_startup_route_does_not_consume_aggregator_namespace(monkeypatch): assert route is None +def test_startup_route_aggregator_native_slug_stays_on_aggregator(monkeypatch): + """On OpenRouter, ``anthropic/claude-...`` is an aggregator-native slug. + + A ``providers.anthropic`` block in the same config must NOT steal the + route — bare vendor slugs resolve WITHIN the aggregator first + (aggregator-aware resolution contract). + """ + monkeypatch.setattr(model_switch, "DIRECT_ALIASES", {}) + monkeypatch.setattr( + "hermes_cli.models._find_openrouter_slug", + lambda name: "anthropic/claude-opus-4.6", + ) + route = model_switch.resolve_startup_model_route( + "anthropic/claude-opus-4.6", + current_provider="openrouter", + user_providers={"anthropic": {"apiKey": "sk-test"}}, + ) + assert route is None + + +def test_startup_route_non_aggregator_current_provider_still_routes(monkeypatch): + monkeypatch.setattr(model_switch, "DIRECT_ALIASES", {}) + route = model_switch.resolve_startup_model_route( + "nous/deepseek-v4-pro", + current_provider="anthropic", + user_providers={"nous": {"base_url": "https://inference.example/v1"}}, + ) + assert route == model_switch.StartupModelRoute("deepseek-v4-pro", "nous", "") + + def test_startup_route_resolves_dict_alias_and_preserves_endpoint(monkeypatch): monkeypatch.setattr( model_switch, @@ -46,6 +76,60 @@ def test_startup_route_resolves_dict_alias_and_preserves_endpoint(monkeypatch): ) +def test_startup_route_url_alias_never_keeps_foreign_provider_label(monkeypatch): + """A URL-bearing alias labelled ``anthropic`` must resolve as ``custom``. + + Keeping the label would let the alias reach the anthropic + explicit-runtime branch with a foreign base_url and put the live vendor + token on the alias host's wire (#28660 / #83612). + """ + monkeypatch.setattr( + model_switch, + "DIRECT_ALIASES", + { + "urlalias": model_switch.DirectAlias( + "qwen3.5:4b", "anthropic", "http://localhost:11434/v1" + ) + }, + ) + route = model_switch.resolve_startup_model_route("urlalias") + assert route is not None + assert route.provider == "custom" + assert route.base_url == "http://localhost:11434/v1" + + +def test_startup_route_alias_carries_own_api_key(monkeypatch): + monkeypatch.setattr( + model_switch, + "DIRECT_ALIASES", + { + "keyed": model_switch.DirectAlias( + "some-model", + "custom", + "https://proxy.example/v1", + api_key="sk-alias-key", + ) + }, + ) + route = model_switch.resolve_startup_model_route("keyed") + assert route is not None + assert route.api_key == "sk-alias-key" + + +def test_startup_route_explicit_provider_wins_over_alias_label(monkeypatch): + monkeypatch.setattr( + model_switch, + "DIRECT_ALIASES", + {"ds": model_switch.DirectAlias("deepseek-chat", "deepseek", "")}, + ) + route = model_switch.resolve_startup_model_route( + "ds", explicit_provider="openrouter" + ) + assert route is not None + assert route.provider == "openrouter" + assert route.model == "deepseek-chat" + + def test_model_aliases_dict_entries_are_loaded(monkeypatch): monkeypatch.setattr( "hermes_cli.config.load_config", @@ -64,4 +148,4 @@ def test_model_aliases_dict_entries_are_loaded(monkeypatch): aliases = model_switch._load_direct_aliases() assert aliases["localqwen"] == model_switch.DirectAlias( "qwen3.5:4b", "custom", "http://localhost:11434/v1" - ) \ No newline at end of file + )