fix(models): same-URL custom endpoints stop evicting each other's cached catalog; no-probe picker opens revalidate
Two picker-freshness defects in cached_fetch_api_models(): 1. The disk cache row was keyed on base_url only, with the credential fingerprint stored inside the row. N custom_providers entries sharing one proxy URL with different keys (#106184) took turns overwriting the single slot; every sibling then failed the fingerprint check, got an empty catalog, and disappeared from the Desktop pickers (which hide zero-model rows). Key on url#fingerprint so each credential owns a row. 2. cache_only opens (Desktop model.options without refresh) served a past-TTL row for up to 7 days without ever revalidating, so a model loaded on a non-current local endpoint stayed invisible until the user found "Refresh Models". Serve the stale row AND spawn the same off-thread SWR refresh the blocking path uses; the caller still never waits on the network. Live repro (two rows, one URL, keys A/B; real loopback /v1/models): GUI no-probe open before {'proxy-a': ['model-A1'], 'proxy-b': ['model-B1']} after {'proxy-a': ['model-A1','model-A2'], 'proxy-b': ['model-B1']}
This commit is contained in:
+12
-6
@@ -2366,23 +2366,26 @@ def cached_fetch_api_models(
|
||||
if not normalized_url: # nothing to key the cache on
|
||||
return None if cache_only else _live()
|
||||
|
||||
cache_key = f"custom:{normalized_url}"
|
||||
# Key on URL AND credential fingerprint: N ``custom_providers`` rows can share one proxy URL
|
||||
# with distinct keys (#106184). A URL-only key let the last probe overwrite its siblings'
|
||||
# slot, so every other same-URL row failed the fingerprint check, got an empty catalog and
|
||||
# vanished from the no-probe pickers.
|
||||
fp = _custom_endpoint_fingerprint(api_key, api_mode, headers)
|
||||
cache_key = f"custom:{normalized_url}#{fp}"
|
||||
cache = _load_provider_models_cache()
|
||||
entry = cache.get(cache_key)
|
||||
now = time.time()
|
||||
valid = not force_refresh and _cache_entry_valid(entry, fp, allow_empty=isinstance(entry, dict) and entry.get("native_catalog") is True)
|
||||
|
||||
if cache_only:
|
||||
# Same trust window as the SWR tier below, minus the revalidation.
|
||||
return _catalog(entry) if valid and now - entry["at"] < _PROVIDER_MODELS_STALE_SERVE_MAX else None
|
||||
|
||||
if valid:
|
||||
age = now - entry["at"]
|
||||
if age < ttl_seconds:
|
||||
return _catalog(entry)
|
||||
if age < _PROVIDER_MODELS_STALE_SERVE_MAX:
|
||||
# Stale-while-revalidate: serve now, refresh off-thread for the next open.
|
||||
# Stale-while-revalidate: serve now, refresh off-thread for the next open. cache_only
|
||||
# opens (GUI pickers that must not block on a stopped local server) take the same
|
||||
# non-blocking refresh: without it a locally loaded model stayed invisible for the
|
||||
# whole 7-day stale window unless the user found "Refresh Models" (#71169 class).
|
||||
def _refresh_custom():
|
||||
live = _live()
|
||||
return _entry(live) if live or isinstance(live, _NativePickerModelList) else None
|
||||
@@ -2390,6 +2393,9 @@ def cached_fetch_api_models(
|
||||
_spawn_swr_refresh(cache_key, _refresh_custom)
|
||||
return _catalog(entry)
|
||||
|
||||
if cache_only:
|
||||
return None
|
||||
|
||||
live = _live()
|
||||
if live or isinstance(live, _NativePickerModelList):
|
||||
stored = _entry(live, now)
|
||||
|
||||
@@ -28,7 +28,7 @@ class TestCachedFetchApiModels:
|
||||
def test_fresh_entry_served_without_live_fetch(self):
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["m1", "m2"], age_seconds=10)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["m1", "m2"], age_seconds=10)}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "_save_provider_models_cache") as save, \
|
||||
@@ -44,7 +44,7 @@ class TestCachedFetchApiModels:
|
||||
slash — config.yaml entries are not guaranteed to be normalized."""
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["m1"], age_seconds=10)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["m1"], age_seconds=10)}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "fetch_api_models") as live:
|
||||
@@ -59,7 +59,7 @@ class TestCachedFetchApiModels:
|
||||
# live fetch (within the window it stale-serves + refreshes off
|
||||
# thread — covered in TestSalvageFollowups).
|
||||
too_old = mod._PROVIDER_MODELS_STALE_SERVE_MAX + 60
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["old"], age_seconds=too_old)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["old"], age_seconds=too_old)}
|
||||
saved = {}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
@@ -70,8 +70,8 @@ class TestCachedFetchApiModels:
|
||||
)
|
||||
assert out == ["fresh-a", "fresh-b"]
|
||||
live.assert_called_once()
|
||||
assert saved["custom:https://gw.example.com/v1"]["models"] == ["fresh-a", "fresh-b"]
|
||||
assert saved["custom:https://gw.example.com/v1"]["fp"] == "fp"
|
||||
assert saved["custom:https://gw.example.com/v1#fp"]["models"] == ["fresh-a", "fresh-b"]
|
||||
assert saved["custom:https://gw.example.com/v1#fp"]["fp"] == "fp"
|
||||
|
||||
def test_rotated_api_key_busts_cache_even_when_fresh(self):
|
||||
"""A same-age entry with a DIFFERENT fingerprint (key rotated, or
|
||||
@@ -79,7 +79,7 @@ class TestCachedFetchApiModels:
|
||||
credentials' catalog."""
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["old-key-models"], 10, fp="old-fp")}
|
||||
cache = {"custom:https://gw.example.com/v1#old-fp": self._entry(["old-key-models"], 10, fp="old-fp")}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="new-fp"), \
|
||||
patch.object(mod, "_save_provider_models_cache"), \
|
||||
@@ -91,7 +91,7 @@ class TestCachedFetchApiModels:
|
||||
def test_force_refresh_bypasses_fresh_cache(self):
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["stale-but-fresh"], age_seconds=5)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["stale-but-fresh"], age_seconds=5)}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "_save_provider_models_cache"), \
|
||||
@@ -108,7 +108,7 @@ class TestCachedFetchApiModels:
|
||||
cached_provider_model_ids')."""
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["last-known-good"], age_seconds=99999, fp="fp")}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["last-known-good"], age_seconds=99999, fp="fp")}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "_save_provider_models_cache") as save, \
|
||||
@@ -130,14 +130,15 @@ class TestCachedFetchApiModels:
|
||||
|
||||
|
||||
class TestCacheOnly:
|
||||
"""``cache_only=True`` is the no-network read used by picker opens that
|
||||
deliberately skip live probing. It must answer from disk or not at all —
|
||||
never a live fetch, never a background revalidation."""
|
||||
"""``cache_only=True`` is the non-blocking read used by picker opens that
|
||||
deliberately skip live probing. The caller's thread never fetches; a
|
||||
past-TTL entry is served AND revalidated off-thread so a newly loaded local
|
||||
model appears on a later open instead of hiding for the whole stale window."""
|
||||
|
||||
def _entry(self, models, age_seconds, fp="fp"):
|
||||
return {"fp": fp, "at": time.time() - age_seconds, "models": list(models)}
|
||||
|
||||
def _call(self, cache, *, fp="fp", **kwargs):
|
||||
def _call(self, cache, *, fp="fp", expect_revalidate=False, **kwargs):
|
||||
import hermes_cli.models as mod
|
||||
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
@@ -148,13 +149,13 @@ class TestCacheOnly:
|
||||
out = mod.cached_fetch_api_models(
|
||||
"sk-key", "https://gw.example.com/v1", cache_only=True, **kwargs
|
||||
)
|
||||
live.assert_not_called()
|
||||
swr.assert_not_called()
|
||||
live.assert_not_called() # the caller's thread never blocks on the network
|
||||
save.assert_not_called()
|
||||
assert swr.called == expect_revalidate
|
||||
return out
|
||||
|
||||
def test_fresh_entry_is_served(self):
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["m1", "m2"], 10)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["m1", "m2"], 10)}
|
||||
assert self._call(cache) == ["m1", "m2"]
|
||||
|
||||
def test_entry_past_ttl_is_still_served_within_the_stale_window(self):
|
||||
@@ -164,27 +165,27 @@ class TestCacheOnly:
|
||||
import hermes_cli.models as mod
|
||||
|
||||
age = mod._PROVIDER_MODELS_CACHE_TTL + 60
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["m1", "m2"], age)}
|
||||
assert self._call(cache) == ["m1", "m2"]
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["m1", "m2"], age)}
|
||||
assert self._call(cache, expect_revalidate=True) == ["m1", "m2"]
|
||||
|
||||
def test_entry_beyond_the_stale_window_is_a_miss(self):
|
||||
import hermes_cli.models as mod
|
||||
|
||||
age = mod._PROVIDER_MODELS_STALE_SERVE_MAX + 60
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["ancient"], age)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["ancient"], age)}
|
||||
assert self._call(cache) is None
|
||||
|
||||
def test_empty_cache_is_a_miss(self):
|
||||
assert self._call({}) is None
|
||||
|
||||
def test_rotated_credentials_are_a_miss(self):
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["old"], 10, fp="old-fp")}
|
||||
cache = {"custom:https://gw.example.com/v1#old-fp": self._entry(["old"], 10, fp="old-fp")}
|
||||
assert self._call(cache, fp="new-fp") is None
|
||||
|
||||
def test_force_refresh_is_a_miss_rather_than_a_live_fetch(self):
|
||||
"""cache_only outranks force_refresh: the caller has said no network,
|
||||
so an un-revalidatable entry is withheld instead of fetched."""
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["m1"], 10)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["m1"], 10)}
|
||||
assert self._call(cache, force_refresh=True) is None
|
||||
|
||||
def test_missing_base_url_is_a_miss_rather_than_a_live_fetch(self):
|
||||
@@ -274,9 +275,26 @@ class TestCachedFetchApiModelsDiskRoundTrip:
|
||||
mod.cached_provider_model_ids("openrouter")
|
||||
|
||||
cache = mod._load_provider_models_cache()
|
||||
assert cache["custom:https://openrouter.ai/v1"]["models"] == ["custom-endpoint-model"]
|
||||
custom_rows = [v for k, v in cache.items() if k.startswith("custom:https://openrouter.ai/v1#")]
|
||||
assert [row["models"] for row in custom_rows] == [["custom-endpoint-model"]]
|
||||
assert cache["openrouter"]["models"] == ["openrouter-curated-model"]
|
||||
|
||||
def test_same_url_distinct_credentials_keep_separate_rows(self, tmp_path, monkeypatch):
|
||||
"""N custom_providers rows sharing one proxy URL with different keys (#106184): a probe
|
||||
for key B must not evict key A's catalog, and a cache-only read for A must still hit."""
|
||||
import hermes_cli.models as mod
|
||||
|
||||
monkeypatch.setenv("HERMES_HOME", str(tmp_path))
|
||||
monkeypatch.setattr(mod, "fetch_api_models", lambda key, *a, **k: [f"models-for-{key}"])
|
||||
|
||||
url = "https://proxy.example.com/v1"
|
||||
assert mod.cached_fetch_api_models("sk-A", url) == ["models-for-sk-A"]
|
||||
assert mod.cached_fetch_api_models("sk-B", url) == ["models-for-sk-B"]
|
||||
|
||||
monkeypatch.setattr(mod, "fetch_api_models", lambda *a, **k: pytest.fail("cache miss"))
|
||||
assert mod.cached_fetch_api_models("sk-A", url, cache_only=True) == ["models-for-sk-A"]
|
||||
assert mod.cached_fetch_api_models("sk-B", url, cache_only=True) == ["models-for-sk-B"]
|
||||
|
||||
|
||||
class TestSalvageFollowups:
|
||||
"""Follow-up behaviors added while salvaging PR #80740: SWR stale-serve
|
||||
@@ -291,7 +309,7 @@ class TestSalvageFollowups:
|
||||
background refresh is spawned for the next open."""
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["stale-ok"], age_seconds=7200)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["stale-ok"], age_seconds=7200)}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "_spawn_swr_refresh") as spawn, \
|
||||
@@ -302,13 +320,13 @@ class TestSalvageFollowups:
|
||||
assert out == ["stale-ok"]
|
||||
live.assert_not_called()
|
||||
spawn.assert_called_once()
|
||||
assert spawn.call_args[0][0] == "custom:https://gw.example.com/v1"
|
||||
assert spawn.call_args[0][0] == "custom:https://gw.example.com/v1#fp"
|
||||
|
||||
def test_entry_beyond_stale_window_blocks_on_live_fetch(self):
|
||||
import hermes_cli.models as mod
|
||||
|
||||
too_old = mod._PROVIDER_MODELS_STALE_SERVE_MAX + 60
|
||||
cache = {"custom:https://gw.example.com/v1": self._entry(["ancient"], age_seconds=too_old)}
|
||||
cache = {"custom:https://gw.example.com/v1#fp": self._entry(["ancient"], age_seconds=too_old)}
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value=cache), \
|
||||
patch.object(mod, "_custom_endpoint_fingerprint", return_value="fp"), \
|
||||
patch.object(mod, "_save_provider_models_cache"), \
|
||||
@@ -336,12 +354,12 @@ class TestSalvageFollowups:
|
||||
with patch.object(mod, "_load_provider_models_cache", return_value={}), \
|
||||
patch.object(mod, "_save_provider_models_cache", side_effect=fake_save):
|
||||
mod._spawn_swr_refresh(
|
||||
"custom:https://gw.example.com/v1",
|
||||
"custom:https://gw.example.com/v1#fp",
|
||||
lambda: {"fp": "fp", "at": time.time(), "models": ["refreshed"]},
|
||||
)
|
||||
assert done.wait(timeout=5), "background refresh did not complete"
|
||||
assert saved["custom:https://gw.example.com/v1"]["models"] == ["refreshed"]
|
||||
assert "custom:https://gw.example.com/v1" not in mod._swr_refresh_inflight
|
||||
assert saved["custom:https://gw.example.com/v1#fp"]["models"] == ["refreshed"]
|
||||
assert "custom:https://gw.example.com/v1#fp" not in mod._swr_refresh_inflight
|
||||
|
||||
def test_corrupt_at_field_degrades_to_live_fetch_instead_of_raising(self):
|
||||
"""provider_models_cache.json is user-editable; a corrupted 'at' must
|
||||
@@ -349,7 +367,7 @@ class TestSalvageFollowups:
|
||||
import hermes_cli.models as mod
|
||||
|
||||
cache = {
|
||||
"custom:https://gw.example.com/v1": {
|
||||
"custom:https://gw.example.com/v1#fp": {
|
||||
"fp": "fp", "at": "yesterday", "models": ["corrupt-row"],
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1776,7 +1776,7 @@ def _seed_custom_model_cache(monkeypatch, models, *, age_seconds=10):
|
||||
|
||||
fp = models_mod._custom_endpoint_fingerprint("", None, None)
|
||||
cache = {
|
||||
f"custom:{_LOCAL_ENDPOINT}": {
|
||||
f"custom:{_LOCAL_ENDPOINT}#{fp}": {
|
||||
"fp": fp,
|
||||
"at": time.time() - age_seconds,
|
||||
"models": list(models),
|
||||
@@ -2061,7 +2061,7 @@ def test_api_mode_rows_do_not_share_a_cached_catalog(monkeypatch):
|
||||
# Only the OpenAI-mode probe (api_mode=None) is on disk.
|
||||
fp = models_mod._custom_endpoint_fingerprint("sk-shared", None, None)
|
||||
cache = {
|
||||
f"custom:{_SHARED_PROXY_URL}": {
|
||||
f"custom:{_SHARED_PROXY_URL}#{fp}": {
|
||||
"fp": fp,
|
||||
"at": time.time() - 10,
|
||||
"models": list(openai_catalog),
|
||||
|
||||
@@ -0,0 +1,66 @@
|
||||
"""Endpoint-probe contract for the Desktop local/custom endpoint validators (#63472).
|
||||
|
||||
httpx honours ``HTTP(S)_PROXY`` (and the Windows system proxy) but never the proxy bypass list,
|
||||
so a system proxy answered ``127.0.0.1`` probes with its own error page. The GUI then reported
|
||||
"advertised no models" for a llama.cpp server the CLI (urllib, honours the bypass) saw fine.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
|
||||
@pytest.mark.parametrize(
|
||||
"url, trusts_env",
|
||||
[
|
||||
("http://127.0.0.1:8080/v1/models", False),
|
||||
("http://localhost:11434/v1/models", False),
|
||||
("http://192.168.1.20:8000/v1/models", False),
|
||||
("https://api.example.com/v1/models", True),
|
||||
],
|
||||
)
|
||||
def test_local_endpoint_probes_bypass_env_proxy(url, trusts_env, monkeypatch):
|
||||
from hermes_cli.web_routers.config_env import _endpoint_probe_client
|
||||
|
||||
monkeypatch.setenv("HTTPS_PROXY", "http://127.0.0.1:1")
|
||||
monkeypatch.setenv("HTTP_PROXY", "http://127.0.0.1:1")
|
||||
client = _endpoint_probe_client(url, 1.0)
|
||||
assert client.trust_env is trusts_env
|
||||
|
||||
|
||||
def test_openai_base_url_probe_names_the_http_status_instead_of_no_models(monkeypatch):
|
||||
"""A reachable endpoint answering non-2xx with no model list is a failure the user can act on,
|
||||
not an empty catalog the GUI turns into 'start a model on that endpoint'."""
|
||||
import hermes_cli.web_routers.config_env as mod
|
||||
from hermes_cli.web_models import EnvVarUpdate
|
||||
|
||||
class _Resp:
|
||||
status_code = 502
|
||||
is_success = False
|
||||
|
||||
def json(self):
|
||||
return {"error": "proxy upstream unavailable"}
|
||||
|
||||
class _Client:
|
||||
def __init__(self, *a, **k):
|
||||
pass
|
||||
|
||||
async def __aenter__(self):
|
||||
return self
|
||||
|
||||
async def __aexit__(self, *a):
|
||||
return False
|
||||
|
||||
async def get(self, *a, **k):
|
||||
return _Resp()
|
||||
|
||||
monkeypatch.setattr(mod, "_endpoint_probe_client", lambda url, timeout: _Client())
|
||||
monkeypatch.setattr(mod, "_require_token", lambda request: None)
|
||||
|
||||
body = EnvVarUpdate(key="OPENAI_BASE_URL", value="http://127.0.0.1:8080/v1", api_key="")
|
||||
out = asyncio.run(mod.validate_provider_credential(body, request=None)) # type: ignore[arg-type]
|
||||
|
||||
assert out["ok"] is False and out["reachable"] is True
|
||||
assert "HTTP 502" in out["message"]
|
||||
Reference in New Issue
Block a user