From e789d79cfe119d3fa7a7cd525269d55ed2f75bbe Mon Sep 17 00:00:00 2001 From: Teknium <127238744+teknium1@users.noreply.github.com> Date: Wed, 2 Sep 2026 22:14:12 -0700 Subject: [PATCH] =?UTF-8?q?refactor(hermes=5Fcli):=20provider=20cluster=20?= =?UTF-8?q?=E2=80=94=20shared=20=5Foverlay=5Fpdef,=20pool-select=20guard?= =?UTF-8?q?=20collapse,=20docstring=20compaction=20(WHY=20kept)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- hermes_cli/providers.py | 71 +++++++++++------------- hermes_cli/runtime_provider.py | 61 +++++++++----------- hermes_cli/runtime_provider_backends.py | 22 +++----- hermes_cli/runtime_provider_custom.py | 74 ++++++++++--------------- 4 files changed, 96 insertions(+), 132 deletions(-) diff --git a/hermes_cli/providers.py b/hermes_cli/providers.py index 06b3a6dc26..eccefc684b 100644 --- a/hermes_cli/providers.py +++ b/hermes_cli/providers.py @@ -177,12 +177,17 @@ def _models_dev_info(canonical: str, allow_network: bool = True): return None -def get_provider(name: str, *, allow_network: bool = True) -> Optional[ProviderDef]: - """Look up a built-in provider by id or alias. +def _overlay_pdef(canonical, ov: HermesOverlay, name, env_vars, base_url, doc, source) -> ProviderDef: + return ProviderDef( + id=canonical, name=name, transport=ov.transport, api_key_env_vars=env_vars, base_url=base_url, + base_url_env_var=ov.base_url_env_var, is_aggregator=ov.is_aggregator, auth_type=ov.auth_type, doc=doc, + source=source, + ) - Order: models.dev catalog merged with the Hermes overlay; Hermes-only overlay (nous, - openai-codex, …); plugin provider profiles with a concrete endpoint. - """ + +def get_provider(name: str, *, allow_network: bool = True) -> Optional[ProviderDef]: + """Look up a built-in provider by id or alias: models.dev catalog merged with the Hermes overlay; + Hermes-only overlay (nous, openai-codex, …); plugin provider profiles with a concrete endpoint.""" canonical = normalize_provider(name) mdev_info = _models_dev_info(canonical, allow_network) overlay = HERMES_OVERLAYS.get(canonical) @@ -192,17 +197,14 @@ def get_provider(name: str, *, allow_network: bool = True) -> Optional[ProviderD for ev in ov.extra_env_vars: if ev not in env_vars: env_vars.append(ev) - return ProviderDef( - id=canonical, name=mdev_info.name, transport=ov.transport, api_key_env_vars=tuple(env_vars), - base_url=ov.base_url_override or mdev_info.api, base_url_env_var=ov.base_url_env_var, - is_aggregator=ov.is_aggregator, auth_type=ov.auth_type, doc=mdev_info.doc, source="models.dev", + return _overlay_pdef( + canonical, ov, mdev_info.name, tuple(env_vars), ov.base_url_override or mdev_info.api, mdev_info.doc, + "models.dev", ) if overlay is not None: - return ProviderDef( - id=canonical, name=_LABEL_OVERRIDES.get(canonical, canonical), transport=overlay.transport, - api_key_env_vars=overlay.extra_env_vars, base_url=overlay.base_url_override, - base_url_env_var=overlay.base_url_env_var, is_aggregator=overlay.is_aggregator, - auth_type=overlay.auth_type, source="hermes", + return _overlay_pdef( + canonical, overlay, _LABEL_OVERRIDES.get(canonical, canonical), overlay.extra_env_vars, + overlay.base_url_override, "", "hermes", ) # Plugin-registered profiles (plugins/model-providers//) absent from models.dev and # HERMES_OVERLAYS would otherwise be "Unknown provider" in /model, --provider and model-switch @@ -264,12 +266,10 @@ def is_routing_aggregator(provider: str) -> bool: def is_official_openai_host(base_url: str) -> bool: - """True when *base_url* points at OpenAI's official API host family. - - Hostname-parsed matching only — never substring — so lookalike hosts - (``api.openai.com.attacker.test``) and path-segment spoofs (``proxy.test/api.openai.com/v1``) - are rejected. A genuine ``*.api.openai.com`` subdomain requires control of openai.com DNS. - """ + """True when *base_url* points at OpenAI's official API host family. Hostname-parsed matching + only — never substring — so lookalike hosts (``api.openai.com.attacker.test``) and path-segment + spoofs (``proxy.test/api.openai.com/v1``) are rejected; a genuine ``*.api.openai.com`` + subdomain requires control of openai.com DNS.""" return base_url_host_matches(base_url, "api.openai.com") @@ -281,14 +281,12 @@ _RESPONSES_NATIVE_HOSTS: frozenset[str] = frozenset({"api.meta.ai", "api.router. def host_mandated_api_mode(base_url: str = "") -> Optional[str]: - """Return the wire protocol a specific endpoint *requires*, or None. - - Some hosts accept exactly one API mode (api.openai.com 400s chat/completions for reasoning - models with tools). These are *mandatory*: a session carrying a stale api_mode (a /model switch - that kept the previous provider's ``chat_completions``) must be overridden, not merely filled - in when empty. Exact-hostname matching only — never substring — so lookalike hosts and - path-segment spoofs are not treated as the real endpoint. - """ + """Return the wire protocol a specific endpoint *requires*, or None. Some hosts accept exactly + one API mode (api.openai.com 400s chat/completions for reasoning models with tools); these are + *mandatory*: a session carrying a stale api_mode (a /model switch that kept the previous + provider's ``chat_completions``) must be overridden, not merely filled in when empty. + Exact-hostname matching only — never substring — so lookalike hosts and path-segment spoofs are + not treated as the real endpoint.""" if not base_url: return None url_lower = base_url.rstrip("/").lower() @@ -383,11 +381,9 @@ def custom_provider_aliases(display_name: str, provider_key: str = "") -> frozen def resolve_custom_provider(name: str, custom_providers: Optional[List[Dict[str, Any]]]) -> Optional[ProviderDef]: - """Resolve a provider from the user's config.yaml ``custom_providers`` list. - - A stored bare ``"custom"`` (corrupt state from a prior model-switch bug) falls back to the first - valid entry so existing configs self-heal. - """ + """Resolve a provider from the user's config.yaml ``custom_providers`` list. A stored bare + ``"custom"`` (corrupt state from a prior model-switch bug) falls back to the first valid entry + so existing configs self-heal.""" if not custom_providers or not isinstance(custom_providers, list): return None requested = (name or "").strip().lower() @@ -459,12 +455,9 @@ def resolve_provider_full( ) -> Optional[ProviderDef]: """Full resolution chain: user ``providers.`` -> lossy-alias registry id -> built-in (models.dev + overlays) -> user providers (canonical, then raw) -> ``custom_providers`` -> - managed llamacpp -> models.dev directly. - - User-defined ``providers.`` is tried FIRST on the raw (pre-alias) name: a configured - ``providers.openai`` pointing at api.openai.com must not be hijacked by the legacy - "openai" -> "openrouter" alias. - """ + managed llamacpp -> models.dev directly. User-defined ``providers.`` is tried FIRST on + the raw (pre-alias) name: a configured ``providers.openai`` pointing at api.openai.com must not + be hijacked by the legacy "openai" -> "openrouter" alias.""" canonical = normalize_provider(name) raw = name.strip().lower() if user_providers: diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 3784c69d68..a1e3fa90b5 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -1,10 +1,9 @@ -"""Shared runtime provider resolution for CLI, gateway, cron, and helpers. - -This module owns the resolution ORDER (:func:`resolve_runtime_provider`), the api_mode / base_url -helpers and the pool / OAuth / explicit paths. Custom-provider lookup lives in -:mod:`hermes_cli.runtime_provider_custom`; Azure Foundry, OpenRouter/bare-custom, Bedrock and -external-process builders in :mod:`hermes_cli.runtime_provider_backends`. Both are re-exported -here so ``hermes_cli.runtime_provider.`` imports and test patches keep working.""" +"""Shared runtime provider resolution for CLI, gateway, cron, and helpers: the resolution ORDER +(:func:`resolve_runtime_provider`), api_mode / base_url helpers and the pool / OAuth / explicit paths. +Custom-provider lookup lives in :mod:`hermes_cli.runtime_provider_custom`; Azure Foundry, +OpenRouter/bare-custom, Bedrock and external-process builders in +:mod:`hermes_cli.runtime_provider_backends` — both re-exported here so +``hermes_cli.runtime_provider.`` imports and test patches keep working.""" from __future__ import annotations @@ -72,12 +71,11 @@ def _resolves_to_custom(name: str) -> bool: def _config_base_url_trustworthy_for_bare_custom(cfg_base_url: str, cfg_provider: str) -> bool: - """Whether ``model.base_url`` may back bare ``custom`` runtime resolution. - - The model picker can select Custom while ``model.provider`` still reflects a previous provider. - Non-loopback URLs are rejected unless the YAML provider is already ``custom`` or a local-server - alias (ollama/vllm/llamacpp — else a legit LAN ollama endpoint silently falls through to - OpenRouter), so a stale OpenRouter/Z.ai base_url cannot hijack local sessions.""" + """Whether ``model.base_url`` may back bare ``custom`` runtime resolution. The picker can select + Custom while ``model.provider`` still names a previous provider, so non-loopback URLs are rejected + unless the YAML provider is already ``custom`` or a local-server alias (ollama/vllm/llamacpp — + else a legit LAN ollama endpoint falls through to OpenRouter): a stale OpenRouter/Z.ai base_url + cannot hijack local sessions.""" cfg_provider_norm = (cfg_provider or "").strip().lower() bu = (cfg_base_url or "").strip() if not bu: @@ -105,11 +103,10 @@ _VALID_API_MODES = {"chat_completions", "codex_responses", "anthropic_messages", def _detect_api_mode_for_url(base_url: str) -> Optional[str]: - """Auto-detect api_mode from the resolved base URL, or None. - - Exact-hostname matches reject lookalike subdomains (api.anthropic.com.attacker.test) and - path-segment spoofing (proxy.test/api.anthropic.com/v1). Official OpenAI hosts (incl. the - data-residency us./eu. regional hosts) need Responses for GPT-5.x tool calls with reasoning.""" + """Auto-detect api_mode from the resolved base URL, or None. Exact-hostname matches reject + lookalike subdomains (api.anthropic.com.attacker.test) and path-segment spoofing + (proxy.test/api.anthropic.com/v1). Official OpenAI hosts (incl. us./eu. data-residency hosts) + need Responses for GPT-5.x tool calls with reasoning.""" normalized = (base_url or "").strip().lower().rstrip("/") hostname = base_url_hostname(base_url) mandated = _HOST_MANDATED_API_MODES.get(hostname) @@ -214,7 +211,6 @@ def _configured_or_fallback_api_mode( provider: str, model_cfg: Dict[str, Any], base_url: str, effective_model: Any, *, opencode_by_model: bool ) -> str: """Persisted ``model.api_mode`` when it belongs to this provider, else URL/transport fallback. - OpenCode Zen/Go serve both anthropic_messages and chat_completions models, so (when ``opencode_by_model``) their mode is always re-derived from the effective model.""" if opencode_by_model and _models.opencode_provider_family(provider) is not None: @@ -320,10 +316,9 @@ def _host_derived_api_key(base_url: str) -> str: def _host_gated_env_key_candidates(base_url: str, *, ollama: bool) -> list: """Env API keys gated on their authoritative hosts, then the host-derived ``_API_KEY``. - Sending OPENAI/OPENROUTER/OLLAMA keys to an unrelated endpoint leaks credentials - (GHSA-76xc-57q6-vm5m); match on HOST, not substring. ``_host_derived_api_key`` skips OLLAMA, - so callers that want it opt in via ``ollama``.""" + (GHSA-76xc-57q6-vm5m); match on HOST, not substring. ``_host_derived_api_key`` skips OLLAMA, so + callers that want it opt in via ``ollama``.""" is_openai = base_url_host_matches(base_url, "openai.com") or base_url_host_matches(base_url, "openai.azure.com") candidates = [] if ollama: @@ -570,14 +565,12 @@ def _resolve_from_pool( if not (pool and pool.has_credentials()): return None entry = pool.select() - pool_api_key = _pool_entry_api_key(entry) if entry is not None else "" - if provider == "nous" and entry is not None: + if entry is None: + return None + pool_api_key = _pool_entry_api_key(entry) + if provider == "nous": entry, pool_api_key = _refresh_nous_pool_entry(pool, entry, pool_api_key) - if ( - entry is not None - and pool_api_key - and credential_pool_matches_provider(pool, provider, base_url=_pool_entry_base_url(entry)) - ): + if pool_api_key and credential_pool_matches_provider(pool, provider, base_url=_pool_entry_base_url(entry)): return _resolve_runtime_from_pool_entry( provider=provider, entry=entry, requested_provider=requested_provider, model_cfg=model_cfg, pool=pool, target_model=target_model, @@ -939,9 +932,8 @@ def resolve_runtime_provider( *, requested: Optional[str] = None, explicit_api_key: Optional[str] = None, explicit_base_url: Optional[str] = None, target_model: Optional[str] = None, ) -> Dict[str, Any]: - """Resolve runtime provider credentials for agent execution. - - Ladder (order is behavior — each rung returns or raises, else falls to the next): + """Resolve runtime provider credentials for agent execution. Ladder (order is behavior — each + rung returns or raises, else falls to the next): 1. disabled-provider guard (``providers..enabled: false``) 2. requested-name shortcuts: moa, anthropic@azure, azure-foundry, vertex 3. named custom provider / llamacpp alias / bare-custom direct alias @@ -951,9 +943,8 @@ def resolve_runtime_provider( 7. OAuth specs (nous/codex/xai/qwen; "auto" swallows AuthError and logs) → minimax-oauth → external-process → anthropic env → bedrock → registry api_key providers 8. OpenRouter / bare-custom fallback - - target_model: overrides model_cfg["default"] when computing provider-specific api_mode - (e.g. OpenCode Zen/Go where different models route through different API surfaces).""" + target_model overrides model_cfg["default"] when computing provider-specific api_mode (e.g. + OpenCode Zen/Go where different models route through different API surfaces).""" requested_provider = resolve_requested_provider(requested) _raise_if_provider_disabled(requested_provider) runtime = _resolve_requested_shortcuts(requested_provider, explicit_api_key, explicit_base_url, target_model) diff --git a/hermes_cli/runtime_provider_backends.py b/hermes_cli/runtime_provider_backends.py index aba924a26e..bbe8c30acd 100644 --- a/hermes_cli/runtime_provider_backends.py +++ b/hermes_cli/runtime_provider_backends.py @@ -1,10 +1,8 @@ """Provider-specific runtime builders for :mod:`hermes_cli.runtime_provider`: Azure Foundry, the -OpenRouter / bare-custom fallback resolver, Bedrock, and external-process providers. - -Origin-internal collaborators are resolved on the origin module at call time via :func:`_rp` so -test patches on ``hermes_cli.runtime_provider.*`` (``_get_model_config``, ``load_config``, -``has_usable_secret``, ``_try_resolve_from_custom_pool``, …) still apply. -""" +OpenRouter / bare-custom fallback resolver, Bedrock, and external-process providers. Origin-internal +collaborators are resolved on the origin module at call time via :func:`_rp` so test patches on +``hermes_cli.runtime_provider.*`` (``_get_model_config``, ``load_config``, ``has_usable_secret``, +``_try_resolve_from_custom_pool``, …) still apply.""" from __future__ import annotations @@ -120,13 +118,11 @@ def _resolve_azure_foundry_runtime( def _resolve_openrouter_runtime( *, requested_provider: str, explicit_api_key: Optional[str] = None, explicit_base_url: Optional[str] = None ) -> Dict[str, Any]: - """Terminal resolver: OpenRouter, or a bare/aliased ``custom`` endpoint. - - base_url precedence: explicit > CUSTOM_BASE_URL > trusted ``model.base_url`` > OPENROUTER_BASE_URL - > default. OPENAI_BASE_URL is deliberately NOT consulted — config.yaml is the single source of - truth for endpoint URLs. OpenRouter contexts prefer OPENROUTER_API_KEY; custom endpoints never - receive the OpenRouter key and only get env keys gated on their authoritative hosts. - """ + """Terminal resolver: OpenRouter, or a bare/aliased ``custom`` endpoint. base_url precedence: + explicit > CUSTOM_BASE_URL > trusted ``model.base_url`` > OPENROUTER_BASE_URL > default. + OPENAI_BASE_URL is deliberately NOT consulted — config.yaml is the single source of truth for + endpoint URLs. OpenRouter contexts prefer OPENROUTER_API_KEY; custom endpoints never receive the + OpenRouter key and only get env keys gated on their authoritative hosts.""" rp = _rp() model_cfg = rp._get_model_config() cfg_base_url = model_cfg.get("base_url") if isinstance(model_cfg.get("base_url"), str) else "" diff --git a/hermes_cli/runtime_provider_custom.py b/hermes_cli/runtime_provider_custom.py index 0dd10f34b8..f1e04c1267 100644 --- a/hermes_cli/runtime_provider_custom.py +++ b/hermes_cli/runtime_provider_custom.py @@ -1,12 +1,9 @@ -"""Custom-provider resolution: ``providers:`` / ``custom_providers:`` lookup, identity -recovery, custom credential pools, and the named-custom runtime builder. - -Extracted from :mod:`hermes_cli.runtime_provider`; every name here is re-exported there. -Origin-internal collaborators (``load_config``, ``_get_model_config``, ``load_pool``, -``has_usable_secret``, ``custom_provider_pool_key_candidates``, …) are looked up on the origin -module AT CALL TIME via :func:`_rp` so ``monkeypatch.setattr(runtime_provider, name, …)`` keeps -working for moved bodies. -""" +"""Custom-provider resolution: ``providers:`` / ``custom_providers:`` lookup, identity recovery, +custom credential pools, and the named-custom runtime builder. Extracted from +:mod:`hermes_cli.runtime_provider` (every name re-exported there); origin-internal collaborators +(``load_config``, ``_get_model_config``, ``load_pool``, ``has_usable_secret``, …) are looked up on +the origin module AT CALL TIME via :func:`_rp` so ``monkeypatch.setattr(runtime_provider, name, …)`` +keeps working for moved bodies.""" from __future__ import annotations @@ -98,23 +95,19 @@ def _lift_common_custom_fields( if api_mode: result["api_mode"] = api_mode _lift_max_output_tokens(entry, result) - capabilities = _filter_capabilities(entry.get("capabilities")) - if capabilities: - result["capabilities"] = capabilities + _lift_model_capabilities(entry, None, result) # ── config lookup ────────────────────────────────────────────────────────────────────────── def _shadowed_by_builtin(requested_norm: str) -> bool: - """Raw names map to custom providers only when they are not canonical built-ins. - - Explicit ``custom:`` keys always target the saved entry, and bare ``custom`` is exempt: a - user may literally name a ``providers:`` entry "custom" (returning None before the config scan - made such cron jobs fail with ``auth_unavailable``). Defer to the built-in only when the raw - name IS the canonical provider (``nous``); an entry matching merely an alias (``kimi`` → - ``kimi-coding``) is the user's target. - """ + """Raw names map to custom providers only when they are not canonical built-ins. Explicit + ``custom:`` keys always target the saved entry, and bare ``custom`` is exempt: a user may + literally name a ``providers:`` entry "custom" (returning None before the config scan made such + cron jobs fail with ``auth_unavailable``). Defer to the built-in only when the raw name IS the + canonical provider (``nous``); an entry matching merely an alias (``kimi`` → ``kimi-coding``) + is the user's target.""" if requested_norm == "custom" or requested_norm.startswith("custom:"): return False rp = _rp() @@ -288,15 +281,13 @@ def find_custom_provider_identity_by_model(model: str) -> Optional[str]: def canonical_custom_identity( *, base_url: Optional[str] = None, config_provider: Optional[str] = None, model: Optional[str] = None ) -> Optional[str]: - """Recover a routable ``custom:`` identity for a bare custom provider. - - Every path that persists or restores a session's provider override must run the resolved - provider through this so a bare ``"custom"`` is upgraded back to its durable ``custom:`` - menu key. Sources in priority order: (1) ``base_url`` reverse lookup — the one fact that always - survives the round-trip when a URL was recorded; (2) ``model`` reverse lookup - (``model``/``default_model``/``models`` catalog); (3) the configured provider (arg, then - ``model.provider``, then ``HERMES_INFERENCE_PROVIDER``) when it names a real entry. - """ + """Recover a routable ``custom:`` identity for a bare custom provider. Every path that + persists or restores a session's provider override must run the resolved provider through this + so a bare ``"custom"`` is upgraded back to its durable menu key. Sources in priority order: + (1) ``base_url`` reverse lookup — the one fact that always survives the round-trip when a URL + was recorded; (2) ``model`` reverse lookup (``model``/``default_model``/``models`` catalog); + (3) the configured provider (arg, ``model.provider``, ``HERMES_INFERENCE_PROVIDER``) when it + names a real entry.""" rp = _rp() if base_url: identity = find_custom_provider_identity(base_url) @@ -336,14 +327,11 @@ def canonical_custom_identity( def is_routable_provider(provider: Optional[str]) -> bool: - """Whether a provider name currently resolves to a routable route. - - Empty/None/``auto`` is vacuously routable (agent build falls back to the configured default). - Bare ``custom`` is the resolved billing class shared by every named entry — not a routable - identity; restore paths must heal it (:func:`canonical_custom_identity`) or fall back. Anything - else is routable iff the full chain (built-in -> ``providers:`` -> ``custom_providers:`` -> - models.dev) resolves it. - """ + """Whether a provider name currently resolves to a routable route. Empty/None/``auto`` is + vacuously routable (agent build falls back to the configured default). Bare ``custom`` is the + resolved billing class shared by every named entry — not a routable identity; restore paths + must heal it (:func:`canonical_custom_identity`) or fall back. Anything else is routable iff the + full chain (built-in -> ``providers:`` -> ``custom_providers:`` -> models.dev) resolves it.""" name = str(provider or "").strip() if not name or name.lower() == "auto": return True @@ -426,11 +414,9 @@ def _apply_custom_provider_extras( def _resolve_llamacpp_runtime(requested_provider: str, explicit_api_key: Optional[str]) -> Dict[str, Any]: """Managed llama.cpp runtime: the supervised (or detected external) server, or a typed error. - No server => say so and stop; falling through to the generic custom path would surface "local server is off" as OpenRouter's baffling "401 Invalid API key". The switch's state picks the - message (server off → point at the switch; else the setup pane). - """ + message (server off → point at the switch; else the setup pane).""" rp = _rp() try: from hermes_cli.local_runtime.endpoint import resolve_llamacpp_endpoint @@ -506,11 +492,9 @@ def _resolve_named_custom_runtime( target_model: Optional[str] = None, ) -> Optional[Dict[str, Any]]: """Runtime for a llamacpp alias, a bare-custom direct alias, or a configured custom entry. - - Aliases resolving to "custom" (ollama, vllm, llamacpp, …) are treated like bare ``custom``. - A llamacpp alias with no explicit base_url resolves to the managed server first; an explicit - base_url always wins. - """ + Aliases resolving to "custom" (ollama, vllm, llamacpp, …) are treated like bare ``custom``. A + llamacpp alias with no explicit base_url resolves to the managed server first; an explicit + base_url always wins.""" rp = _rp() requested_norm = (requested_provider or "").strip().lower() if requested_norm in _LLAMACPP_ALIASES and not explicit_base_url: