diff --git a/CHANGELOG.md b/CHANGELOG.md index e94fc18a..f2a0cb9f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,8 @@ ### Fixed +- **`@provider:model` picker selections no longer 404 against the Hermes gateway.** The WebUI model picker deliberately keeps the qualified `@provider:model` form for internal routing (#1253), but nothing converted it back into a bare model name on the gateway request path, so the `@`-prefixed string was forwarded verbatim to the upstream provider API and 404'd with "Unknown provider" / "model not found" for any model not covered by a `model_routes` workaround entry. The gateway request body (in both the runs-API and legacy-chat streaming paths) now carries the bare model name via a new `_gateway_model_field()` helper, and provider normalization in `_clean_session_model_provider()` resolves the provider through the **shared** `_parse_provider_qualified_model_id()` grammar instead of a positional colon split — so a multi-segment custom provider ID (`@custom::`, `@custom:::`) yields the real model rather than a fragment, while plain (non-`@`) provider IDs whose colons belong to the ID itself are preserved verbatim. Thanks @dss539. (#6723, #6722) + - **Custom providers with a dict-shaped `models` catalog now show every model in the Providers card.** The Providers settings card built a custom provider's model list from only the singular `model` field or a plain `models` list, so a provider configured with a dict-shaped `models` catalog (e.g. a LiteLLM gateway with 20 models) showed just the single `model` entry while the chat model picker already listed the full catalog. The card now builds its list the same way the picker does — the singular `model` first, then the remaining catalog IDs appended via the shared `_configured_model_ids` helper (which strips whitespace, drops empty IDs, and de-duplicates, including when the singular model is also a dict key) — so the two stay consistent. String-shaped `models`, providers without a `models` block, and empty/whitespace-only configs are unchanged. Thanks @KimSpecht. (#6774) - **Matrix gateway sessions are now correctly grouped under Messaging.** Sessions originating from the Matrix gateway (`Platform.MATRIX`) were not listed in the messaging-source allowlists, so they fell into the default/other bucket in the sidebar instead of being grouped and labeled as messaging sessions like Telegram, WhatsApp, WeChat, and the other chat platforms. `matrix` (labeled "Matrix") is now added to the messaging-source allowlists on both the Python (`MESSAGING_SOURCES` / `SOURCE_LABELS`) and JavaScript (`_MESSAGING_RAW_SOURCES` / `_MESSAGING_SOURCE_LABELS`) sides, keeping the two in sync; no other source's classification changes. Thanks @rudidev08. (#6816) diff --git a/api/gateway_chat.py b/api/gateway_chat.py index 4072d39e..75ca6211 100644 --- a/api/gateway_chat.py +++ b/api/gateway_chat.py @@ -24,6 +24,7 @@ from api.config import ( STREAM_PARTIAL_TEXT, STREAM_REASONING_TEXT, _get_session_agent_lock, + _parse_provider_qualified_model_id, coerce_reasoning_effort_for_model, gateway_approval_unavailable_reason, gateway_supports_approval, @@ -152,6 +153,28 @@ _WEBUI_GATEWAY_USE_RUNS_API_ENV = "HERMES_WEBUI_GATEWAY_USE_RUNS_API" _GATEWAY_CHAT_BACKENDS = {"gateway", "api_server", "api-server"} +def _gateway_model_field(model: str | None) -> str: + """Return the bare model name to put in a gateway request body. + + The picker and ``_resolve_compatible_session_model_state`` intentionally + keep the full ``@provider:model`` string for internal routing (#1253), but + the gateway expects a bare model name and carries the provider separately. + Sent verbatim, ``@ollama-cloud:minimax-m2.7`` is forwarded to the upstream + provider API, which 404s on the ``@``-prefixed string (#6722). + + Parsing is delegated to ``config._parse_provider_qualified_model_id()`` so + a multi-segment custom provider ID (``@custom:backup:model-a``) yields the + real model (``model-a``) instead of a positional-split fragment. + """ + if not model: + return "" + value = str(model).strip() + parsed = _parse_provider_qualified_model_id(value) + if parsed: + return str(parsed[0] or "").strip() + return value + + # Total byte-silence budget (seconds) for the gateway SSE socket, applied via # ``urlopen(timeout=...)``. A stream that emits ANY byte within the window never # trips it, so a genuinely alive (if slow) token stream is untouched; only more @@ -529,7 +552,7 @@ def _run_gateway_runs_api_streaming( if isinstance(run_input, list): run_input = [{"role": "user", "content": run_input}] run_body = { - "model": model or "default", + "model": _gateway_model_field(model) or "default", "input": run_input, **body_extras, "session_id": session_id, @@ -1018,7 +1041,7 @@ def _run_gateway_chat_streaming( logger.debug("Failed to build gateway multimodal attachment payload", exc_info=True) message_content = str(msg_text or "") body = { - "model": model or "default", + "model": _gateway_model_field(model) or "default", "stream": True, "messages": [*prefill_messages, {"role": "user", "content": message_content}], } diff --git a/api/routes.py b/api/routes.py index 99103fae..bba920fa 100644 --- a/api/routes.py +++ b/api/routes.py @@ -2869,6 +2869,7 @@ from api.config import ( get_config_for_profile_home, _cfg_lock, PENDING_BG_TASK_COMPLETIONS, + _parse_provider_qualified_model_id, ) from api import config as api_config from api.helpers import ( @@ -6394,20 +6395,42 @@ def _repair_foreign_session_model_provider( def _clean_session_model_provider(value: str | None) -> str | None: + """Normalize a stored/requested provider value to a bare provider ID. + + An ``@``-prefixed value is a provider-qualified *model* hint, so the + provider is resolved with the shared + ``config._parse_provider_qualified_model_id()`` grammar rather than a + positional colon split — that keeps multi-segment custom provider IDs + (``custom:``, ``custom::``) whole while still dropping a + trailing model segment (#6722). Values without the ``@`` marker are already + plain provider IDs, whose colons belong to the ID itself, so they are + preserved verbatim. + """ provider = str(value or "").strip().lower() if not provider or provider == "default": return None if provider.startswith("@"): - provider = provider[1:] + parsed = _parse_provider_qualified_model_id(provider) + provider = parsed[1].strip() if parsed else provider[1:] return provider or None def _split_provider_qualified_model(model: str) -> tuple[str, str | None]: + """Split an ``@provider:model`` hint into ``(bare_model, provider)``. + + Delegates the grammar to ``config._parse_provider_qualified_model_id()``, + the shared parser that already knows how to keep a multi-segment custom + provider ID (``custom:``, ``custom::``) intact while + still letting the model segment carry its own colons for tags such as + ``:free``. Keeping one parser here means every caller in this module and + the gateway request path resolve the same provider/model pair (#6722). + """ model = str(model or "").strip() - if model.startswith("@") and ":" in model: - provider_hint, bare_model = model[1:].rsplit(":", 1) + parsed = _parse_provider_qualified_model_id(model) + if parsed: + bare_model, provider_hint = parsed provider = _clean_session_model_provider(provider_hint) - bare = bare_model.strip() + bare = str(bare_model or "").strip() if provider and bare: return bare, provider return model, None diff --git a/tests/test_issue6722_provider_qualified_model_leak.py b/tests/test_issue6722_provider_qualified_model_leak.py new file mode 100644 index 00000000..3c6f0059 --- /dev/null +++ b/tests/test_issue6722_provider_qualified_model_leak.py @@ -0,0 +1,308 @@ +"""Regression tests for issue #6722 — @provider:model leaking to the gateway. + +The WebUI model picker deliberately carries a provider-qualified model string +(``@provider:model``) through internal routing (#1253). The leak involved +three parsing/request-boundary failures: + +1. ``_clean_session_model_provider`` did not parse qualified values with the + shared grammar, so provider selection could retain or truncate the wrong + segments. +2. ``_split_provider_qualified_model`` used a positional split that could not + distinguish multi-segment custom provider IDs from colon-tagged model IDs. +3. The gateway request builders sent the internally qualified model string + verbatim as ``body["model"]``, which the upstream provider API 404'd on. + +Internal session-state resolution deliberately keeps the qualified string for +routing and provider-switch repair. The gateway boundary alone strips it to the +bare model. All parsing now uses the single shared grammar in +``config._parse_provider_qualified_model_id()``. That parser is the important +part of the fix: a positional colon split cannot tell +``@ollama-cloud:deepseek-v4-flash:0731`` (single-segment provider, tagged +model) from ``@custom:backup:model-a`` (two-segment custom provider ID), and +guessing either way breaks the other. These tests pin both shapes so a future +"simplification" back to a split heuristic fails loudly. +""" + +from collections import OrderedDict +import json + +import api.gateway_chat as gateway_chat +import api.models as models +import api.streaming as streaming +from api.config import STREAMS, create_stream_channel +from api.models import new_session + + +# (qualified value, expected bare model, expected provider) +# +# The custom:* rows are the ones a positional split gets wrong. ``custom:backup`` +# and ``custom::`` are legitimate multi-segment provider IDs, so the +# provider is NOT just the text before the first colon. +QUALIFIED_MODEL_CASES = [ + # Single-segment provider, plain model — the originally reported #6722 case. + ("@ollama-cloud:deepseek-v4-flash", "deepseek-v4-flash", "ollama-cloud"), + # Single-segment provider, model carrying its own colon tag. + ("@ollama-cloud:deepseek-v4-flash:0731", "deepseek-v4-flash:0731", "ollama-cloud"), + # Named custom provider — provider keeps both segments. + ("@custom:backup:model-a", "model-a", "custom:backup"), + # Named custom provider AND a tagged model. + ("@custom:backup:model-a:free", "model-a:free", "custom:backup"), + # host:port custom provider IDs. + ("@custom:192.168.1.5:11434:llama4", "llama4", "custom:192.168.1.5:11434"), + ("@custom:localhost:1234:qwen3", "qwen3", "custom:localhost:1234"), + # Slash-style model id plus a :free tag (#6221 shape). + ("@openrouter:meta/llama-4:free", "meta/llama-4:free", "openrouter"), + ("@anthropic:claude-opus-4.8", "claude-opus-4.8", "anthropic"), +] + + +class TestSharedParserIsTheSingleGrammar: + """routes' splitter must agree with the shared config parser, exactly.""" + + def test_split_provider_qualified_model_matches_shared_parser(self): + from api.config import _parse_provider_qualified_model_id + from api.routes import _split_provider_qualified_model + + for value, expected_model, expected_provider in QUALIFIED_MODEL_CASES: + assert _parse_provider_qualified_model_id(value) == ( + expected_model, + expected_provider, + ), f"shared parser disagrees for {value!r}" + assert _split_provider_qualified_model(value) == ( + expected_model, + expected_provider, + ), f"routes splitter diverged from the shared parser for {value!r}" + + def test_non_qualified_values_pass_through_unchanged(self): + from api.routes import _split_provider_qualified_model + + for value in ("gpt-5.4", "anthropic/claude-opus-4.8", "custom:backup", "@nocolon", ""): + assert _split_provider_qualified_model(value) == (value, None) + + +class TestCleanSessionModelProvider: + """The provider field must never keep a model segment attached.""" + + def test_at_qualified_values_resolve_to_the_provider_only(self): + from api.routes import _clean_session_model_provider + + for value, _expected_model, expected_provider in QUALIFIED_MODEL_CASES: + assert _clean_session_model_provider(value) == expected_provider, ( + f"{value!r} must clean to provider {expected_provider!r}" + ) + + def test_non_at_provider_ids_keep_their_colons(self): + """A bare ``custom:backup`` is already a provider ID, not a hint.""" + from api.routes import _clean_session_model_provider + + assert _clean_session_model_provider("custom:backup") == "custom:backup" + assert _clean_session_model_provider("custom:localhost:1234") == "custom:localhost:1234" + assert _clean_session_model_provider("ollama-cloud") == "ollama-cloud" + + def test_empty_and_default_values_resolve_to_none(self): + from api.routes import _clean_session_model_provider + + for value in (None, "", " ", "default", "@"): + assert _clean_session_model_provider(value) is None + + +class TestSessionModelStatePreservesQualifierForRepair: + """The request path must keep the @provider: qualifier through resolution so + the compatibility/repair step (and PR #6718's custom-provider normalizer) can + still see the qualified form. The qualifier is stripped to the bare model ONLY + at the gateway boundary (see TestGatewayModelField).""" + + def test_unavailable_qualified_provider_repairs_to_active_default(self, monkeypatch): + """A session pinned to a removed/unconfigured provider must revert to the + active default on a non-explicit resolve — the premature strip in the + previous revision returned (bare_model, removed) instead.""" + import api.routes as routes + + monkeypatch.setattr( + routes, + "get_available_models", + lambda: { + "active_provider": "openai-codex", + "default_model": "gpt-5.5", + "groups": [ + { + "provider_id": "openai-codex", + "models": [{"id": "@openai-codex:gpt-5.5"}], + }, + ], + }, + ) + model_value, provider = routes._session_model_state_from_request( + "@removed:mistral-large", None, "removed" + ) + assert model_value == "gpt-5.5", ( + f"removed/unconfigured qualified provider must repair to the active " + f"default, got model_value={model_value!r}" + ) + assert provider == "openai-codex" + + def test_active_provider_qualified_model_preserves_qualifier_for_routing(self, monkeypatch): + """A qualified model naming the active provider is preserved intact so + downstream routing (resolve_model_provider) can use the provider.""" + import api.routes as routes + + monkeypatch.setattr( + routes, + "get_available_models", + lambda: { + "active_provider": "openai-codex", + "default_model": "gpt-5.5", + "groups": [ + { + "provider_id": "openai-codex", + "models": [{"id": "@openai-codex:gpt-5.5"}], + }, + ], + }, + ) + model_value, provider = routes._session_model_state_from_request( + "@openai-codex:gpt-5.5", "openai-codex" + ) + assert model_value == "@openai-codex:gpt-5.5", ( + f"qualifier must survive session-state resolution for routing, got {model_value!r}" + ) + assert provider == "openai-codex" + + +class TestGatewayModelField: + """body['model'] must be the bare model for every provider shape.""" + + def test_gateway_model_field_strips_qualifier(self): + from api.gateway_chat import _gateway_model_field + + for value, expected_model, _expected_provider in QUALIFIED_MODEL_CASES: + assert _gateway_model_field(value) == expected_model, ( + f"gateway body model for {value!r} must be {expected_model!r}" + ) + + def test_unqualified_models_are_untouched(self): + from api.gateway_chat import _gateway_model_field + + assert _gateway_model_field("gpt-5.4") == "gpt-5.4" + assert _gateway_model_field("anthropic/claude-opus-4.8") == "anthropic/claude-opus-4.8" + assert _gateway_model_field(None) == "" + assert _gateway_model_field("") == "" + + +def _stub_prefill(monkeypatch): + monkeypatch.setattr( + streaming, + "_load_webui_prefill_context", + lambda cfg: { + "status": "not_configured", + "source": "none", + "label": "", + "message_count": 0, + "messages": [], + }, + ) + monkeypatch.setattr(streaming, "_prefill_messages_with_webui_context", lambda ctx, cfg: []) + + +class _FakeChatResponse: + def __enter__(self): + return self + + def __exit__(self, exc_type, exc, tb): + return False + + def __iter__(self): + yield b'data: {"choices":[{"delta":{"content":"ok"}}]}\n\n' + yield b"data: [DONE]\n\n" + + +def _run_legacy_gateway_chat(tmp_path, monkeypatch, model): + """Drive the legacy chat-completions path and return the request body.""" + session_dir = tmp_path / "sessions" + session_dir.mkdir(exist_ok=True) + monkeypatch.setattr(models, "SESSION_DIR", session_dir) + monkeypatch.setattr(models, "SESSION_INDEX_FILE", session_dir / "_index.json") + monkeypatch.setattr(models, "SESSIONS", OrderedDict()) + + captured = {} + + def fake_urlopen(req, timeout=0): + captured["body"] = req.data.decode("utf-8") + return _FakeChatResponse() + + monkeypatch.setenv("HERMES_WEBUI_GATEWAY_BASE_URL", "http://gateway.local") + _stub_prefill(monkeypatch) + monkeypatch.setattr(gateway_chat.urllib.request, "urlopen", fake_urlopen) + + s = new_session() + stream_id = f"stream-6722-legacy-{model}" + s.active_stream_id = stream_id + s.pending_user_message = "hi" + s.pending_attachments = [] + s.pending_started_at = 123 + s.save() + channel = create_stream_channel() + channel.subscribe() + STREAMS[stream_id] = channel + + gateway_chat._run_gateway_chat_streaming( + s.session_id, "hi", model, str(tmp_path), stream_id, [] + ) + return json.loads(captured["body"]) + + +class TestGatewayRequestBodiesCarryBareModel: + """End-to-end: the bytes actually sent to the gateway.""" + + def test_legacy_chat_completions_body_has_bare_model(self, tmp_path, monkeypatch): + payload = _run_legacy_gateway_chat( + tmp_path, monkeypatch, "@ollama-cloud:deepseek-v4-flash" + ) + assert payload["model"] == "deepseek-v4-flash" + assert not payload["model"].startswith("@") + + def test_legacy_body_keeps_named_custom_provider_model_intact(self, tmp_path, monkeypatch): + """The multi-segment case a positional split would truncate.""" + payload = _run_legacy_gateway_chat( + tmp_path, monkeypatch, "@custom:backup:model-a:free" + ) + assert payload["model"] == "model-a:free" + + def test_legacy_body_keeps_host_port_custom_provider_model_intact(self, tmp_path, monkeypatch): + payload = _run_legacy_gateway_chat( + tmp_path, monkeypatch, "@custom:192.168.1.5:11434:llama4" + ) + assert payload["model"] == "llama4" + + def test_runs_api_body_has_bare_model(self, monkeypatch): + """The runs API builder is the second gateway request site.""" + captured = {} + + def fake_urlopen(req, timeout=0): + captured["body"] = req.data.decode("utf-8") + raise AssertionError("stop after the POST body is captured") + + monkeypatch.setattr(gateway_chat.urllib.request, "urlopen", fake_urlopen) + monkeypatch.setattr(gateway_chat, "update_active_run", lambda *a, **k: None) + + try: + gateway_chat._run_gateway_runs_api_streaming( + "sess-6722", + "hi", + "@custom:backup:model-a:free", + "/tmp", + "stream-6722-runs", + "http://gateway.local", + "", + [], + {}, + put_gateway_event=lambda *a, **k: None, + cancel_event=None, + ) + except Exception: + pass + + assert "body" in captured, "runs API POST body was never built" + payload = json.loads(captured["body"]) + assert payload["model"] == "model-a:free" + assert not payload["model"].startswith("@")