diff --git a/hermes_cli/runtime_provider.py b/hermes_cli/runtime_provider.py index 83e925da03..285f4e3f2b 100644 --- a/hermes_cli/runtime_provider.py +++ b/hermes_cli/runtime_provider.py @@ -1009,7 +1009,19 @@ def canonical_custom_identity( # Only return it when it actually resolves to a configured custom entry, # so we never invent a `custom:` that resolution can't honor. try: - if _get_named_custom_provider(candidate) is not None: + entry = _get_named_custom_provider(candidate) + if entry is not None: + # ``candidate`` matched, but it may be the entry's DISPLAY NAME — + # ``_get_named_custom_provider`` accepts either spelling. For a + # keyed ``providers:`` entry the display name is not the durable + # identity, so re-resolve through the endpoint the matched entry + # owns and return the same config-key slug every other path + # returns (7b5a18817). Without this, a display name that differs + # from its key heals to ``custom:`` and stops + # matching the persisted identity. + identity = find_custom_provider_identity(str(entry.get("base_url") or "")) + if identity: + return identity if candidate_norm.startswith("custom:"): return candidate_norm return f"custom:{candidate_norm}" diff --git a/tests/hermes_cli/test_canonical_custom_identity.py b/tests/hermes_cli/test_canonical_custom_identity.py new file mode 100644 index 0000000000..80ef6c3624 --- /dev/null +++ b/tests/hermes_cli/test_canonical_custom_identity.py @@ -0,0 +1,100 @@ +"""``canonical_custom_identity`` must return the durable config-key identity. + +A keyed ``providers:`` entry's identity is its config key, not its display +name — ``custom_provider_slug`` encodes that, and the endpoint- and +model-based recovery sources both honour it. The configured-provider fallback +built its slug from whatever string the caller had, so a display name that +differs from its key healed to ``custom:``: a second identity +for the same endpoint that no longer matches what persistence and routing +store. + +Both spellings match the entry (``_get_named_custom_provider`` accepts +either), so the test asserts they converge on one identity rather than +asserting any particular spelling is rejected. +""" + +from __future__ import annotations + +import pytest + +from hermes_cli import runtime_provider as rp + +PROVIDER_KEY = "my-endpoint" +DISPLAY_NAME = "My Endpoint Display" +BASE_URL = "https://example.invalid/v1" +MODEL = "cool-model-1" + +CANONICAL = f"custom:{PROVIDER_KEY}" + + +@pytest.fixture +def keyed_provider_config(monkeypatch): + """A ``providers:`` entry whose display name differs from its config key.""" + config = { + "providers": { + PROVIDER_KEY: { + "name": DISPLAY_NAME, + "api": BASE_URL, + "api_key": "sk-test", + "default_model": MODEL, + "models": [MODEL], + } + } + } + monkeypatch.setattr(rp, "load_config", lambda *a, **k: config) + monkeypatch.setattr("hermes_cli.config.load_config", lambda *a, **k: config) + monkeypatch.setattr(rp, "_get_model_config", lambda: {}) + return config + + +def test_display_name_heals_to_the_config_key_identity(keyed_provider_config): + """The regression: the display-name spelling must not mint a second identity.""" + assert rp.canonical_custom_identity(config_provider=DISPLAY_NAME) == CANONICAL + + +def test_config_model_provider_display_name_heals_too(keyed_provider_config, monkeypatch): + """Same path reached through ``config.model.provider`` rather than an argument.""" + monkeypatch.setattr(rp, "_get_model_config", lambda: {"provider": DISPLAY_NAME}) + assert rp.canonical_custom_identity() == CANONICAL + + +def test_config_key_spelling_still_resolves(keyed_provider_config): + """The spelling that already worked keeps working.""" + assert rp.canonical_custom_identity(config_provider=PROVIDER_KEY) == CANONICAL + + +def test_all_recovery_sources_agree_on_one_identity(keyed_provider_config): + """Endpoint, model and configured-provider recovery must not disagree. + + Three sources feeding the same session-identity slot is only safe while + they agree; a divergent one silently splits an endpoint in two. + """ + by_url = rp.canonical_custom_identity(base_url=BASE_URL) + by_model = rp.canonical_custom_identity(model=MODEL) + by_config = rp.canonical_custom_identity(config_provider=DISPLAY_NAME) + + assert {by_url, by_model, by_config} == {CANONICAL} + + +def test_unconfigured_candidate_still_returns_none(keyed_provider_config): + """Fail-closed contract: never invent an identity resolution can't honour.""" + assert rp.canonical_custom_identity(config_provider="not-a-configured-entry") is None + + +def test_legacy_unkeyed_entry_keeps_its_name_identity(monkeypatch): + """``custom_providers:`` entries have no key, so the name stays the identity.""" + config = { + "custom_providers": [ + { + "name": "Legacy Endpoint", + "base_url": "https://legacy.invalid/v1", + "api_key": "sk-legacy", + "models": ["legacy-model"], + } + ] + } + monkeypatch.setattr(rp, "load_config", lambda *a, **k: config) + monkeypatch.setattr("hermes_cli.config.load_config", lambda *a, **k: config) + monkeypatch.setattr(rp, "_get_model_config", lambda: {}) + + assert rp.canonical_custom_identity(config_provider="Legacy Endpoint") == "custom:legacy-endpoint"