fix(models): discovered catalogs are discovery metadata, not allowlists
Hermes persists discovery results as models: {...} + models_discovered: true.
_custom_provider_allowlist_ids() treated any plural models value as an
explicit allowlist, so after one discovery /api/models/live was permanently
pinned to the first-discovery set and newly available upstream models were
silently dropped (gate repro: new-live-c missing vs merge-base).
Gate the helper on api.config._provider_models_are_discovered_catalog() —
the same predicate the /api/models path uses — and return no allowlist when
it holds. discover_models: false still pins via the predicate; a hand-written
models: list without the discovered marker still filters; probe-failure
fallback still serves saved ids.
Real-handler tests: discovered catalog preserves full live catalog (RED on
the previous head), discover_models: false control stays pinned, failed-probe
control still returns saved models.
This commit is contained in:
1 parent
39017f6013
commit
18842cc707
2 files changed
+108
No files matched your search
@@ -21891,6 +21891,19 @@ def _handle_live_models(handler, parsed):
|
||||
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 mapping still filters.
|
||||
|
||||
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
|
||||
@@ -21900,6 +21913,10 @@ def _handle_live_models(handler, parsed):
|
||||
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
|
||||
|
||||
@@ -14,6 +14,9 @@ Covered shapes:
|
||||
* malformed serialized ``models`` → safe, treated as scalar name
|
||||
* allowlisted model absent from live catalog → appended
|
||||
* live probe failure → falls back to config ids
|
||||
* ``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
|
||||
@@ -251,3 +254,91 @@ def test_empty_allowlist_is_treated_as_not_configured(monkeypatch, models_value)
|
||||
"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"]
|
||||
Reference in new issue
Block a user