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")