jinghui/hermes-webui
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge PR #7165 into stage/7165-custom-allowlist

Browse Source
This commit is contained in:
nesquena-hermes committed 2026-09-23 10:22:41 +00:00
2 parents 5e16dd860f 0fcd584274
commit dc554cbd1c
5 files changed
+646 -183
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

api/routes.py
+126 -7
View File
@@ -21896,6 +21896,99 @@ def _handle_live_models(handler, parsed):
_env = str(_cp.get("key_env") or "").strip()
return os.getenv(_env, "").strip() if _env else ""
def _custom_provider_models_discover_is_false(_cp):
"""True when ``discover_models`` is an explicit ``false`` opt-out.
Mirrors ``api.config._provider_discover_allowed`` / Hermes
Agent ``model_switch_providers._discover_flag``: ``discover_models``
defaults to True, and the string forms ``"false"``/``"no"``/``"0"``
(case-insensitive) mean False. Used to decide whether a
dict-shaped ``models`` mapping is a hand-pinned allowlist (only
when discovery is off) rather than per-model metadata.
"""
_discover = _cp.get("discover_models", True) if isinstance(_cp, dict) else True
if isinstance(_discover, str):
return _discover.strip().lower() in {"false", "no", "0"}
return not bool(_discover)
def _custom_provider_allowlist_ids(_cp):
"""Plural ``models`` list only — the explicit allowlist signal.
The singular ``model`` field is sticky/default metadata, NOT an
allowlist: it must not gate live-catalog filtering, otherwise a
provider configured with only ``model: assistant`` (no ``models``
list) would be collapsed to a single model. Only an explicit
``models`` allowlist expresses "show exactly these models".
An auto-discovered catalog is NOT an allowlist: when Hermes
persisted discovery results back into config (``models: {...}``
plus ``models_discovered: true``), that mapping is a snapshot
of what the gateway exposed at discovery time. Gating on it
would permanently pin the live catalog to the first-discovery
set, silently dropping any model the user pulls in later
(LM Studio / Ollama). ``_provider_models_are_discovered_catalog()``
is the shared predicate the ``/api/models`` path already uses;
when it says "discovered", return no allowlist and let the
live probe win. An explicit ``discover_models: false`` opt-out
re-pins the catalog (the predicate accounts for it), so a
hand-pinned discovered catalog still filters.
A dict-shaped ``models`` mapping that is NOT marked discovered is
*per-model metadata* written by the Hermes Agent setup flow
(``hermes_cli/model_switch.py::_save_custom_provider`` and the
setup wizard) — e.g. ``{chat-a: {context_length: 128000}}``. It is
not a catalog narrow: treating its keys as an allowlist would
collapse the live picker to the single saved default (keyless
Ollama) while the CLI live-probe shows the full catalog. Only an
explicit ``discover_models: false`` opts into treating the dict
keys as a pinned allowlist. List/JSON-array-string/Python-literal
shapes remain plain allowlists.
The plural value is decoded through ``_parse_config_string_list()``
because ``hermes config set`` / JSON-mode editor saves persist
lists as quoted JSON-array strings (``'["chat-a","chat-b"]'``) or
Python literals (``"['chat-a']"``). Handling only ``dict``/``list``
made a serialized allowlist fall through to ``[]``, which the
caller reads as "no allowlist configured" and floods the picker
with the full upstream catalog — the same class of bug as the
``skills.disabled`` regression (#7120 / #7134).
"""
from api.config import _provider_models_are_discovered_catalog
if _provider_models_are_discovered_catalog(_cp):
return []
_ids = []
_models = _cp.get("models")
# Serialized shapes only: ``hermes config set`` / JSON-mode
# editor saves persist lists as quoted JSON-array strings
# (``'["chat-a","chat-b"]'``) or Python literals
# (``"['chat-a']"``). Decode those through the shared helper so
# they are honored; native dict/list values are walked below so
# dict *entries* keep their id|model|name metadata (the decoder
# str()-ifies list members, which would mangle them).
if isinstance(_models, str):
_models = _parse_config_string_list(_models)
if isinstance(_models, dict):
# A dict-shaped ``models`` is per-model metadata written by
# the Hermes Agent setup flow, NOT a catalog narrow. Only a
# ``discover_models: false`` opt-out treats the dict keys as a
# pinned allowlist; otherwise return the empty allowlist so the
# live probe returns the full catalog.
if not _custom_provider_models_discover_is_false(_cp):
return []
for _mid in _models:
if isinstance(_mid, str) and _mid.strip():
_ids.append(_mid.strip())
elif isinstance(_models, (list, tuple)):
for _item in _models:
if isinstance(_item, str) and _item.strip():
_ids.append(_item.strip())
elif isinstance(_item, dict):
_mid = _item.get("id") or _item.get("model") or _item.get("name")
if _mid and str(_mid).strip():
_ids.append(str(_mid).strip())
return _ids
# For 'custom' and 'custom:*' providers, provider_model_ids()
# returns [] because they aren't real hermes_cli endpoints.
# Fall back to the custom_providers entries from config.yaml so
@@ -21904,11 +21997,13 @@ def _handle_live_models(handler, parsed):
# Collect config-specified model IDs separately so they don't
# prevent the live fetch below from running (#3718).
_config_ids = []
_allowlist_ids = []
if provider == "custom" or provider.startswith("custom:"):
for _cp in _custom_provider_entries_for_request():
if custom_provider_entry is None:
custom_provider_entry = _cp
_config_ids.extend(_custom_provider_model_ids(_cp))
_allowlist_ids.extend(_custom_provider_allowlist_ids(_cp))
# Always try live fetch for custom providers — config entries are a
# fallback, not a replacement. The live endpoint should return ALL
@@ -21979,15 +22074,39 @@ def _handle_live_models(handler, parsed):
except Exception as _fetch_err:
logger.debug("Live fetch from custom provider failed: %s", _fetch_err)
# If live fetch succeeded, merge with config entries (live takes
# priority). If live fetch failed, fall back to config-only list.
# If live fetch succeeded, filter the live catalog down to the
# config-declared models allowlist first, then append any
# allowlisted models the live endpoint didn't return. Custom
# providers (esp. New-API-style gateways) expose their ENTIRE
# catalog via /v1/models — including image/audio models that
# are not chat models. The picker must not surface models the
# user never declared in config.yaml. Only a NON-EMPTY explicit
# ``models`` allowlist gates the filter — a provider configured
# with just a singular ``model`` (no ``models`` list) keeps the
# unfiltered live catalog. When no allowlist is configured,
# return the live list as-is (preserves the pre-filter
# discovery behaviour).
#
# An empty allowlist (``models: []``, ``models: "[]"``, or a
# value that decodes to no usable ids) is deliberately treated
# as "not configured", NOT as "allow nothing". Gating on
# declared-ness instead would emit an EMPTY picker and make the
# provider unselectable — a harder failure than surfacing a few
# extra models, and unrecoverable from the UI because
# ``custom_providers`` is hand-edited in config.yaml (the WebUI
# has no write path for it). ``[]`` in practice means a
# leftover/placeholder key, not an intentional deny-all, and no
# deny-all use case exists: a provider the user wants hidden is
# removed from ``custom_providers`` outright.
if ids:
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
if _allowlist_ids:
_allowlist_set = set(_allowlist_ids)
ids = [m for m in ids if m in _allowlist_set] or []
for _aid in _allowlist_ids:
if _aid not in ids:
ids.append(_aid)
else:
ids = list(_config_ids)
ids = list(_allowlist_ids or _config_ids)
# ── OpenAI-compat live fetch fallback ──────────────────────────────────
# When provider_model_ids() is unavailable or returns [] for a provider
docs/CONTRACTS.md
+9
View File
@@ -89,6 +89,15 @@ contributor guidance; it does not change runtime behavior or CI gates.
`models_cache.json` refresh timestamps), the stat-identity fallbacks, and the
schema/version stamps. Start here before changing model-catalog caching, the
`/api/models` cache keys, or the Codex catalog dependency (#2443, #7540).
- [`docs/architecture/live-models-allowlist.md`](architecture/live-models-allowlist.md):
current contract for how `/api/models/live` filters a custom provider's
upstream catalog: the four signals in evaluation order (discovered catalog
defers to the live probe, explicit plural `models:` allowlist filters,
singular `model:` never gates, no allowlist shows the full catalog), the
serialized-list shapes `hermes config set` persists, the deliberate
empty-allowlist-is-not-configured rule, and the probe-failure fallback.
Start here before changing custom-provider model filtering or
discovery-vs-allowlist semantics (#7165, #7404).
- [`docs/architecture/profile-home-resolve-cache.md`](architecture/profile-home-resolve-cache.md):
current contract for the call-scoped memoization around
`_resolve_profile_home_param()` in `api/workspace.py`: what it caches, the
docs/architecture/live-models-allowlist.md
+123
View File
@@ -0,0 +1,123 @@
# Live model picker allowlist contract (`/api/models/live`)
This document records the runtime contract for how `/api/models/live` decides
which models a `custom` / `custom:*` provider contributes to the chat model
picker. It describes shipped behavior and changes no runtime behavior. It was
added because the #7165 review flagged the contract as undocumented: the
filtering rules live in `_handle_live_models` (`api/routes.py`) and had only
code comments backing them.
Start here before changing how the live catalog of a custom provider is
filtered, how discovery snapshots are distinguished from allowlists, or what
the endpoint returns when the upstream probe fails.
## Why the endpoint filters at all
OpenAI-compatible gateways (New-API-style aggregators, LM Studio, Ollama
proxies) expose their **entire** catalog from `/v1/models`, including
image-generation, audio, TTS, and embedding models that are not chat models.
The background enrichment path must not surface models the user never opted
into — but it also must not hide models the gateway gained since config was
last edited. The contract below is how those two goals are reconciled.
## The four signals, in evaluation order
For a custom provider the handler consults these signals:
1. **Discovered catalog → no allowlist (live probe wins).**
When the provider entry carries `models_discovered: true` and discovery is
allowed (`discover_models` is not an explicit opt-out), the `models:`
mapping is a *snapshot Hermes persisted from a past discovery*, not a
hand-curated list. It must not gate the live probe: a model the user pulls
into LM Studio / Ollama after discovery has to appear in the picker.
Detected by the shared predicate
`api.config._provider_models_are_discovered_catalog()` — the same
predicate the `/api/models` catalog path uses, so both endpoints agree.
With this signal, no allowlist is derived and the full live catalog is
returned.
2. **Explicit plural `models:` allowlist → filter.**
A non-empty plural `models:` value on an entry that is *not* a discovered
catalog expresses "show exactly these models". The live catalog is
intersected with it, then any allowlisted model missing from the live
response is appended (so an allowlisted-but-offline model stays
selectable). This is the #7165 filter. Only **list** and **string**
(serialized) shapes are plain allowlists — see "Dict-shaped `models:`"
below.
3. **Singular `model:` → never gates.**
The singular field is sticky/default metadata (the model to preselect),
not an allowlist. A provider configured only with `model: assistant` keeps
the full unfiltered live catalog; gating on it would collapse discovery to
a single model.
4. **No allowlist → full live catalog.**
Preserves the pre-#7165 discovery behavior for setups that rely on it.
### Serialized shapes
`hermes config set` and the JSON-mode editor persist lists as quoted
JSON-array strings (`'["chat-a","chat-b"]'`) or Python literals
(`"['chat-a']"`). The plural value is decoded through the shared
`_parse_config_string_list()` before id extraction — but only when the value
is a `str`; native `dict` / `list` values are walked directly so dict *entries*
keep their `id|model|name` metadata (the decoder stringifies members). A
dict-only parser drops serialized allowlists to "no allowlist" and floods the
picker — the same bug class as `skills.disabled` (#7120 / #7134).
### Dict-shaped `models:`
A dict-shaped `models:` mapping (e.g. `{chat-a: {context_length: 128000}}`)
is *per-model metadata* written by the Hermes Agent setup flow —
`hermes_cli/model_switch.py::_save_custom_provider` and the setup wizard —
**not** a catalog narrow. Treating its keys as an allowlist would collapse the
live picker to the single saved default (keyless Ollama) while the CLI
live-probe shows the full catalog. So a dict is **never** a plain allowlist:
- without an explicit `discover_models: false`, a dict-shaped `models:`
contributes **no allowlist** and the full live catalog is returned;
- with `discover_models: false` (bool `False` or the string forms
`"false"`/`"no"`/`"0"`, case-insensitive — mirroring
`model_switch_providers.py:557`), the dict **keys** are honored as a pinned
allowlist.
This complements the discovered-catalog rule: `models_discovered: true`
without the opt-out is already ignored (signal 1). The dict guard covers the
Agent-setup shape that is *not* flagged discovered.
If someone later "fixes" the dict shape back into an always-allowlist, the
regression test `test_dict_models_without_discover_false_does_not_gate` fails
and forces the trade-off to be re-argued.
### Empty allowlist semantics
`models: []`, `models: "[]"`, an empty mapping, or a blank scalar are
deliberately treated as **not configured**, never as "allow nothing". The
filter runs only on a non-empty allowlist because gating on declared-ness
would emit an empty picker and make the provider unselectable — a harder
failure than showing a few extra models, and unrecoverable from the UI
(`custom_providers` has no WebUI write path; it is hand-edited in
config.yaml). There is no deny-all use case: a provider the user wants hidden
is removed from `custom_providers` outright.
### Probe failure / empty probe
When the upstream probe fails or returns no models, the endpoint falls back
to `allowlist or config ids` — the non-empty plural allowlist first, so a
singular `model:` outside the allowlist cannot leak in precisely when the
probe is unavailable; with no allowlist, the configured ids (singular +
plural) are served. A discovered catalog therefore still serves its saved
models on a transient probe failure instead of emptying the picker.
## Tests
- `tests/test_issue3718_live_models_custom_probe.py` — real-handler
(`_handle_live_models`) coverage of every signal above: plural list /
JSON-array-string / Python-literal / list-of-dicts filtering, dict-shaped
`models:` (not gated) and dict + `discover_models: false` (pinned),
singular passthrough, malformed safety, discovered-catalog passthrough,
`discover_models: false` pinning, and both fallback branches.
- `tests/test_issue7404_models_discovered_not_allowlist.py` — the
`/api/models` side of the discovered-catalog contract.
- `tests/test_byok_model_dropdown.py` — picker rendering, profile-scoped
caching, and current-session model preservation around this endpoint.
tests/test_byok_model_dropdown.py
+8 -1
View File
@@ -291,6 +291,13 @@ class TestLiveModelsCustomProviderFallback:
"model": "gpt-5.5",
"models": {"gpt-5.5-mini": {}},
"base_url": "https://right.codes/codex/v1",
# dict-shaped ``models`` is per-model metadata (Agent setup
# flow) and only narrows the live catalog when discovery is
# explicitly off — the new #7165 contract. This test still
# wants provider-scoping (no sibling leak), so pin the
# catalog via ``discover_models: false`` instead of relying
# on the removed dict-as-allowlist behavior.
"discover_models": False,
},
{
"name": "infini-ai",
@@ -308,7 +315,7 @@ class TestLiveModelsCustomProviderFallback:
resp = self._call_live_models(monkeypatch, cfg, "custom:rightcode-codex")
assert resp["provider"] == "custom:rightcode-codex"
assert [m["id"] for m in resp["models"]] == ["gpt-5.5", "gpt-5.5-mini"]
assert [m["id"] for m in resp["models"]] == ["gpt-5.5-mini"]
def test_bare_custom_fallback_ignores_named_custom_provider_models(self, monkeypatch):
"""Bare custom only represents unnamed custom entries, not named siblings."""
tests/test_issue3718_live_models_custom_probe.py
+380 -175
View File
@@ -1,193 +1,398 @@
"""Regression test for #3718 -- /api/models/live skips probe for custom providers.
"""Handler-level regression tests for /api/models/live custom-provider probing.
When a custom provider (``custom_providers`` entry in config.yaml) has a
``model:`` field but no ``models:`` allowlist, the live endpoint previously
populated ``ids`` from the config entry and then skipped the live ``/v1/models``
probe (guarded by ``if not ids``). The fix collects config-specified model IDs
in a separate ``_config_ids`` list so the live fetch always runs, and merges
config entries as a fallback after the fetch.
These tests invoke the real ``routes._handle_live_models`` and inspect the
emitted ``models`` payload. Earlier revisions of this file re-implemented the
filtering loop locally, which meant they passed unchanged even with the
production filter deleted (a vacuous oracle). Every case here must fail if the
allowlist gating in ``api/routes.py`` regresses.
Covered shapes:
* native ``models`` list → live catalog filtered
* JSON-array-string ``models`` → live catalog filtered (#7120 class)
* Python-literal-string ``models`` → live catalog filtered
* singular ``model`` only, no ``models`` → live catalog NOT gated
* malformed serialized ``models`` → safe, treated as scalar name
* allowlisted model absent from live catalog → appended
* live probe failure → falls back to config ids
* dict-shaped ``models`` (Agent setup) → live catalog NOT gated (per-model
metadata, not an allowlist)
* dict-shaped ``models`` + ``discover_models:
false`` → pinned to dict keys (explicit opt-out)
* ``models_discovered: true`` catalog → live catalog NOT gated (discovery
metadata is not an allowlist)
* discovered + ``discover_models: false`` → still pinned (explicit opt-out)
"""
import io
import json
import pathlib
import unittest
from unittest import mock
import sys
import types
from urllib.parse import urlparse
REPO = pathlib.Path(__file__).parent.parent
ROUTES_PY = REPO / "api" / "routes.py"
import pytest
class TestLiveModelsCustomProviderProbe(unittest.TestCase):
"""Live endpoint must probe the upstream /v1/models even when config has entries."""
def _install_provider_model_ids(monkeypatch, fn):
"""Stub hermes_cli.models.provider_model_ids (returns [] for custom:*)."""
hermes_cli = types.ModuleType("hermes_cli")
hermes_cli.__path__ = []
models = types.ModuleType("hermes_cli.models")
models.provider_model_ids = fn
monkeypatch.setitem(sys.modules, "hermes_cli", hermes_cli)
monkeypatch.setitem(sys.modules, "hermes_cli.models", models)
def test_custom_provider_with_model_field_probes_upstream(self):
"""When a custom provider has model: in config, live fetch must still run.
Before the fix, _custom_provider_model_ids() populated ids=["assistant"],
and the `if not ids` guard prevented the upstream /v1/models probe from
running. The endpoint returned only ["assistant"] instead of the full
upstream catalog.
class _FakeResponse(io.BytesIO):
def __enter__(self):
return self
We test the fix by simulating the logic directly: config provides
model IDs, the upstream probe returns a different set, and the result
must contain BOTH the live models and the config fallback.
"""
# Simulate config-specified model IDs
_config_ids = ["assistant"]
def __exit__(self, *_exc):
self.close()
return False
# Simulate upstream /v1/models response
live_models = [
"assistant",
"assistant-pro",
"assistant-zdr",
"gpt-5.5:pt",
"claude-sonnet-4.6:pt",
]
# Simulate the merge logic from the fix:
# if ids: merge config entries not in live set
# else: fall back to config-only list
ids = list(live_models) # live fetch succeeded
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" from config should not duplicate the live result
self.assertEqual(ids, live_models)
# All live models must be present
for m in live_models:
self.assertIn(m, ids)
def test_custom_provider_falls_back_to_config_when_probe_fails(self):
"""When the upstream probe fails, config entries must be used as fallback."""
_config_ids = ["assistant", "custom-only-model"]
# Simulate failed live fetch: ids stays empty
ids = []
# Fallback logic from the fix
if not ids:
ids = list(_config_ids)
self.assertEqual(ids, ["assistant", "custom-only-model"])
def test_config_only_model_appended_when_not_in_live_results(self):
"""Config-specified models not in the live results must be appended."""
_config_ids = ["assistant", "local-finetune"]
live_models = ["assistant", "assistant-pro", "gpt-5.5:pt"]
ids = list(live_models)
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
# "assistant" is in both, "local-finetune" is config-only
self.assertIn("local-finetune", ids)
self.assertIn("assistant-pro", ids)
self.assertEqual(ids.count("assistant"), 1, "assistant must not be duplicated")
def test_live_fetch_guard_no_longer_checks_ids(self):
"""The live fetch block must not be guarded by `if not ids`.
This is a structural regression guard: the fix replaced
`if not ids and (provider == "custom" ...)` with
`if provider == "custom" ...` so the probe always runs regardless
of whether config entries populated ids.
"""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the "Always try live fetch" comment (added by the fix)
marker = "Always try live fetch for custom providers"
self.assertIn(marker, source, (
"routes.py must contain the 'Always try live fetch' comment (#3718)"
))
# Extract the block between the marker and the next major section
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
self.assertNotEqual(next_section, -1, "could not find next section marker")
block = source[marker_pos:next_section]
# The old `if not ids and` guard must NOT appear in this block
self.assertNotIn("if not ids and", block, (
"Live fetch for custom providers must not be guarded by 'if not ids' (#3718)"
))
def test_mocked_live_fetch_returns_full_catalog_plus_config_entry(self):
"""Integration test: mock urlopen and exercise the real fetch/parse/merge path.
This test patches urllib.request.urlopen to return a fake /v1/models
response, then runs the same fetch+merge logic that the live handler
uses, verifying that config entries are merged and live models are
included.
"""
fake_models = {"data": [
{"id": "assistant"},
{"id": "assistant-pro"},
{"id": "gpt-5.5:pt"},
]}
fake_body = json.dumps(fake_models).encode("utf-8")
fake_resp = mock.MagicMock()
fake_resp.read.return_value = fake_body
fake_resp.__enter__ = mock.MagicMock(return_value=fake_resp)
fake_resp.__exit__ = mock.MagicMock(return_value=False)
_config_ids = ["assistant", "local-only"]
with mock.patch("urllib.request.urlopen", return_value=fake_resp):
# Replicate the fetch+parse logic from routes.py
def _install_live_catalog(monkeypatch, routes, catalog, *, fail=False):
"""Stub the upstream /v1/models probe used by the custom-provider branch."""
import urllib.request
ids = []
_req = urllib.request.Request(
"http://localhost:4000/v1/models",
headers={"Authorization": "Bearer test-key"},
requested: list[str] = []
def _fake_urlopen(req, timeout=None):
requested.append(getattr(req, "full_url", str(req)))
if fail:
raise OSError("probe failed")
body = {"data": [{"id": mid} for mid in catalog]}
return _FakeResponse(json.dumps(body).encode())
monkeypatch.setattr(urllib.request, "urlopen", _fake_urlopen)
return requested
def _run_live_models(monkeypatch, routes, custom_provider, *, catalog, fail=False):
"""Invoke the real handler for provider=custom:test-gateway."""
import api.config as config
import api.profiles as profiles
routes._clear_live_models_cache()
monkeypatch.setattr(
routes, "j", lambda _handler, payload, status=200, extra_headers=None: payload
)
with urllib.request.urlopen(_req, timeout=5) as _resp:
_body = json.loads(_resp.read())
monkeypatch.setattr(config, "_resolve_provider_alias", lambda provider: provider)
monkeypatch.setattr(profiles, "get_active_profile_name", lambda: "default")
if isinstance(_body, dict):
_data = _body.get("data", [])
if isinstance(_data, list):
ids = [m.get("id", "") for m in _data if m.get("id")]
config_data = {
"model": {"provider": "custom:test-gateway"},
"custom_providers": [custom_provider],
}
monkeypatch.setattr(config, "get_config", lambda: config_data)
monkeypatch.setattr(routes, "cfg", config_data, raising=False)
# Apply the merge logic from the fix
if ids:
_live_set = set(ids)
for _cid in _config_ids:
if _cid not in _live_set:
ids.append(_cid)
else:
ids = list(_config_ids)
# provider_model_ids() returns [] for custom:* → drives the custom branch.
_install_provider_model_ids(monkeypatch, lambda provider: [])
requested = _install_live_catalog(monkeypatch, routes, catalog, fail=fail)
# Live models must all be present
self.assertIn("assistant", ids)
self.assertIn("assistant-pro", ids)
self.assertIn("gpt-5.5:pt", ids)
# Config-only entry must be appended
self.assertIn("local-only", ids)
# No duplicates
self.assertEqual(ids.count("assistant"), 1)
def test_timeout_uses_config_constant(self):
"""The live fetch must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS, not a hardcoded value."""
source = ROUTES_PY.read_text(encoding="utf-8")
# Find the custom-provider live fetch block
marker = "Always try live fetch for custom providers"
marker_pos = source.find(marker)
next_section = source.find("OpenAI-compat live fetch fallback", marker_pos)
block = source[marker_pos:next_section]
# Must use the constant, not a bare number
self.assertIn("CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS", block, (
"Live fetch timeout must use CUSTOM_MODELS_ENDPOINT_TIMEOUT_SECONDS "
"instead of a hardcoded value (#3718 review feedback)"
))
self.assertNotIn("timeout=8", block, (
"Live fetch must not use hardcoded timeout=8 (#3718 review feedback)"
))
parsed = urlparse("/api/models/live?provider=custom:test-gateway")
payload = routes._handle_live_models(object(), parsed)
return payload, requested
if __name__ == "__main__":
unittest.main()
def _ids(payload):
return [m["id"] for m in payload.get("models", [])]
_LIVE_CATALOG = [
"chat-a",
"chat-b",
"image-gen-1", # non-chat noise the picker must not surface
"audio-tts-1",
]
_BASE_PROVIDER = {
"name": "Test Gateway",
"base_url": "https://gateway.example/v1",
"api_key": "sk-test",
}
@pytest.mark.parametrize(
"models_value",
[
pytest.param(["chat-a", "chat-b"], id="native-list"),
pytest.param('["chat-a","chat-b"]', id="json-array-string"),
pytest.param("['chat-a', 'chat-b']", id="python-literal-string"),
pytest.param(
[{"id": "chat-a"}, {"model": "chat-b"}], id="list-of-dicts"
),
],
)
def test_allowlist_filters_live_catalog(monkeypatch, models_value):
"""An explicit plural allowlist must trim the live catalog to itself.
The JSON-array-string and Python-literal cases are the regression: those
shapes are what ``hermes config set`` / JSON-mode editor saves persist, and
a dict/list-only parser drops them to ``[]`` → read as "no allowlist" → the
full upstream catalog floods the picker.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"]
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_dict_models_without_discover_false_does_not_gate(monkeypatch):
"""A dict-shaped ``models`` is per-model metadata — NOT an allowlist (#7165).
Hermes Agent's setup flow and ``hermes_cli/model_switch.py::_save_custom_provider``
persist per-model metadata as a mapping, e.g. ``{chat-a: {context_length: 128000}}``.
Treating its keys as an allowlist would collapse the live picker to the saved
default while the CLI live-probe shows the full catalog. Without an explicit
``discover_models: false`` opt-out the dict must NOT gate — full live catalog wins.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == _LIVE_CATALOG, (
"dict-shaped models is per-model metadata, not an allowlist — the "
"full live catalog must be surfaced"
)
def test_dict_models_with_discover_false_pins_to_keys(monkeypatch):
"""``discover_models: false`` opts a dict-shaped ``models`` into an allowlist.
The explicit opt-out (bool ``False`` or the string forms ``"false"``/``"no"``/``"0"``)
means the dict keys are a hand-pinned allowlist and must filter the live catalog.
"""
import api.routes as routes
for discover in (False, "false", "no", "0"):
provider = dict(
_BASE_PROVIDER,
models={"chat-a": {"context_length": 128000}, "chat-b": {"context_length": 32000}},
discover_models=discover,
)
payload, requested = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["chat-a", "chat-b"], (
f"discover_models={discover!r} must pin the dict keys as the allowlist"
)
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_singular_model_only_does_not_gate_live_catalog(monkeypatch):
"""``model:`` is default metadata, not an allowlist — keep full discovery."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-a")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG
def test_malformed_serialized_models_is_safe(monkeypatch):
"""A broken JSON-array string must not crash or silently drop the gate.
``_parse_config_string_list()`` treats an undecodable value as one literal
name, so the allowlist stays non-empty and the filter still runs.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models='["chat-a", "chat-b"') # missing ]
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
# Treated as a single (unmatched) name → appended, catalog not flooded.
assert "image-gen-1" not in _ids(payload)
assert "audio-tts-1" not in _ids(payload)
def test_allowlisted_model_missing_from_live_is_appended(monkeypatch):
"""A declared model the upstream didn't return must still be selectable."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=["chat-a", "chat-offline"])
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == ["chat-a", "chat-offline"]
def test_contradictory_singular_model_outside_plural_allowlist_is_ignored(monkeypatch):
"""A singular default outside the plural allowlist must not leak into the picker.
Paired test: whether the live probe succeeds or fails, only the explicit
plural allowlist is exposed.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default", models=["chat-a"])
# 1. Successful live fetch -> only plural allowlist
payload_success, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=False
)
assert _ids(payload_success) == ["chat-a"]
# 2. Failed live fetch -> still only plural allowlist (not chat-default)
payload_fail, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload_fail) == ["chat-a"]
def test_probe_failure_without_allowlist_falls_back_to_singular_model(monkeypatch):
"""When no plural allowlist is configured and probe fails, singular model is used."""
import api.routes as routes
provider = dict(_BASE_PROVIDER, model="chat-default")
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG, fail=True
)
assert _ids(payload) == ["chat-default"]
@pytest.mark.parametrize(
"models_value",
[
pytest.param([], id="native-empty-list"),
pytest.param("[]", id="serialized-empty-list"),
pytest.param({}, id="empty-mapping"),
pytest.param(" ", id="blank-scalar"),
],
)
def test_empty_allowlist_is_treated_as_not_configured(monkeypatch, models_value):
"""An empty ``models`` value must NOT be read as "allow nothing".
Pins a deliberate semantic decision. Gating the filter on *declared-ness*
rather than on a non-empty allowlist would make ``models: []`` emit an
EMPTY picker, leaving the provider unselectable — a harder failure than
surfacing a few extra models, and unrecoverable from the UI because
``custom_providers`` has no WebUI write path (hand-edited config.yaml only).
There is also no deny-all use case: a provider the user wants hidden is
removed from ``custom_providers`` outright.
If someone later "fixes" the empty case into a deny-all gate, this test
fails and forces the trade-off to be re-argued rather than silently shipped.
"""
import api.routes as routes
provider = dict(_BASE_PROVIDER, models=models_value)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=_LIVE_CATALOG
)
assert _ids(payload) == _LIVE_CATALOG, (
"empty allowlist must fall through to full live discovery, "
"never collapse the picker to zero models"
)
assert _ids(payload), "picker must never be emptied by an empty allowlist"
def test_models_discovered_catalog_is_not_an_allowlist(monkeypatch):
"""``models_discovered: true`` marks ``models:`` as discovery metadata (#7165).
Hermes persists *discovery results* back into config as ``models: {...}``
plus ``models_discovered: true``. That mapping is a snapshot of what the
gateway exposed at discovery time — NOT a hand-curated allowlist. Treating
it as one permanently pins the live catalog to the first-discovery set, so
a model the user later pulls into LM Studio / Ollama is silently dropped
from ``/api/models/live`` (the exact regression the gate reproduced: merge
base returned ``saved-a``, ``saved-b`` AND ``new-live-c``; the pinned head
dropped ``new-live-c``).
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, requested = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert requested, "live /v1/models probe was never attempted"
assert _ids(payload) == ["saved-a", "saved-b", "new-live-c"], (
"a discovered catalog must not gate the live probe — models that "
"appear upstream after discovery (new-live-c) must reach the picker"
)
def test_discovered_catalog_with_discover_models_false_still_pins(monkeypatch):
"""``discover_models: false`` re-pins a discovered catalog (control).
The explicit opt-out wins over ``models_discovered: true`` — same rule as
the ``/api/models`` catalog path (`api.config._provider_discover_allowed`).
Filtering to the configured set must survive the discovered-catalog fix.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
discover_models=False,
)
payload, _ = _run_live_models(
monkeypatch,
routes,
provider,
catalog=["saved-a", "saved-b", "new-live-c"],
)
assert _ids(payload) == ["saved-a", "saved-b"]
def test_discovered_catalog_probe_failure_falls_back_to_saved_models(monkeypatch):
"""Discovered + failed probe → saved models, not an empty picker (control).
Releasing the allowlist must not empty the fallback: when the live probe
fails, the previously discovered ``models:`` mapping is still the best
answer and must be served from ``_config_ids``.
"""
import api.routes as routes
provider = dict(
_BASE_PROVIDER,
models={
"saved-a": {"context_length": 128000},
"saved-b": {"context_length": 32000},
},
models_discovered=True,
)
payload, _ = _run_live_models(
monkeypatch, routes, provider, catalog=[], fail=True
)
assert _ids(payload) == ["saved-a", "saved-b"]