diff --git a/CHANGELOG.md b/CHANGELOG.md index 37cddfbf..6b9ffc1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,10 @@ ## [Unreleased] +### Fixed + +- **A session no longer gets stuck sending to the wrong provider after its stored provider becomes stale.** When a session's persisted model/provider no longer matches where that model actually lives (e.g. the model moved groups, or an older session carried a now-foreign provider), chat start now repairs the binding to the correct provider — but only on positive proof: it reroutes solely when the live model catalog shows the stored group genuinely lacks the model AND exactly one other healthy group owns it. It checks both the primary and overflow (`extra_models`) model buckets before deciding a group doesn't own a model, and it fails safe — preserving the stored provider without rerouting — whenever any relevant catalog probe failed (a transient discovery error is never treated as proof the model is absent). Ordinary matching-provider sends make no catalog call, and explicit/ambiguous/unknown/stored-owner picks are all left untouched. Thanks @rodboev. (#5873, #5731) + ### Internal - **Centralized WebUI static-asset resolution.** The app-shell (`index.html`), `/static/*`, manifest, service worker, and favicon now resolve through a single shared resolver instead of scattered `__file__`-relative logic — behavior-preserving (same bytes, headers, cache signature, CSRF-per-request injection, and path-traversal rejection), with new resolver regression tests. Thanks @rodboev. (#5557, #2695) diff --git a/api/routes.py b/api/routes.py index 1f1a7a6d..726d3542 100644 --- a/api/routes.py +++ b/api/routes.py @@ -6001,6 +6001,85 @@ def _catalog_model_id_matches(candidate: str, model: str) -> bool: return candidate.replace("-", ".").lower() == model.replace("-", ".").lower() +def _catalog_group_owns_exact_model(group: dict, model: str) -> bool: + provider_id = str(group.get("provider_id") or "").strip() + wrapper = f"@{provider_id}:" + for bucket in ("models", "extra_models"): + for entry in group.get(bucket) or []: + if not isinstance(entry, dict): + continue + candidate = str(entry.get("id") or "").strip() + if candidate.lower().startswith(wrapper.lower()): + candidate = candidate[len(wrapper):] + if candidate == model or _catalog_model_id_matches(candidate, model): + return True + return False + + +def _repair_foreign_session_model_provider( + session, + *, + requested_model: str, + requested_provider: str | None, + resolved_model: str, + resolved_provider: str | None, + explicit_model_pick: bool, + profile_provider: str | None, +) -> str | None: + """Repair a stale provider only when the cached catalog names one owner.""" + stored_model = str(getattr(session, "model", "") or "").strip() + stored_provider = _clean_session_model_provider(getattr(session, "model_provider", None)) + requested_provider = _clean_session_model_provider(requested_provider) + resolved_provider = _clean_session_model_provider(resolved_provider) + profile_provider = _clean_session_model_provider(profile_provider) + _, qualified_provider = _split_provider_qualified_model(requested_model) + if ( + explicit_model_pick + or qualified_provider + or not stored_model + or not stored_provider + or ( + str(requested_model or "").strip() != stored_model + and not _catalog_model_id_matches(str(requested_model or "").strip(), stored_model) + ) + or requested_provider != stored_provider + or ( + resolved_model != stored_model + and not _catalog_model_id_matches(resolved_model, stored_model) + ) + or resolved_provider != stored_provider + or not profile_provider + or profile_provider == stored_provider + ): + return resolved_provider + + try: + catalog = get_available_models(prefer_cache=True) + except Exception: + return resolved_provider + groups = [group for group in catalog.get("groups") or [] if isinstance(group, dict)] + stored_groups = [ + group + for group in groups + if str(group.get("provider_id") or "").strip().lower() == stored_provider + ] + if ( + not stored_groups + or any(group.get("models_endpoint_error") for group in stored_groups) + or any(_catalog_group_owns_exact_model(group, stored_model) for group in stored_groups) + ): + return resolved_provider + owners = [ + group + for group in groups + if str(group.get("provider_id") or "").strip().lower() != stored_provider + and _catalog_group_owns_exact_model(group, stored_model) + ] + if len(owners) != 1: + return resolved_provider + return str(owners[0].get("provider_id") or "").strip() or resolved_provider + + def _clean_session_model_provider(value: str | None) -> str | None: provider = str(value or "").strip().lower() if not provider or provider == "default": @@ -20934,6 +21013,20 @@ def _handle_chat_start(handler, body, diag=None): profile_config=_pp_cfg, explicit_model_pick=explicit_model_pick, ) + catalog_profile_provider = _pp_provider + if catalog_profile_provider is None and isinstance(_pp_cfg, dict): + profile_model_config = _pp_cfg.get("model") or {} + if isinstance(profile_model_config, dict): + catalog_profile_provider = profile_model_config.get("provider") + model_provider = _repair_foreign_session_model_provider( + s, + requested_model=requested_model, + requested_provider=requested_provider, + resolved_model=model, + resolved_provider=model_provider, + explicit_model_pick=explicit_model_pick, + profile_provider=catalog_profile_provider, + ) if model_provider == "moa" and moa_config is None: if webui_gateway_chat_enabled(get_config()): return bad(handler, "MoA override is unavailable on gateway-backed sessions", 409) diff --git a/tests/test_issue5731_session_model_provider_repair.py b/tests/test_issue5731_session_model_provider_repair.py new file mode 100644 index 00000000..887fb1ad --- /dev/null +++ b/tests/test_issue5731_session_model_provider_repair.py @@ -0,0 +1,269 @@ +"""Regression coverage for catalog-backed session provider repair (#5731).""" + +from types import SimpleNamespace + +import pytest + +import api.routes as routes + + +def _catalog(*groups): + return {"groups": list(groups)} + + +def _group(provider_id, *models): + return {"provider_id": provider_id, "models": [{"id": model} for model in models]} + + +def _session(*, model="kilo/minimax/minimax-m3", provider="ollama"): + return SimpleNamespace(model=model, model_provider=provider) + + +def _repair( + session, + catalog, + *, + requested_model=None, + requested_provider=None, + resolved_model=None, + profile_provider="kilocode", + explicit_model_pick=False, +): + return routes._repair_foreign_session_model_provider( + session, + requested_model=requested_model if requested_model is not None else session.model, + requested_provider=requested_provider if requested_provider is not None else session.model_provider, + resolved_model=resolved_model if resolved_model is not None else session.model, + resolved_provider=session.model_provider, + explicit_model_pick=explicit_model_pick, + profile_provider=profile_provider, + ) + + +def test_poisoned_pair_repairs_at_chat_start(monkeypatch, tmp_path): + """A plain send repairs the documented Kilo model before normal persistence.""" + session = SimpleNamespace( + session_id="issue-5731", + workspace=str(tmp_path), + model="kilo/minimax/minimax-m3", + model_provider="ollama", + profile="default", + messages=[], + context_messages=[], + pending_user_message=None, + save=lambda: None, + ) + captured = {} + catalog_calls = [] + + def start_run(s, **kwargs): + captured.update(kwargs) + routes._prepare_chat_start_session_for_stream( + s, + msg=kwargs["msg"], + attachments=kwargs["attachments"], + workspace=kwargs["workspace"], + model=kwargs["model"], + model_provider=kwargs["model_provider"], + stream_id="issue-5731-stream", + ) + return {"stream_id": "issue-5731-stream"} + + monkeypatch.setattr(routes, "_get_or_materialize_session", lambda _sid, **_kwargs: session) + monkeypatch.setattr(routes, "_resolve_chat_workspace_with_recovery", lambda _s, _w: str(tmp_path)) + monkeypatch.setattr(routes, "_read_profile_model_config", lambda _s, _p: (None, None, {"model": {"provider": "kilocode"}})) + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: ( + catalog_calls.append(prefer_cache) + or _catalog( + _group("ollama", "llama3.2"), + _group("kilocode", "@kilocode:kilo/minimax/minimax-m3"), + ) + ), + ) + monkeypatch.setattr(routes, "_start_run", start_run) + monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200: payload) + + routes._handle_chat_start(None, {"session_id": session.session_id, "message": "continue"}) + + assert captured["model_provider"] == "kilocode", captured["model_provider"] + assert session.model == "kilo/minimax/minimax-m3" + assert session.model_provider == "kilocode" + assert catalog_calls == [True] + + +def test_catalog_equivalent_owner_repairs_poisoned_pair(monkeypatch): + session = _session(model="gpt-4o-mini", provider="ollama") + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: _catalog( + _group("ollama", "llama3.2"), + _group("kilocode", "GPT.4O.MINI"), + ), + ) + + assert _repair(session, None) == "kilocode" + + +def test_equivalent_request_model_repairs_poisoned_pair(monkeypatch): + session = _session(model="GPT.4O.MINI", provider="ollama") + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: _catalog( + _group("ollama", "llama3.2"), + _group("kilocode", "gpt-4o-mini"), + ), + ) + + assert _repair( + session, + None, + requested_model="gpt-4o-mini", + resolved_model="gpt-4o-mini", + ) == "kilocode" + + +@pytest.mark.parametrize("provider", ["ollama", "lmstudio"]) +def test_self_hosted_exact_owner_is_preserved(monkeypatch, provider): + session = _session(model="vendor/model/with/slashes", provider=provider) + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: _catalog( + _group(provider, "vendor/model/with/slashes"), + _group("kilocode", "other-model"), + ), + ) + + assert _repair(session, None) == provider + + +def test_stored_owner_in_extra_models_is_preserved(monkeypatch): + session = _session() + stored_group = _group("ollama", "llama3.2") + stored_group["extra_models"] = [{"id": "kilo/minimax/minimax-m3"}] + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: _catalog( + stored_group, + _group("kilocode", "kilo/minimax/minimax-m3"), + ), + ) + + assert _repair(session, None) == "ollama" + + +def test_stored_provider_discovery_failure_is_preserved(monkeypatch): + session = _session() + stored_group = _group("ollama", "llama3.2") + stored_group["models_endpoint_error"] = "catalog unavailable" + monkeypatch.setattr( + routes, + "get_available_models", + lambda *, prefer_cache=False: _catalog( + stored_group, + _group("kilocode", "kilo/minimax/minimax-m3"), + ), + ) + + assert _repair(session, None) == "ollama" + + +@pytest.mark.parametrize( + ("requested_model", "explicit_model_pick"), + [ + ("kilo/minimax/minimax-m3", True), + ("@ollama:kilo/minimax/minimax-m3", False), + ], +) +def test_explicit_or_qualified_model_selection_is_preserved(monkeypatch, requested_model, explicit_model_pick): + session = _session() + monkeypatch.setattr(routes, "get_available_models", lambda **_kwargs: pytest.fail("catalog must not be read")) + + assert _repair( + session, + None, + requested_model=requested_model, + explicit_model_pick=explicit_model_pick, + ) == "ollama" + + +@pytest.mark.parametrize( + "catalog", + [ + _catalog(_group("kilocode", "kilo/minimax/minimax-m3")), + _catalog(_group("ollama", "llama3.2")), + _catalog( + _group("ollama", "llama3.2"), + _group("kilocode", "kilo/minimax/minimax-m3"), + _group("other", "kilo/minimax/minimax-m3"), + ), + ], +) +def test_missing_or_ambiguous_catalog_evidence_is_preserved(monkeypatch, catalog): + session = _session() + monkeypatch.setattr(routes, "get_available_models", lambda *, prefer_cache=False: catalog) + + assert _repair(session, catalog) == "ollama" + + +def test_matching_profile_provider_skips_catalog(monkeypatch): + session = _session() + monkeypatch.setattr(routes, "get_available_models", lambda **_kwargs: pytest.fail("catalog must not be read")) + + assert _repair(session, None, profile_provider="ollama") == "ollama" + + +@pytest.mark.parametrize( + ("body", "catalog"), + [ + ({}, _catalog(_group("ollama", "kilo/minimax/minimax-m3"))), + ({"explicit_model_pick": True}, _catalog(_group("kilocode", "kilo/minimax/minimax-m3"))), + ({"model": "@ollama:kilo/minimax/minimax-m3"}, _catalog(_group("ollama", "kilo/minimax/minimax-m3"))), + ( + {}, + _catalog( + _group("ollama", "llama3.2"), + _group("kilocode", "kilo/minimax/minimax-m3"), + _group("other", "kilo/minimax/minimax-m3"), + ), + ), + ({}, RuntimeError("catalog unavailable")), + ], +) +def test_preservation_cases_still_reach_chat_start(monkeypatch, tmp_path, body, catalog): + session = SimpleNamespace( + session_id="issue-5731-preserve", + workspace=str(tmp_path), + model="kilo/minimax/minimax-m3", + model_provider="ollama", + profile="default", + messages=[], + context_messages=[], + pending_user_message=None, + ) + captured = {} + + def get_catalog(*, prefer_cache=False): + if isinstance(catalog, Exception): + raise catalog + return catalog + + monkeypatch.setattr(routes, "_get_or_materialize_session", lambda _sid, **_kwargs: session) + monkeypatch.setattr(routes, "_resolve_chat_workspace_with_recovery", lambda _s, _w: str(tmp_path)) + monkeypatch.setattr(routes, "_read_profile_model_config", lambda _s, _p: (None, None, {"model": {"provider": "kilocode"}})) + monkeypatch.setattr(routes, "get_available_models", get_catalog) + monkeypatch.setattr(routes, "_start_run", lambda _s, **kwargs: captured.update(kwargs) or {"ok": True}) + monkeypatch.setattr(routes, "j", lambda _handler, payload, status=200: payload) + + routes._handle_chat_start( + None, + {"session_id": session.session_id, "message": "continue", **body}, + ) + + assert captured["model_provider"] == "ollama"