fix: harden startup route salvage — aggregator-slug guard, alias credential ownership, oneshot dedup
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.
This commit is contained in:
@@ -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:<preset>`` 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:<preset>`` 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().
|
||||
|
||||
@@ -1,61 +0,0 @@
|
||||
<svg xmlns="http://www.w3.org/2000/svg" width="1200" height="620" viewBox="0 0 1200 620" role="img" aria-labelledby="title desc">
|
||||
<title id="title">Hermes model startup routing fix</title>
|
||||
<desc id="desc">A diagram showing aliases and provider/model inputs converging at startup routing before runtime provider resolution.</desc>
|
||||
<defs>
|
||||
<linearGradient id="bg" x1="0" x2="1" y1="0" y2="1"><stop stop-color="#07111f"/><stop offset="1" stop-color="#101b31"/></linearGradient>
|
||||
<linearGradient id="good" x1="0" x2="1"><stop stop-color="#064e3b"/><stop offset="1" stop-color="#065f46"/></linearGradient>
|
||||
<linearGradient id="bad" x1="0" x2="1"><stop stop-color="#7f1d1d"/><stop offset="1" stop-color="#991b1b"/></linearGradient>
|
||||
<filter id="shadow"><feDropShadow dx="0" dy="8" stdDeviation="8" flood-color="#000" flood-opacity=".35"/></filter>
|
||||
<marker id="arrow" markerWidth="10" markerHeight="10" refX="8" refY="5" orient="auto"><path d="M0,0 L10,5 L0,10 z" fill="#67e8f9"/></marker>
|
||||
<pattern id="grid" width="40" height="40" patternUnits="userSpaceOnUse"><path d="M40 0H0V40" fill="none" stroke="#334155" stroke-opacity=".22"/></pattern>
|
||||
<style>
|
||||
.h{font:700 28px 'Segoe UI',sans-serif;fill:#e2e8f0}.s{font:14px 'Segoe UI',sans-serif;fill:#94a3b8}.t{font:600 17px 'Segoe UI',sans-serif;fill:#f8fafc}.m{font:14px Consolas,monospace;fill:#cbd5e1}.small{font:12px 'Segoe UI',sans-serif;fill:#cbd5e1}.label{font:700 12px 'Segoe UI',sans-serif;letter-spacing:1.4px;fill:#67e8f9}
|
||||
</style>
|
||||
</defs>
|
||||
<rect width="1200" height="620" rx="22" fill="url(#bg)"/>
|
||||
<rect x="20" y="20" width="1160" height="580" rx="16" fill="url(#grid)"/>
|
||||
<text x="60" y="70" class="h">Issue #87189 · startup model routing</text>
|
||||
<text x="60" y="100" class="s">Resolve the requested route before the configured default provider can take ownership.</text>
|
||||
|
||||
<rect x="60" y="145" width="310" height="105" rx="12" fill="#172554" stroke="#818cf8" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="82" y="177" class="label">INPUT</text>
|
||||
<text x="82" y="207" class="m">--model nous/deepseek-v4-pro</text>
|
||||
<text x="82" y="230" class="small">or a configured model alias</text>
|
||||
|
||||
<rect x="445" y="130" width="310" height="135" rx="12" fill="url(#good)" stroke="#34d399" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="467" y="163" class="label" style="fill:#a7f3d0">FIXED CHOKE POINT</text>
|
||||
<text x="467" y="195" class="t">resolve_startup_model_route</text>
|
||||
<text x="467" y="220" class="small">alias → model + provider + base_url</text>
|
||||
<text x="467" y="243" class="small">prefix → configured provider only</text>
|
||||
|
||||
<rect x="830" y="145" width="310" height="105" rx="12" fill="#164e63" stroke="#22d3ee" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="852" y="177" class="label">RUNTIME</text>
|
||||
<text x="852" y="207" class="m">provider = nous</text>
|
||||
<text x="852" y="230" class="m">model = deepseek-v4-pro</text>
|
||||
|
||||
<path d="M370 197H435" stroke="#67e8f9" stroke-width="3" marker-end="url(#arrow)"/>
|
||||
<path d="M755 197H820" stroke="#67e8f9" stroke-width="3" marker-end="url(#arrow)"/>
|
||||
|
||||
<rect x="60" y="330" width="310" height="105" rx="12" fill="url(#bad)" stroke="#fb7185" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="82" y="362" class="label" style="fill:#fecdd3">BEFORE</text>
|
||||
<text x="82" y="392" class="m">model = nous/deepseek-v4-pro</text>
|
||||
<text x="82" y="417" class="m">provider = anthropic</text>
|
||||
|
||||
<rect x="445" y="315" width="310" height="135" rx="12" fill="#1e293b" stroke="#64748b" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="467" y="348" class="label" style="fill:#fda4af">WRONG FALLBACK</text>
|
||||
<text x="467" y="380" class="t">configured default wins</text>
|
||||
<text x="467" y="408" class="small">requested provider never participates</text>
|
||||
<text x="467" y="432" class="small">route reaches the wrong endpoint</text>
|
||||
|
||||
<rect x="830" y="330" width="310" height="105" rx="12" fill="#3f1d2e" stroke="#fb7185" stroke-width="2" filter="url(#shadow)"/>
|
||||
<text x="852" y="362" class="label" style="fill:#fecdd3">FAILURE</text>
|
||||
<text x="852" y="392" class="m">api.anthropic.com</text>
|
||||
<text x="852" y="417" class="m">HTTP 404: model not found</text>
|
||||
|
||||
<path d="M215 260V320" stroke="#fb7185" stroke-width="3" stroke-dasharray="7 6" marker-end="url(#arrow)"/>
|
||||
<path d="M600 275V305" stroke="#fb7185" stroke-width="3" stroke-dasharray="7 6" marker-end="url(#arrow)"/>
|
||||
<path d="M755 382H820" stroke="#fb7185" stroke-width="3" stroke-dasharray="7 6" marker-end="url(#arrow)"/>
|
||||
|
||||
<rect x="60" y="505" width="1080" height="58" rx="10" fill="#0f172a" stroke="#334155"/>
|
||||
<circle cx="87" cy="534" r="7" fill="#34d399"/><text x="106" y="540" class="small">Regression coverage: CLI startup · oneshot · aliases · configured provider/model prefixes · aggregator namespace preservation</text>
|
||||
</svg>
|
||||
|
Before Width: | Height: | Size: 4.8 KiB |
@@ -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 {})
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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"
|
||||
)
|
||||
)
|
||||
|
||||
Reference in New Issue
Block a user