diff --git a/hermes_cli/models.py b/hermes_cli/models.py index e8fa909234..cbc288a69d 100644 --- a/hermes_cli/models.py +++ b/hermes_cli/models.py @@ -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) diff --git a/tests/hermes_cli/test_cached_fetch_api_models.py b/tests/hermes_cli/test_cached_fetch_api_models.py index 24246fd0ea..0424b7ff87 100644 --- a/tests/hermes_cli/test_cached_fetch_api_models.py +++ b/tests/hermes_cli/test_cached_fetch_api_models.py @@ -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"], } } diff --git a/tests/hermes_cli/test_model_switch_custom_providers.py b/tests/hermes_cli/test_model_switch_custom_providers.py index b4262c01a2..a5e4768742 100644 --- a/tests/hermes_cli/test_model_switch_custom_providers.py +++ b/tests/hermes_cli/test_model_switch_custom_providers.py @@ -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), diff --git a/tests/hermes_cli/test_web_routers_endpoint_probe.py b/tests/hermes_cli/test_web_routers_endpoint_probe.py new file mode 100644 index 0000000000..2b30242864 --- /dev/null +++ b/tests/hermes_cli/test_web_routers_endpoint_probe.py @@ -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"]