jinghui/hermes-webui
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix: route overlapping custom_providers models by active endpoint (#7107)

Browse Source 
* fix: route overlapping custom_providers models by active endpoint

When two custom_providers[] entries list the same bare model id (e.g.
dogapi and packyapi both advertising 'claude-sonnet-5'),
resolve_model_provider scanned them in config write order and returned
the first match, silently ignoring the active provider's base_url. A
user whose model.base_url pointed at packyapi got routed to dogapi
because dogapi appeared earlier in config.

Now, when the active provider resolves to a named custom provider
(custom:<slug>, including via model.base_url -> named-slug matching),
that provider claims models it owns before the ordered scan runs. Falls
through to the legacy first-match scan when the active provider is bare
'custom' with no base_url to disambiguate, so existing single-endpoint
setups are unaffected.

Adds 3 regression tests covering: shared-model active-endpoint wins,
provider-unique model still routes by ownership, and bare-custom
no-base_url preserves legacy order.

* fix: extend active-endpoint routing to aux, handoff, and slug-collision paths

Review follow-up: the overlapping-id fix must reach three sibling
resolution paths that still routed on the wrong endpoint.

1. Auxiliary slot base_url (config.py set_auxiliary_model): a bare
   resolve_model_provider(model) ignored the SELECTED aux provider, so a
   custom:A slot was persisted with the active custom:B endpoint when both
   listed the model. Look up the selected provider's own custom entry via
   resolve_custom_provider_connection. (NOT model_with_provider_context
   here: the @custom:<slug>:model form resolves base_url to None, which
   would drop the base_url entirely.)

2. Handoff summary (routes.py): read s_obj.model_provider and pass
   model_with_provider_context(model, model_provider) so a session pinned
   to custom:A is not rerouted to the active custom:B. base_url is
   backfilled from the provider's own custom entry downstream.

3. Normalized-name slug collision (config.py resolve_model_provider): two
   distinct provider names can normalize to the same slug (e.g. "Foo Bar"
   and "foo-bar"). The active-slug guard now matches the EXACT entry by
   base_url instead of re-deriving it from the non-unique slug, and fails
   closed (falls through to the ordered scan) on genuine ambiguity.

Adds 4 regression tests: normalized-slug base_url disambiguation,
slug-collision fail-closed, aux-slot selected-provider base_url, and
session-provider-context handoff routing.

* fix: fail closed on custom-provider slug collisions and break aux deadlock

resolve_model_provider() returns a provider slug (custom:<slug>) and the
credential lookup (resolve_custom_provider_connection) later resolves the
API key from the FIRST same-slug entry regardless of base_url. So when two
distinct provider names normalize to the same slug (e.g. "Foo Bar" and
"foo-bar") and both list the requested model, returning the slug silently
pairs one entry's endpoint with another entry's credential. Now raise a
clear AmbiguousCustomProviderError instead of guessing, on both the
active-slug guard and the ordered-scan fall-through paths.

Also: an explicit, unambiguous named custom provider now stays authoritative
even when model.base_url is stale/absent and points at neither entry — a
stale URL is no longer evidence to demote it to config order.

And fix a self-deadlock in set_auxiliary_model(): it held the non-reentrant
_cfg_lock while calling resolve_custom_provider_connection() -> get_config()
-> reload_config_if_stale(), which re-acquires the same lock. The selected
custom provider's base_url is now resolved inline from the config_data
already loaded under the lock.

Tests: collision cases now assert the ambiguity error; added a stale-URL
explicit-provider regression and a watchdog-guarded aux no-deadlock
regression (both fail before the fix).

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: fail closed on custom-provider slug collisions across all four paths

The previous cut only guarded the symmetric bare-model case, and it built
collision membership from entries that OWN the requested model. But credential
resolution scans ALL same-slug entries and first-matches regardless of
ownership, so several slug-only boundaries could still pair one entry's
endpoint with another entry's credential — including the asymmetric case where
only one colliding entry lists the model.

Introduce one shared, pure/lock-safe helper (_custom_provider_slug_key +
_unique_custom_provider_entry) that groups ALL named custom_providers entries by
a single canonical slug normalization and raises AmbiguousCustomProviderError
whenever a consumed slug maps to >=2 entries. Apply it at every slug-only
boundary so endpoint and credential can never come from different entries:

- bare resolution (resolve_model_provider): all-entry membership for both the
  active-slug guard and the ordered ownership scan (was owner-only).
- provider-qualified @custom:<slug>:model return (the session/send/handoff
  model_with_provider_context shape), which previously bypassed the check.
- credential lookup (resolve_custom_provider_connection), replacing its private
  first-match-by-slug scan and its duplicate _slugify.
- auxiliary persistence (set_auxiliary_model), replacing its own inline
  first-match-by-slug; the ambiguity now propagates (fails the save) instead of
  being swallowed, while other best-effort errors stay non-fatal. Still resolves
  from the in-scope config_data so it remains lock-safe (no deadlock regression).

Tests: add fail-closed regressions for the asymmetric A-first-non-owner/B-owner
bare case (active-endpoint and bare-custom paths), the model_with_provider_context
qualified handoff shape, the credential boundary called directly, and the
auxiliary-save ambiguity (asserts nothing is persisted). All five fail before
this change and pass after.

Co-authored-by: Cursor <cursoragent@cursor.com>

* fix: one canonical custom-provider slug identity, point-of-return ambiguity, profile-scoped credentials

Deep re-gate follow-up (5 findings):

1. Collision detection now derives from the single slug PRODUCER
   (_custom_provider_slug_from_name) instead of a looser private key, so
   producer-level collisions on punctuated/parenthesized names (e.g.
   'Foo (Bar)' vs 'foo-bar', both -> custom:foo-bar) are detected. Previously
   the looser key normalized 'Foo (Bar)' to 'foo-(bar)' and missed the
   collision, reintroducing the endpoint-A/credential-B bug for those names.

2. The all-entry ambiguity check now runs ONLY at the point a custom:<slug> is
   about to be returned (via a _finalize() guard covering every custom-return in
   resolve_model_provider, including the final active-provider fallback), not up
   front. An unrelated collision on the active slug no longer blocks lanes that
   resolve to a different provider (@openrouter, a non-colliding @custom:other).

3. streaming.py now resolves endpoint AND credential inside ONE profile scope:
   the main path extends the scope over runtime-key + custom-provider resolution,
   and both self-heal retry paths bind the session profile. This stops a detached
   worker from pairing a named profile's endpoint with the default profile's key.

4. Handoff summary: AmbiguousCustomProviderError now returns HTTP 400 with the
   actionable rename message instead of degrading to a 200 local fallback the UI
   treats as success.

5. UI: aux + default model saves surface the server's message and abort (retain
   dirty state) instead of swallowing it into a generic failure / "settings
   saved"; the handoff catch shows the 400 rename message verbatim.

AmbiguousCustomProviderError gains a .message attribute for verbatim forwarding.

Tests: parenthesized-name collision fails closed on all direct paths; unrelated
lanes are not blocked by an active collision; custom overrides resolve inside the
passed profile scope; handoff returns 400 (not a persisted fallback); JS
error-surfacing source guards. Verified the finding #1/#2 cases fail before and
pass after (old key misses 'Foo (Bar)'/'foo-bar'; old up-front check over-blocks).

Co-authored-by: Cursor <cursoragent@cursor.com>

---------

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: nesquena-hermes <nesquena+hermes@gmail.com>
This commit is contained in:
rouVlingCursornesquena-hermes
 authored and GitHub committed 2026-08-20 07:09:25 +00:00
1 parent 6bc16a68bc
commit 4528279cb2
9 files changed
+1113 -95
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

