diff --git a/hermes_cli/auth.py b/hermes_cli/auth.py index 1d77fffa92f20..3611c5849202a 100644 --- a/hermes_cli/auth.py +++ b/hermes_cli/auth.py @@ -41,7 +41,7 @@ import httpx import yaml -from hermes_cli.config import get_hermes_home, get_config_path, read_raw_config +from hermes_cli.config import get_hermes_home, get_config_path, read_raw_config, get_env_value from hermes_constants import OPENROUTER_BASE_URL from utils import atomic_replace, atomic_yaml_write, is_truthy_value @@ -3424,7 +3424,9 @@ def get_api_key_provider_status(provider_id: str) -> Dict[str, Any]: env_url = "" if pconfig.base_url_env_var: - env_url = os.getenv(pconfig.base_url_env_var, "").strip() + # Use get_env_value so ~/.hermes/.env is consulted, matching + # how the API key itself is resolved. See issue #18757. + env_url = (get_env_value(pconfig.base_url_env_var) or "").strip() if provider_id in ("kimi-coding", "kimi-coding-cn"): base_url = _resolve_kimi_base_url(api_key, pconfig.inference_base_url, env_url) @@ -3456,7 +3458,12 @@ def get_external_process_provider_status(provider_id: str) -> Dict[str, Any]: ) raw_args = os.getenv("HERMES_COPILOT_ACP_ARGS", "").strip() args = shlex.split(raw_args) if raw_args else ["--acp", "--stdio"] - base_url = os.getenv(pconfig.base_url_env_var, "").strip() if pconfig.base_url_env_var else "" + # ~/.hermes/.env should win over a stale shell export — see issue #18757. + base_url = ( + (get_env_value(pconfig.base_url_env_var) or "").strip() + if pconfig.base_url_env_var + else "" + ) if not base_url: base_url = pconfig.inference_base_url @@ -3528,7 +3535,10 @@ def resolve_api_key_provider_credentials(provider_id: str) -> Dict[str, Any]: env_url = "" if pconfig.base_url_env_var: - env_url = os.getenv(pconfig.base_url_env_var, "").strip() + # Read from ~/.hermes/.env as well as os.environ so providers with + # a custom base URL stored only in the dotenv file (e.g. Xiaomi) are + # routed to the right endpoint. See issue #18757. + env_url = (get_env_value(pconfig.base_url_env_var) or "").strip() if provider_id in ("kimi-coding", "kimi-coding-cn"): base_url = _resolve_kimi_base_url(api_key, pconfig.inference_base_url, env_url) @@ -3557,7 +3567,12 @@ def resolve_external_process_provider_credentials(provider_id: str) -> Dict[str, code="invalid_provider", ) - base_url = os.getenv(pconfig.base_url_env_var, "").strip() if pconfig.base_url_env_var else "" + # Honour ~/.hermes/.env for the base URL — see issue #18757. + base_url = ( + (get_env_value(pconfig.base_url_env_var) or "").strip() + if pconfig.base_url_env_var + else "" + ) if not base_url: base_url = pconfig.inference_base_url diff --git a/hermes_cli/model_switch.py b/hermes_cli/model_switch.py index 07455eb6fa4fa..026a35c5c9073 100644 --- a/hermes_cli/model_switch.py +++ b/hermes_cli/model_switch.py @@ -1050,7 +1050,13 @@ def _record_builtin_endpoint(slug: str) -> None: return url = "" if getattr(pcfg, "base_url_env_var", ""): - url = os.environ.get(pcfg.base_url_env_var, "") or "" + # Prefer ~/.hermes/.env over os.environ for parity with how the + # rest of Hermes resolves provider base URLs (issue #18757). + try: + from hermes_cli.config import get_env_value + url = (get_env_value(pcfg.base_url_env_var) or "") + except Exception: + url = os.environ.get(pcfg.base_url_env_var, "") or "" if not url: url = getattr(pcfg, "inference_base_url", "") or "" normed = _norm_url(url) diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index dfdc9115699e3..a9dd4e5218934 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -844,7 +844,11 @@ def _resolve_explicit_runtime( if pconfig and pconfig.auth_type == "api_key": env_url = "" if pconfig.base_url_env_var: - env_url = os.getenv(pconfig.base_url_env_var, "").strip().rstrip("/") + # Read ~/.hermes/.env in addition to the process environment so + # custom base URLs stored only in the dotenv file (e.g. Xiaomi) + # are honoured here too. See issue #18757. + from hermes_cli.config import get_env_value + env_url = (get_env_value(pconfig.base_url_env_var) or "").strip().rstrip("/") base_url = explicit_base_url if not base_url: diff --git a/tests/hermes_cli/test_base_url_dotenv_resolution.py b/tests/hermes_cli/test_base_url_dotenv_resolution.py new file mode 100644 index 0000000000000..e3a7326630d2b --- /dev/null +++ b/tests/hermes_cli/test_base_url_dotenv_resolution.py @@ -0,0 +1,192 @@ +"""Regression tests for issue #18757. + +`base_url_env_var` resolution must consult ``~/.hermes/.env`` via +``get_env_value()`` — not just ``os.getenv()`` — so that providers configured +exclusively in the dotenv file (with no shell export) end up using the right +endpoint instead of silently falling back to ``inference_base_url``. + +Before the fix, ``resolve_api_key_provider_credentials("xiaomi")`` would +return the registry default (``api.xiaomimimo.com``) even when the user had +written ``XIAOMI_BASE_URL=https://token-plan-cn.xiaomimimo.com/v1`` to +``~/.hermes/.env``, causing 401s on auxiliary tasks (issue #18757). The same +class of bug existed in five additional places — this module exercises each +of them. +""" + +from __future__ import annotations + +from pathlib import Path +from typing import Iterator + +import pytest + + +# --------------------------------------------------------------------------- +# Fixture: a real ~/.hermes/.env file containing only the base URL override. +# --------------------------------------------------------------------------- + + +@pytest.fixture +def hermes_home_with_dotenv_base_url( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> Iterator[Path]: + """Write a Hermes home with .env that overrides XIAOMI_BASE_URL only. + + The shell environment is intentionally cleared of XIAOMI_BASE_URL so the + test can prove that resolution falls through to the dotenv file. + """ + home = tmp_path / "hermes_home" + home.mkdir() + env_file = home / ".env" + env_file.write_text( + "XIAOMI_API_KEY=sk-from-dotenv\n" + "XIAOMI_BASE_URL=https://token-plan-cn.xiaomimimo.com/v1\n" + ) + + # Point Hermes at this directory and clear cached lookups. + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("XIAOMI_BASE_URL", raising=False) + monkeypatch.delenv("XIAOMI_API_KEY", raising=False) + + # Bust the in-process cache that load_env() uses, if present. + try: + from hermes_cli import config as _config + + if hasattr(_config, "_ENV_CACHE"): + _config._ENV_CACHE = None # type: ignore[attr-defined] + except Exception: + pass + + yield home + + +# --------------------------------------------------------------------------- +# auth.py — both API-key paths +# --------------------------------------------------------------------------- + + +class TestAuthApiKeyResolution: + """resolve_api_key_provider_credentials() / get_api_key_provider_status().""" + + def test_resolve_credentials_reads_base_url_from_dotenv( + self, hermes_home_with_dotenv_base_url: Path + ) -> None: + from hermes_cli.auth import resolve_api_key_provider_credentials + + creds = resolve_api_key_provider_credentials("xiaomi") + assert creds["base_url"] == "https://token-plan-cn.xiaomimimo.com/v1" + assert creds["api_key"] == "sk-from-dotenv" + + def test_status_reads_base_url_from_dotenv( + self, hermes_home_with_dotenv_base_url: Path + ) -> None: + from hermes_cli.auth import get_api_key_provider_status + + status = get_api_key_provider_status("xiaomi") + assert status["base_url"] == "https://token-plan-cn.xiaomimimo.com/v1" + assert status["configured"] is True + + def test_shell_env_still_wins_when_set( + self, + hermes_home_with_dotenv_base_url: Path, + monkeypatch: pytest.MonkeyPatch, + ) -> None: + """Explicit shell exports continue to win — get_env_value() prefers + os.environ, matching how API keys are already resolved.""" + from hermes_cli.auth import resolve_api_key_provider_credentials + + monkeypatch.setenv("XIAOMI_BASE_URL", "https://shell.example/v1") + creds = resolve_api_key_provider_credentials("xiaomi") + assert creds["base_url"] == "https://shell.example/v1" + + +# --------------------------------------------------------------------------- +# auth.py — external-process providers (Copilot ACP) +# --------------------------------------------------------------------------- + + +class TestAuthExternalProcessResolution: + """resolve_external_process_provider_credentials() / *_status().""" + + def _write_dotenv(self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path: + home = tmp_path / "hermes_home" + home.mkdir() + (home / ".env").write_text( + "COPILOT_ACP_BASE_URL=acp+tcp://127.0.0.1:9999\n" + ) + monkeypatch.setenv("HERMES_HOME", str(home)) + monkeypatch.delenv("COPILOT_ACP_BASE_URL", raising=False) + try: + from hermes_cli import config as _config + + if hasattr(_config, "_ENV_CACHE"): + _config._ENV_CACHE = None # type: ignore[attr-defined] + except Exception: + pass + return home + + def test_resolve_credentials_reads_base_url_from_dotenv( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._write_dotenv(tmp_path, monkeypatch) + from hermes_cli.auth import resolve_external_process_provider_credentials + + creds = resolve_external_process_provider_credentials("copilot-acp") + assert creds["base_url"] == "acp+tcp://127.0.0.1:9999" + + def test_status_reads_base_url_from_dotenv( + self, tmp_path: Path, monkeypatch: pytest.MonkeyPatch + ) -> None: + self._write_dotenv(tmp_path, monkeypatch) + from hermes_cli.auth import get_external_process_provider_status + + status = get_external_process_provider_status("copilot-acp") + assert status["base_url"] == "acp+tcp://127.0.0.1:9999" + + +# --------------------------------------------------------------------------- +# runtime_provider.py — the one PR #17246 missed entirely +# --------------------------------------------------------------------------- + + +class TestRuntimeProviderResolution: + """The runtime resolver path used by gateway / auxiliary clients.""" + + def test_runtime_resolver_reads_base_url_from_dotenv( + self, hermes_home_with_dotenv_base_url: Path + ) -> None: + from hermes_cli.runtime_provider import resolve_runtime_provider + + runtime = resolve_runtime_provider(requested="xiaomi") + assert runtime["base_url"].rstrip("/") == ( + "https://token-plan-cn.xiaomimimo.com/v1" + ) + + +# --------------------------------------------------------------------------- +# model_switch.py — built-in endpoint dedup helper +# --------------------------------------------------------------------------- + + +class TestModelSwitchBuiltinEndpointDedup: + """The built-in endpoint dedup logic must read ~/.hermes/.env so that + user-defined custom_providers entries pointing at the same host get + deduplicated correctly (issue #18757). + + The helper is a closure inside _refresh_curated_models. Inspect the + source of the patched module to confirm it now goes through + ``get_env_value`` rather than ``os.environ.get`` directly. + """ + + def test_dedup_helper_uses_get_env_value(self) -> None: + import inspect + from hermes_cli import model_switch as ms + + src = inspect.getsource(ms) + # The fixed call path must reference get_env_value when reading + # base_url_env_var. Direct os.environ.get(pcfg.base_url_env_var ...) + # is the regression we're guarding against. + assert "get_env_value(pcfg.base_url_env_var)" in src, ( + "model_switch must resolve base_url_env_var via get_env_value " + "so ~/.hermes/.env is honoured (issue #18757)." + )