From 0b8e67c20d078a2d3d5f3354e8a56cb01c158613 Mon Sep 17 00:00:00 2001 From: Gordie Date: Tue, 30 Jun 2026 22:32:42 -0500 Subject: [PATCH] fix(#5301): admit active models-only custom provider without reopening dedup regression Maintainer review found regression 2 on commit 575996a7: the new _has_provider_route gate (api/config.py) required api/base_url/api_key/key_env before admitting a configured provider into the picker, which dropped a models-only custom provider config (the lmstudio-style shape from #1970, tests/test_pr1970_lmstudio_base_url_fallback.py::test_provider_catalog_preserves_dict_shaped_raw_key_lookup). A naive fix (admit any config with models) would re-break test_unknown_duplicate_copilot_provider_config_is_not_rendered, since a spurious alias like copilot-2: {name: copilot, models: {...}} must stay rejected. Fix: admit a models-only provider config as evidence only when its canonical id matches the active/configured provider (threaded via active_provider, already in scope). Non-active models-only configs (including duplicate aliases of known providers) are still rejected. Added a regression test: tests/test_pr1970_lmstudio_base_url_fallback.py::test_provider_catalog_rejects_non_active_models_only_custom_provider covering both the active-admits and non-active-rejects cases side by side. Verification: ./scripts/test.sh tests/test_issue5057_moa_webui_route.py tests/test_copilot_provider_model_settings_not_allowlist.py tests/test_moa_model_picker_provider.py tests/test_pr1970_lmstudio_base_url_fallback.py -q 26 passed, 1 skipped in 5.58s Broader sanity pass: ./scripts/test.sh tests/ -k "config or provider or model or copilot or moa or lmstudio" -q 1728 passed, 5 skipped, 9654 deselected (4 pre-existing failures confirmed unrelated: 2 reproduce identically on the pre-fix commit via git stash, 2 are order-dependent and pass in isolation). --- api/config.py | 17 +++++++++- .../test_pr1970_lmstudio_base_url_fallback.py | 33 +++++++++++++++++++ 2 files changed, 49 insertions(+), 1 deletion(-) diff --git a/api/config.py b/api/config.py index bbd3dfae..e74fe911 100644 --- a/api/config.py +++ b/api/config.py @@ -6008,7 +6008,22 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa str(_provider_cfg.get(_route_key) or "").strip() for _route_key in ("api", "base_url", "api_key", "key_env") ) - if not (_is_known_provider or _has_provider_route): + # A models-only provider config (no api/base_url/api_key/key_env) + # is only admitted as evidence when it's the active/configured + # provider (e.g. the lmstudio-style custom shape from #1970). + # This must NOT re-open the door for a spurious duplicate alias + # of a known provider (e.g. ``copilot-2: {name: "copilot", + # models: {...}}`` from #644/dedup regression) — that case is + # still rejected because it isn't the active provider and it + # isn't a route-bearing config in its own right. + _has_models_only_active_route = ( + not _has_provider_route + and _is_provider_config + and isinstance(_provider_cfg.get("models"), (dict, list)) + and _provider_cfg["models"] + and _canonical == _canonicalise_provider_id(active_provider) + ) + if not (_is_known_provider or _has_provider_route or _has_models_only_active_route): continue _canonical_to_raw_provider_key.setdefault(_canonical, _pid_key) diff --git a/tests/test_pr1970_lmstudio_base_url_fallback.py b/tests/test_pr1970_lmstudio_base_url_fallback.py index c3951417..d8488dc2 100644 --- a/tests/test_pr1970_lmstudio_base_url_fallback.py +++ b/tests/test_pr1970_lmstudio_base_url_fallback.py @@ -417,6 +417,39 @@ def test_provider_catalog_preserves_dict_shaped_raw_key_lookup(monkeypatch): assert "another-model" in model_ids +def test_provider_catalog_rejects_non_active_models_only_custom_provider(monkeypatch): + """A models-only custom provider config must NOT render when it is not the + active/configured provider (#5301 review finding: admitting any + models-only config re-opens the copilot-2 duplicate-alias regression).""" + _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"], + }, + "OtherCustom": { + "models": ["unrelated-model"], + }, + }, + }) + config.invalidate_models_cache() + + result = config.get_available_models() + + groups = _groups_by_provider_id(result) + assert "clippoxy" in groups, f"Active models-only provider should render, got: {list(groups)}" + assert "othercustom" not in groups, ( + f"Non-active models-only provider must not render as a phantom group, got: {list(groups)}" + ) + + 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)