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).
This commit is contained in:
1 parent
cf1ab5bbbc
commit
0b8e67c20d
2 files changed
+49
-1
No files matched your search
+16
-1
@@ -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)
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in new issue
Block a user