diff --git a/CHANGELOG.md b/CHANGELOG.md index fb96b9ce..6bec8aa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/api/config.py b/api/config.py index 7bba9db8..e5e0435f 100644 --- a/api/config.py +++ b/api/config.py @@ -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: — 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": diff --git a/tests/test_model_resolver.py b/tests/test_model_resolver.py index 165e4d3c..4c0a7c42 100644 --- a/tests/test_model_resolver.py +++ b/tests/test_model_resolver.py @@ -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)