From 0c6d52da9af7c19bbbc746259d9b55e4bc1f4056 Mon Sep 17 00:00:00 2001 From: Lukasz Raczylo Date: Thu, 27 Aug 2026 22:26:30 +0100 Subject: [PATCH] fix(registry): resolve bare model ids against the caller's own group bareWinner picked the first provider in global sorted order that served a bare model id, ignoring which providers the caller's group could actually use. resolveAgainst then denied the request whenever that provider fell outside the group, even when another provider serving the identical id was already permitted. A caller entitled to a model could receive errModelDenied purely because an unrelated, invisible provider sorted first alphabetically. bareWinner now takes the group and prefers the sorted-first provider it may use, falling back to the global sorted-first provider only when the group is authorized for none of them. That fallback keeps a genuinely unauthorized request returning errModelDenied, not the wrong errModelUnknown a "no visible owner" result would produce. resolve, resolveAliasTarget, and the alias-shadow collision check all route through this corrected selection. listFor now computes each bare id's owner from the group's own visible providers and adds "provider/model" disambiguating entries only when the group can see two or more owners of that id. A provider invisible to the group never appears in that group's catalog, bare or prefixed. warnCollisionOnce keeps logging the global collision unchanged, since a provider collision is a fact about the fleet's configuration that every operator must see regardless of which group lists models first. Its message now says winner is the global default, because a group's own request can resolve to a different provider than the one logged. Adds registry_test.go coverage for the production repro, the both-allowed and neither-allowed non-regression cases, the listing behavior, and the alias path. Adds a yaegi-check probe proving the fix resolves and serves under the real interpreter, and updates one existing listFor test that asserted the pre-fix prefixed-form behavior. --- registry.go | 132 +++++++++++++++++---- registry_test.go | 242 +++++++++++++++++++++++++++++++++++++- tools/yaegi-check/main.go | 165 +++++++++++++++++++++++++- 3 files changed, 508 insertions(+), 31 deletions(-) diff --git a/registry.go b/registry.go index 205a643..df6d4da 100644 --- a/registry.go +++ b/registry.go @@ -1089,14 +1089,39 @@ func (m *modelRegistry) splitConfiguredProvider(id string) (providerName, rest s // bareWinner returns the provider that owns bare id when more than one // provider's known model set contains it: the first, in sorted -// provider-name order (ruling (g)). -func (m *modelRegistry) bareWinner(id string) (string, bool) { +// provider-name order, among providers grp is authorized to use (ruling +// (g), revised 2026-08-27 — bareWinner used to ignore grp entirely and +// always return the GLOBAL sorted-first owning provider, so a caller was +// denied a model they were actually entitled to whenever some OTHER, +// group-invisible provider happened to sort earlier and also served that +// id). grp.allowsModel is NOT consulted here — only allowsProvider; model- +// level authorization still happens once, in resolveAgainst, against +// whichever provider this returns. +// +// When grp allows none of id's owning providers, this still returns the +// GLOBAL sorted-first owner with ok true, exactly as the pre-fix version +// always did, rather than reporting "not found": id is still a genuinely +// known model, just not one this group may reach, and the caller +// (resolveAgainst) must see it as known-but-denied (errModelDenied) — +// the same outcome grp got before this fix — not the wrong +// errModelUnknown a "no visible owner" result would produce. +// +// Single pass, same O(providerNames) bound as before: the fallback and the +// grp-preferred search share one loop instead of two. +func (m *modelRegistry) bareWinner(id string, grp *group) (string, bool) { + fallback, fallbackOK := "", false for _, name := range m.providerNames { - if m.states[name].hasModel(id) { + if !m.states[name].hasModel(id) { + continue + } + if !fallbackOK { + fallback, fallbackOK = name, true + } + if grp.allowsProvider(name) { return name, true } } - return "", false + return fallback, fallbackOK } // resolve maps a client-requested model id to the adapter that serves it. @@ -1119,9 +1144,12 @@ func (m *modelRegistry) bareWinner(id string) (string, bool) { // // id in "provider/model" form (ruling (a): only when "provider" names a // configured provider) resolves directly against that provider. Any other -// id is a bare id, resolved against the first configured provider (sorted -// name order) whose known model set contains it — ruling (g)'s collision -// rule. Either way, authorization requires both grp.allowsModel and +// id is a bare id, resolved against the first provider — in sorted name +// order, among providers grp is authorized to use, falling back to the +// global sorted-first owner only when grp is authorized for none of them +// (bareWinner's own doc comment) — whose known model set contains it, +// ruling (g)'s collision rule as revised 2026-08-27 for group-awareness. +// Either way, authorization requires both grp.allowsModel and // grp.allowsProvider for the resolved provider (ruling (c)); a model that // exists but fails authorization returns errModelDenied, distinct from // errModelUnknown for a model no configured provider knows at all. @@ -1132,8 +1160,14 @@ func (m *modelRegistry) resolve(id string, grp *group) (providerAdapter, string, // alias would otherwise pay it just to decide not to log again. // Once warned, this collapses to one cheap mutex-guarded map // lookup per request instead. + // + // grp is passed through here too even though this call only uses + // the boolean "does id collide at all" — bareWinner's ok result is + // group-independent (it falls back to the global owner rather than + // reporting not-found when grp allows none of them), so this stays + // the same collision signal it always was. if !m.aliasShadowWarned(id) { - if _, discoveredCollision := m.bareWinner(id); discoveredCollision { + if _, discoveredCollision := m.bareWinner(id, grp); discoveredCollision { m.warnAliasShadowsDiscoveredOnce(id) } } @@ -1142,7 +1176,7 @@ func (m *modelRegistry) resolve(id string, grp *group) (providerAdapter, string, if providerName, rest, ok := m.splitConfiguredProvider(id); ok { return m.resolveAgainst(providerName, rest, id, "", grp, errModelUnknown) } - providerName, ok := m.bareWinner(id) + providerName, ok := m.bareWinner(id, grp) if !ok { return nil, "", "", errModelUnknown } @@ -1190,7 +1224,7 @@ func (m *modelRegistry) resolveAliasTarget(alias, target string, grp *group) (pr if providerName, rest, ok := m.splitConfiguredProvider(target); ok { return m.resolveAgainst(providerName, rest, target, alias, grp, notFound) } - providerName, ok := m.bareWinner(target) + providerName, ok := m.bareWinner(target, grp) if !ok { return nil, "", "", notFound } @@ -1297,6 +1331,24 @@ func (m *modelRegistry) resolveAgainst(providerName, upstreamModel, requestedID, // warnCollisionOnce logs, at most once per colliding bare id for this // registry's lifetime, that provs (sorted) all provide id and winner owns // its bare form. +// +// Decision (2026-08-27, group-aware bare-winner fix): stays keyed on the +// GLOBAL owner set and the global sorted-first winner, not narrowed per +// calling group, even though bareWinner (above) now prefers a +// group-visible owner over this global one when resolving an actual +// request. A provider collision on one model id is a fact about the +// fleet's configuration — two providers were pointed at the same upstream +// model — true and worth an operator's attention regardless of which +// group's request happens to trigger listFor first. Narrowing this to +// what one group can see would either (a) log the same underlying +// collision repeatedly, once per differently-scoped group, forcing an +// operator to de-duplicate by hand, or (b) suppress it entirely for any +// collision no single group can see both sides of, hiding a real +// configuration duplicate from the operator who needs to see it to fix +// it (rename one provider's model, or accept the collision knowingly). +// The log message is worded below to say winner is only the global +// default, since a given group's own request may now resolve to a +// different provider than winner. func (m *modelRegistry) warnCollisionOnce(id, winner string, provs []string) { m.warnedMu.Lock() defer m.warnedMu.Unlock() @@ -1304,7 +1356,7 @@ func (m *modelRegistry) warnCollisionOnce(id, winner string, provs []string) { return } m.warned[id] = true - m.warnf(fmt.Sprintf("model registry: model id %q is provided by multiple providers %v; %q wins the bare id", id, provs, winner)) + m.warnf(fmt.Sprintf("model registry: model id %q is provided by multiple providers %v; %q is the global bare-id winner (a caller's group may instead be routed to whichever of these providers it is authorized to use)", id, provs, winner)) } // warnf writes msg through the warn field, falling back to log when no @@ -1378,13 +1430,19 @@ func (m *modelRegistry) modelsJSON(grp *group) []byte { // listFor returns grp's visible model catalog as OpenAI-compatible model // objects ({"id","object":"model","owned_by"}), sorted by id. A bare id -// owned by only one provider is listed once, bare. A bare id owned by more -// than one provider (ruling (g)/(h)) is listed once bare — under its -// collision winner — plus once more per owning provider in "provider/id" +// owned by only one provider grp can see is listed once, bare. A bare id +// owned by more than one provider grp can see (ruling (g)/(h), revised +// 2026-08-27 for group-awareness) is listed once bare — under the +// sorted-first of THOSE group-visible owners, not the global sorted-first +// owner — plus once more per group-visible owning provider in "provider/id" // form, so every provider's copy stays reachable through explicit -// addressing even when it lost the bare-id collision. Each listed entry is -// independently filtered by grp.allowsModel and grp.allowsProvider for the -// provider it names. +// addressing even when it lost the bare-id collision. A provider grp +// cannot use is never a candidate for the bare id and never gets a +// "provider/id" entry either: a prefix that only disambiguates against an +// invisible provider is noise, not information, to this group. Each listed +// entry is independently filtered by grp.allowsModel; grp.allowsProvider +// filtering happens once, up front, when the group-visible owner subset is +// built, rather than per candidate entry. func (m *modelRegistry) listFor(grp *group) []map[string]any { owners := make(map[string][]string) for _, name := range m.providerNames { @@ -1402,22 +1460,44 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any { // from the listing entirely (neither the real entry nor the alias // appeared), even though resolve still serves it through the alias. listed := make(map[string]bool, len(owners)) + visible := make([]string, 0, 4) // reused per id; provs is at most len(m.providerNames) long for id, provs := range owners { - winner := provs[0] - if grp.allowsModel(id) && grp.allowsProvider(winner) { - out = append(out, modelObject(id, winner, m.resolveMetaFor(winner, id))) + // warnCollisionOnce stays keyed on the GLOBAL owner set, not grp's + // view of it (decision, see registry.go's collision-logging note + // below resolveAgainst): a config collision is a fact about the + // fleet, true regardless of which group happens to list models + // first, and operators need one stable signal to act on rather + // than a different, group-shaped warning per requester. + if len(provs) >= 2 { + m.warnCollisionOnce(id, provs[0], provs) + } + + visible = visible[:0] + for _, p := range provs { + if grp.allowsProvider(p) { + visible = append(visible, p) // provs is sorted, so visible stays sorted too + } + } + if len(visible) == 0 { + continue // grp cannot reach any provider serving id at all + } + + groupWinner := visible[0] + if grp.allowsModel(id) { + out = append(out, modelObject(id, groupWinner, m.resolveMetaFor(groupWinner, id))) listed[id] = true } - if len(provs) < 2 { - continue + if len(visible) < 2 { + continue // only one provider serving id is visible to grp: no disambiguating prefix to add } - m.warnCollisionOnce(id, winner, provs) - for _, p := range provs { + for _, p := range visible { pid := p + "/" + id // allowsModel does no prefix-stripping (auth.go): check both the // prefixed form and its bare suffix, same as resolveAgainst does - // for the equivalent client request. - if (grp.allowsModel(pid) || grp.allowsModel(id)) && grp.allowsProvider(p) { + // for the equivalent client request. allowsProvider(p) is already + // established by p's membership in visible, so it is not + // re-checked per candidate here. + if grp.allowsModel(pid) || grp.allowsModel(id) { out = append(out, modelObject(pid, p, m.resolveMetaFor(p, id))) listed[pid] = true } diff --git a/registry_test.go b/registry_test.go index e92ff58..08386ab 100644 --- a/registry_test.go +++ b/registry_test.go @@ -351,6 +351,128 @@ func TestModelRegistry_Resolve_ProviderPrefixed_AlwaysReachesLosingProvider(t *t } } +// --- resolve: GROUP-AWARE collision precedence (2026-08-27 fix) --- + +// TestModelRegistry_Resolve_BareIDCollision_GroupAwareWinner is the exact +// production repro from the bug report: "alpha" and "zeta" both serve +// "shared". alpha sorts first, so bareWinner used to pick it globally +// regardless of the caller — a group authorized ONLY for zeta got +// errModelDenied for a model it was actually entitled to use, purely +// because alpha (a provider the group cannot even see) happened to sort +// earlier. The fix restricts bareWinner's candidate search to providers +// grp.allowsProvider admits, falling back to the global winner only when +// none of them qualify. +// Mutation that must make this test fail: revert bareWinner to its +// pre-fix, group-blind body (`for _, name := range m.providerNames { if +// m.states[name].hasModel(id) { return name, true } }`, ignoring grp +// entirely) — resolve then picks "alpha", grp.allowsProvider("alpha") is +// false, and this returns errModelDenied instead of serving zeta. Verified: +// applying that exact revert turns this failure red (errModelDenied, want +// nil) before restoring the fix. +func TestModelRegistry_Resolve_BareIDCollision_GroupAwareWinner(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "alpha": newFakeAdapter("alpha"), + "zeta": newFakeAdapter("zeta"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "alpha": {Models: []string{"shared"}}, + "zeta": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + grp := &group{name: "friends", providers: []string{"zeta"}} + adapter, upstreamModel, canonical, err := reg.resolve("shared", grp) + if err != nil { + t.Fatalf("resolve: %v, want success (zeta serves \"shared\" and this group may use zeta)", err) + } + if adapter != adapters["zeta"] { + t.Errorf("adapter = %v, want zeta", adapter) + } + if upstreamModel != "shared" { + t.Errorf("upstreamModel = %q, want %q", upstreamModel, "shared") + } + if canonical != "zeta/shared" { + t.Errorf("canonical = %q, want %q", canonical, "zeta/shared") + } +} + +// TestModelRegistry_Resolve_BareIDCollision_BothAllowed_MatchesGlobalWinner +// is the required non-regression: a group explicitly authorized for BOTH +// colliding providers must resolve to the exact same provider it did +// before this fix — the sorted-first one, "alpha" — with no behavior +// change at all for a caller who could already reach today's global +// winner. +// Mutation that must make this test fail: reverse bareWinner's provider +// iteration order (`for i := len(m.providerNames) - 1; i >= 0; i--`) — +// this group can reach both providers, so the now-sorted-last provider +// "zeta" would win instead of "alpha". Verified: applying that reversal +// turns this failure red (adapter = zeta, want alpha) before restoring +// the fix. +func TestModelRegistry_Resolve_BareIDCollision_BothAllowed_MatchesGlobalWinner(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "alpha": newFakeAdapter("alpha"), + "zeta": newFakeAdapter("zeta"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "alpha": {Models: []string{"shared"}}, + "zeta": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + grp := &group{name: "both", providers: []string{"alpha", "zeta"}} + adapter, _, canonical, err := reg.resolve("shared", grp) + if err != nil { + t.Fatalf("resolve: %v", err) + } + if adapter != adapters["alpha"] { + t.Errorf("adapter = %v, want alpha (sorted-first, unchanged from today)", adapter) + } + if canonical != "alpha/shared" { + t.Errorf("canonical = %q, want %q", canonical, "alpha/shared") + } +} + +// TestModelRegistry_Resolve_BareIDCollision_NeitherAllowed_SameDenialAsToday +// covers the other required non-regression: a group authorized for +// NEITHER colliding provider must still get errModelDenied — the same +// sentinel a known-but-unauthorized model returns today — never the wrong +// errModelUnknown a "no owner found" result would produce. +// Mutation that must make this test fail: make bareWinner return ("", +// false) when grp allows none of id's owners, instead of falling back to +// the global sorted-first owner — resolve's `if !ok { return ..., +// errModelUnknown }` branch then fires, and this test's errModelDenied +// assertion fails against the wrong sentinel. Verified: that change turns +// this failure red (err = errModelUnknown, want errModelDenied) before +// restoring the fallback. +func TestModelRegistry_Resolve_BareIDCollision_NeitherAllowed_SameDenialAsToday(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "alpha": newFakeAdapter("alpha"), + "zeta": newFakeAdapter("zeta"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "alpha": {Models: []string{"shared"}}, + "zeta": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + grp := &group{name: "outsiders", providers: []string{"unrelated"}} + if _, _, _, err := reg.resolve("shared", grp); err != errModelDenied { + t.Errorf("err = %v, want errModelDenied", err) + } +} + // --- newModelRegistry construction --- func TestNewModelRegistry_InvalidDiscoveryInterval_ReturnsConstructorError(t *testing.T) { @@ -869,7 +991,18 @@ func TestModelRegistry_ListFor_Collision_LogsWarningOnceRegardlessOfCallCount(t } } -func TestModelRegistry_ListFor_Collision_LosingProviderStillReachableViaPrefixedForm(t *testing.T) { +// TestModelRegistry_ListFor_Collision_SingleVisibleProviderGetsBareID is +// the group-aware listing fix (2026-08-27): "alpha" and "beta" both serve +// "shared", but a group that may only reach "beta" sees no reason to know +// "alpha" exists at all. Before the fix, listFor always computed the bare +// id's owner from the GLOBAL sorted-first provider ("alpha" here) and +// only fell back to a "provider/id"-prefixed entry for a group that could +// not use it — so this exact group saw "beta/shared", a prefix that +// disambiguates against a provider it can never see in the first place. +// Mutation that must make this test fail: revert listFor's winner +// selection to the unconditional `winner := provs[0]` global choice (this +// commit's registry.go diff) — the entry becomes "beta/shared" again. +func TestModelRegistry_ListFor_Collision_SingleVisibleProviderGetsBareID(t *testing.T) { t.Parallel() adapters := map[string]providerAdapter{ "beta": newFakeAdapter("beta"), @@ -889,8 +1022,65 @@ func TestModelRegistry_ListFor_Collision_LosingProviderStillReachableViaPrefixed if len(got) != 1 { t.Fatalf("listFor = %v, want exactly 1 entry", got) } - if got[0]["id"] != "beta/shared" || got[0]["owned_by"] != "beta" { - t.Errorf("entry = %v, want {id:beta/shared, owned_by:beta}", got[0]) + if got[0]["id"] != "shared" || got[0]["owned_by"] != "beta" { + t.Errorf("entry = %v, want {id:shared, owned_by:beta} (bare, not prefixed)", got[0]) + } +} + +// TestModelRegistry_ListFor_Collision_TwoVisibleProvidersKeepBothPrefixedForms +// covers the OTHER half of the same fix: when a group can see two or more +// of an id's owning providers, the collision is real FROM THAT GROUP'S OWN +// VIEW, so both "provider/id" forms stay listed (letting the group reach +// either explicitly) alongside a bare-id entry attributed to whichever of +// the group's own visible providers sorts first — "alpha" is configured +// but invisible to this group entirely and must not appear anywhere in the +// output, including as the bare id's owner. +// Mutation that must make this test fail: change listFor's prefixed-form +// gate back to the global `len(provs) < 2` (this commit's registry.go +// diff) — with alpha invisible but still counted, the gate does not +// change here (provs already has 2 entries either way), so instead mutate +// the winner used for the bare entry back to the unconditional +// `provs[0]` — the bare entry's owned_by becomes "alpha", a provider this +// group is never authorized to reach. +func TestModelRegistry_ListFor_Collision_TwoVisibleProvidersKeepBothPrefixedForms(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "gamma": newFakeAdapter("gamma"), + "beta": newFakeAdapter("beta"), + "alpha": newFakeAdapter("alpha"), + } + cfg := &Config{Providers: map[string]*ProviderConfig{ + "gamma": {Models: []string{"shared"}}, + "beta": {Models: []string{"shared"}}, + "alpha": {Models: []string{"shared"}}, + }} + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + // alpha sorts first globally but is invisible to this group. + grp := &group{name: "beta-and-gamma", providers: []string{"beta", "gamma"}} + got := reg.listFor(grp) + + byID := make(map[string]map[string]any, len(got)) + for _, entry := range got { + byID[entry["id"].(string)] = entry + } + if len(got) != 3 { + t.Fatalf("listFor = %v, want exactly 3 entries (bare + 2 prefixed)", got) + } + if entry, ok := byID["shared"]; !ok || entry["owned_by"] != "beta" { + t.Errorf("bare entry = %v, want owned_by beta (sorted-first of the group's OWN visible providers)", entry) + } + if entry, ok := byID["beta/shared"]; !ok || entry["owned_by"] != "beta" { + t.Errorf("beta/shared entry = %v, want owned_by beta", entry) + } + if entry, ok := byID["gamma/shared"]; !ok || entry["owned_by"] != "gamma" { + t.Errorf("gamma/shared entry = %v, want owned_by gamma", entry) + } + if _, ok := byID["alpha/shared"]; ok { + t.Errorf("listFor = %v, must never list alpha/shared: alpha is invisible to this group", got) } } @@ -1273,6 +1463,52 @@ func TestModelRegistry_Resolve_Alias_BareTarget_ResolvesViaCollisionWinner(t *te } } +// TestModelRegistry_Resolve_Alias_BareTarget_GroupAwareWinner is the alias +// path's counterpart to TestModelRegistry_Resolve_BareIDCollision_ +// GroupAwareWinner (2026-08-27 fix): resolveAliasTarget's own bareWinner +// call must apply the identical group-aware collision rule the direct +// bare-id path does, not the pre-fix, group-blind one — the brief +// explicitly calls out this call site since it is easy to fix bareWinner +// itself and forget one of its two callers. +// Mutation that must make this test fail: hardcode resolveAliasTarget's +// `m.bareWinner(target, grp)` call to pass `&group{}` (allow-all) instead +// of the real grp — resolveAliasTarget would then always prefer "alpha" +// (the global winner) regardless of what this group may use, and this +// test's zeta assertion fails. Verified: that change turns this failure +// red (adapter = alpha, want zeta; also errModelDenied surfaces once +// resolveAgainst's own provider check runs against alpha) before +// restoring the real grp parameter. +func TestModelRegistry_Resolve_Alias_BareTarget_GroupAwareWinner(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "alpha": newFakeAdapter("alpha"), + "zeta": newFakeAdapter("zeta"), + } + cfg := &Config{ + Providers: map[string]*ProviderConfig{ + "alpha": {Models: []string{"shared"}}, + "zeta": {Models: []string{"shared"}}, + }, + ModelAliases: map[string]string{"aliased/x": "shared"}, + } + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + + grp := &group{name: "friends", providers: []string{"zeta"}} + adapter, _, canonical, err := reg.resolve("aliased/x", grp) + if err != nil { + t.Fatalf("resolve: %v, want success via the alias's group-visible target provider", err) + } + if adapter != adapters["zeta"] { + t.Errorf("adapter = %v, want zeta", adapter) + } + if canonical != "zeta/shared" { + t.Errorf("canonical = %q, want %q", canonical, "zeta/shared") + } +} + // --- resolve: alias authorization (spec §5's "alias name OR target, // either grants") --- diff --git a/tools/yaegi-check/main.go b/tools/yaegi-check/main.go index a9cf29f..7e31f69 100644 --- a/tools/yaegi-check/main.go +++ b/tools/yaegi-check/main.go @@ -491,6 +491,32 @@ func run() error { })) defer failoverBUpstream.Close() + // bareWinnerAUpstream/bareWinnerBUpstream back exerciseBareWinnerGroupAware + // (fix/group-aware-bare-winner, 2026-08-27): two providers configured + // with the IDENTICAL bare model id (bareWinnerModelID) — the same + // collision shape failoverAUpstream/failoverBUpstream above drive, but + // exercising the bareWinner AUTHORIZATION fix, not failover. A must + // NEVER be hit: bareWinnerGroupName can only use "bare-winner-b", so a + // correct group-aware bareWinner never even considers "bare-winner-a" + // as a candidate for this group's request, unlike a failover fall- + // through (which WOULD hit A first). bareWinnerAHits catches a + // regression back to the pre-fix, group-blind selection, which would + // route here and get an error response instead of the expected 200. + bareWinnerAHits := new(int64) + bareWinnerAUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt64(bareWinnerAHits, 1) + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusInternalServerError) + _, _ = w.Write([]byte(`{"error":{"message":"bare-winner-a must never be reached by bareWinnerGroupName","type":"server_error"}}`)) + })) + defer bareWinnerAUpstream.Close() + bareWinnerBUpstream := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + w.WriteHeader(http.StatusOK) + _, _ = w.Write([]byte(`{"id":"c-bare-winner","object":"chat.completion","model":"` + bareWinnerModelID + `","choices":[{"index":0,"message":{"role":"assistant","content":"served by bare-winner-b"},"finish_reason":"stop"}],"usage":{"prompt_tokens":1,"completion_tokens":1}}`)) + })) + defer bareWinnerBUpstream.Close() + // failover.enabled must be set explicitly here: FailoverConfig.Enabled // defaults to false (coordinator ruling — a version upgrade with no // config change must preserve prior behavior), so without this the @@ -533,7 +559,19 @@ func run() error { // callAnthropicMessagesPassthrough. `"anthropic":{"type":"anthropic","baseUrl":"` + anthropicProbeUpstream.URL + `","apiKey":"sk-anth","models":["claude-test"]},` + // #nosec G101 -- test fixture literal, not a real credential `"failover-a":{"type":"openai","baseUrl":"` + failoverAUpstream.URL + `","apiKey":"sk-up","models":["` + failoverModelID + `"]},` + - `"failover-b":{"type":"openai","baseUrl":"` + failoverBUpstream.URL + `","apiKey":"sk-up","models":["` + failoverModelID + `"]}},` + + `"failover-b":{"type":"openai","baseUrl":"` + failoverBUpstream.URL + `","apiKey":"sk-up","models":["` + failoverModelID + `"]},` + + // bareWinnerModelID's own const doc comment (above) has the full + // account of why "bare-winner-a"/"bare-winner-b" exist alongside + // failover-a/failover-b: same collision shape, different fix under + // test. + `"bare-winner-a":{"type":"openai","baseUrl":"` + bareWinnerAUpstream.URL + `","apiKey":"sk-up","models":["` + bareWinnerModelID + `"]},` + + `"bare-winner-b":{"type":"openai","baseUrl":"` + bareWinnerBUpstream.URL + `","apiKey":"sk-up","models":["` + bareWinnerModelID + `"]}},` + + // groups: ADDS bareWinnerGroupName to the map Unmarshal already + // populated from .traefik.yml's own testData.groups.default — map + // keys merge (run()'s own doc comment, above, on why "brk" and the + // other new provider keys above are additive rather than + // replacing), so "default" (providers: [], allow-all) is untouched. + `"groups":{"` + bareWinnerGroupName + `":{"providers":["bare-winner-b"]}},` + // modelMeta (feature v0.23): a config-override entry for // testDataWantModel, so exerciseHandler's GET /v1/models // assertion below proves resolveModelMeta's config-override @@ -564,7 +602,12 @@ func run() error { // still renders a real (zero-traffic) llmgateway_requests_total // series, since writeUsageMetrics (metrics.go) lists every // active user regardless of traffic. - `"admin":{"enabled":true},"users":{"inline":[{"name":"tester","group":"default","apiKey":"` + testDataUserAPIKey + `"},{"name":"admin1","group":"default","apiKey":"` + attemptAccountingAdminAPIKey + `","admin":true},{"name":` + string(trickyNameJSON) + `,"group":"default","apiKey":"sk-tricky"}]}}` + // + // The fourth inline user (bareWinnerFriendAPIKey) is + // exerciseBareWinnerGroupAware's own — group bareWinnerGroupName, + // authorized for "bare-winner-b" only (the "groups" override + // above). + `"admin":{"enabled":true},"users":{"inline":[{"name":"tester","group":"default","apiKey":"` + testDataUserAPIKey + `"},{"name":"admin1","group":"default","apiKey":"` + attemptAccountingAdminAPIKey + `","admin":true},{"name":` + string(trickyNameJSON) + `,"group":"default","apiKey":"sk-tricky"},{"name":"bare-winner-friend","group":"` + bareWinnerGroupName + `","apiKey":"` + bareWinnerFriendAPIKey + `"}]}}` if err = json.Unmarshal([]byte(attemptAccountingOverride), cfgVal.Interface()); err != nil { return fmt.Errorf("decode attempt-accounting harness override into the interpreted Config: %w", err) } @@ -602,6 +645,16 @@ func run() error { if err := exerciseHandler(handler, builtinLookupContextTokens, failoverBHits); err != nil { return err } + // exerciseBareWinnerGroupAware (fix/group-aware-bare-winner, + // 2026-08-27) runs right after exerciseHandler and before + // exerciseBreaker: it drives its own, freshly named providers and + // model id (bareWinnerModelID's own doc comment above), so it cannot + // perturb exerciseBreaker's "brk"-specific hit counting or any later + // probe's /metrics assertions, which are all keyed to OTHER provider/ + // model names. + if err := exerciseBareWinnerGroupAware(handler, bareWinnerAHits); err != nil { + return err + } // exerciseBreaker (feat/provider-health, adversarial-review round 2): // runs after exerciseHandler, against the SAME interpreted handler — // proves the discovery circuit breaker's open/half-open/closed state @@ -656,6 +709,31 @@ const builtinLookupModelID = "gpt-4o" // exerciseFailover (below) drives. const failoverModelID = "failover-shared" +// bareWinnerModelID is the bare model id both "bare-winner-a" and +// "bare-winner-b" (run(), above) are configured with — deliberately the +// identical collision SHAPE as failoverModelID above, but exercising +// registry.go's bareWinner AUTHORIZATION fix (2026-08-27) rather than +// failover: bareWinnerGroupName (below) is authorized for "bare-winner-b" +// only, yet "bare-winner-a" sorts first alphabetically. Before the fix, +// bareWinner always returned the GLOBAL sorted-first owner regardless of +// the caller, so this exact group got errModelDenied for a model it was +// actually entitled to use — the production repro this whole fix exists +// for (uni/macstudio serving text-embedding-multilingual-e5-base, the +// "friends" group entitled to uni but denied because macstudio sorts +// first). exerciseBareWinnerGroupAware (below) proves the fix resolves +// and serves under the REAL interpreter, not merely compiled. +const bareWinnerModelID = "bare-winner-shared" + +// bareWinnerGroupName is the group exerciseBareWinnerGroupAware +// authenticates as: authorized for "bare-winner-b" only (run()'s "groups" +// override), never "bare-winner-a" — see bareWinnerModelID's own doc +// comment for why that specific asymmetry is the point. +const bareWinnerGroupName = "bare-winner-friends" + +// bareWinnerFriendAPIKey authenticates bareWinnerGroupName's one inline +// user (run()'s "users" override). +const bareWinnerFriendAPIKey = "sk-bare-winner-friend" // #nosec G101 -- test fixture literal, not a real credential + // mcpProbeServerName is the federated MCP server run() configures against // mcpProbeUpstream, and mcpProbeToolName is a tool id carrying its // "_" prefix so resolveFederatedTool routes POST /mcp's @@ -1737,6 +1815,89 @@ func exerciseFailover(handler http.Handler, bHits *int64) error { return nil } +// exerciseBareWinnerGroupAware proves registry.go's group-aware bareWinner +// fix (2026-08-27) under the REAL interpreter: "bare-winner-a" and +// "bare-winner-b" both serve bareWinnerModelID; bareWinnerGroupName may +// use "bare-winner-b" only, yet "bare-winner-a" sorts first +// alphabetically. Before the fix, resolve's bareWinner call always +// returned the GLOBAL sorted-first owner regardless of the caller's +// group, so this exact request got errModelDenied for a model the group +// was actually entitled to use — this is the production repro the fix +// exists for (bug report: "friends" group entitled to "uni", denied a +// bare id because "macstudio" sorts first and also serves it). +// +// Unlike exerciseFailover (immediately above, same colliding-bare-id +// shape): a correct group-aware bareWinner never even CONSIDERS +// "bare-winner-a" as a candidate for this group's request — it is not a +// failover fall-through after a failed attempt. aHits staying at 0 proves +// that: a pre-fix, group-blind bareWinner would route here first, and +// "bare-winner-a"'s upstream (run(), above) answers 500 with a body +// naming the exact wrong outcome, so a regression is unambiguous either +// way (aHits != 0, or a non-200 status, or the wrong response body). +// +// MUTATION VERIFIED: temporarily reverting bareWinner (registry.go) to +// its pre-fix, group-blind body (ignore grp entirely, always return the +// first provider in m.providerNames whose known model set contains id) +// made this fail with "status = 403" (errModelDenied) — bareWinnerGroupName +// cannot use "bare-winner-a", the pre-fix global winner. Reverted before +// committing. +func exerciseBareWinnerGroupAware(handler http.Handler, aHits *int64) error { + req := httptest.NewRequest(http.MethodPost, "/v1/chat/completions", strings.NewReader( + `{"model":"`+bareWinnerModelID+`","messages":[{"role":"user","content":"hi"}]}`, + )) + req.Header.Set("Authorization", "Bearer "+bareWinnerFriendAPIKey) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + handler.ServeHTTP(rec, req) + + if rec.Code != http.StatusOK { + return fmt.Errorf("POST /v1/chat/completions (fix/group-aware-bare-winner harness): status = %d, want 200 (%s is entitled to \"bare-winner-b\", which serves the bare id), body=%s", rec.Code, bareWinnerGroupName, rec.Body.String()) + } + if !strings.Contains(rec.Body.String(), "served by bare-winner-b") { + return fmt.Errorf("POST /v1/chat/completions (fix/group-aware-bare-winner harness): body does not contain bare-winner-b's own content, want the group-visible provider's response: %s", rec.Body.String()) + } + if got := atomic.LoadInt64(aHits); got != 0 { + return fmt.Errorf("fix/group-aware-bare-winner harness: bare-winner-a upstream hit count = %d, want exactly 0 (%s can never reach bare-winner-a)", got, bareWinnerGroupName) + } + + // GET /v1/models, same group: listFor's own half of the fix + // (registry.go) — the bare id must be attributed to "bare-winner-b" + // (the group's own visible winner), and "bare-winner-a" must never + // appear anywhere in this group's catalog, bare or prefixed. + listReq := httptest.NewRequest(http.MethodGet, "/v1/models", nil) + listReq.Header.Set("Authorization", "Bearer "+bareWinnerFriendAPIKey) + listRec := httptest.NewRecorder() + handler.ServeHTTP(listRec, listReq) + if listRec.Code != http.StatusOK { + return fmt.Errorf("GET /v1/models (fix/group-aware-bare-winner harness): status = %d, want 200, body=%s", listRec.Code, listRec.Body.String()) + } + if strings.Contains(listRec.Body.String(), "bare-winner-a") { + return fmt.Errorf("GET /v1/models (fix/group-aware-bare-winner harness): body names bare-winner-a, want it invisible to %s entirely: %s", bareWinnerGroupName, listRec.Body.String()) + } + var listBody struct { + Data []map[string]any `json:"data"` + } + if err := json.Unmarshal(listRec.Body.Bytes(), &listBody); err != nil { + return fmt.Errorf("decode GET /v1/models body (fix/group-aware-bare-winner harness): %w (body=%s)", err, listRec.Body.String()) + } + found := false + for _, entry := range listBody.Data { + if entry["id"] != bareWinnerModelID { + continue + } + found = true + if entry["owned_by"] != "bare-winner-b" { + return fmt.Errorf("GET /v1/models entry %v: owned_by = %v, want %q (this group's own visible winner)", entry, entry["owned_by"], "bare-winner-b") + } + } + if !found { + return fmt.Errorf("GET /v1/models body has no bare entry for %q: %s", bareWinnerModelID, listRec.Body.String()) + } + + fmt.Println("yaegi-check: group-aware bareWinner resolved and listed bare-winner-shared via bare-winner-b, never touching bare-winner-a") + return nil +} + // exerciseRequestTimeout proves the request-timeout feature aborts a // hung provider under the REAL interpreter, not merely in compiled // tests: timeoutProviderName's own ProviderConfig.RequestTimeout ("80ms")