Merge pull request #5521 from nesquena/release/stage-5511
Release — route user-defined providers: models (#5511), hardened
This commit is contained in:
commit
2981f71181
3 changed files with 259 additions and 9 deletions
|
|
@ -14,6 +14,7 @@
|
|||
|
||||
### Fixed
|
||||
|
||||
- **Models from a user-defined provider in `config.yaml` now route correctly.** If you defined a custom provider under the `providers:` block with an explicit `models:` allowlist, a bare model id from that list wasn't matched by request-time routing — it silently fell back to the default provider and often 404'd. Resolution now scans your `providers:` allowlists (exact match, same as `custom_providers:`), so those models reach the right endpoint. Copilot is deliberately excluded from this scan because `providers.copilot.models` is a per-model settings map (reasoning effort, limits), not a routing allowlist. Thanks @akay64. (#5511)
|
||||
- **The transcript no longer scrolls up when the composer grows.** When you were pinned to the bottom of a conversation and the composer got taller — multi-line typing, `Shift+Enter`, or a multi-line / dictation paste — the transcript viewport shrank by the same amount and left you stranded a row or two above the bottom (it read as the chat "randomly scrolling up" during normal use). The transcript now re-pins to the bottom whenever the composer grows, but only when you were genuinely still pinned there — if you'd scrolled up to read history, nothing yanks you back down. (#5516, #5514, #5515)
|
||||
- **A deleted conversation no longer comes back as a read-only "ghost" after a restart.** Deleting a WebUI session removed its file but left a recoverable row in the agent's state database, so after a server restart and page reload the deleted conversation could reappear as a read-only "Agent" session still showing its old transcript. Deleted WebUI sessions are now recorded in a durable tombstone that the sidebar projection, startup backup-recovery, audit, and session-claim paths all honor, so a deleted session stays gone (the backend keeps returning the 404 the client relies on to clear the stale route). Genuine crash recovery is unaffected — a session whose sidecar is lost without a delete is still restored — imported external-channel (messaging) sessions are exempt and keep their transcript, and a deleted id is automatically un-tombstoned if the same session is later re-created or re-imported. Thanks @rodboev. (#5504, #5498)
|
||||
- **A drag that ends over the sidebar no longer switches conversations by accident.** Releasing the mouse (a stray `pointerup`) over a session row after a drag that began elsewhere in the main content area could switch you into that conversation unintentionally. Session switching now ignores a `pointerup` that isn't a genuine click on the row. Thanks @webtecnica. (#5487, #5462)
|
||||
|
|
|
|||
|
|
@ -1444,12 +1444,15 @@ def _canonicalise_provider_id(name: object) -> str:
|
|||
# keep as-is to avoid round-tripping through aliases (e.g. x-ai → xai).
|
||||
if raw in _PROVIDER_DISPLAY or raw in _PROVIDER_MODELS:
|
||||
return raw
|
||||
# Try alias resolution. Only accept the result if it's itself a
|
||||
# canonical id in _PROVIDER_DISPLAY — that prevents aliases pointing
|
||||
# at non-canonical strings (legacy, hermes_cli-specific) from leaking
|
||||
# in. Falls back to the normalised input otherwise.
|
||||
# Try alias resolution. Accept the result if it's a canonical id known to
|
||||
# either _PROVIDER_DISPLAY OR _PROVIDER_MODELS (mirroring the direct-hit
|
||||
# check above) — some canonical targets (e.g. `gemini`) are indexed in
|
||||
# _PROVIDER_MODELS but not _PROVIDER_DISPLAY, so a _DISPLAY-only check
|
||||
# rejected valid aliases like `google-gemini`→`gemini`, leaving the id
|
||||
# uncanonicalised and silently breaking provider-ownership checks (#5511).
|
||||
# This still blocks aliases that point at non-canonical/legacy strings.
|
||||
resolved = _resolve_provider_alias(raw)
|
||||
if resolved and resolved.lower() in _PROVIDER_DISPLAY:
|
||||
if resolved and (resolved.lower() in _PROVIDER_DISPLAY or resolved.lower() in _PROVIDER_MODELS):
|
||||
return resolved.lower()
|
||||
return raw
|
||||
|
||||
|
|
@ -2522,16 +2525,47 @@ def resolve_model_provider(model_id: str) -> tuple:
|
|||
)
|
||||
_default_model = model_cfg.get('default') if isinstance(model_cfg, dict) else None
|
||||
# Owns model if it appears in the static catalog for the configured provider.
|
||||
# _PROVIDER_MODELS is keyed by CANONICAL slug (e.g. 'zai', not the 'z-ai'
|
||||
# alias a user may write in config), so canonicalise config_provider before
|
||||
# the lookup — otherwise an aliased active provider gets an empty ownership
|
||||
# set and _skip_custom_providers guard-2 silently fails, letting another
|
||||
# providers.<slug>.models entry hijack an active-owned model (#5511).
|
||||
_canon_config_provider = _canonicalise_provider_id(config_provider) if config_provider else ""
|
||||
_provider_models_set: set[str] = set()
|
||||
if (
|
||||
config_provider is not None
|
||||
and config_provider in _PROVIDER_MODELS
|
||||
and isinstance(_PROVIDER_MODELS[config_provider], list)
|
||||
_canon_config_provider
|
||||
and _canon_config_provider in _PROVIDER_MODELS
|
||||
and isinstance(_PROVIDER_MODELS[_canon_config_provider], list)
|
||||
):
|
||||
_provider_models_set = {
|
||||
m.get('id', '') for m in _PROVIDER_MODELS[config_provider]
|
||||
m.get('id', '') for m in _PROVIDER_MODELS[_canon_config_provider]
|
||||
if isinstance(m, dict) and isinstance(m.get('id'), str)
|
||||
}
|
||||
# The active provider may be defined ENTIRELY via config.yaml `providers:`
|
||||
# (no static _PROVIDER_MODELS entry) with its own `models:` allowlist. Fold
|
||||
# that allowlist into the ownership set too, so the active provider owns its
|
||||
# own declared models and can't be hijacked by another providers.<slug>
|
||||
# entry that happens to list the same bare id earlier in config order (#5511).
|
||||
if _canon_config_provider:
|
||||
_providers_cfg_own = cfg.get('providers', {})
|
||||
if isinstance(_providers_cfg_own, dict):
|
||||
for _slug, _pdef in _providers_cfg_own.items():
|
||||
if not isinstance(_pdef, dict):
|
||||
continue
|
||||
if _canonicalise_provider_id(_slug) != _canon_config_provider:
|
||||
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()
|
||||
)
|
||||
_skip_custom_providers = (
|
||||
_is_explicit_non_custom_provider
|
||||
and (
|
||||
|
|
@ -2563,6 +2597,50 @@ def resolve_model_provider(model_id: str) -> tuple:
|
|||
provider_hint = _custom_provider_slug_from_name(entry_name)
|
||||
return model_id, provider_hint, entry_base_url or None
|
||||
|
||||
# Check user-defined providers (config.yaml → providers:).
|
||||
# Mirrors the custom_providers scan above — exact match against each
|
||||
# entry's declared models list (case-sensitive to match custom_providers).
|
||||
providers_cfg = cfg.get('providers', {})
|
||||
if isinstance(providers_cfg, dict):
|
||||
target = model_id.strip()
|
||||
# Honor the same active/default ownership guard as the custom_providers
|
||||
# scan (_skip_custom_providers, config.py:2535): when the active provider
|
||||
# explicitly owns this model (it's the configured default or in the
|
||||
# active provider's model set), another provider's overlapping
|
||||
# `providers.<slug>.models` entry must NOT hijack routing away from the
|
||||
# active provider (#5511 gate finding — e.g. active ai-gateway + default
|
||||
# gpt-5 was being pulled to providers.openai.models.gpt-5). In that case
|
||||
# restrict the scan to the active provider's own canonical slug.
|
||||
_active_slug = _canon_config_provider
|
||||
for slug, pdef in providers_cfg.items():
|
||||
if not isinstance(pdef, dict):
|
||||
continue
|
||||
# Copilot is the documented exception: `providers.copilot.models` is
|
||||
# a per-model SETTINGS map (reasoning_effort, limits, etc.), NOT a
|
||||
# routable allowlist (see the exception at the catalog-build site).
|
||||
# Scanning it here would let a Copilot per-model settings entry
|
||||
# hijack that model's routing away from its real provider (#5511).
|
||||
if _canonicalise_provider_id(slug) == "copilot":
|
||||
continue
|
||||
# Ownership guard: when the active provider owns this model, only its
|
||||
# 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
|
||||
|
||||
# @provider:model format — explicit provider hint from the dropdown.
|
||||
# Route through that provider directly (resolve_runtime_provider will
|
||||
# resolve credentials in streaming.py).
|
||||
|
|
|
|||
|
|
@ -748,3 +748,174 @@ def test_bare_custom_provider_no_base_url_with_known_prefix_keeps_custom_and_ful
|
|||
)
|
||||
assert model == 'google/gemma-2-9b'
|
||||
assert base_url is None
|
||||
|
||||
|
||||
# ── providers: (config.yaml user-defined provider) scan (#5511) ─────────────
|
||||
|
||||
def _resolve_with_providers(model_id, providers_cfg, *, provider=None, default=None):
|
||||
"""Helper: temporarily set config.cfg['providers'] + model, call resolve, restore."""
|
||||
old_cfg = dict(config.cfg)
|
||||
model_cfg = {}
|
||||
if provider:
|
||||
model_cfg['provider'] = provider
|
||||
if default:
|
||||
model_cfg['default'] = default
|
||||
config.cfg['model'] = model_cfg
|
||||
config.cfg['providers'] = providers_cfg
|
||||
try:
|
||||
return config.resolve_model_provider(model_id)
|
||||
finally:
|
||||
config.cfg.clear()
|
||||
config.cfg.update(old_cfg)
|
||||
|
||||
|
||||
def test_providers_scan_routes_user_defined_allowlist_5511():
|
||||
"""A user-defined providers.<slug>.models allowlist routes a bare model id
|
||||
to that provider (the feature #5511 adds)."""
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'my-model-1',
|
||||
{'myprov': {'base_url': 'https://my.example/v1', 'models': ['my-model-1', 'my-model-2']}},
|
||||
provider='openai',
|
||||
default='gpt-5',
|
||||
)
|
||||
assert provider == 'myprov', f"user-defined provider allowlist must route; got {provider!r}"
|
||||
assert model == 'my-model-1'
|
||||
assert base_url == 'https://my.example/v1'
|
||||
|
||||
|
||||
def test_providers_scan_skips_copilot_settings_map_5511():
|
||||
"""providers.copilot.models is a per-model SETTINGS map, NOT a routable
|
||||
allowlist — a Copilot per-model settings entry must NOT hijack routing away
|
||||
from the model's real provider (#5511 gate-cert CORE finding)."""
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'gpt-5',
|
||||
{'copilot': {'models': {'gpt-5': {'reasoning_effort': 'high'}}}},
|
||||
provider='openai',
|
||||
default='gpt-5',
|
||||
)
|
||||
assert provider == 'openai', (
|
||||
"Copilot settings-map entry must NOT hijack routing; "
|
||||
f"gpt-5 must stay on openai, got {provider!r}"
|
||||
)
|
||||
assert model == 'gpt-5'
|
||||
|
||||
|
||||
def test_providers_scan_copilot_list_shape_also_skipped_5511():
|
||||
"""Defense in depth: even if providers.copilot.models is a list shape, the
|
||||
Copilot exclusion still prevents a routing hijack."""
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'gpt-5',
|
||||
{'copilot': {'models': ['gpt-5', 'gpt-5-mini']}},
|
||||
provider='openai',
|
||||
default='gpt-5',
|
||||
)
|
||||
assert provider == 'openai', (
|
||||
f"Copilot (list shape) must not hijack routing; got {provider!r}"
|
||||
)
|
||||
assert model == 'gpt-5'
|
||||
|
||||
|
||||
def test_providers_scan_honors_active_provider_ownership_5511():
|
||||
"""When the active provider owns the model (it's the configured default),
|
||||
another provider's overlapping providers.<slug>.models entry must NOT hijack
|
||||
routing away from the active provider (#5511 gate finding — active ai-gateway
|
||||
+ default gpt-5 was being pulled to providers.openai.models.gpt-5)."""
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'gpt-5',
|
||||
{'openai': {'models': ['gpt-5']}},
|
||||
provider='ai-gateway',
|
||||
default='gpt-5',
|
||||
)
|
||||
assert provider == 'ai-gateway', (
|
||||
"active provider that owns the default model must keep routing; "
|
||||
f"gpt-5 must stay on ai-gateway, got {provider!r}"
|
||||
)
|
||||
assert model == 'gpt-5'
|
||||
|
||||
|
||||
def test_providers_scan_active_provider_own_entry_still_matches_5511():
|
||||
"""The ownership guard still lets the ACTIVE provider's own providers: entry
|
||||
match (e.g. active myprov + a model in providers.myprov.models resolves to
|
||||
myprov with its base_url)."""
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'gpt-5',
|
||||
{'myprov': {'base_url': 'https://my.example/v1', 'models': ['gpt-5']}},
|
||||
provider='myprov',
|
||||
default='gpt-5',
|
||||
)
|
||||
assert provider == 'myprov', f"active provider's own entry must match; got {provider!r}"
|
||||
assert base_url == 'https://my.example/v1'
|
||||
|
||||
|
||||
def test_providers_scan_ownership_guard_canonicalises_aliased_active_provider_5511():
|
||||
"""An ALIASED active provider (e.g. 'z-ai' → canonical 'zai') must still be
|
||||
recognized as owning its catalog models, so another providers.<slug>.models
|
||||
entry can't hijack an active-owned model (#5511 latent-bug gate finding —
|
||||
_provider_models_set was built with the raw alias, missing the canonical
|
||||
_PROVIDER_MODELS key, so the ownership guard silently failed)."""
|
||||
import api.config as config
|
||||
# Pick a real catalog model id owned by the canonical 'zai' provider.
|
||||
zai_models = config._PROVIDER_MODELS.get('zai') or []
|
||||
zai_ids = [m.get('id') for m in zai_models if isinstance(m, dict) and m.get('id')]
|
||||
if not zai_ids:
|
||||
import pytest
|
||||
pytest.skip("no zai catalog models to exercise the alias ownership guard")
|
||||
owned = zai_ids[0]
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
owned,
|
||||
{'openai': {'models': [owned]}},
|
||||
provider='z-ai', # aliased form the user may write in config
|
||||
)
|
||||
assert config._canonicalise_provider_id('z-ai') == 'zai'
|
||||
assert provider == 'z-ai', (
|
||||
"aliased active provider (z-ai→zai) that owns the model must not be "
|
||||
f"hijacked by providers.openai.models; got {provider!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_providers_scan_ownership_guard_canonicalises_gemini_alias_5511():
|
||||
"""A Gemini-family alias (google-gemini → gemini) must canonicalise so the
|
||||
active provider is recognized as owning its catalog models (#5511 latent bug:
|
||||
`gemini` is in _PROVIDER_MODELS but not _PROVIDER_DISPLAY, so the alias was
|
||||
rejected and the ownership guard silently failed)."""
|
||||
import api.config as config
|
||||
assert config._canonicalise_provider_id('google-gemini') == 'gemini', (
|
||||
"google-gemini must canonicalise to gemini"
|
||||
)
|
||||
gem_models = config._PROVIDER_MODELS.get('gemini') or []
|
||||
gem_ids = [m.get('id') for m in gem_models if isinstance(m, dict) and m.get('id')]
|
||||
if not gem_ids:
|
||||
import pytest
|
||||
pytest.skip("no gemini catalog models to exercise the alias ownership guard")
|
||||
owned = gem_ids[0]
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
owned,
|
||||
{'openai': {'models': [owned]}},
|
||||
provider='google-gemini',
|
||||
)
|
||||
assert provider == 'google-gemini', (
|
||||
"aliased active Gemini provider that owns the model must not be hijacked "
|
||||
f"by providers.openai.models; got {provider!r}"
|
||||
)
|
||||
|
||||
|
||||
def test_providers_scan_active_own_providers_entry_owns_over_other_slug_5511():
|
||||
"""An active provider defined purely via config.yaml `providers:` (no static
|
||||
catalog entry) owns the models in its OWN providers.<active>.models allowlist,
|
||||
so another provider's entry listing the same bare id (even earlier in config
|
||||
order) must NOT hijack it (#5511 gate finding 5)."""
|
||||
# 'openai' entry lists gpt-x first, but the ACTIVE provider (myprov) also
|
||||
# declares gpt-x in its own providers.myprov.models — active must win.
|
||||
model, provider, base_url = _resolve_with_providers(
|
||||
'gpt-x',
|
||||
{
|
||||
'openai': {'models': ['gpt-x']},
|
||||
'myprov': {'base_url': 'https://my.example/v1', 'models': ['gpt-x']},
|
||||
},
|
||||
provider='myprov',
|
||||
)
|
||||
assert provider == 'myprov', (
|
||||
"active provider's own providers: allowlist must own its model over "
|
||||
f"another slug's overlapping entry; got {provider!r}"
|
||||
)
|
||||
assert base_url == 'https://my.example/v1'
|
||||
|
|
|
|||
Loading…
Reference in a new issue