From 7d29404a819b5b76f82421428ef3f9c893877eca Mon Sep 17 00:00:00 2001 From: Wolfram Ravenwolf Date: Fri, 11 Sep 2026 01:25:16 +0200 Subject: [PATCH] fix(providers): preserve profile aliases and endpoint configuration Keep canonical plugin profile identity through CLI aliases, separate URL variables from credentials, and honor matching persisted configuration across registered aliases. Retain native custom and built-in routing. Retain real discovery/runtime test provenance from the contributor while omitting the already absorbed bridge and separately owned transport fix. Co-authored-by: david-bowiegxw <834563048@qq.com> Co-Authored-By: Claude Opus 4.8 --- contributors/emails/834563048@qq.com | 2 + hermes_cli/providers.py | 18 +- hermes_cli/runtime_provider.py | 13 +- .../test_provider_profile_identity.py | 266 ++++++++++++++++++ 4 files changed, 293 insertions(+), 6 deletions(-) create mode 100644 contributors/emails/834563048@qq.com create mode 100644 tests/hermes_cli/test_provider_profile_identity.py diff --git a/contributors/emails/834563048@qq.com b/contributors/emails/834563048@qq.com new file mode 100644 index 0000000000000..682fd5b60514c --- /dev/null +++ b/contributors/emails/834563048@qq.com @@ -0,0 +1,2 @@ +david-bowiegxw +# PR #52549 retains discovery test provenance from #53055 diff --git a/hermes_cli/providers.py b/hermes_cli/providers.py index 54380eb718631..db1e686f4169a 100644 --- a/hermes_cli/providers.py +++ b/hermes_cli/providers.py @@ -210,18 +210,24 @@ def get_provider(name: str, *, allow_network: bool = True) -> Optional[ProviderD 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 - # even though the picker lists them. Only profiles with a concrete endpoint resolve here: + # even though the picker lists them. Profiles may declare a literal or env-configured endpoint: # placeholder profiles like ``custom`` (aliases ollama/local/vllm) ship an empty base_url and # are completed by config.yaml custom_providers — resolving them would preempt # resolve_provider_full's custom step and collapse keyed ``custom:`` ids to bare custom. try: from providers import get_provider_profile as _profile _prof = _profile(canonical) - if _prof is not None and (_prof.base_url or "").strip(): + if _prof is not None: + _env_vars = tuple(_prof.env_vars or ()) + _url_vars = tuple(v for v in _env_vars if v.endswith(("_BASE_URL", "_URL"))) + _key_vars = tuple(v for v in _env_vars if v not in _url_vars) + if not ((_prof.base_url or "").strip() or (_prof.auth_type == "api_key" and _key_vars and _url_vars)): + return None _api_mode_to_transport = {v: k for k, v in TRANSPORT_TO_API_MODE.items()} - return ProviderDef(id=canonical, name=_prof.display_name or _prof.name or canonical, + return ProviderDef(id=_prof.name, name=_prof.display_name or _prof.name or canonical, transport=_api_mode_to_transport.get(_prof.api_mode, "openai_chat"), - api_key_env_vars=tuple(_prof.env_vars or ()), base_url=_prof.base_url or "", + api_key_env_vars=_key_vars, base_url=_prof.base_url or "", + base_url_env_var=next(iter(_url_vars), ""), auth_type=_prof.auth_type or "api_key", source="plugin-profile") except Exception: pass @@ -495,6 +501,10 @@ def resolve_provider_full(name: str, user_providers: Optional[Dict[str, Any]] = return pdef pdef = get_provider(canonical) if pdef is not None: + if pdef.source == "plugin-profile" and user_providers: + user_pdef = resolve_user_provider(pdef.id, user_providers) + if user_pdef is not None: + return user_pdef return pdef if user_providers: for candidate in (canonical, raw): diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 37dc514bcfa03..57292ee070ce9 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -162,11 +162,20 @@ def _resolve_plain_custom_api_mode(model_cfg: Dict[str, Any], base_url: str) -> return configured_mode or detected_mode or "chat_completions" +def _same_registered_provider(provider: str, configured_provider: str) -> bool: + """Profile aliases share an auth registry ID; unrelated routes must stay distinct.""" + if provider == configured_provider: + return True + pconfig = PROVIDER_REGISTRY.get(provider) + configured = PROVIDER_REGISTRY.get(configured_provider) + return bool(pconfig and configured and pconfig.id == configured.id) + + def _provider_supports_explicit_api_mode(provider: Optional[str], configured_provider: Optional[str] = None) -> bool: """Whether a persisted api_mode may be honored for ``provider`` — only when the config's provider matches (or none is recorded), so a stale mode never leaks across a switch.""" p, c = (provider or "").strip().lower(), (configured_provider or "").strip().lower() - return not c or (c == "custom" or c.startswith("custom:") if p == "custom" else c == p) + return not c or (c == "custom" or c.startswith("custom:") if p == "custom" else _same_registered_provider(p, c)) def _configured_api_mode(provider: str, model_cfg: Dict[str, Any]) -> Optional[str]: @@ -267,7 +276,7 @@ def _config_base_url_for_provider(model_cfg: Dict[str, Any], provider: str) -> s configured_provider = _cfg_provider(model_cfg) if provider == "actual": configured_provider = _models.normalize_provider(configured_provider) - return str(model_cfg.get("base_url") or "").strip().rstrip("/") if configured_provider == provider else "" + return str(model_cfg.get("base_url") or "").strip().rstrip("/") if _same_registered_provider(provider, configured_provider) else "" def _anthropic_base_url_override_ok(base_url: str) -> bool: diff --git a/tests/hermes_cli/test_provider_profile_identity.py b/tests/hermes_cli/test_provider_profile_identity.py new file mode 100644 index 0000000000000..e8b88235f95de --- /dev/null +++ b/tests/hermes_cli/test_provider_profile_identity.py @@ -0,0 +1,266 @@ +"""ProviderProfile identity/config regressions through discovery, CLI and runtime. + +The on-disk discovery fixture is adapted from david-bowiegxw's #53054 tests +(commit fcbc30e5defc112a66ef834f6e2e4e46d3cbcec4). The current auth bridge +and transport fallback remain upstream-owned; these tests cover residual +identity and configuration behavior. +""" + +from __future__ import annotations + +import sys + +import pytest +import yaml + +from hermes_cli import auth as auth_mod +from hermes_cli import runtime_provider as rp +from hermes_cli.model_switch import switch_model +from hermes_cli.providers import get_provider, resolve_provider_full +from hermes_constants import get_hermes_home + + +PLUGIN_NAME = "testgw" +PLUGIN_ALIAS = "testgw-alias" +PLUGIN_OTHER_ALIAS = "testgw-other" +PLUGIN_ENV_VAR = "TESTGW_API_KEY" +PLUGIN_URL_VAR = "TESTGW_BASE_URL" +PLUGIN_BASE_URL = "https://gw.example.com/api/coding" +ENV_BASE_URL = "https://env.example.test/api/coding" +CONFIG_BASE_URL = "https://configured.example.test/api/coding" +TEST_KEY = "test-profile-credential" + + +@pytest.fixture(params=[PLUGIN_BASE_URL, ""], ids=["default-url", "env-only-url"]) +def registered_plugin_provider(request, monkeypatch): + """Discover an actual plugin file, then use the production auth bridge. + + Keep previously discovered bundled profiles and restore shared auth state; + only this fixture's imported module is removed on teardown. + """ + import providers as profiles + + monkeypatch.setattr(profiles, "_REGISTRY", dict(profiles._REGISTRY)) + monkeypatch.setattr(profiles, "_ALIASES", dict(profiles._ALIASES)) + monkeypatch.setattr(profiles, "_PROVIDER_LIST_CACHE", None) + monkeypatch.setattr(profiles, "_discovered", False) + auth_before = dict(auth_mod.PROVIDER_REGISTRY) + module_name = "_hermes_user_provider_testgw" + monkeypatch.delitem(sys.modules, module_name, raising=False) + monkeypatch.setattr("agent.models_dev.fetch_models_dev", lambda *a, **kw: {}) + monkeypatch.setenv(PLUGIN_ENV_VAR, TEST_KEY) + monkeypatch.delenv(PLUGIN_URL_VAR, raising=False) + monkeypatch.delenv("TESTGW_URL", raising=False) + if not request.param: + monkeypatch.setenv(PLUGIN_URL_VAR, ENV_BASE_URL) + + plugin_dir = get_hermes_home() / "plugins" / "model-providers" / PLUGIN_NAME + plugin_dir.mkdir(parents=True) + (plugin_dir / "plugin.yaml").write_text( + f"name: {PLUGIN_NAME}\nkind: model-provider\nversion: 0.0.1\n" + "description: Provider identity regression fixture\n" + ) + (plugin_dir / "__init__.py").write_text( + "from providers import register_provider\n" + "from providers.base import ProviderProfile\n\n" + "register_provider(ProviderProfile(\n" + f" name={PLUGIN_NAME!r},\n" + f" aliases={(PLUGIN_ALIAS, PLUGIN_OTHER_ALIAS)!r},\n" + " display_name='Test Gateway',\n" + " api_mode='anthropic_messages',\n" + f" env_vars={(PLUGIN_URL_VAR, PLUGIN_ENV_VAR, 'TESTGW_URL')!r},\n" + f" base_url={request.param!r},\n" + " auth_type='api_key',\n" + "))\n" + "register_provider(ProviderProfile(\n" + " name='foreign-provider', env_vars=('FOREIGN_TESTGW_API_KEY',),\n" + " base_url='https://foreign.example.test/v1', api_mode='codex_responses',\n" + "))\n" + "register_provider(ProviderProfile(\n" + " name='testgw-process', auth_type='external_process',\n" + " base_url='acp+tcp://127.0.0.1:56789', env_vars=(),\n" + "))\n" + ) + try: + for profile in profiles.list_providers(): + if profile.name not in auth_mod.PROVIDER_REGISTRY: + auth_mod._register_plugin_provider(profile) + profile = profiles.get_provider_profile(PLUGIN_NAME) + assert profile is not None + config = auth_mod.PROVIDER_REGISTRY[PLUGIN_NAME] + assert config is auth_mod.PROVIDER_REGISTRY[PLUGIN_ALIAS] + assert config.api_key_env_vars == (PLUGIN_ENV_VAR,) + assert config.base_url_env_var == PLUGIN_URL_VAR + yield profile + finally: + auth_mod.PROVIDER_REGISTRY.clear() + auth_mod.PROVIDER_REGISTRY.update(auth_before) + sys.modules.pop(module_name, None) + + +def _write_model_config(provider, **settings): + (get_hermes_home() / "config.yaml").write_text( + yaml.safe_dump({"model": {"provider": provider, "default": "test-model", **settings}}) + ) + + +@pytest.mark.parametrize("requested", [PLUGIN_NAME, PLUGIN_ALIAS, PLUGIN_OTHER_ALIAS]) +@pytest.mark.parametrize("override", [None, "canonical", "raw"]) +def test_residual_profile_cli_identity(registered_plugin_provider, monkeypatch, requested, override): + profile = registered_plugin_provider + _write_model_config(PLUGIN_NAME) + # Validation is an external model probe; discovery, auth, config and the + # switch's runtime credential resolution are all real. + monkeypatch.setattr( + "hermes_cli.models_validate.validate_requested_model", + lambda *a, **kw: {"accepted": True, "persist": True, "recognized": True, "message": None}, + ) + user_providers = {} + expected_url = profile.base_url or ENV_BASE_URL + expected_id = profile.name + if override: + user_providers[profile.name] = { + "name": "Configured Gateway", "base_url": CONFIG_BASE_URL, + "key_env": PLUGIN_ENV_VAR, + } + expected_url = CONFIG_BASE_URL + if override == "raw": + user_providers[requested] = { + "name": "Raw Override", "base_url": "https://raw.example.test/api/coding", + "key_env": PLUGIN_ENV_VAR, + } + expected_url = user_providers[requested]["base_url"] + expected_id = requested + + result = switch_model( + "test-model", current_provider="openrouter", current_model="previous-model", + current_api_key="foreign-provider-key", explicit_provider=requested, + user_providers=user_providers, custom_providers=[], + ) + assert result.success, result.error_message + assert result.target_provider == expected_id + assert result.base_url == expected_url + assert result.api_key == TEST_KEY + + definition = resolve_provider_full(requested, user_providers=user_providers) + assert definition is not None + assert definition.id == expected_id + assert definition.api_key_env_vars == (PLUGIN_ENV_VAR,) + if not override: + assert definition.base_url_env_var == PLUGIN_URL_VAR + assert definition.source == "plugin-profile" + assert result.api_mode == profile.api_mode + else: + assert definition.source == "user-config" + assert definition.base_url == expected_url + + +@pytest.mark.parametrize("route", ["pooled", "explicit-key", "no-pool"]) +@pytest.mark.parametrize( + "requested,persisted", + [ + (PLUGIN_NAME, PLUGIN_NAME), + (PLUGIN_NAME, PLUGIN_ALIAS), + (PLUGIN_ALIAS, PLUGIN_NAME), + (PLUGIN_ALIAS, PLUGIN_OTHER_ALIAS), + (PLUGIN_OTHER_ALIAS, "foreign-provider"), + (PLUGIN_OTHER_ALIAS, ""), + ], +) +def test_residual_profile_runtime_config(registered_plugin_provider, monkeypatch, route, requested, persisted): + from agent.credential_pool import CredentialPool + + profile = registered_plugin_provider + # A URL without a configured provider deliberately selects the bare-custom + # bypass. Leave it absent when testing the no-provider api_mode contract. + _write_model_config(persisted, base_url=CONFIG_BASE_URL if persisted else "", api_mode="chat_completions") + kwargs = {} + if route == "explicit-key": + kwargs["explicit_api_key"] = TEST_KEY + elif route == "no-pool": + # Force the fallback route with a real empty pool; credentials still + # resolve through the real provider config and scoped environment. + monkeypatch.setattr(rp, "load_pool", lambda provider: CredentialPool(provider, [])) + runtime = rp.resolve_runtime_provider(requested=requested, **kwargs) + + same_profile = persisted in (PLUGIN_NAME, PLUGIN_ALIAS, PLUGIN_OTHER_ALIAS) + expected_url = profile.base_url or ENV_BASE_URL + if same_profile and (route == "no-pool" or (route == "pooled" and profile.base_url)): + expected_url = CONFIG_BASE_URL + assert runtime["provider"] == profile.name + assert runtime["requested_provider"] == requested + assert runtime["base_url"] == expected_url + assert runtime["api_key"] == TEST_KEY + assert runtime["api_mode"] == ("chat_completions" if same_profile or not persisted else profile.api_mode) + if route == "pooled": + assert isinstance(runtime["credential_pool"], CredentialPool) + assert runtime["source"] == f"env:{PLUGIN_ENV_VAR}" + else: + assert "credential_pool" not in runtime + if route == "explicit-key": + assert runtime["source"] == "explicit" + + +@pytest.mark.parametrize("route", ["pooled", "explicit-key", "no-pool", "explicit-url"]) +def test_residual_profile_url_precedence(registered_plugin_provider, monkeypatch, route): + from agent.credential_pool import CredentialPool + + profile = registered_plugin_provider + _write_model_config(PLUGIN_ALIAS, base_url=CONFIG_BASE_URL) + monkeypatch.setenv(PLUGIN_URL_VAR, "https://api.openai.com/v1") + kwargs = {} + if route in ("explicit-key", "explicit-url"): + kwargs["explicit_api_key"] = TEST_KEY + if route == "explicit-url": + kwargs["explicit_base_url"] = PLUGIN_BASE_URL + if route == "no-pool": + monkeypatch.setattr(rp, "load_pool", lambda provider: CredentialPool(provider, [])) + + runtime = rp.resolve_runtime_provider(requested=PLUGIN_OTHER_ALIAS, **kwargs) + expected_url = { + "pooled": "https://api.openai.com/v1", "explicit-key": "https://api.openai.com/v1", + "no-pool": CONFIG_BASE_URL, "explicit-url": PLUGIN_BASE_URL, + }[route] + assert runtime["provider"] == profile.name + assert runtime["base_url"] == expected_url + assert runtime["api_mode"] == ( + "codex_responses" if route in ("pooled", "explicit-key") else profile.api_mode + ) + + +def test_residual_profile_url_is_not_a_credential(registered_plugin_provider, monkeypatch): + from hermes_cli.auth import AuthError + + monkeypatch.delenv(PLUGIN_ENV_VAR) + monkeypatch.setenv(PLUGIN_URL_VAR, ENV_BASE_URL) + definition = get_provider(PLUGIN_ALIAS, allow_network=False) + assert definition is not None + assert definition.api_key_env_vars == (PLUGIN_ENV_VAR,) + with pytest.raises(AuthError, match="No usable credentials"): + rp.resolve_runtime_provider(requested=PLUGIN_ALIAS) + + +def test_residual_external_process_route_keeps_process_auth(registered_plugin_provider, monkeypatch): + """Control: resolving an external profile does not require an API key or launch it.""" + _write_model_config(PLUGIN_ALIAS, base_url=CONFIG_BASE_URL, api_mode="anthropic_messages") + monkeypatch.setattr( + "hermes_cli.models_validate.validate_requested_model", + lambda *a, **kw: {"accepted": True, "persist": True, "recognized": True, "message": None}, + ) + definition = get_provider("testgw-process", allow_network=False) + assert definition is not None + assert definition.auth_type == "external_process" + assert definition.api_key_env_vars == () + runtime = rp.resolve_runtime_provider(requested=definition.id) + assert runtime["source"] == "process" + assert runtime["api_key"] == definition.id + assert runtime["base_url"] == definition.base_url + assert runtime["api_mode"] == "chat_completions" + result = switch_model( + "test-model", current_provider=PLUGIN_NAME, current_model="previous-model", + explicit_provider=definition.id, user_providers={}, custom_providers=[], + ) + assert result.success, result.error_message + assert result.target_provider == definition.id + assert result.api_key == runtime["api_key"] + assert result.base_url == runtime["base_url"]