diff --git a/CHANGELOG.md b/CHANGELOG.md index 582fd46d..c7ecc5de 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,8 @@ ### Fixed +- **Custom providers that Hermes auto-discovered now show their full live `/v1/models` catalog again instead of just the saved models.** When a provider entry carries `models_discovered: true` alongside a `models:` mapping of per-model metadata (vision, context length), that mapping is discovery metadata — not a hand-curated allowlist — but the WebUI treated it as one and suppressed the live catalog, collapsing the model picker to only the already-saved IDs. Such a provider now defers to its live `/v1/models` catalog (matching the Hermes Agent's own `_models_config_is_allowlist` semantics), while an explicit `discover_models: false` opt-out (including the Agent-compatible string forms `false`/`no`/`0`) still pins the configured list, and a transient empty live probe falls back to the configured models rather than dropping the provider from the picker. Thanks @evgenyponomarev. (#7409, closes #7404) + - **Ctrl-C and `ctl.sh`-launched daemons now shut down gracefully instead of leaving the server running.** The WebUI installed a SIGTERM handler that drains in-flight work through `serve_forever()`'s `finally`, but SIGINT fell through to Python's default, so a daemon started via `./ctl.sh start` could only be stopped with `kill ` and the Settings "Stop server" button did not stop it. SIGINT now shares the same idempotent graceful-shutdown handler as SIGTERM. Thanks @aniruddhaadak80. (#7315, closes #7078) - **Static assets are served with correct MIME types instead of a `text/plain` catch-all.** `_serve_static()` defaulted every extension outside its small built-in allowlist to `text/plain; charset=utf-8`, mislabeling PDFs, APKs, WASM and other binaries. Unknown extensions now resolve through Python's standard `mimetypes` database, with an explicit deterministic entry for `.apk` (whose type varies across host MIME databases), and fail closed to `application/octet-stream` for unknown or pre-encoded suffixes (`.svgz`/`.tgz`/`.js.gz`) so still-compressed bytes are never advertised with their decoded media type. Path containment, gzip/ETag/cache handling, and the text charset path are unchanged. Thanks @Oidaladn0. (#7372) diff --git a/api/config.py b/api/config.py index a90bf7fb..6578c8e1 100644 --- a/api/config.py +++ b/api/config.py @@ -1399,6 +1399,37 @@ def _configured_model_ids(raw_models: object) -> list[str]: return model_ids +def _provider_discover_allowed(provider_cfg: object) -> bool: + """Mirror the Hermes Agent ``discover_models`` opt-out (``model_switch_providers._discover_flag``). + + ``discover_models`` defaults to True; the string forms ``"false"``/``"no"``/``"0"`` + (case-insensitive) mean False. A provider that pins its catalog with + ``discover_models: false`` keeps its configured ``models:`` even when the entry is + also marked ``models_discovered: true`` — the explicit opt-out wins. + """ + if not isinstance(provider_cfg, dict): + return True + discover = provider_cfg.get("discover_models", True) + if isinstance(discover, str): + return discover.strip().lower() not in {"false", "no", "0"} + return bool(discover) + + +def _provider_models_are_discovered_catalog(provider_cfg: object) -> bool: + """True when ``models:`` is an auto-discovered catalog that should defer to the live probe. + + A provider entry marked ``models_discovered: true`` carries a per-model *metadata* + mapping written by Hermes discovery, not a hand-curated allowlist — so the live + ``/v1/models`` catalog is authoritative. But an explicit ``discover_models: false`` + re-pins the configured mapping as the source of truth, so honor that opt-out. + """ + return ( + isinstance(provider_cfg, dict) + and provider_cfg.get("models_discovered") is True + and _provider_discover_allowed(provider_cfg) + ) + + def _configured_model_options(raw_models: object) -> list[dict[str, str]]: """Return picker option rows from supported config allowlist shapes.""" labels: dict[str, str] = {} @@ -1418,6 +1449,29 @@ def _configured_model_options(raw_models: object) -> list[dict[str, str]]: ] +def _merge_model_option_rows(*row_lists: object) -> list[dict[str, str]]: + """Merge picker option rows from multiple sources, first-seen order, deduped by id. + + Used to preserve a discovered provider's configured model IDs (ordered first) as a + fallback when a live ``/v1/models`` probe transiently returns nothing, merged with + any static built-in catalog without producing duplicate ids. + """ + merged: list[dict[str, str]] = [] + seen: set[str] = set() + for rows in row_lists: + if not isinstance(rows, (list, tuple)): + continue + for row in rows: + if not isinstance(row, dict): + continue + model_id = str(row.get("id") or "").strip() + if not model_id or model_id in seen: + continue + seen.add(model_id) + merged.append(row) + return merged + + def _named_custom_provider_slugs(config_obj: dict | None = None) -> set[str]: return { slug @@ -5741,8 +5795,12 @@ def _static_models_catalog_without_live_probes() -> dict: raw_key = canonical_to_raw_provider_key.get(pid, pid) provider_cfg = _get_provider_cfg(raw_key) raw_models = [] - if isinstance(provider_cfg, dict) and "models" in provider_cfg: - raw_models = _configured_model_options(provider_cfg["models"]) + if ( + isinstance(provider_cfg, dict) + and "models" in provider_cfg + and not _provider_models_are_discovered_catalog(provider_cfg) + ): + raw_models = _configured_model_options(provider_cfg.get("models")) if not raw_models: raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, [])) # Plugin-only providers (e.g. 9router) are not in _PROVIDER_MODELS @@ -8143,13 +8201,16 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa # whichever model had local settings. Only Copilot skips the # config-models allowlist branch and asks Hermes CLI for the # live catalog first (static _PROVIDER_MODELS is fallback only). - _uses_models_as_settings_map = pid == "copilot" + _uses_models_as_settings_map = ( + pid == "copilot" + or _provider_models_are_discovered_catalog(provider_cfg) + ) if ( not _uses_models_as_settings_map and isinstance(provider_cfg, dict) and "models" in provider_cfg ): - raw_models = _configured_model_options(provider_cfg["models"]) + raw_models = _configured_model_options(provider_cfg.get("models")) if not raw_models: if pid == "moa": @@ -8165,6 +8226,19 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa pid, _read_live_provider_model_ids(pid), ) + if ( + not raw_models + and _provider_models_are_discovered_catalog(provider_cfg) + ): + # A transient live-catalog failure must not drop a + # provider's persisted discovered models (the empty + # result would then be cached for up to 24h). Fall + # back to the configured discovered IDs, ordered + # first, merged with any static fallback (deduped). + raw_models = _merge_model_option_rows( + _configured_model_options(provider_cfg.get("models")), + copy.deepcopy(_PROVIDER_MODELS.get(pid, [])), + ) if not raw_models: raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, [])) diff --git a/tests/test_issue7404_models_discovered_not_allowlist.py b/tests/test_issue7404_models_discovered_not_allowlist.py new file mode 100644 index 00000000..2aceef8a --- /dev/null +++ b/tests/test_issue7404_models_discovered_not_allowlist.py @@ -0,0 +1,178 @@ +"""Regression coverage for #7404: models_discovered suppresses live catalog. + +When a provider sets ``models_discovered: true`` in its config alongside a +``models:`` dict of per-model metadata, WebUI must not treat the dict as a +strict allowlist. It should fall through to the live ``/v1/models`` probe, +matching the upstream Hermes Agent behaviour. +""" + + +def _provider_group(payload: dict, provider_id: str) -> dict: + for group in payload.get("groups", []): + if group.get("provider_id") == provider_id: + return group + raise AssertionError(f"provider group {provider_id!r} not found: {payload.get('groups')!r}") + + +def test_models_discovered_skips_config_allowlist_and_uses_live_catalog(monkeypatch, tmp_path): + import api.config as config + + cfg = { + "model": {"default": "model-a", "provider": "custom-llm"}, + "providers": { + "custom-llm": { + "name": "Custom LLM", + "api": "https://llm.example.com", + "transport": "chat_completions", + "models_discovered": True, + "models": { + "model-a": {"supports_vision": True}, + "model-b": {"context_length": 128000}, + }, + } + }, + } + + live_ids = ["model-a", "model-b", "model-c", "model-d", "model-e"] + + monkeypatch.setattr(config, "cfg", cfg, raising=False) + monkeypatch.setattr(config, "_get_config_path", lambda: tmp_path / "config.yaml") + monkeypatch.setattr(config, "_get_auth_store_path", lambda: tmp_path / "auth.json") + monkeypatch.setattr(config, "_get_models_cache_path", lambda: tmp_path / "models_cache.json") + monkeypatch.setattr(config, "_models_cache_source_fingerprint", lambda: {"test": "fingerprint"}) + monkeypatch.setattr(config, "reload_config_if_stale", lambda: None) + monkeypatch.setattr(config, "reload_config", lambda: None) + monkeypatch.setattr(config, "_cfg_mtime", 0.0, raising=False) + monkeypatch.setattr(config, "_LIVE_REBUILD_BUDGET_SECONDS", 0.0, raising=False) + monkeypatch.setattr( + config, "_read_live_provider_model_ids", + lambda pid: live_ids if pid == "custom-llm" else [], + ) + + config.invalidate_models_cache() + payload = config.get_available_models(force_refresh=True) + group = _provider_group(payload, "custom-llm") + ids = [m["id"] for m in group["models"]] + + assert ids == live_ids + + +def _setup_provider(monkeypatch, tmp_path, provider_cfg, live_ids): + """Wire config with a single custom-llm provider and a controllable live catalog.""" + import api.config as config + + cfg = { + "model": {"default": "model-a", "provider": "custom-llm"}, + "providers": {"custom-llm": provider_cfg}, + } + monkeypatch.setattr(config, "cfg", cfg, raising=False) + monkeypatch.setattr(config, "_get_config_path", lambda: tmp_path / "config.yaml") + monkeypatch.setattr(config, "_get_auth_store_path", lambda: tmp_path / "auth.json") + monkeypatch.setattr(config, "_get_models_cache_path", lambda: tmp_path / "models_cache.json") + monkeypatch.setattr(config, "_models_cache_source_fingerprint", lambda: {"test": "fingerprint"}) + monkeypatch.setattr(config, "reload_config_if_stale", lambda: None) + monkeypatch.setattr(config, "reload_config", lambda: None) + monkeypatch.setattr(config, "_cfg_mtime", 0.0, raising=False) + monkeypatch.setattr(config, "_LIVE_REBUILD_BUDGET_SECONDS", 0.0, raising=False) + monkeypatch.setattr( + config, "_read_live_provider_model_ids", + lambda pid: live_ids if pid == "custom-llm" else [], + ) + config.invalidate_models_cache() + return config + + +def test_discover_models_false_pins_configured_allowlist_despite_discovered_flag(monkeypatch, tmp_path): + """`discover_models: false` overrides `models_discovered: true` (explicit opt-out wins). + + A provider that persisted a discovered catalog but then pinned it with + ``discover_models: false`` must keep exactly its configured ``models:`` and NOT + probe/expose the broader live catalog. + """ + provider_cfg = { + "name": "Custom LLM", + "api": "https://llm.example.com", + "transport": "chat_completions", + "models_discovered": True, + "discover_models": False, + "models": { + "model-a": {"supports_vision": True}, + "model-b": {"context_length": 128000}, + }, + } + live_ids = ["model-a", "model-b", "model-c", "model-d", "model-e"] + config = _setup_provider(monkeypatch, tmp_path, provider_cfg, live_ids) + + payload = config.get_available_models(force_refresh=True) + group = _provider_group(payload, "custom-llm") + ids = [m["id"] for m in group["models"]] + + assert ids == ["model-a", "model-b"] + + +def test_discover_models_false_string_form_also_pins(monkeypatch, tmp_path): + """Agent-compatible string opt-out (`discover_models: "false"`) also pins the allowlist.""" + provider_cfg = { + "name": "Custom LLM", + "api": "https://llm.example.com", + "transport": "chat_completions", + "models_discovered": True, + "discover_models": "false", + "models": {"model-a": {}, "model-b": {}}, + } + live_ids = ["model-a", "model-b", "model-c"] + config = _setup_provider(monkeypatch, tmp_path, provider_cfg, live_ids) + + payload = config.get_available_models(force_refresh=True) + group = _provider_group(payload, "custom-llm") + ids = [m["id"] for m in group["models"]] + + assert ids == ["model-a", "model-b"] + + +def test_discovered_catalog_survives_transient_live_probe_failure(monkeypatch, tmp_path): + """A transient empty live probe must not drop a discovered provider's configured models. + + With ``models_discovered: true`` (and discovery allowed), the live catalog is + authoritative — but when the probe transiently returns nothing, the provider group + must fall back to the configured discovered IDs rather than vanishing (which would be + cached empty for up to 24h). A provider absent from the static built-in catalog is + the exact case #7404's fix must not regress. + """ + provider_cfg = { + "name": "Custom LLM", + "api": "https://llm.example.com", + "transport": "chat_completions", + "models_discovered": True, + "models": {"model-a": {"supports_vision": True}, "model-b": {"context_length": 128000}}, + } + # Live probe returns NOTHING (transient failure), provider not in _PROVIDER_MODELS. + config = _setup_provider(monkeypatch, tmp_path, provider_cfg, []) + + payload = config.get_available_models(force_refresh=True) + group = _provider_group(payload, "custom-llm") + ids = [m["id"] for m in group["models"]] + + assert ids == ["model-a", "model-b"] + + +def test_discovered_flag_without_models_key_and_empty_probe_does_not_500(monkeypatch, tmp_path): + """models_discovered:true with NO models: key + empty live probe must not raise (KeyError->500). + + _provider_models_are_discovered_catalog is True on models_discovered alone, so the + probe-failure fallback must read models via .get() (absent -> no configured rows), + degrading to the static catalog rather than crashing /api/models. + """ + provider_cfg = { + "name": "Custom LLM", + "api": "https://llm.example.com", + "transport": "chat_completions", + "models_discovered": True, + # no "models" key at all + } + config = _setup_provider(monkeypatch, tmp_path, provider_cfg, []) + + # Must not raise; provider absent from the static catalog degrades to an empty + # group (filtered out) rather than a 500. + payload = config.get_available_models(force_refresh=True) + assert isinstance(payload.get("groups"), list)