diff --git a/api/config.py b/api/config.py index a12c3ef1..0260fb67 100644 --- a/api/config.py +++ b/api/config.py @@ -1918,7 +1918,7 @@ def _get_provider_base_url(provider_id): Returns the URL stripped of trailing ``/`` if configured, otherwise None. """ - prov_cfg = cfg.get("providers", {}).get(provider_id, {}) or {} + prov_cfg = _get_provider_cfg(provider_id) explicit = (prov_cfg.get("base_url") or "").strip().rstrip("/") if explicit: return explicit @@ -1932,6 +1932,16 @@ def _get_provider_base_url(provider_id): return None +def _get_providers_cfg() -> dict: + providers_cfg = cfg.get("providers") + return providers_cfg if isinstance(providers_cfg, dict) else {} + + +def _get_provider_cfg(provider_id) -> dict: + provider_cfg = _get_providers_cfg().get(provider_id, {}) + return provider_cfg if isinstance(provider_cfg, dict) else {} + + def resolve_model_provider(model_id: str) -> tuple: """Resolve model name, provider, and base_url for AIAgent. @@ -4181,7 +4191,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: # and a phantom ``Opencode_Go`` group for the config-key form (#1568). # The same applies to mixed-case ids like ``OpenCode-Go`` and to # legitimate aliases like ``z-ai`` → ``zai``. - _cfg_providers = cfg.get("providers", {}) + _cfg_providers = _get_providers_cfg() # Map canonical provider IDs back to raw config keys so the # generic-provider branch can preserve mixed-case/underscore # provider_cfg values (#2245). @@ -4927,9 +4937,9 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: # `cfg["providers"]["lmstudio"]["base_url"]` or # `cfg["model"]["base_url"]` (via _get_provider_base_url), # so the historical model-block config shape still works. - lm_cfg = cfg.get("providers", {}).get("lmstudio", {}) or {} + lm_cfg = _get_provider_cfg("lmstudio") lm_base_url = _get_provider_base_url("lmstudio") or "" - lm_api_key = str(lm_cfg.get("api_key") or "").strip() if isinstance(lm_cfg, dict) else "" + lm_api_key = str(lm_cfg.get("api_key") or "").strip() if lm_base_url: headers = {"User-Agent": "OpenAI/Python 1.0"} if lm_api_key: @@ -4968,7 +4978,7 @@ def get_available_models(*, prefer_cache: bool = False) -> dict: # (#2245). Fall back to the canonical pid for providers # that appear in _PROVIDER_MODELS but not in cfg. _raw_key = _canonical_to_raw_provider_key.get(pid, pid) - provider_cfg = cfg.get("providers", {}).get(_raw_key, {}) + provider_cfg = _get_provider_cfg(_raw_key) raw_models = [] # User-configured model allowlists are explicit local diff --git a/tests/test_pr1970_lmstudio_base_url_fallback.py b/tests/test_pr1970_lmstudio_base_url_fallback.py index cb6c01d2..c3951417 100644 --- a/tests/test_pr1970_lmstudio_base_url_fallback.py +++ b/tests/test_pr1970_lmstudio_base_url_fallback.py @@ -16,7 +16,11 @@ can't accidentally drop the model.base_url fallback again. """ from __future__ import annotations +import sys +import types + import api.config as config +import pytest class _RestoreCfg: @@ -25,11 +29,49 @@ class _RestoreCfg: def __enter__(self): import copy self._snapshot = copy.deepcopy(config.cfg) + self._cfg_mtime = config._cfg_mtime return self def __exit__(self, *exc): config.cfg.clear() config.cfg.update(self._snapshot) + config._cfg_mtime = self._cfg_mtime + + +def _set_cfg(cfg_dict: dict): + config.cfg.clear() + config.cfg.update(cfg_dict) + try: + config._cfg_mtime = config.Path(config._get_config_path()).stat().st_mtime + except Exception: + config._cfg_mtime = 0.0 + + +def _groups_by_provider_id(result: dict) -> dict: + return {group["provider_id"]: group for group in result["groups"]} + + +def _stub_hermes_cli(monkeypatch): + fake_models = types.ModuleType("hermes_cli.models") + fake_models.list_available_providers = lambda: [] + fake_models.provider_model_ids = lambda _pid: [] + fake_auth = types.ModuleType("hermes_cli.auth") + fake_auth.get_auth_status = lambda _pid: {"key_source": "none"} + monkeypatch.setitem(sys.modules, "hermes_cli.models", fake_models) + monkeypatch.setitem(sys.modules, "hermes_cli.auth", fake_auth) + monkeypatch.setattr( + config, + "_get_auth_store_path", + lambda: config.Path("/tmp/does-not-exist-auth.json"), + ) + + +@pytest.fixture(autouse=True) +def _isolate_models_cache(tmp_path, monkeypatch): + monkeypatch.setattr(config, "_models_cache_path", tmp_path / "models_cache.json") + config.invalidate_models_cache() + yield + config.invalidate_models_cache() def test_get_provider_base_url_finds_explicit_providers_entry(): @@ -127,6 +169,34 @@ def test_get_provider_base_url_explicit_wins_over_model_fallback(): assert config._get_provider_base_url("lmstudio") == "http://correct:1234/v1" +def test_get_provider_base_url_treats_list_providers_as_unconfigured(): + """List-shaped providers must behave like a missing providers map.""" + with _RestoreCfg(): + for providers_cfg in ([], ["lmstudio"]): + _set_cfg({ + "model": { + "provider": "lmstudio", + "base_url": "http://192.168.1.22:1234/v1", + }, + "providers": providers_cfg, + }) + assert config._get_provider_base_url("lmstudio") == "http://192.168.1.22:1234/v1" + assert config._get_provider_base_url("openai") is None + + +def test_get_provider_base_url_treats_malformed_provider_entry_as_unconfigured(): + """Truth-y non-dict provider entries must not reach ``.get("base_url")``.""" + with _RestoreCfg(): + _set_cfg({ + "model": { + "provider": "lmstudio", + "base_url": "http://192.168.1.22:1234/v1", + }, + "providers": {"lmstudio": "enabled"}, + }) + assert config._get_provider_base_url("lmstudio") == "http://192.168.1.22:1234/v1" + + def test_lmstudio_fallback_works_when_hermes_cli_unavailable(tmp_path, monkeypatch): """The lmstudio branch must populate models from the urlopen fallback even @@ -142,7 +212,6 @@ def test_lmstudio_fallback_works_when_hermes_cli_unavailable(tmp_path, monkeypat """ import json as _json import socket as _socket - import sys import urllib.request as _urlreq import api.config as config @@ -218,3 +287,153 @@ providers: sys.meta_path.remove(blocker) except ValueError: pass + + +def test_lmstudio_fallback_handles_malformed_providers_list(tmp_path, monkeypatch): + """LM Studio fallback must ignore list-shaped providers instead of crashing.""" + import json as _json + import socket as _socket + import urllib.request as _urlreq + + blocked_modules = [name for name in list(sys.modules) if name == "hermes_cli" or name.startswith("hermes_cli.")] + for name in blocked_modules: + monkeypatch.delitem(sys.modules, name, raising=False) + + class _Blocker: + def find_module(self, name, path=None): + if name == "hermes_cli" or name.startswith("hermes_cli."): + return self + return None + + def load_module(self, name): + raise ImportError(f"hermes_cli blocked for test: {name}") + + blocker = _Blocker() + sys.meta_path.insert(0, blocker) + try: + cfgfile = tmp_path / "config.yaml" + cfgfile.write_text( + """ +model: + provider: lmstudio + default: qwen3.6-35b-a3b@q6_k + base_url: http://10.0.0.5:1234/v1 +providers: + - lmstudio +""", + encoding="utf-8", + ) + monkeypatch.setattr(config, "_get_config_path", lambda: cfgfile) + config.reload_config() + config.invalidate_models_cache() + + class _ModelsResponse: + def __enter__(self): + return self + + def __exit__(self, *args): + pass + + def read(self): + return _json.dumps( + {"data": [{"id": "qwen3.6-35b-a3b@q6_k"}, {"id": "another-model"}]} + ).encode() + + monkeypatch.setattr(_urlreq, "urlopen", lambda *_a, **_kw: _ModelsResponse()) + monkeypatch.setattr( + _socket, + "getaddrinfo", + lambda *_a, **_kw: [ + (_socket.AF_INET, _socket.SOCK_STREAM, 6, "", ("10.0.0.5", 0)) + ], + ) + + result = config.get_available_models() + groups = _groups_by_provider_id(result) + + assert "lmstudio" in groups, ( + f"lmstudio group missing for malformed providers list; groups={list(groups)}" + ) + model_ids = {model["id"] for model in groups["lmstudio"]["models"]} + assert "qwen3.6-35b-a3b@q6_k" in model_ids + assert "another-model" in model_ids + finally: + try: + sys.meta_path.remove(blocker) + except ValueError: + pass + + +def test_provider_catalog_treats_malformed_providers_as_unconfigured(monkeypatch): + """Catalog lookup must not crash when providers is a non-dict value.""" + _stub_hermes_cli(monkeypatch) + monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: []) + + with _RestoreCfg(): + _set_cfg({ + "model": { + "default": "claude-sonnet-4-5", + "provider": "anthropic", + }, + "providers": ["anthropic"], + }) + config.invalidate_models_cache() + + result = config.get_available_models() + + groups = _groups_by_provider_id(result) + assert "anthropic" in groups, f"Expected anthropic group, got: {list(groups)}" + assert groups["anthropic"]["models"], "anthropic group should still fall back to models" + + +def test_provider_catalog_preserves_dict_shaped_raw_key_lookup(monkeypatch): + """Dict-shaped providers must still resolve configured models through the raw key.""" + _stub_hermes_cli(monkeypatch) + monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: []) + + with _RestoreCfg(): + _set_cfg({ + "model": { + "default": "my-model", + "provider": "CLIPpoxy", + }, + "providers": { + "CLIPpoxy": { + "models": ["my-model", "another-model"], + }, + }, + }) + config.invalidate_models_cache() + + result = config.get_available_models() + + groups = _groups_by_provider_id(result) + assert "clippoxy" in groups, f"Expected clippoxy group, got: {list(groups)}" + model_ids = [] + for model in groups["clippoxy"]["models"]: + mid = model["id"] + model_ids.append(mid.split(":", 1)[-1] if mid.startswith("@") and ":" in mid else mid) + assert "my-model" in model_ids + assert "another-model" in model_ids + + +def test_provider_catalog_treats_malformed_provider_entry_as_unconfigured(monkeypatch): + """Provider catalog raw-key lookup must ignore truth-y non-dict entries.""" + _stub_hermes_cli(monkeypatch) + monkeypatch.setattr("socket.getaddrinfo", lambda *a, **k: []) + + with _RestoreCfg(): + _set_cfg({ + "model": { + "default": "claude-sonnet-4-5", + "provider": "anthropic", + }, + "providers": {"anthropic": "enabled"}, + }) + config.invalidate_models_cache() + + result = config.get_available_models() + + groups = _groups_by_provider_id(result) + assert "anthropic" in groups, f"Expected anthropic group, got: {list(groups)}" + assert groups["anthropic"]["models"], "anthropic group should still fall back to models"