api/config.py
+235 -43
View File
@@ -2616,6 +2616,85 @@ def _get_provider_cfg(provider_id) -> dict:
return provider_cfg if isinstance(provider_cfg, dict) else {}
class AmbiguousCustomProviderError(ValueError):
"""Raised when two+ custom_providers[] entries normalize to the same slug.
A custom provider is identified downstream by a SLUG (``custom:<slug>``):
``resolve_model_provider()`` returns it, and the credential lookup
(``resolve_custom_provider_connection``) resolves the API key + base_url by
scanning ``custom_providers[]`` for the FIRST entry whose name normalizes to
that slug — independent of model ownership or the endpoint chosen earlier. So
when two distinct provider names normalize to the same slug (e.g. ``Foo Bar``
and ``foo-bar`` both -> ``custom:foo-bar``), consuming the slug on ANY path
could pair one entry's endpoint with another entry's credential — including
the asymmetric case where only one of the colliding entries lists the
requested model. Rather than guess, every slug-only boundary fails closed and
surfaces the collision so the user can rename one provider. Subclasses
``ValueError`` so existing ``except ValueError`` / broad-``except`` fallbacks
continue to catch it.
"""
def __init__(self, message: str):
super().__init__(message)
# Expose the actionable rename text as ``.message`` so HTTP handlers can
# forward it verbatim (as the JSON ``error``) without ``str(e)`` casts,
# and it reaches the user on the handoff + save paths instead of being
# swallowed into a generic fallback.
self.message = message
def _custom_provider_slug_key(value: object) -> str:
"""Canonical bare slug for custom-provider identity + collision detection.
Derived from the SINGLE authoritative slug PRODUCER
``_custom_provider_slug_from_name()`` — the same function that mints the
``custom:<slug>`` ids resolve_model_provider() actually returns, persists,
and routes on. Using the producer's normalization (not a private variant)
everywhere means every slug-only boundary — bare + qualified resolution,
credential lookup, auxiliary persistence — compares against ONE identity, so
names that genuinely collide at the producer level (e.g. ``Foo (Bar)`` and
``foo-bar`` both -> ``custom:foo-bar``) are detected as collisions instead of
slipping through a looser key. Returns the bare slug (no ``custom:`` prefix).
Accepts a bare provider name or a ``custom:<slug>`` id.
"""
produced = _custom_provider_slug_from_name(value)
return produced.split(":", 1)[1] if produced.startswith("custom:") else produced
def _unique_custom_provider_entry(custom_providers: object, slug_key: str) -> dict | None:
"""Return the single named ``custom_providers`` entry matching ``slug_key``.
Pure and lock-safe: operates only on the passed-in list, so it can be called
while holding ``_cfg_lock`` (unlike ``get_config()``-based resolvers).
Membership is built from ALL named entries, INDEPENDENT of model ownership,
because slug-only credential resolution scans every same-slug entry and
returns the first match. Raises ``AmbiguousCustomProviderError`` when 2+
entries share the key so an endpoint and an API key can never be resolved
from different entries on any path. Returns the matching entry, or ``None``
when no entry matches.
"""
if not slug_key or not isinstance(custom_providers, list):
return None
matches: list[dict] = []
for entry in custom_providers:
if not isinstance(entry, dict):
continue
name = str(entry.get("name") or "").strip()
if not name:
continue
if _custom_provider_slug_key(name) == slug_key:
matches.append(entry)
if len(matches) >= 2:
names = [str(e.get("name") or "").strip() for e in matches]
raise AmbiguousCustomProviderError(
f"Custom providers {names!r} all normalize to the same provider slug "
f"{slug_key!r}; an endpoint and API key could be resolved from "
f"different entries. Rename one so each custom provider has a unique slug."
)
return matches[0] if matches else None
def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) -> tuple:
"""Resolve model name, provider, and base_url for AIAgent.
@@ -2669,9 +2748,30 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
if isinstance(config_provider, str) and config_provider.strip().lower() == "local":
config_provider = "custom"
def _finalize(model: object, provider: object, base_url: object) -> tuple:
"""Point-of-return collision guard.
Fail closed HERE — immediately before handing a ``custom:<slug>`` back —
never up front. A slug collision on one custom pair must not block a
request that resolves to a DIFFERENT provider (an unrelated
``@openrouter:...`` / ``@custom:safe-provider:...`` lane, or the bare
``custom`` proxy), so the ambiguity check runs only on the slug actually
being returned. ``_unique_custom_provider_entry`` raises
AmbiguousCustomProviderError when >=2 config entries share the slug, so a
downstream credential lookup can never first-match a different entry than
the one whose endpoint we resolved. No-op for bare ``custom`` and every
non-custom provider.
"""
if isinstance(provider, str) and provider.startswith("custom:"):
_unique_custom_provider_entry(
cfg.get('custom_providers', []),
_custom_provider_slug_key(provider),
)
return model, provider, base_url
model_id = (model_id or "").strip()
if not model_id:
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# Custom providers declared in config.yaml should win over slash-based
# OpenRouter heuristics. Their model IDs commonly contain '/' too.
@@ -2732,6 +2832,57 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
)
custom_providers = cfg.get('custom_providers', [])
if isinstance(custom_providers, list) and not _skip_custom_providers:
# Disambiguation guard: when two custom_providers[] entries both list the
# same bare model id (e.g. dogapi and packyapi both advertise
# 'claude-sonnet-5'), a plain first-match scan routes on config WRITE
# ORDER — silently hijacking the model to whichever entry appears first,
# regardless of the active provider the user actually configured. If the
# ACTIVE provider is itself a named custom provider (config_provider
# resolved to 'custom:<slug>', including via model.base_url → named-slug
# matching), prefer THAT entry when it also owns the model, so an explicit
# active endpoint wins over an overlapping earlier entry. Falls through to
# the ordered scan below when the active provider is bare 'custom' / not a
# named custom entry, or when it doesn't list this model.
_active_custom_slug = ''
if isinstance(config_provider, str) and config_provider.startswith('custom:'):
_active_custom_slug = config_provider
def _entry_owns_model(entry: dict) -> bool:
entry_model = (entry.get('model') or '').strip()
ids = set()
if entry_model:
ids.add(entry_model)
ids.update(_configured_model_ids(entry.get('models')))
return model_id in ids
# Explicit active named provider wins over config order when it owns the
# model. Only CONSUME the active slug (and thus let _finalize fail closed
# on a collision) when the active provider actually lists this model: if
# it doesn't, fall through so an unrelated collision on the active slug
# never blocks a request that resolves to a different provider.
if _active_custom_slug:
_active_key = _custom_provider_slug_key(_active_custom_slug)
_active_owner = next(
(
e for e in custom_providers
if isinstance(e, dict)
and _entry_owns_model(e)
and _custom_provider_slug_key(e.get('name')) == _active_key
),
None,
)
# Exactly one entry carries the active slug AND owns the model ->
# authoritative, even when model.base_url is stale/absent or points at
# a different endpoint. An explicit, unambiguous named provider must
# never lose to config order: a stale URL is not evidence to discard
# it. _finalize() fails closed if the active slug is shared by >=2
# entries (endpoint + credential could then split).
if _active_owner is not None:
return _finalize(
model_id,
_active_custom_slug,
(_active_owner.get('base_url') or '').strip() or None,
)
for entry in custom_providers:
if not isinstance(entry, dict):
continue
@@ -2744,7 +2895,9 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
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
# _finalize() applies the all-entry collision guard on this
# bare-'custom' / fall-through path before returning the slug.
return _finalize(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
@@ -2803,14 +2956,21 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
parsed_provider_hint = _parse_provider_qualified_model_id(model_id)
if parsed_provider_hint is not None:
bare_model, provider_hint = parsed_provider_hint
# Session/send/handoff shapes encode the provider as @custom:<slug>:model
# and reach here after the ownership scan only saw the ENCODED string.
# _finalize() applies the all-entry uniqueness check before returning the
# slug so the downstream credential lookup can't first-match a different
# colliding entry — but ONLY on the custom:<slug> actually returned, so an
# unrelated collision never blocks this explicit hint (@openrouter, a
# non-colliding @custom:other, ...).
if (
provider_hint.startswith("custom:")
and config_base_url
and _is_local_server_provider(config_provider)
and provider_hint.lower() in _custom_endpoint_slugs_for_base_url(config_base_url)
):
return bare_model, config_provider, config_base_url
return bare_model, provider_hint, _get_provider_base_url(provider_hint)
return _finalize(bare_model, config_provider, config_base_url)
return _finalize(bare_model, provider_hint, _get_provider_base_url(provider_hint))
if "/" in model_id:
prefix, bare = model_id.split("/", 1)
@@ -2828,11 +2988,11 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
# #854 / #894 for Nous, where this guard was originally added).
_PORTAL_PROVIDERS = {"nous", "opencode-zen", "opencode-go", "nvidia"}
if config_provider in _PORTAL_PROVIDERS:
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# If prefix matches config provider exactly, strip it and use that provider directly.
# e.g. config=anthropic, model=anthropic/claude-... → bare name to anthropic API
if config_provider and prefix == config_provider:
return bare, config_provider, config_base_url
return _finalize(bare, config_provider, config_base_url)
# The OpenAI Codex provider uses a real base_url, but its default
# ChatGPT endpoint cannot serve OpenRouter-style provider/model IDs.
# Keep that narrow exception before the custom endpoint protection so
@@ -2858,7 +3018,7 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
if isinstance(_entry, dict) and _entry.get("name", "").strip() == prefix:
_slug = _custom_provider_slug_from_name(prefix)
_base = (_entry.get("base_url") or "").strip()
return model_id, _slug, _base or None
return _finalize(model_id, _slug, _base or None)
# If a custom endpoint base_url is configured, don't reroute through OpenRouter
# just because the model name contains a slash (e.g. google/gemma-4-26b-a4b).
@@ -2872,7 +3032,7 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
# pointing at a loopback/private host.
if (_is_local_server_provider(config_provider)
or _base_url_points_at_local_server(config_base_url)):
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# Strip the provider prefix only when it's a known provider namespace
# AND stripping is the right call for this configured provider:
#
@@ -2920,22 +3080,22 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
# model.models / custom_providers[].models). Authoritative and
# network-free, so #5979 survives a cold restart — preserve.
if _model_id_declared_in_config(model_id, config_provider):
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# (2) The endpoint's live/cached catalog advertised it.
_advertised = _endpoint_advertised_model_ids(config_provider)
if _advertised:
# Full id advertised → route on it verbatim (#5979/#3872/#548).
if model_id in _advertised:
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# ONLY the bare id advertised → the prefix is a redundant
# leftover the relay rejects; strip it (#433). Keep the
# ``prefix in _PROVIDER_MODELS`` belt so an adversarial catalog
# advertising a bare id can't strip an unknown-vendor prefix.
if bare in _advertised and prefix in _PROVIDER_MODELS:
return bare, config_provider, config_base_url
return _finalize(bare, config_provider, config_base_url)
# Advertised but neither exact shape matched → intrinsic /
# unknown prefix the proxy routes on; preserve it whole.
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# (3) Provenance genuinely unavailable (cold/unbuilt or
# fingerprint-mismatched catalog AND not config-declared).
# Distinguish a DELIBERATE selection from a stale leftover:
@@ -2961,17 +3121,17 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
# the static first-party catalog silently flipped routing
# (exactly how #5979 regressed).
if explicitly_picked:
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
if prefix in _PROVIDER_MODELS and _is_first_party_model(prefix, bare):
return bare, config_provider, config_base_url
return model_id, config_provider, config_base_url
return _finalize(bare, config_provider, config_base_url)
return _finalize(model_id, config_provider, config_base_url)
# Non-custom first-party provider pointed at an OpenAI-compatible
# proxy (e.g. provider=openai + base_url=litellm): the bare id is
# what it expects — "openai/gpt-5.4" → "gpt-5.4" (#433).
if prefix in _PROVIDER_MODELS:
return bare, config_provider, config_base_url
return _finalize(bare, config_provider, config_base_url)
# Intrinsic / unknown prefix — pass the full model_id through unchanged.
return model_id, config_provider, config_base_url
return _finalize(model_id, config_provider, config_base_url)
# If prefix does NOT match config provider, the user picked a cross-provider model
# from the OpenRouter dropdown (e.g. config=anthropic but picked openai/gpt-5.4-mini).
@@ -2993,7 +3153,11 @@ def resolve_model_provider(model_id: str, *, explicitly_picked: bool = False) ->
):
return model_id, "openrouter", None
return model_id, config_provider, config_base_url
# Final active-provider fallback: when nothing more specific matched, route
# on the configured provider. _finalize() fails closed here too if that
# provider is a collision-shared custom:<slug> (per finding #2 — the check
# must reach this last fallback, not just the earlier explicit paths).
return _finalize(model_id, config_provider, config_base_url)
def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, str | None]:
@@ -3007,13 +3171,7 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st
if not pid.startswith("custom:"):
return None, None
def _slugify(value: str) -> str:
s = str(value or "").strip().lower().replace("_", "-").replace(" ", "-")
while "--" in s:
s = s.replace("--", "-")
return s.strip("-")
slug = _slugify(pid.split(":", 1)[1].strip())
slug = _custom_provider_slug_key(pid)
if not slug:
return None, None
@@ -3041,18 +3199,13 @@ def resolve_custom_provider_connection(provider_id: str) -> tuple[str | None, st
if not isinstance(custom_providers, list):
custom_providers = []
for entry in custom_providers:
if not isinstance(entry, dict):
continue
name = str(entry.get("name") or "").strip()
if not name:
continue
entry_slug = _slugify(name)
if entry_slug != slug:
continue
base_url = str(entry.get("base_url") or "").strip() or None
api_key = _resolve_key(entry.get("api_key"), entry.get("key_env"), pid)
# Fail closed when the slug maps to multiple entries (raises); otherwise use
# the single matching entry. Shared with resolve_model_provider so endpoint
# and credential are always resolved from the SAME entry.
matched_entry = _unique_custom_provider_entry(custom_providers, slug)
if matched_entry is not None:
base_url = str(matched_entry.get("base_url") or "").strip() or None
api_key = _resolve_key(matched_entry.get("api_key"), matched_entry.get("key_env"), pid)
return api_key, base_url
# If exactly one custom provider is configured, use it as a pragmatic
@@ -4849,12 +5002,51 @@ def set_auxiliary_model(task: str, provider: str, model: str, advanced: dict | N
slot_cfg["provider"] = provider or "auto"
slot_cfg["model"] = model or ""
if provider and (provider.startswith("custom:") or provider == "custom"):
try:
_, _, resolved_base_url = resolve_model_provider(model)
if resolved_base_url:
slot_cfg["base_url"] = str(resolved_base_url).strip().rstrip("/")
except Exception:
pass
# Resolve the auxiliary slot's base_url against the SELECTED
# provider, not the active main provider. A bare
# resolve_model_provider(model) ignores `provider` and routes the
# model through whatever main provider is active — so when the
# selected auxiliary provider (custom:A) and the active main
# provider (custom:B) both list the same model id, the slot was
# persisted with provider=custom:A but base_url=B's endpoint
# (overlapping-id misroute, sibling of the resolve_model_provider
# fix). For a named custom:<slug> selection, look up that
# provider's OWN custom_providers[] entry directly. Note we do
# NOT route through model_with_provider_context here: the
# @custom:<slug>:model form resolves base_url to None (the
# @provider path doesn't carry a custom entry's base_url), which
# would drop the base_url entirely. Fall back to the bare resolve
# only for the unnamed `custom` case, which has no own entry.
resolved_base_url = None
if provider.startswith("custom:"):
# Resolve the selected provider's base_url from the
# config_data already loaded under _cfg_lock above. Do NOT
# call resolve_custom_provider_connection() / get_config()
# here: they re-acquire the non-reentrant _cfg_lock we
# already hold, self-deadlocking whenever the cache is stale
# or the profile path changed. Use the shared uniqueness
# helper on the in-scope dict so this slug-only save fails
# closed on a collision (raises AmbiguousCustomProviderError)
# exactly like every other path — otherwise the ambiguity
# would be swallowed and the wrong endpoint persisted.
_cp_match = _unique_custom_provider_entry(
config_data.get("custom_providers", []),
_custom_provider_slug_key(provider),
)
if _cp_match is not None:
resolved_base_url = str(_cp_match.get("base_url") or "").strip() or None
if not resolved_base_url:
# Best-effort fallback for the unnamed `custom` case (no own
# entry). Keep it non-fatal for unexpected errors, but let a
# genuine ambiguity propagate so the save fails closed.
try:
_, _, resolved_base_url = resolve_model_provider(model)
except AmbiguousCustomProviderError:
raise
except Exception:
resolved_base_url = None
if resolved_base_url:
slot_cfg["base_url"] = str(resolved_base_url).strip().rstrip("/")
if advanced is not None:
try:
_apply_advanced_model_options(slot_cfg, advanced)
api/routes.py
+25 -1
View File
@@ -27051,14 +27051,28 @@ def _handle_handoff_summary(handler, body):
resolved_model = None
resolved_provider = None
resolved_base_url = None
session_model_provider = None
try:
from api.models import get_session
s_obj = get_session(sid)
resolved_model = getattr(s_obj, "model", None)
# Carry the session's OWN selected provider into resolution. Without
# it, a bare resolve_model_provider(model) routes the summary through
# whatever main provider is active — so a session pinned to custom:A
# gets its handoff summary rerouted to the active custom:B when both
# providers list the same model id (overlapping-id misroute, sibling
# of the resolve_model_provider fix). model_with_provider_context
# encodes it as @custom:A:model so the resolver honors the session's
# endpoint; base_url is backfilled from that provider's own custom
# entry by the resolve_custom_provider_connection block below.
session_model_provider = getattr(s_obj, "model_provider", None)
except Exception:
pass
resolved_model, resolved_provider, resolved_base_url = _cfg.resolve_model_provider(resolved_model)
model_for_resolution = _cfg.model_with_provider_context(
resolved_model, session_model_provider
)
resolved_model, resolved_provider, resolved_base_url = _cfg.resolve_model_provider(model_for_resolution)
resolved_api_key = None
try:
@@ -27187,6 +27201,16 @@ def _handle_handoff_summary(handler, body):
"type": "agent_runtime_stale",
"retryable": True,
}, status=409)
except api_config.AmbiguousCustomProviderError as e:
# A custom-provider slug collision is a user-fixable misconfiguration,
# not a transient summary failure. Return 400 with the actionable rename
# message so the UI shows it, instead of degrading to a 200 local
# fallback that the client treats as success and that hides the fix.
logger.warning("Handoff summary blocked by ambiguous custom provider: %s", e.message)
return j(handler, {
"error": e.message,
"type": "custom_provider_ambiguous",
}, status=400)
except Exception as e:
logger.warning("Handoff summary generation failed: %s", e)
summary_text = _fallback_handoff_summary(msgs)
api/streaming.py
+82 -42
View File
@@ -549,6 +549,7 @@ def _resolve_custom_provider_runtime_overrides(
resolved_provider: str | None,
resolved_api_key: str | None,
resolved_base_url: str | None,
profile_name: str | None = None,
) -> tuple[str | None, str | None, str | None]:
"""Return provider/key/base_url overrides for ``custom:*`` endpoints.
@@ -557,11 +558,24 @@ def _resolve_custom_provider_runtime_overrides(
without authentication, so a missing key should not fail before the first
request; pass a harmless placeholder to the SDK and let the endpoint accept
it or return its own auth error.
``profile_name`` binds the SESSION's own profile while
``resolve_custom_provider_connection`` reads the endpoint AND the credential.
That helper resolves both from ONE ``get_config()``/env snapshot, so binding
the profile here keeps them from splitting across profiles: on a detached
worker thread (which doesn't inherit the request-profile TLS/env) an unbound
call would read the DEFAULT profile and pair a named profile's endpoint with
the default profile's API key (finding #3). No-op for the default/root
profile; when ``profile_name`` is None the ambient scope (if any) is used.
"""
if not (isinstance(resolved_provider, str) and resolved_provider.startswith("custom:")):
return resolved_provider, resolved_api_key, resolved_base_url
_cp_key, _cp_base = resolve_custom_provider_connection(resolved_provider)
from api import profiles as _profiles_api
with _profiles_api.profile_scope_for_detached_worker(
profile_name, "custom provider connection", logger_override=logger
):
_cp_key, _cp_base = resolve_custom_provider_connection(resolved_provider)
if not resolved_api_key and _cp_key:
resolved_api_key = _cp_key
if not resolved_base_url and _cp_base:
@@ -9433,46 +9447,54 @@ def _run_agent_streaming(
_sig_provider = getattr(s, "model_provider", None) or provider_context
_current_sig = _mk_sig(_sig_model, _sig_provider)
_explicitly_picked = bool(_picked_sig) and _picked_sig == _current_sig
# Resolve the endpoint AND the credential inside ONE profile scope so
# they always come from the same profile-owned snapshot. Previously
# only resolve_model_provider() ran under the scope; the runtime-key
# and custom-provider resolution below ran after it closed, so a
# detached worker for a NAMED profile paired that profile's endpoint
# with the DEFAULT profile's API key (finding #3). No-op for the
# default/root profile.
with profiles_api.profile_scope_for_detached_worker(
_resolved_profile_name, "model resolution", logger_override=logger
_resolved_profile_name, "model + credential resolution", logger_override=logger
):
warm_models_catalog_provenance_if_cold()
resolved_model, resolved_provider, resolved_base_url = resolve_model_provider(
model_with_provider_context(model, provider_context),
explicitly_picked=_explicitly_picked,
)
configured_base_url = resolved_base_url
configured_base_url = resolved_base_url
# Resolve API key via Hermes runtime provider (matches gateway behaviour).
# Pass the resolved provider so non-default providers get their own credentials.
resolved_api_key = None
try:
from api.oauth import resolve_runtime_provider_with_anthropic_env_lock
from hermes_cli.runtime_provider import resolve_runtime_provider
_rt = resolve_runtime_provider_with_anthropic_env_lock(
resolve_runtime_provider,
requested=resolved_provider,
target_model=resolved_model,
)
resolved_api_key = _rt.get("api_key")
if not resolved_provider:
resolved_provider = _rt.get("provider")
resolved_base_url = _runtime_preferred_base_url(
_rt, resolved_provider, configured_base_url
)
except Exception as _e:
print(f"[webui] WARNING: resolve_runtime_provider failed: {_e}", flush=True)
# Resolve API key via Hermes runtime provider (matches gateway behaviour).
# Pass the resolved provider so non-default providers get their own credentials.
resolved_api_key = None
try:
from api.oauth import resolve_runtime_provider_with_anthropic_env_lock
from hermes_cli.runtime_provider import resolve_runtime_provider
_rt = resolve_runtime_provider_with_anthropic_env_lock(
resolve_runtime_provider,
requested=resolved_provider,
target_model=resolved_model,
)
resolved_api_key = _rt.get("api_key")
if not resolved_provider:
resolved_provider = _rt.get("provider")
resolved_base_url = _runtime_preferred_base_url(
_rt, resolved_provider, configured_base_url
)
except Exception as _e:
print(f"[webui] WARNING: resolve_runtime_provider failed: {_e}", flush=True)
# Named custom providers (custom:slug) may not be resolvable by
# hermes_cli.runtime_provider directly. Fall back to config.yaml
# custom_providers[] so WebUI can pass explicit creds/base_url.
# Preserve the pre-canonicalization identity so image routing can
# still select the exact custom_providers entry after the rewrite
# to "custom" below.
_session_requested_provider = resolved_provider
resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides(
resolved_provider, resolved_api_key, resolved_base_url
)
# Named custom providers (custom:slug) may not be resolvable by
# hermes_cli.runtime_provider directly. Fall back to config.yaml
# custom_providers[] so WebUI can pass explicit creds/base_url.
# Preserve the pre-canonicalization identity so image routing can
# still select the exact custom_providers entry after the rewrite
# to "custom" below.
_session_requested_provider = resolved_provider
resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides(
resolved_provider, resolved_api_key, resolved_base_url,
profile_name=_resolved_profile_name,
)
# Read per-profile config at call time (not module-level snapshot).
# The streaming worker is a detached thread that does NOT inherit the
@@ -10508,10 +10530,18 @@ def _run_agent_streaming(
# Before emitting the error, try re-reading credentials
# and retrying once with a fresh agent.
_heal_result = None
_heal_rt = _attempt_credential_self_heal(
resolved_provider or '', session_id, _agent_lock,
target_model=resolved_model,
)
# Bind the session's profile so the self-heal re-resolve
# AND the custom-provider override below read one
# profile-owned snapshot (finding #3): otherwise a named
# profile's endpoint pairs with the default profile's key.
from api import profiles as _profiles_api
with _profiles_api.profile_scope_for_detached_worker(
_resolved_profile_name, "credential self-heal", logger_override=logger
):
_heal_rt = _attempt_credential_self_heal(
resolved_provider or '', session_id, _agent_lock,
target_model=resolved_model,
)
if _heal_rt is not None:
logger.info('[webui] self-heal: retrying stream after credential refresh')
# Rebuild runtime variables from the refreshed resolve
@@ -10530,7 +10560,8 @@ def _run_agent_streaming(
if not _session_requested_provider:
_session_requested_provider = resolved_provider
resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides(
resolved_provider, resolved_api_key, resolved_base_url
resolved_provider, resolved_api_key, resolved_base_url,
profile_name=_resolved_profile_name,
)
# Rebuild agent kwargs and create a fresh agent
_agent_kwargs['api_key'] = resolved_api_key
@@ -11749,10 +11780,18 @@ def _run_agent_streaming(
elif _exc_is_auth:
if not _self_healed:
# ── Credential self-heal on 401 (#1401) ──
_heal_rt = _attempt_credential_self_heal(
resolved_provider or '', session_id, _agent_lock,
target_model=resolved_model,
)
# Bind the session's profile so the self-heal re-resolve AND the
# custom-provider override below read one profile-owned snapshot
# (finding #3): otherwise a named profile's endpoint pairs with
# the default profile's key.
from api import profiles as _profiles_api
with _profiles_api.profile_scope_for_detached_worker(
_resolved_profile_name, "credential self-heal", logger_override=logger
):
_heal_rt = _attempt_credential_self_heal(
resolved_provider or '', session_id, _agent_lock,
target_model=resolved_model,
)
if _heal_rt is not None:
logger.info('[webui] self-heal (except path): retrying stream after credential refresh')
_self_healed = True
@@ -11771,7 +11810,8 @@ def _run_agent_streaming(
if not _session_requested_provider:
_session_requested_provider = resolved_provider
resolved_provider, resolved_api_key, resolved_base_url = _resolve_custom_provider_runtime_overrides(
resolved_provider, resolved_api_key, resolved_base_url
resolved_provider, resolved_api_key, resolved_base_url,
profile_name=_resolved_profile_name,
)
# Build a fresh agent with the new credentials
_heal_kwargs = dict(_agent_kwargs) if '_agent_kwargs' in dir() else {}
static/panels.js
+26 -8
View File
@@ -12690,7 +12690,13 @@ async function _applyAuxModels(){
saved++;
}catch(e){
console.warn('[settings] failed to save aux task',task.task,e);
if(typeof showToast==='function') showToast(t('settings_aux_save_failed')||'Failed to save auxiliary model');
// Surface the server's actionable message (e.g. an ambiguous custom-provider
// slug collision: rename one provider so its slug is unique) instead of a
// generic failure, and abort the loop so the dirty selection is retained for
// the user to fix and retry — the reload that would clear it is skipped.
const _msg=(e&&e.message)?e.message:'';
const _base=t('settings_aux_save_failed')||'Failed to save auxiliary model';
if(typeof showToast==='function') showToast(_msg?(_base+': '+_msg):_base,6000,'error');
return;
}
}
@@ -12804,11 +12810,17 @@ async function saveSettings(andClose){
const saved=await _enqueueSettingsPost({method:'POST',body:JSON.stringify(payload)});
if(modelChanged && model){
try{
await api('/api/default-model',{method:'POST',body:JSON.stringify({model,provider:modelState.model_provider||null})});
body.default_model=model;
body.default_model_provider=(modelState&&modelState.model===model)?(modelState.model_provider||null):null;
await api('/api/default-model',{method:'POST',body:JSON.stringify({model,provider:modelState.model_provider||null})});
body.default_model=model;
body.default_model_provider=(modelState&&modelState.model===model)?(modelState.model_provider||null):null;
}catch(_modelErr){
if(typeof showToast==='function') showToast('Failed to update default model — settings saved');
// A 400 here (e.g. an ambiguous custom-provider slug collision: rename
// one provider) is user-fixable, not a partial success. Surface the
// message, abort before "settings saved", and retain dirty state so the
// user can fix and retry instead of the error being swallowed.
const _msg=(_modelErr&&_modelErr.message)?_modelErr.message:'';
if(typeof showToast==='function') showToast('Failed to update default model'+(_msg?(': '+_msg):''),6000,'error');
return;
}
}
_applySavedSettingsUi(saved, body, {sendKey,showTokenUsage,showQuotaChip,showConversationOutline,showBusyPlaceholderHint,showTps,fadeTextEffect,showCliSessions,theme,skin,language,sidebarDensity,fontSize});
@@ -12837,9 +12849,15 @@ async function saveSettings(andClose){
await api('/api/default-model',{method:'POST',body:JSON.stringify({model,provider:modelState.model_provider||null})});
body.default_model=model;
body.default_model_provider=(modelState&&modelState.model===model)?(modelState.model_provider||null):null;
}catch(_modelErr){
if(typeof showToast==='function') showToast('Failed to update default model — settings saved');
}
}catch(_modelErr){
// A 400 here (e.g. an ambiguous custom-provider slug collision: rename
// one provider) is user-fixable, not a partial success. Surface the
// message, abort before "settings saved", and retain dirty state so the
// user can fix and retry instead of the error being swallowed.
const _msg=(_modelErr&&_modelErr.message)?_modelErr.message:'';
if(typeof showToast==='function') showToast('Failed to update default model'+(_msg?(': '+_msg):''),6000,'error');
return;
}
}
_applySavedSettingsUi(saved, body, {sendKey,showTokenUsage,showQuotaChip,showConversationOutline,showBusyPlaceholderHint,showTps,fadeTextEffect,showCliSessions,theme,skin,language,sidebarDensity,fontSize});
showToast(t('settings_saved'));
static/sessions.js
+9 -1
View File
@@ -2918,12 +2918,20 @@ async function _generateHandoffSummary(sid, rounds) {
} catch (e) {
console.warn('Handoff summary failed:', e);
if (S.session && S.session.session_id === sid && typeof setHandoffUi === 'function') {
// A 400 carries an actionable, user-fixable message (e.g. an ambiguous
// custom-provider slug collision: rename one provider so its slug is
// unique). Surface it verbatim rather than degrading to the generic
// "try again" card — previously the server answered 200 with a warning
// that this handler ignored, hiding the fix from the user.
const errorText = (e && e.status === 400 && e.message)
? e.message
: ('Summary generation failed: ' + (e && e.message ? e.message : 'unknown error'));
setHandoffUi({
sessionId: sid,
phase: 'error',
channel,
rounds,
errorText: 'Summary generation failed: ' + e.message,
errorText,
});
}
}
tests/test_auxiliary_models_settings.py
+190
View File
@@ -937,3 +937,193 @@ class TestAuxiliaryModelsBackend:
assert result["status"] == 400
assert "Unknown auxiliary task slot" in result["error"]
def test_aux_slot_base_url_uses_selected_provider_not_active_endpoint(
self, monkeypatch, tmp_path
):
"""Overlapping-id sibling fix: persisting an auxiliary slot for a named
custom provider must record THAT provider's own base_url, not the active
main provider's endpoint.
Repro: main provider is custom:dogapi (base_url dogapi), and both dogapi
and packyapi list 'shared-model'. Selecting custom:packyapi for the vision
slot previously persisted {provider: custom:packyapi, base_url: dogapi's}
because the base_url was resolved with a bare resolve_model_provider(model)
that ignores the selected provider. It must persist packyapi's base_url.
"""
from api import config
shared_cfg = {
"model": {
"default": "shared-model",
"provider": "custom",
"base_url": "https://www.dogapi.cc/v1",
},
"custom_providers": [
{"name": "dogapi", "base_url": "https://www.dogapi.cc/v1",
"models": ["shared-model"]},
{"name": "packyapi", "base_url": "https://www.packyapi.ai/v1",
"models": ["shared-model"]},
],
}
config_path = tmp_path / "config.yaml"
import yaml
config_path.write_text(yaml.safe_dump(shared_cfg), encoding="utf-8")
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
monkeypatch.setattr(config, "reload_config", lambda: None)
# base_url resolution reads the in-memory cfg / get_config snapshot.
monkeypatch.setattr(config, "cfg", dict(shared_cfg))
monkeypatch.setattr(config, "get_config", lambda: dict(shared_cfg))
config.set_auxiliary_model("vision", "custom:packyapi", "shared-model")
saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"]
assert saved["provider"] == "custom:packyapi"
assert saved["base_url"] == "https://www.packyapi.ai/v1", (
f"aux slot must persist the SELECTED provider's base_url, got "
f"{saved.get('base_url')!r}"
)
def test_aux_slot_custom_provider_base_url_no_deadlock(self, monkeypatch, tmp_path):
"""Regression: saving a named custom auxiliary model must not self-deadlock.
set_auxiliary_model() holds the non-reentrant _cfg_lock while resolving
the selected custom provider's base_url. The pre-fix code called
resolve_custom_provider_connection() -> get_config() ->
reload_config_if_stale(), which re-acquires _cfg_lock and hangs forever
whenever the config cache is stale or the profile path changed. This test
uses the REAL get_config (only reload_config, which runs AFTER the lock is
released, is stubbed) and forces get_config()'s reload branch, then runs
the call on a watchdog thread that fails the test if it hangs.
"""
import threading
import yaml
from api import config
shared_cfg = {
"model": {
"default": "shared-model",
"provider": "custom",
"base_url": "https://www.dogapi.cc/v1",
},
"custom_providers": [
{"name": "dogapi", "base_url": "https://www.dogapi.cc/v1",
"models": ["shared-model"]},
{"name": "packyapi", "base_url": "https://www.packyapi.ai/v1",
"models": ["shared-model"]},
],
}
config_path = tmp_path / "config.yaml"
config_path.write_text(yaml.safe_dump(shared_cfg), encoding="utf-8")
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
# reload_config runs AFTER _cfg_lock is released; stub it so the test
# doesn't mutate global module state. get_config stays REAL — that is the
# path the pre-fix code re-entered while holding the lock.
monkeypatch.setattr(config, "reload_config", lambda: None)
# Force the reload branch inside the real get_config the pre-fix code
# called: a mismatched cached path makes path_changed True.
monkeypatch.setattr(config, "_cfg_path", None, raising=False)
result_box: dict = {}
error_box: dict = {}
def _run():
try:
result_box["r"] = config.set_auxiliary_model(
"vision", "custom:packyapi", "shared-model"
)
except Exception as exc: # pragma: no cover - surfaced via join
error_box["e"] = exc
worker = threading.Thread(target=_run, daemon=True)
worker.start()
worker.join(timeout=10)
assert not worker.is_alive(), (
"set_auxiliary_model deadlocked while resolving a custom provider "
"base_url under _cfg_lock"
)
if "e" in error_box:
raise error_box["e"]
saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"]
assert saved["provider"] == "custom:packyapi"
assert saved["base_url"] == "https://www.packyapi.ai/v1", (
f"aux slot must persist the SELECTED provider's base_url, got "
f"{saved.get('base_url')!r}"
)
def test_aux_slot_custom_provider_slug_collision_fails_closed(self, monkeypatch, tmp_path):
"""Saving a named custom auxiliary model whose slug collides with another
config entry must fail closed, not silently persist the wrong endpoint.
The inline base_url resolution added for the deadlock fix shares the same
all-entry uniqueness helper, so a custom:foo-bar save with colliding
'Foo Bar' + 'foo-bar' entries raises AmbiguousCustomProviderError and
writes nothing (the slot stays 'auto'). Operates on the in-scope
config_data, so it remains lock-safe.
"""
import yaml
from api import config
shared_cfg = {
"auxiliary": {"vision": {"provider": "auto", "model": ""}},
"custom_providers": [
{"name": "Foo Bar", "base_url": "https://a.example/v1"},
{"name": "foo-bar", "base_url": "https://b.example/v1",
"models": ["shared-model"]},
],
}
config_path = tmp_path / "config.yaml"
config_path.write_text(yaml.safe_dump(shared_cfg), encoding="utf-8")
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
monkeypatch.setattr(config, "reload_config", lambda: None)
with pytest.raises(config.AmbiguousCustomProviderError):
config.set_auxiliary_model("vision", "custom:foo-bar", "shared-model")
# The ambiguous save must not have persisted: the slot stays 'auto'.
saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"]
assert saved.get("provider") == "auto", (
f"ambiguous aux save must not persist, got {saved!r}"
)
def test_aux_slot_parenthesized_name_collision_fails_closed(self, monkeypatch, tmp_path):
"""Finding #1 on the aux persistence path: a parenthesized-name collision
that a looser slug key MISSED ('Foo (Bar)' vs 'foo-bar', both producing
custom:foo-bar) must also fail closed on save.
This is the case the earlier collision key got wrong: it normalized
'Foo (Bar)' to 'foo-(bar)' and never saw the collision, so the aux save
could persist endpoint A while the credential lookup later returned B.
"""
import yaml
from api import config
shared_cfg = {
"auxiliary": {"vision": {"provider": "auto", "model": ""}},
"custom_providers": [
{"name": "Foo (Bar)", "base_url": "https://a.example/v1"},
{"name": "foo-bar", "base_url": "https://b.example/v1",
"models": ["shared-model"]},
],
}
config_path = tmp_path / "config.yaml"
config_path.write_text(yaml.safe_dump(shared_cfg), encoding="utf-8")
monkeypatch.setattr(config, "_get_config_path", lambda: config_path)
monkeypatch.setattr(config, "reload_config", lambda: None)
with pytest.raises(config.AmbiguousCustomProviderError):
config.set_auxiliary_model("vision", "custom:foo-bar", "shared-model")
saved = config._load_yaml_config_file(config_path)["auxiliary"]["vision"]
assert saved.get("provider") == "auto", (
f"ambiguous aux save must not persist, got {saved!r}"
)
tests/test_issue1013_handoff_dock.py
+99
View File
@@ -108,6 +108,28 @@ def test_handoff_delete_clears_local_storage_markers():
assert "ids.forEach(_clearHandoffStorageForSession);" in SESSIONS_JS
def test_handoff_catch_surfaces_400_message_verbatim():
"""Finding #5 (handoff UI): a 400 from the summary endpoint carries an
actionable message (e.g. an ambiguous custom-provider slug collision). The
catch must surface it verbatim instead of the generic 'try again' text."""
assert "e && e.status === 400 && e.message" in SESSIONS_JS
# The generic 'try again' text must no longer be the sole error message.
assert "errorText," in SESSIONS_JS
def test_model_save_catches_surface_error_and_abort():
"""Finding #5 (settings UI): aux + default model saves must surface the
server's actionable message and abort (retain dirty state) rather than
swallowing it into a generic failure / 'settings saved'."""
panels_js = (ROOT / "static" / "panels.js").read_text(encoding="utf-8")
# Default-model save no longer degrades to "settings saved" after a failure.
assert "Failed to update default model — settings saved" not in panels_js
# It surfaces the server message and aborts.
assert "const _msg=(_modelErr&&_modelErr.message)?_modelErr.message:''" in panels_js
# Aux save surfaces the server message too.
assert "const _base=t('settings_aux_save_failed')||'Failed to save auxiliary model';" in panels_js
def test_handoff_summary_renders_as_transcript_card_not_dock_card():
assert "function setHandoffUi" in SESSIONS_JS or "function setHandoffUi" in (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
ui_js = (ROOT / "static" / "ui.js").read_text(encoding="utf-8")
@@ -396,6 +418,83 @@ def test_exception_handoff_summary_persists_fallback_summary(monkeypatch):
assert persisted[0]["rounds"] == models.CONVERSATION_ROUND_THRESHOLD
def test_ambiguous_custom_provider_handoff_returns_400_not_fallback(monkeypatch):
"""Finding #4: a custom-provider slug collision on a handoff must surface as a
400 with the actionable rename message — NOT a 200 local fallback.
Previously AmbiguousCustomProviderError fell into the generic ``except
Exception`` handler that returns 200 with a ``warning`` the UI ignores,
marking the handoff handled and hiding the fix from the user.
"""
import api.routes as routes
import api.config as cfg
import api.models as models
monkeypatch.setattr(routes, "require", lambda body, *keys: None)
monkeypatch.setattr(
routes, "j",
lambda _handler, payload, status=200, extra_headers=None: {**payload, "_status": status},
)
monkeypatch.setattr(
routes, "bad",
lambda _handler, msg, status=400: {"ok": False, "error": msg, "_status": status},
)
persisted = []
monkeypatch.setattr(
routes, "_persist_handoff_summary",
lambda sid, summary, channel, rounds, fallback=False: persisted.append(sid) or {"ok": True},
)
monkeypatch.setattr(models, "count_conversation_rounds", lambda sid, since=None: models.CONVERSATION_ROUND_THRESHOLD)
monkeypatch.setattr(
models, "get_cli_session_messages",
lambda sid: [
{"role": "user", "content": "Could you check this?", "timestamp": 1.0},
{"role": "assistant", "content": "Sure, I can help", "timestamp": 2.0},
],
)
# require_ai_agent_class() imports run_agent and hermes_cli.runtime_provider
# before model resolution; stub them so execution reaches resolve_model_provider.
fake_runtime_module = types.ModuleType("hermes_cli.runtime_provider")
fake_runtime_module.resolve_runtime_provider = lambda requested=None: {
"api_key": "x", "provider": "openrouter", "base_url": None,
}
fake_hermes_cli = types.ModuleType("hermes_cli")
fake_hermes_cli.__path__ = []
fake_hermes_cli.runtime_provider = fake_runtime_module
monkeypatch.setitem(sys.modules, "hermes_cli", fake_hermes_cli)
monkeypatch.setitem(sys.modules, "hermes_cli.runtime_provider", fake_runtime_module)
class _DummyAgent:
def __init__(self, *args, **kwargs):
pass
fake_run_agent = types.ModuleType("run_agent")
fake_run_agent.AIAgent = _DummyAgent
monkeypatch.setitem(sys.modules, "run_agent", fake_run_agent)
rename_msg = (
"Custom providers ['Foo (Bar)', 'foo-bar'] all normalize to the same "
"provider slug 'foo-bar'; rename one so each has a unique slug."
)
def _raise_ambiguous(*args, **kwargs):
raise cfg.AmbiguousCustomProviderError(rename_msg)
monkeypatch.setattr(cfg, "resolve_model_provider", _raise_ambiguous)
response = routes._handle_handoff_summary(object(), {"session_id": "session-ambiguous"})
assert response.get("_status") == 400, response
assert response.get("type") == "custom_provider_ambiguous", response
assert response.get("error") == rename_msg
# Must NOT have silently degraded to a persisted fallback summary.
assert persisted == [], "ambiguity must not persist a 200 fallback summary"
assert "summary" not in response
def test_handoff_summary_retries_once_when_length_limit_reached(monkeypatch):
"""finish_reason='length' should trigger one retry with larger budget."""
import api.routes as routes
tests/test_issue2271_keyless_custom_provider.py
+48
View File
@@ -55,6 +55,54 @@ def test_named_custom_provider_keeps_existing_runtime_base_url(monkeypatch):
assert base_url == "http://runtime.example/v1"
def test_custom_provider_overrides_resolve_connection_within_profile_scope(monkeypatch):
"""Finding #3: the custom endpoint AND credential are resolved inside the
session's OWN profile scope.
``resolve_custom_provider_connection`` reads both the base_url and the API key
from one ``get_config()``/env snapshot. On a detached worker thread (no
inherited request-profile TLS/env) an unbound call reads the DEFAULT profile,
pairing a named profile's endpoint with the default profile's key. The
overrides helper must run that lookup under the passed profile scope.
"""
import contextlib
import api.streaming as streaming
import api.profiles as profiles
scope_state = {"active": False, "name": None}
@contextlib.contextmanager
def _fake_scope(name, purpose="", logger_override=None):
scope_state["active"] = True
scope_state["name"] = name
try:
yield
finally:
scope_state["active"] = False
monkeypatch.setattr(profiles, "profile_scope_for_detached_worker", _fake_scope)
seen = {}
def _fake_conn(provider):
# Endpoint + credential must both be read WHILE the profile scope is active.
seen["scope_active"] = scope_state["active"]
seen["scope_name"] = scope_state["name"]
return ("team-key", "https://team.example/v1")
monkeypatch.setattr(streaming, "resolve_custom_provider_connection", _fake_conn)
provider, api_key, base_url = streaming._resolve_custom_provider_runtime_overrides(
"custom:team", None, None, profile_name="team",
)
assert seen["scope_active"] is True, "connection resolved OUTSIDE the profile scope"
assert seen["scope_name"] == "team"
assert provider == "custom"
assert api_key == "team-key"
assert base_url == "https://team.example/v1"
def test_non_custom_provider_is_unchanged(monkeypatch):
import api.streaming as streaming
tests/test_model_resolver.py
+399
View File
@@ -253,6 +253,405 @@ def test_custom_provider_model_with_slash_routes_to_named_custom_provider():
assert base_url == 'http://lmstudio.local:1234/v1'
# ── Overlapping custom_providers[] model ids — active endpoint wins ─────────
def test_overlapping_custom_providers_active_base_url_wins_not_config_order():
"""When two custom_providers[] list the SAME bare model id, routing must
follow the ACTIVE provider (resolved from model.base_url → named slug), NOT
config write order.
Real-world repro: dogapi and packyapi both advertise 'claude-sonnet-5';
dogapi is written FIRST in config. A plain first-match scan hijacked the
model to dogapi even though model.base_url pointed at packyapi. The active
endpoint the user configured must win.
"""
custom_providers = [
{'name': 'dogapi', 'base_url': 'https://www.dogapi.cc/v1',
'models': ['claude-sonnet-5', 'gpt-5.6-sol', 'musk-4.5']},
{'name': 'packyapi', 'base_url': 'https://www.packyapi.ai/v1',
'models': ['claude-sonnet-5', 'claude-opus-5']},
]
# Active endpoint is packyapi (bare 'custom' + base_url resolves to the slug).
model, provider, base_url = _resolve_with_config(
'claude-sonnet-5',
provider='custom',
base_url='https://www.packyapi.ai/v1',
custom_providers=custom_providers,
)
assert provider == 'custom:packyapi', (
f"shared model must route to the ACTIVE packyapi endpoint, got {provider!r}"
)
assert base_url == 'https://www.packyapi.ai/v1'
def test_overlapping_custom_providers_unique_model_still_routes_by_ownership():
"""A model that only ONE overlapping provider lists still routes to that
provider even when the active endpoint is the other one — the active-slug
guard only claims models the active provider actually owns, then falls
through to the ordered ownership scan.
"""
custom_providers = [
{'name': 'dogapi', 'base_url': 'https://www.dogapi.cc/v1',
'models': ['claude-sonnet-5', 'gpt-5.6-sol', 'musk-4.5']},
{'name': 'packyapi', 'base_url': 'https://www.packyapi.ai/v1',
'models': ['claude-sonnet-5', 'claude-opus-5']},
]
# Active endpoint packyapi, but 'gpt-5.6-sol' is dogapi-only → must go dogapi.
model, provider, base_url = _resolve_with_config(
'gpt-5.6-sol',
provider='custom',
base_url='https://www.packyapi.ai/v1',
custom_providers=custom_providers,
)
assert provider == 'custom:dogapi', (
f"dogapi-only model must still route to dogapi, got {provider!r}"
)
assert base_url == 'https://www.dogapi.cc/v1'
def test_overlapping_custom_providers_bare_custom_no_base_url_keeps_order():
"""With a bare 'custom' provider and NO base_url to disambiguate, the active
slug can't be resolved, so the legacy config-order first-match behaviour is
preserved (no regression for users who never set a base_url).
"""
custom_providers = [
{'name': 'dogapi', 'base_url': 'https://www.dogapi.cc/v1',
'models': ['claude-sonnet-5']},
{'name': 'packyapi', 'base_url': 'https://www.packyapi.ai/v1',
'models': ['claude-sonnet-5']},
]
model, provider, base_url = _resolve_with_config(
'claude-sonnet-5',
provider='custom',
custom_providers=custom_providers,
)
assert provider == 'custom:dogapi', (
f"with no base_url to disambiguate, first-match order is preserved, got {provider!r}"
)
def test_overlapping_custom_providers_normalized_slug_collision_fails_closed():
"""Two DISTINCT provider names that normalize to the SAME slug must fail
closed, even when the active base_url pins one exact entry.
'Foo Bar' and 'foo-bar' both normalize to slug custom:foo-bar. Even with the
active base_url pointing at the SECOND entry (B), resolve_model_provider can
only return the shared slug custom:foo-bar. The downstream credential lookup
(resolve_custom_provider_connection) then resolves the API key from the FIRST
same-slug entry (A) regardless of base_url — so endpoint B would be paired
with credential A. Rather than emit that broken pairing, resolution must
raise so the collision surfaces to the user.
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1', 'models': ['shared-model']},
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
]
with pytest.raises(config.AmbiguousCustomProviderError):
_resolve_with_config(
'shared-model',
provider='custom',
base_url='https://b.example/v1',
custom_providers=custom_providers,
)
def test_overlapping_custom_providers_slug_collision_no_base_url_fails_closed():
"""When two same-slug entries exist and there is NO base_url to disambiguate,
resolution must fail closed rather than silently guessing the first entry.
The ordered scan would return the shared slug custom:foo-bar, whose credential
lookup can't be pinned to a single entry — so it raises instead.
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1', 'models': ['shared-model']},
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
]
# Active provider is bare 'custom' with no base_url: nothing disambiguates.
with pytest.raises(config.AmbiguousCustomProviderError):
_resolve_with_config(
'shared-model',
provider='custom',
custom_providers=custom_providers,
)
def test_overlapping_custom_providers_explicit_slug_wins_over_stale_base_url():
"""An explicitly selected named custom provider with a UNIQUE slug must win
even when model.base_url is stale and points at neither entry.
A (dogapi) is written first; B (packyapi) is explicitly selected. Both own
'shared-model'. model.base_url is set to a THIRD, stale URL that matches no
entry. The explicit, unambiguous provider B must be authoritative — a stale
URL is not evidence to demote it to config order (which would return A).
"""
custom_providers = [
{'name': 'dogapi', 'base_url': 'https://www.dogapi.cc/v1',
'models': ['shared-model']},
{'name': 'packyapi', 'base_url': 'https://www.packyapi.ai/v1',
'models': ['shared-model']},
]
model, provider, base_url = _resolve_with_config(
'shared-model',
provider='custom:packyapi',
base_url='https://stale.example/v1', # stale: matches neither entry
custom_providers=custom_providers,
)
assert provider == 'custom:packyapi', (
f"explicit provider must win over stale base_url + config order, got {provider!r}"
)
assert base_url == 'https://www.packyapi.ai/v1', (
f"must use the selected provider's own base_url, got {base_url!r}"
)
def test_session_provider_context_routes_handoff_to_session_endpoint_not_active():
"""Handoff-summary sibling fix: a session pinned to custom:A must resolve
through A, not the active custom:B, when both list the same model id.
The handoff summary path reads s_obj.model + s_obj.model_provider and passes
model_with_provider_context(model, model_provider) into resolve_model_provider.
This pins the routing behavior that fix relies on: encoding the SESSION's own
provider overrides the active endpoint. (The handoff path then backfills
base_url from resolve_custom_provider_connection, so base_url=None here is
expected and fine.)
"""
custom_providers = [
{'name': 'dogapi', 'base_url': 'https://www.dogapi.cc/v1',
'models': ['shared-model']},
{'name': 'packyapi', 'base_url': 'https://www.packyapi.ai/v1',
'models': ['shared-model']},
]
old_cfg = dict(config.cfg)
config.cfg['model'] = {
'default': 'shared-model', 'provider': 'custom',
'base_url': 'https://www.dogapi.cc/v1', # active endpoint = dogapi
}
config.cfg['custom_providers'] = custom_providers
try:
# Session is pinned to packyapi even though dogapi is active.
encoded = config.model_with_provider_context('shared-model', 'custom:packyapi')
model, provider, _base = config.resolve_model_provider(encoded)
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
assert provider == 'custom:packyapi', (
f"session's own provider must win over the active endpoint, got {provider!r}"
)
def test_overlapping_custom_providers_asymmetric_collision_fails_closed():
"""ASYMMETRIC normalized-slug collision must fail closed too.
Reviewer-reproduced shape: entry A ('Foo Bar') appears FIRST and does NOT
list 'shared-model'; entry B ('foo-bar') lists it and is the active endpoint.
Both normalize to custom:foo-bar. Ownership-only collision detection would
see just B and happily return B's endpoint + custom:foo-bar — but the
credential lookup scans by slug and first-matches A, pairing B's endpoint
with A's credential. Membership must be built from ALL named entries
(ownership-independent), so this raises.
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1'}, # first, NON-owner
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
]
with pytest.raises(config.AmbiguousCustomProviderError):
_resolve_with_config(
'shared-model',
provider='custom',
base_url='https://b.example/v1', # active endpoint = B
custom_providers=custom_providers,
)
def test_overlapping_custom_providers_asymmetric_collision_bare_custom_fails_closed():
"""Same asymmetric collision on the bare-'custom', no-base_url ordered-scan
path: the owning entry B is returned as custom:foo-bar, but non-owner A
shares the slug, so the ordered scan must also fail closed.
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1'}, # first, NON-owner
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
]
with pytest.raises(config.AmbiguousCustomProviderError):
_resolve_with_config(
'shared-model',
provider='custom',
custom_providers=custom_providers,
)
def test_provider_qualified_custom_hint_collision_fails_closed():
"""The @custom:<slug>:model qualified path (session/send/handoff shape via
model_with_provider_context) must apply the same all-entry uniqueness check.
Without it, the qualified return hands back custom:foo-bar and the credential
backfill first-matches the wrong entry. Uses the asymmetric shape so the
check cannot rely on model ownership of the encoded string.
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1'}, # first, NON-owner
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
]
old_cfg = dict(config.cfg)
config.cfg['model'] = {
'default': 'shared-model', 'provider': 'custom',
'base_url': 'https://a.example/v1',
}
config.cfg['custom_providers'] = custom_providers
try:
encoded = config.model_with_provider_context('shared-model', 'custom:foo-bar')
with pytest.raises(config.AmbiguousCustomProviderError):
config.resolve_model_provider(encoded)
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
def test_resolve_custom_provider_connection_collision_fails_closed(monkeypatch):
"""The credential boundary itself must fail closed on a slug collision so an
endpoint and API key can never be resolved from different entries — even when
called directly (e.g. the handoff/context-length credential backfill).
"""
custom_providers = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1', 'api_key': 'key-A'},
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'api_key': 'key-B'},
]
monkeypatch.setattr(config, 'get_config', lambda: {'custom_providers': custom_providers})
with pytest.raises(config.AmbiguousCustomProviderError):
config.resolve_custom_provider_connection('custom:foo-bar')
# ── Finding #1: one canonical slug identity (_custom_provider_slug_from_name) ──
# The collision key MUST match the slug PRODUCER. A parenthesized / punctuated
# name is where a looser key diverged: 'Foo (Bar)' is produced as custom:foo-bar
# but a naive lower+space/underscore key yields 'foo-(bar)' — so the collision
# with a literal 'foo-bar' entry was MISSED, reintroducing the original bug
# (endpoint A + credential B) for these names. These regressions collide two
# names ONLY via the producer.
def test_slug_key_matches_producer_for_parenthesized_name():
"""The collision key derives from the SAME producer that mints the returned
slug, so genuinely-colliding names are seen as identical."""
assert config._custom_provider_slug_from_name('Foo (Bar)') == 'custom:foo-bar'
assert config._custom_provider_slug_from_name('foo-bar') == 'custom:foo-bar'
# Both map to the same bare collision key.
assert config._custom_provider_slug_key('Foo (Bar)') == config._custom_provider_slug_key('foo-bar')
assert config._custom_provider_slug_key('custom:foo-bar') == 'foo-bar'
def _parenthesized_collision_providers():
# 'Foo (Bar)' and 'foo-bar' BOTH produce custom:foo-bar, but only collide
# under the producer's normalization (a looser key would miss them).
return [
{'name': 'Foo (Bar)', 'base_url': 'https://a.example/v1', 'api_key': 'key-A',
'models': ['shared-model']},
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'api_key': 'key-B',
'models': ['shared-model']},
]
def test_parenthesized_name_collision_fails_closed_bare_resolution():
"""Finding #1: bare resolution must fail closed on a producer-level collision
that a looser key missed ('Foo (Bar)' vs 'foo-bar')."""
with pytest.raises(config.AmbiguousCustomProviderError):
_resolve_with_config(
'shared-model',
provider='custom',
base_url='https://b.example/v1',
custom_providers=_parenthesized_collision_providers(),
)
def test_parenthesized_name_collision_fails_closed_qualified_hint():
"""Finding #1: the @custom:<slug>:model qualified path must also catch the
parenthesized-name collision."""
old_cfg = dict(config.cfg)
config.cfg['model'] = {'default': 'shared-model', 'provider': 'custom',
'base_url': 'https://a.example/v1'}
config.cfg['custom_providers'] = _parenthesized_collision_providers()
try:
encoded = config.model_with_provider_context('shared-model', 'custom:foo-bar')
with pytest.raises(config.AmbiguousCustomProviderError):
config.resolve_model_provider(encoded)
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
def test_parenthesized_name_collision_fails_closed_credential_boundary(monkeypatch):
"""Finding #1: the credential lookup must fail closed on the parenthesized
collision so endpoint and key can never split."""
monkeypatch.setattr(
config, 'get_config',
lambda: {'custom_providers': _parenthesized_collision_providers()},
)
with pytest.raises(config.AmbiguousCustomProviderError):
config.resolve_custom_provider_connection('custom:foo-bar')
# ── Finding #2: the ambiguity check runs only at the point of return, so an
# unrelated collision never blocks a request that resolves to a DIFFERENT
# provider. With a colliding slug ACTIVE, unrelated lanes must still route.
def _collision_plus_safe_lane_config():
old_cfg = dict(config.cfg)
config.cfg['model'] = {
'default': 'shared-model',
'provider': 'custom:foo-bar', # the ACTIVE provider is the colliding slug
'base_url': 'https://b.example/v1',
}
# 'Foo Bar' + 'foo-bar' collide under BOTH the old and new normalizers, so
# the PRE-FIX up-front check reproduces the over-block (every lane raises);
# the fix moves the check to the point of return so only the colliding lane
# fails. (Parenthesized-name collisions are covered by the finding #1 tests.)
config.cfg['custom_providers'] = [
{'name': 'Foo Bar', 'base_url': 'https://a.example/v1', 'models': ['shared-model']},
{'name': 'foo-bar', 'base_url': 'https://b.example/v1', 'models': ['shared-model']},
{'name': 'safe-provider', 'base_url': 'https://safe.example/v1',
'models': ['safe-model']},
]
return old_cfg
def test_unrelated_openrouter_lane_not_blocked_by_active_collision():
"""Finding #2: an @openrouter hint must resolve even though the ACTIVE custom
slug (custom:foo-bar) is ambiguous — the check must not run up front."""
old_cfg = _collision_plus_safe_lane_config()
try:
model, provider, _base = config.resolve_model_provider('@openrouter:gpt-x')
assert provider == 'openrouter', provider
assert model == 'gpt-x', model
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
def test_unrelated_custom_lane_not_blocked_by_active_collision():
"""Finding #2: a DIFFERENT, non-colliding @custom:safe-provider hint must
resolve even though the active slug collides."""
old_cfg = _collision_plus_safe_lane_config()
try:
encoded = config.model_with_provider_context('safe-model', 'custom:safe-provider')
model, provider, base_url = config.resolve_model_provider(encoded)
assert provider == 'custom:safe-provider', provider
assert model == 'safe-model', model
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
def test_active_collision_still_fails_closed_for_owned_model():
"""Finding #2 counterpart: when the request DOES resolve to the colliding
active slug (its owned default model), it must still fail closed — the
point-of-return check fires exactly when the slug is consumed."""
old_cfg = _collision_plus_safe_lane_config()
try:
with pytest.raises(config.AmbiguousCustomProviderError):
config.resolve_model_provider('shared-model')
finally:
config.cfg.clear()
config.cfg.update(old_cfg)
# ── #3872: bare ``custom`` provider is a vendor-routing proxy — preserve the
# full model id (the prefix is intrinsic). #433's redundant-prefix strip is
# scoped to real first-party providers (provider=openai + proxy base_url),