Centralize custom-provider model-ID extraction into _configured_model_ids() + _configured_model_options(), used by both catalog construction and runtime routing so the picker and resolver can't drift. Fixes list-shaped custom_providers[].models allowlists silently staying on the default provider instead of routing to their own (#6121). Existing mapping / list-of-strings configs unaffected. Co-authored-by: nesquena-hermes <agent@nesquena-hermes> Co-authored-by: franksong2702 <franksong2702@users.noreply.github.com>
This commit is contained in:
3 files changed
+139
-117
No files matched your search
@@ -5,6 +5,8 @@
|
||||
|
||||
### Fixed
|
||||
|
||||
- **Models selected from a list-shaped custom-provider allowlist now route to the right provider.** A `custom_providers[].models` allowlist can be written as either a mapping or a list, and the model picker accepted both — but runtime routing only read mapping keys, so a model chosen from a *list*-shaped allowlist could silently stay on the active/default provider instead of its own. The supported-model-ID extraction is now centralized in one helper used by both catalog construction and routing, so the picker and runtime can no longer drift. Existing mapping-shaped and list-of-strings configs are unaffected. Thanks @franksong2702. (#6127, #6121)
|
||||
|
||||
- **Stale sidebar approval attention badge now clears once its gateway approval has drained.** A parent session could keep showing the red approval "attention" dot in the sidebar after the underlying gateway approval was already resolved/gone — the live gateway queue drained but the sidebar summary kept the mirrored approval alive. `_session_attention_summary()` now reconciles the gateway pending-mirror before counting attention (the same self-healing repair the approval-pending route already does), so the badge clears correctly. Live approvals stay visible and actionable and the clarify fallback is unchanged. This is the WebUI read-path mitigation; the child-session context-propagation root cause stays tracked in #6100. Thanks @rodboev. (#6117, #6100)
|
||||
|
||||
### Added
|
||||
|
||||
+58
-117
@@ -1351,6 +1351,46 @@ def _custom_provider_entries(config_obj: dict | None = None) -> list[dict]:
|
||||
return [entry for entry in entries if isinstance(entry, dict)]
|
||||
|
||||
|
||||
def _configured_model_ids(raw_models: object) -> list[str]:
|
||||
"""Return ordered model IDs from supported config allowlist shapes."""
|
||||
if isinstance(raw_models, dict):
|
||||
candidates = (key for key in raw_models if isinstance(key, str))
|
||||
elif isinstance(raw_models, list):
|
||||
candidates = raw_models
|
||||
else:
|
||||
return []
|
||||
|
||||
model_ids: list[str] = []
|
||||
for item in candidates:
|
||||
if isinstance(item, dict):
|
||||
candidate = item.get("id") or item.get("model") or item.get("name")
|
||||
else:
|
||||
candidate = item
|
||||
model_id = str(candidate or "").strip()
|
||||
if model_id and model_id not in model_ids:
|
||||
model_ids.append(model_id)
|
||||
return model_ids
|
||||
|
||||
|
||||
def _configured_model_options(raw_models: object) -> list[dict[str, str]]:
|
||||
"""Return picker option rows from supported config allowlist shapes."""
|
||||
labels: dict[str, str] = {}
|
||||
if isinstance(raw_models, list):
|
||||
for item in raw_models:
|
||||
if not isinstance(item, dict):
|
||||
continue
|
||||
candidate = item.get("id") or item.get("model") or item.get("name")
|
||||
model_id = str(candidate or "").strip()
|
||||
if not model_id or model_id in labels:
|
||||
continue
|
||||
label = str(item.get("label") or model_id).strip() or model_id
|
||||
labels[model_id] = label
|
||||
return [
|
||||
{"id": model_id, "label": labels.get(model_id, model_id)}
|
||||
for model_id in _configured_model_ids(raw_models)
|
||||
]
|
||||
|
||||
|
||||
def _named_custom_provider_slugs(config_obj: dict | None = None) -> set[str]:
|
||||
return {
|
||||
slug
|
||||
@@ -2376,12 +2416,7 @@ def _model_id_declared_in_config(model_id: str, config_provider: str | None) ->
|
||||
if str(model_cfg.get("default") or "").strip() == model:
|
||||
return True
|
||||
_declared = model_cfg.get("models")
|
||||
if isinstance(_declared, dict) and model in _declared:
|
||||
return True
|
||||
if isinstance(_declared, list) and any(
|
||||
(str(m.get("id") or "").strip() if isinstance(m, dict) else str(m or "").strip()) == model
|
||||
for m in _declared
|
||||
):
|
||||
if model in _configured_model_ids(_declared):
|
||||
return True
|
||||
# Named custom:<slug> — scan its custom_providers[] entry for a verbatim id.
|
||||
prov = str(config_provider or "").strip().lower()
|
||||
@@ -2394,13 +2429,7 @@ def _model_id_declared_in_config(model_id: str, config_provider: str | None) ->
|
||||
continue
|
||||
if str(entry.get("model") or "").strip() == model:
|
||||
return True
|
||||
_em = entry.get("models")
|
||||
if isinstance(_em, dict) and model in _em:
|
||||
return True
|
||||
if isinstance(_em, list) and any(
|
||||
(str(m.get("id") or "").strip() if isinstance(m, dict) else str(m or "").strip()) == model
|
||||
for m in _em
|
||||
):
|
||||
if model in _configured_model_ids(entry.get("models")):
|
||||
return True
|
||||
return False
|
||||
|
||||
@@ -2664,16 +2693,7 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
|
||||
continue
|
||||
if _canon_config_provider == "copilot":
|
||||
continue # copilot.models is a settings map, not an allowlist
|
||||
_own_models = _pdef.get('models')
|
||||
if isinstance(_own_models, list):
|
||||
for _m in _own_models:
|
||||
_mid = str(_m.get('id') or '') if isinstance(_m, dict) else str(_m or '')
|
||||
if _mid.strip():
|
||||
_provider_models_set.add(_mid.strip())
|
||||
elif isinstance(_own_models, dict):
|
||||
_provider_models_set.update(
|
||||
str(k).strip() for k in _own_models if isinstance(k, str) and str(k).strip()
|
||||
)
|
||||
_provider_models_set.update(_configured_model_ids(_pdef.get('models')))
|
||||
_skip_custom_providers = (
|
||||
_is_explicit_non_custom_provider
|
||||
and (
|
||||
@@ -2694,13 +2714,7 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
|
||||
entry_model_ids = set()
|
||||
if entry_model:
|
||||
entry_model_ids.add(entry_model)
|
||||
entry_models = entry.get('models')
|
||||
if isinstance(entry_models, dict):
|
||||
entry_model_ids.update(
|
||||
key.strip()
|
||||
for key in entry_models.keys()
|
||||
if isinstance(key, str) and key.strip()
|
||||
)
|
||||
entry_model_ids.update(_configured_model_ids(entry.get('models')))
|
||||
if entry_name and model_id in entry_model_ids:
|
||||
provider_hint = _custom_provider_slug_from_name(entry_name)
|
||||
return model_id, provider_hint, entry_base_url or None
|
||||
@@ -2734,20 +2748,9 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
|
||||
# own providers: entry may match; skip all other slugs.
|
||||
if _skip_custom_providers and _canonicalise_provider_id(slug) != _active_slug:
|
||||
continue
|
||||
p_models = pdef.get('models')
|
||||
if isinstance(p_models, list):
|
||||
for m in p_models:
|
||||
mid = str(m.get('id') or '') if isinstance(m, dict) else str(m or '')
|
||||
if not mid:
|
||||
continue
|
||||
if mid.strip() == target:
|
||||
p_base_url = str(pdef.get('base_url') or '').strip()
|
||||
return model_id, slug, p_base_url or None
|
||||
elif isinstance(p_models, dict):
|
||||
for mid in p_models:
|
||||
if isinstance(mid, str) and mid.strip() == target:
|
||||
p_base_url = str(pdef.get('base_url') or '').strip()
|
||||
return model_id, slug, p_base_url or None
|
||||
if target in _configured_model_ids(pdef.get('models')):
|
||||
p_base_url = str(pdef.get('base_url') or '').strip()
|
||||
return model_id, slug, p_base_url or None
|
||||
|
||||
# @provider:model format — explicit provider hint from the dropdown.
|
||||
# Route through that provider directly (resolve_runtime_provider will
|
||||
@@ -5103,20 +5106,9 @@ def _static_models_catalog_without_live_probes() -> dict:
|
||||
for key in ("api_key", "key_env", "base_url")
|
||||
)
|
||||
provider_models = provider_cfg.get("models")
|
||||
if isinstance(provider_models, dict):
|
||||
for model_id in provider_models:
|
||||
_append_model_id(canonical, model_id)
|
||||
has_local_signal = True
|
||||
elif isinstance(provider_models, list):
|
||||
for item in provider_models:
|
||||
if isinstance(item, dict):
|
||||
_append_model_id(
|
||||
canonical,
|
||||
item.get("id") or item.get("model") or item.get("name"),
|
||||
)
|
||||
else:
|
||||
_append_model_id(canonical, item)
|
||||
has_local_signal = True
|
||||
for model_id in _configured_model_ids(provider_models):
|
||||
_append_model_id(canonical, model_id)
|
||||
has_local_signal = True
|
||||
if has_local_signal:
|
||||
detected_providers.add(canonical)
|
||||
|
||||
@@ -5166,21 +5158,9 @@ def _static_models_catalog_without_live_probes() -> dict:
|
||||
model_id = str(entry.get("model") or "").strip()
|
||||
if model_id:
|
||||
configured_ids.append(model_id)
|
||||
raw_models = entry.get("models")
|
||||
if isinstance(raw_models, dict):
|
||||
for key in raw_models:
|
||||
if isinstance(key, str) and key.strip() and key.strip() not in configured_ids:
|
||||
configured_ids.append(key.strip())
|
||||
elif isinstance(raw_models, list):
|
||||
for item in raw_models:
|
||||
if isinstance(item, dict):
|
||||
candidate = str(
|
||||
item.get("id") or item.get("model") or item.get("name") or ""
|
||||
).strip()
|
||||
else:
|
||||
candidate = str(item or "").strip()
|
||||
if candidate and candidate not in configured_ids:
|
||||
configured_ids.append(candidate)
|
||||
for configured_id in _configured_model_ids(entry.get("models")):
|
||||
if configured_id not in configured_ids:
|
||||
configured_ids.append(configured_id)
|
||||
|
||||
for configured_id in configured_ids:
|
||||
label = _get_label_for_model(configured_id, [])
|
||||
@@ -5251,28 +5231,7 @@ def _static_models_catalog_without_live_probes() -> dict:
|
||||
provider_cfg = _get_provider_cfg(raw_key)
|
||||
raw_models = []
|
||||
if isinstance(provider_cfg, dict) and "models" in provider_cfg:
|
||||
cfg_models = provider_cfg["models"]
|
||||
if isinstance(cfg_models, dict):
|
||||
raw_models = [{"id": key, "label": key} for key in cfg_models.keys()]
|
||||
elif isinstance(cfg_models, list):
|
||||
raw_models = []
|
||||
for item in cfg_models:
|
||||
if isinstance(item, dict):
|
||||
model_id = (
|
||||
item.get("id")
|
||||
or item.get("model")
|
||||
or item.get("name")
|
||||
)
|
||||
if not model_id:
|
||||
continue
|
||||
raw_models.append(
|
||||
{
|
||||
"id": model_id,
|
||||
"label": item.get("label", model_id),
|
||||
}
|
||||
)
|
||||
elif item:
|
||||
raw_models.append({"id": item, "label": item})
|
||||
raw_models = _configured_model_options(provider_cfg["models"])
|
||||
if not raw_models:
|
||||
raw_models = copy.deepcopy(_PROVIDER_MODELS.get(pid, []))
|
||||
# Plugin-only providers (e.g. 9router) are not in _PROVIDER_MODELS
|
||||
@@ -7132,21 +7091,9 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa
|
||||
_cp_model = _cp.get("model", "")
|
||||
if _cp_model:
|
||||
_cp_model_ids.append(_cp_model)
|
||||
_cp_models_dict = _cp.get("models")
|
||||
if isinstance(_cp_models_dict, dict):
|
||||
for _m_id in _cp_models_dict:
|
||||
if isinstance(_m_id, str) and _m_id.strip() and _m_id not in _cp_model_ids:
|
||||
_cp_model_ids.append(_m_id.strip())
|
||||
elif isinstance(_cp_models_dict, list):
|
||||
for _item in _cp_models_dict:
|
||||
if isinstance(_item, str):
|
||||
_mid = _item.strip()
|
||||
if _mid and _mid not in _cp_model_ids:
|
||||
_cp_model_ids.append(_mid)
|
||||
elif isinstance(_item, dict):
|
||||
_mid = str(_item.get("id") or _item.get("model") or _item.get("name") or "").strip()
|
||||
if _mid and _mid not in _cp_model_ids:
|
||||
_cp_model_ids.append(_mid)
|
||||
for _cp_model_id in _configured_model_ids(_cp.get("models")):
|
||||
if _cp_model_id not in _cp_model_ids:
|
||||
_cp_model_ids.append(_cp_model_id)
|
||||
|
||||
for _cp_model in _cp_model_ids:
|
||||
_dedup_key = f"{_slug}:{_cp_model}" if _slug else _cp_model
|
||||
@@ -7617,13 +7564,7 @@ def get_available_models(*, prefer_cache: bool = False, force_refresh: bool = Fa
|
||||
and isinstance(provider_cfg, dict)
|
||||
and "models" in provider_cfg
|
||||
):
|
||||
cfg_models = provider_cfg["models"]
|
||||
if isinstance(cfg_models, dict):
|
||||
raw_models = [{"id": k, "label": k} for k in cfg_models.keys()]
|
||||
elif isinstance(cfg_models, list):
|
||||
raw_models = [{"id": k["id"] if isinstance(k, dict) else k,
|
||||
"label": k.get("label", k["id"]) if isinstance(k, dict) else k}
|
||||
for k in cfg_models]
|
||||
raw_models = _configured_model_options(provider_cfg["models"])
|
||||
|
||||
if not raw_models:
|
||||
if pid == "moa":
|
||||
|
||||
@@ -800,6 +800,46 @@ def test_custom_provider_models_dict_routes_to_named_custom_provider():
|
||||
assert base_url == 'http://127.0.0.1:8080/v1'
|
||||
|
||||
|
||||
def test_custom_provider_models_string_list_routes_to_named_custom_provider_6121():
|
||||
"""#6121: picker-visible string-list models must route to their custom provider."""
|
||||
model, provider, base_url = _resolve_with_config(
|
||||
'qwen3.6:35b-a3b',
|
||||
provider='anthropic',
|
||||
default='claude-haiku-4-5',
|
||||
custom_providers=[{
|
||||
'name': 'storm-ollama',
|
||||
'base_url': 'http://ollama.test:11434/v1',
|
||||
'models': ['qwen3.6:35b-a3b', 'violet-lotus:latest'],
|
||||
}],
|
||||
)
|
||||
assert model == 'qwen3.6:35b-a3b'
|
||||
assert provider == 'custom:storm-ollama'
|
||||
assert base_url == 'http://ollama.test:11434/v1'
|
||||
|
||||
|
||||
def test_custom_provider_models_object_list_routes_to_named_custom_provider_6121():
|
||||
"""#6121: resolver accepts the same object-list ids as the model picker."""
|
||||
for model_entry in (
|
||||
{'id': 'picker-model-id'},
|
||||
{'model': 'picker-model-name'},
|
||||
{'name': 'picker-model-label'},
|
||||
):
|
||||
requested_model = next(iter(model_entry.values()))
|
||||
model, provider, base_url = _resolve_with_config(
|
||||
requested_model,
|
||||
provider='anthropic',
|
||||
default='claude-haiku-4-5',
|
||||
custom_providers=[{
|
||||
'name': 'Picker Parity',
|
||||
'base_url': 'https://models.example.test/v1',
|
||||
'models': [model_entry],
|
||||
}],
|
||||
)
|
||||
assert model == requested_model
|
||||
assert provider == 'custom:picker-parity'
|
||||
assert base_url == 'https://models.example.test/v1'
|
||||
|
||||
|
||||
# ── Issue #2047: parenthesized local provider names with ports ────────────
|
||||
|
||||
def test_custom_provider_name_with_parenthesized_port_uses_safe_slug():
|
||||
@@ -968,6 +1008,45 @@ def test_default_provider_models_not_prefixed(monkeypatch):
|
||||
assert not any(mid.startswith('@anthropic:') for mid in returned_ids), returned_ids
|
||||
|
||||
|
||||
def test_provider_config_object_list_catalog_uses_picker_supported_keys_6121(monkeypatch):
|
||||
"""Provider allowlists in the live catalog accept id/model/name object keys."""
|
||||
import api.config as _cfg
|
||||
old_cfg = dict(_cfg.cfg)
|
||||
_cfg.cfg['model'] = {
|
||||
'provider': 'myprov',
|
||||
'default': 'by-model-key',
|
||||
}
|
||||
_cfg.cfg['providers'] = {
|
||||
'myprov': {
|
||||
'models': [
|
||||
{'model': 'by-model-key', 'label': 'By Model Key'},
|
||||
{'name': 'by-name-key'},
|
||||
{'id': 'by-id-key', 'label': 'By Id Key'},
|
||||
],
|
||||
},
|
||||
}
|
||||
try:
|
||||
_cfg._cfg_mtime = _cfg.Path(_cfg._get_config_path()).stat().st_mtime
|
||||
except Exception:
|
||||
_cfg._cfg_mtime = 0.0
|
||||
monkeypatch.setattr(_cfg, "_read_live_provider_model_ids", lambda pid: [])
|
||||
try:
|
||||
result = _cfg.get_available_models()
|
||||
finally:
|
||||
_cfg.cfg.clear()
|
||||
_cfg.cfg.update(old_cfg)
|
||||
|
||||
groups = {group['provider_id']: group['models'] for group in result['groups']}
|
||||
model_rows = groups.get('myprov') or []
|
||||
model_ids = [row['id'] for row in model_rows]
|
||||
assert 'by-model-key' in model_ids
|
||||
assert 'by-name-key' in model_ids
|
||||
assert 'by-id-key' in model_ids
|
||||
labels = {row['id']: row['label'] for row in model_rows}
|
||||
assert labels['by-model-key'] == 'By Model Key'
|
||||
assert labels['by-id-key'] == 'By Id Key'
|
||||
|
||||
|
||||
# ── get_available_models(): phantom "Custom" group regression ─────────────
|
||||
#
|
||||
# When the user has model.provider set to a real provider (e.g. openai-codex)
|
||||
|
||||
Reference in new issue
Block a user