fix(cli): heal a bare custom provider to its config key, not its display name
7b5a18817 migrated the sibling slug sites to custom_provider_slug, which
keeps a keyed providers: entry's config key as its durable identity. It
covered find_custom_provider_identity_by_model; canonical_custom_identity's
third recovery source - the configured-provider fallback - still built
f"custom:{normalized}" out of whatever string the caller happened to hold.
_get_named_custom_provider matches on either spelling, so a display name
that differs from its config key matches the entry and then heals to
custom:<display-name>. That is a second identity for one endpoint: the
endpoint- and model-based sources of the same function return
custom:<config-key>, and so does everything that persists or restores a
session's provider override. canonical_custom_identity exists precisely to
make a bare "custom" routable again, and tui_gateway calls it on the
session-persist, resume and recovery paths - so the divergence lands in
stored session identity.
Re-resolve through the endpoint the matched entry owns, reusing the
function's own URL-based canonicaliser rather than duplicating the match
logic. Legacy unkeyed custom_providers: entries keep their name identity,
and an unconfigured candidate still returns None.
This commit is contained in:
@@ -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:<x>` 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:<display-name>`` 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}"
|
||||
|
||||
@@ -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:<display-name>``: 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"
|
||||
Reference in New Issue
Block a user