1 Commits
Author SHA1 Message Date
4528279cb2 fix: route overlapping custom_providers models by active endpoint (#7107)
* 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>
2026-08-20 07:09:25 +00:00