fix(aux): provider "openai" resolves the same on the runtime and aux-client paths
auxiliary.<task>.provider: openai was expanded to custom + the user's OpenAI endpoint only by agent/auxiliary_client.py (compression, vision, title generation). hermes_cli/runtime_provider.py::resolve_runtime_provider — the path background_review, curator, MoA slots and delegation use — had no such expansion, so "openai" hit auth.resolve_provider's registry lookup and raised "Unknown provider 'openai'". The alias table now lives once in runtime_provider_custom.py (the direct-alias/custom sibling) and both paths call it; resolve_runtime_provider applies it before the ladder. _host_gated_env_key_candidates also pairs OPENAI_API_KEY with a base_url that is exactly OPENAI_BASE_URL: the alias lands on that proxy when no block base_url is set, and the key was issued for it — the host gate otherwise sent the "no-key-required" placeholder there while the aux-client path used the key. Slim redo of #116083 (same direction: shared alias, applied in the runtime resolver) without the extra key gate and effective_provider threading. Co-authored-by: Mohamad Kanso <91088196+MohamadKanso@users.noreply.github.com>
This commit is contained in:
@@ -5937,12 +5937,6 @@ def _get_cached_client(
|
||||
return client, _compat_model(client, model, default_model)
|
||||
|
||||
|
||||
# Aliases for direct REST APIs not modeled in PROVIDER_REGISTRY, so ``auxiliary.<task>.provider:
|
||||
# openai`` resolves to a working ``custom`` endpoint (OPENAI_API_KEY + api.openai.com) instead of
|
||||
# silently falling back to the main provider and sending OpenAI model names elsewhere.
|
||||
_AUX_DIRECT_API_BASE_URLS: Dict[str, str] = {"openai": "https://api.openai.com/v1"}
|
||||
|
||||
|
||||
# MoA virtual provider: an *explicit* `provider: moa` override (either the caller-passed `provider` arg or
|
||||
# `auxiliary.<task>.provider` in config.yaml) reaches this function directly — it never goes through
|
||||
# _resolve_auto_route(), which only unwraps the *implicit* "main provider is moa" case (#53827). Left as-is, "moa"
|
||||
@@ -5962,25 +5956,6 @@ def _unwrap_moa_provider(prov: str, mdl: Optional[str]) -> Tuple[str, Optional[s
|
||||
return prov, mdl
|
||||
|
||||
|
||||
def _expand_direct_api_alias(prov: Optional[str], existing_base: Optional[str]) -> Tuple[Optional[str], Optional[str]]:
|
||||
"""``provider: openai`` → custom + the user's OpenAI endpoint, api.openai.com/v1 only as the last resort.
|
||||
|
||||
A ``providers.openai`` entry keeps the provider name so the named-custom branch applies its base_url and
|
||||
key; otherwise ``OPENAI_BASE_URL`` (a proxy/gateway the OPENAI_API_KEY was issued for) wins over the
|
||||
public endpoint — sending the proxy key to api.openai.com 401s and then quarantines a valid key.
|
||||
"""
|
||||
if not prov:
|
||||
return prov, existing_base
|
||||
target_base = _AUX_DIRECT_API_BASE_URLS.get(prov.strip().lower())
|
||||
if target_base is None:
|
||||
return prov, existing_base
|
||||
with contextlib.suppress(Exception):
|
||||
from hermes_cli.runtime_provider import _get_named_custom_provider
|
||||
if _get_named_custom_provider(prov) is not None:
|
||||
return prov, existing_base
|
||||
return "custom", existing_base or _scoped_key_env("OPENAI_BASE_URL").rstrip("/") or target_base
|
||||
|
||||
|
||||
def _preserve_provider_with_base_url(prov: Optional[str]) -> bool:
|
||||
"""True when a first-class provider keeps its identity alongside an explicit base_url."""
|
||||
normalized = str(prov or "").strip().lower()
|
||||
@@ -6042,10 +6017,13 @@ def _resolve_task_provider_model(
|
||||
resolved_model = cfg_model
|
||||
cfg_base_url = None
|
||||
cfg_api_key = None
|
||||
# One shared alias table with resolve_runtime_provider(): ``provider: openai`` routes the same
|
||||
# way here (compression/vision/title) and on the runtime path (background review, curator, MoA).
|
||||
from hermes_cli.runtime_provider_custom import expand_direct_api_alias
|
||||
if provider:
|
||||
provider, base_url = _expand_direct_api_alias(provider, base_url)
|
||||
provider, base_url = expand_direct_api_alias(provider, base_url)
|
||||
if cfg_provider:
|
||||
cfg_provider, cfg_base_url = _expand_direct_api_alias(cfg_provider, cfg_base_url)
|
||||
cfg_provider, cfg_base_url = expand_direct_api_alias(cfg_provider, cfg_base_url)
|
||||
# An explicit provider without base_url adopts the task's configured endpoint (same or
|
||||
# unnamed provider) so the early return below carries it. Explicit "auto" is excluded — it
|
||||
# must keep flowing through auto-resolution.
|
||||
|
||||
@@ -356,6 +356,10 @@ def _host_gated_env_key_candidates(base_url: str, *, ollama: bool) -> list:
|
||||
(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")
|
||||
# OPENAI_BASE_URL names the proxy/gateway the OPENAI_API_KEY was issued for (the ``openai`` alias
|
||||
# expands onto it); an exact match is the user's own pairing, not a leak to an unrelated host.
|
||||
env_openai_base = get_secret_str("OPENAI_BASE_URL", "").strip().rstrip("/")
|
||||
is_openai = is_openai or (bool(env_openai_base) and (base_url or "").strip().rstrip("/") == env_openai_base)
|
||||
candidates = [get_secret_str("OLLAMA_API_KEY", "").strip() if base_url_host_matches(base_url, "ollama.com") else ""] if ollama else []
|
||||
return candidates + [get_secret_str("OPENAI_API_KEY", "").strip() if is_openai else "",
|
||||
get_secret_str("OPENROUTER_API_KEY", "").strip() if base_url_host_matches(base_url, "openrouter.ai") else "",
|
||||
@@ -461,7 +465,8 @@ from hermes_cli.runtime_provider_custom import ( # noqa: E402,F401
|
||||
_LLAMACPP_ALIASES, _apply_custom_provider_extras, _custom_provider_request_overrides, _filter_capabilities, _find_custom_identity,
|
||||
_get_named_custom_provider, _lift_common_custom_fields, _lift_extra_headers,
|
||||
_lift_model_capabilities, _normalize_base_url_for_match, _normalize_custom_provider_name, _resolve_named_custom_runtime,
|
||||
_try_resolve_from_custom_pool, canonical_custom_identity, codex_model_provider_id, find_custom_provider_identity,
|
||||
_try_resolve_from_custom_pool, canonical_custom_identity, codex_model_provider_id, expand_direct_api_alias,
|
||||
find_custom_provider_identity,
|
||||
find_custom_provider_identity_by_model, has_named_custom_provider, is_routable_provider,
|
||||
)
|
||||
from hermes_cli.runtime_provider_backends import ( # noqa: E402,F401
|
||||
@@ -934,6 +939,9 @@ def resolve_runtime_provider(*, requested: Optional[str] = None, explicit_api_ke
|
||||
OpenCode Zen/Go where different models route through different API surfaces)."""
|
||||
requested_provider = resolve_requested_provider(requested)
|
||||
_raise_if_provider_disabled(requested_provider)
|
||||
# Same alias expansion the auxiliary client applies, so ``provider: openai`` means one thing on
|
||||
# every path (background review, curator, MoA slots, delegation) instead of "Unknown provider".
|
||||
requested_provider, explicit_base_url = expand_direct_api_alias(requested_provider, explicit_base_url)
|
||||
_raise_if_local_alias_missing_endpoint(requested_provider, explicit_base_url)
|
||||
runtime = next(r for r in _ladder_rungs(requested_provider, explicit_api_key, explicit_base_url, target_model) if r)
|
||||
_raise_for_credentialless_bare_custom(requested_provider, runtime)
|
||||
|
||||
@@ -9,7 +9,7 @@ from __future__ import annotations
|
||||
|
||||
import logging
|
||||
import os
|
||||
from typing import Any, Callable, Dict, Optional
|
||||
from typing import Any, Callable, Dict, Optional, Tuple
|
||||
|
||||
from hermes_cli.providers import custom_provider_aliases, custom_provider_slug
|
||||
from agent.secret_scope import get_secret_str
|
||||
@@ -458,6 +458,29 @@ def _custom_runtime(rp, base_url: str, api_key: Any, api_mode: Optional[str], **
|
||||
api_key or "no-key-required", **extra)
|
||||
|
||||
|
||||
# Aliases for direct REST APIs not modeled in PROVIDER_REGISTRY, so ``provider: openai`` (aux slots,
|
||||
# background review, curator, MoA slots, the main model) resolves to a working ``custom`` endpoint
|
||||
# instead of "Unknown provider" and a silent fall-back to the main model (#116055).
|
||||
_DIRECT_API_BASE_URLS: Dict[str, str] = {"openai": "https://api.openai.com/v1"}
|
||||
|
||||
|
||||
def expand_direct_api_alias(provider: Optional[str], existing_base: Optional[str]) -> Tuple[Optional[str], Optional[str]]:
|
||||
"""``provider: openai`` → custom + the user's OpenAI endpoint, api.openai.com/v1 only as the last resort.
|
||||
|
||||
The ONE normalization both aux paths (``agent.auxiliary_client`` and ``resolve_runtime_provider``)
|
||||
apply, so the same ``auxiliary.<task>.provider`` value routes identically everywhere. A
|
||||
``providers.openai`` entry keeps the provider name so the named-custom branch applies its base_url
|
||||
and key; otherwise ``OPENAI_BASE_URL`` (a proxy/gateway the OPENAI_API_KEY was issued for) wins over
|
||||
the public endpoint — sending the proxy key to api.openai.com 401s and then quarantines a valid key.
|
||||
"""
|
||||
if not provider:
|
||||
return provider, existing_base
|
||||
target_base = _DIRECT_API_BASE_URLS.get(provider.strip().lower())
|
||||
if target_base is None or _rp()._get_named_custom_provider(provider) is not None:
|
||||
return provider, existing_base
|
||||
return "custom", (existing_base or "").strip() or get_secret_str("OPENAI_BASE_URL", "").strip().rstrip("/") or target_base
|
||||
|
||||
|
||||
def _resolve_direct_alias_runtime(requested_provider: str, explicit_api_key: Optional[str],
|
||||
explicit_base_url: str) -> Dict[str, Any]:
|
||||
"""Bare ``custom`` + explicit base_url (e.g. a ``model_aliases:`` direct alias)."""
|
||||
|
||||
@@ -8,6 +8,7 @@ from types import SimpleNamespace
|
||||
import pytest
|
||||
|
||||
from agent import auxiliary_client as aux
|
||||
from hermes_cli.runtime_provider_custom import expand_direct_api_alias
|
||||
|
||||
SESSION = {"provider": "openai-api", "model": "gpt-5.4",
|
||||
"base_url": "https://proxy.example:8443/v1", "api_key": "sk-session"}
|
||||
@@ -15,11 +16,11 @@ SESSION = {"provider": "openai-api", "model": "gpt-5.4",
|
||||
|
||||
def test_openai_alias_prefers_configured_endpoint_over_public_default(monkeypatch):
|
||||
monkeypatch.setenv("OPENAI_BASE_URL", "https://llm-proxy.corp.example/v1")
|
||||
provider, base = aux._expand_direct_api_alias("openai", None)
|
||||
provider, base = expand_direct_api_alias("openai", None)
|
||||
assert provider == "custom"
|
||||
assert base == "https://llm-proxy.corp.example/v1"
|
||||
monkeypatch.delenv("OPENAI_BASE_URL")
|
||||
assert aux._expand_direct_api_alias("openai", None) == ("custom", "https://api.openai.com/v1")
|
||||
assert expand_direct_api_alias("openai", None) == ("custom", "https://api.openai.com/v1")
|
||||
|
||||
|
||||
@pytest.mark.parametrize("rejecting_base", [
|
||||
|
||||
@@ -34,9 +34,10 @@ def test_base_urls_follow_the_scoped_key_not_default_environ(monkeypatch, second
|
||||
from hermes_cli import auth_nous
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setattr(aux, "_get_named_custom_provider", lambda name: None, raising=False)
|
||||
from hermes_cli.runtime_provider_custom import expand_direct_api_alias
|
||||
monkeypatch.setattr("hermes_cli.runtime_provider._get_named_custom_provider", lambda name: None)
|
||||
|
||||
_, base = aux._expand_direct_api_alias("openai", None)
|
||||
_, base = expand_direct_api_alias("openai", None)
|
||||
assert "default.example" not in (base or "")
|
||||
assert aux._scoped_key_env("OPENAI_BASE_URL") == ""
|
||||
assert auth_nous._nous_inference_env_override() is None
|
||||
|
||||
@@ -2133,3 +2133,33 @@ def test_openai_runtime_unset_keeps_wire_api_mode(monkeypatch, rung, openai_runt
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: model_cfg)
|
||||
|
||||
assert rp.resolve_runtime_provider(requested="openai-codex", **kwargs)["api_mode"] == "codex_responses"
|
||||
|
||||
|
||||
# ── #116055: ``provider: openai`` means the same thing on both auxiliary paths ──────────────────
|
||||
|
||||
def test_openai_alias_resolves_identically_on_runtime_and_aux_client_paths(monkeypatch):
|
||||
"""background_review/curator/MoA (resolve_runtime_provider) and compression/vision/title
|
||||
(_resolve_task_provider_model) must land on the same endpoint for the same aux block."""
|
||||
from agent import auxiliary_client as aux
|
||||
block = {"provider": "openai", "model": "review-model", "base_url": "https://gateway.example/v1", "api_key": "gw-key"}
|
||||
monkeypatch.setattr(aux, "_get_auxiliary_task_config", lambda task: block if task == "background_review" else {})
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: {"provider": "custom:mylocal", "default": "local-main"})
|
||||
|
||||
aux_provider, aux_model, aux_base, aux_key, _ = aux._resolve_task_provider_model("background_review")
|
||||
runtime = rp.resolve_runtime_provider(requested=block["provider"], target_model=block["model"],
|
||||
explicit_api_key=block["api_key"], explicit_base_url=block["base_url"])
|
||||
|
||||
assert (aux_provider, aux_base, aux_key) == ("custom", "https://gateway.example/v1", "gw-key")
|
||||
assert (runtime["provider"], runtime["base_url"], runtime["api_key"]) == (aux_provider, aux_base, aux_key)
|
||||
|
||||
|
||||
def test_openai_alias_without_base_url_pairs_openai_key_with_openai_base_url(monkeypatch):
|
||||
"""No aux base_url: the alias lands on OPENAI_BASE_URL (the proxy the key was issued for) and the
|
||||
runtime path pairs OPENAI_API_KEY with it instead of sending a placeholder key to the proxy."""
|
||||
monkeypatch.setenv("OPENAI_BASE_URL", "https://llm-proxy.corp.example/v1")
|
||||
monkeypatch.setenv("OPENAI_API_KEY", "sk-proxy-issued")
|
||||
monkeypatch.setattr(rp, "_get_model_config", lambda: {"provider": "custom:mylocal", "default": "local-main"})
|
||||
|
||||
runtime = rp.resolve_runtime_provider(requested="openai", target_model="gpt-x")
|
||||
|
||||
assert (runtime["provider"], runtime["base_url"], runtime["api_key"]) == ("custom", "https://llm-proxy.corp.example/v1", "sk-proxy-issued")
|
||||
|
||||
Reference in New Issue
Block a user