diff --git a/registry.go b/registry.go index df6d4da..125e7e2 100644 --- a/registry.go +++ b/registry.go @@ -1452,14 +1452,14 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any { } out := make([]map[string]any, 0, len(owners)) - // listed tracks every id actually APPENDED to out below — not merely - // present in the raw, authorization-blind owners map — so the alias - // dedupe loop further down only skips an alias whose real-model entry - // this group can actually see (review fix, second pass): keying the - // dedupe on owners itself made a group-denied collision vanish the id - // 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)) + // aliasEntries is every alias grp may actually use, resolved ONCE up + // front (spec §5) and keyed by alias id, so the id loop below can see + // which bare ids an alias shadows before it emits anything for them. + // An alias whose target does not resolve right now, or which grp is + // authorized for neither by alias name nor by target, is absent here + // and shadows nothing — the discovered model keeps its own bare entry + // in that case. + aliasEntries := m.aliasEntriesFor(grp) visible := make([]string, 0, 4) // reused per id; provs is at most len(m.providerNames) long for id, provs := range owners { // warnCollisionOnce stays keyed on the GLOBAL owner set, not grp's @@ -1482,12 +1482,23 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any { continue // grp cannot reach any provider serving id at all } + // A discovered model whose bare id collides with a usable alias id + // is SHADOWED (bug fix, 2026-09-10): resolve gives the exact alias + // match precedence over every discovered bare id (its own doc + // comment), so the listing must agree — emitting the discovered + // model's own entry here would advertise that provider's owner and + // metadata for an id every request routes to the alias target + // instead. The alias's entry is appended after this loop; the + // shadowed model itself stays listed, and addressable, in + // "provider/id" form below, which is exactly how a client must + // address it once the alias owns the bare id. + _, shadowed := aliasEntries[id] + groupWinner := visible[0] - if grp.allowsModel(id) { + if grp.allowsModel(id) && !shadowed { out = append(out, modelObject(id, groupWinner, m.resolveMetaFor(groupWinner, id))) - listed[id] = true } - if len(visible) < 2 { + if len(visible) < 2 && !shadowed { continue // only one provider serving id is visible to grp: no disambiguating prefix to add } for _, p := range visible { @@ -1499,43 +1510,18 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any { // 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 } } } - // Aliases (spec §5, v0.2): listed alongside real models only when - // both their target actually resolves right now (a target awaiting - // its provider's first discovery fetch is not listed — there is - // nothing yet to name as owned_by) and grp is authorized for it. - // resolveAliasTarget applies the exact same "alias name OR target" - // authorization rule request-time resolve uses (its own doc - // comment), so this listing can never promise access resolve would - // then deny. owned_by is recovered from the successful call's own - // canonical return value ("provider/upstreamModel") by splitting on - // the first "/" — exact, because a registry provider name can never - // itself contain one (configNamePattern, providers.go). - // - // An alias id that also happens to collide with a since-discovered - // model's own bare id (impossible for an EXPLICIT model — - // validateModelAliases already rejects that at construction) is - // SKIPPED here (review fix) only when that real model's own entry was - // actually EMITTED above (listed[alias], not merely present in - // owners) — otherwise adding it a second time would list it twice, - // duplicate, order-nondeterministic-looking entries in an - // OpenAI-shaped model list. resolve's own precedence (its doc - // comment) still has the alias win at REQUEST time regardless of - // which entry the listing shows; only the listing itself is deduped. - for alias, target := range m.aliases { - if listed[alias] { - continue - } - _, _, canonical, err := m.resolveAliasTarget(alias, target, grp) - if err != nil { - continue - } - providerName, bareTarget, _ := strings.Cut(canonical, "/") - out = append(out, modelObject(alias, providerName, m.resolveMetaForAlias(alias, providerName, bareTarget))) + // Aliases (spec §5, v0.2): appended after every real model, exactly + // once each. An alias id can never duplicate an entry emitted above — + // the id loop suppresses the bare entry of any id aliasEntries + // shadows, and an alias id can never take the "provider/id" form of a + // prefixed entry either (validateModelAliases rejects an alias whose + // prefix names a configured provider at construction). + for _, entry := range aliasEntries { + out = append(out, entry) } sort.Slice(out, func(i, j int) bool { @@ -1544,6 +1530,38 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any { return out } +// aliasEntriesFor resolves every configured alias grp may actually use +// into its finished listing entry, keyed by alias id (spec §5, v0.2). An +// alias is present only when its target resolves right now (a target +// awaiting its provider's first discovery fetch has nothing yet to name +// as owned_by) and grp is authorized for it: resolveAliasTarget applies +// the exact same "alias name OR target" authorization rule request-time +// resolve uses (its own doc comment), so a listing built from this can +// never promise access resolve would then deny. owned_by is recovered +// from the successful call's own canonical return value +// ("provider/upstreamModel") by splitting on the first "/" — exact, +// because a registry provider name can never itself contain one +// (configNamePattern, providers.go). +// +// Resolved once per listFor call rather than per candidate id: listFor +// needs the same set twice — to know which bare ids an alias shadows, +// and to emit the alias entries themselves. +func (m *modelRegistry) aliasEntriesFor(grp *group) map[string]map[string]any { + if len(m.aliases) == 0 { + return nil // the v0.1 no-aliases case allocates nothing + } + entries := make(map[string]map[string]any, len(m.aliases)) + for alias, target := range m.aliases { + _, _, canonical, err := m.resolveAliasTarget(alias, target, grp) + if err != nil { + continue + } + providerName, bareTarget, _ := strings.Cut(canonical, "/") + entries[alias] = modelObject(alias, providerName, m.resolveMetaForAlias(alias, providerName, bareTarget)) + } + return entries +} + // modelObject builds one OpenAI-compatible model list entry, extended // with two OpenAI-compat-safe extension fields (feature v0.23): // "context_window" (int) and "pricing" ({"input_per_mtok_usd", diff --git a/registry_test.go b/registry_test.go index 08386ab..6eb8799 100644 --- a/registry_test.go +++ b/registry_test.go @@ -1785,3 +1785,73 @@ func TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision(t *testin t.Errorf("owned_by = %q, want %q (the alias's own target provider)", ownedBy, "openai") } } + +// TestModelRegistry_ListFor_Alias_ShadowedCollisionKeepsAliasEntry proves the +// listing follows resolve's OWN precedence when a discovered model's bare id +// collides with a configured alias id: the single bare entry must be the +// ALIAS's (its target's owner and its resolved metadata), never the +// discovered model's, because every request for that id lands on the alias +// target instead (resolve's doc comment). The shadowed provider's own copy +// stays listed in "provider/id" form so it remains both visible and +// addressable. +// +// Live-fleet regression this fixes (2026-09-10): the alias +// "deepseek-v4-flash-vision-exp" -> "gx10/current" was listed as the cloud +// provider's identically-named model, owned_by that cloud provider and +// carrying NO context_window at all, while every request for the id ran +// against the local 1M-context model — so clients silently fell back to their +// own 128k default. +// +// Mutation that must make this test fail: restore listFor's old +// listed[alias]-keyed skip, which emitted the discovered model's entry and +// dropped the alias's. +func TestModelRegistry_ListFor_Alias_ShadowedCollisionKeepsAliasEntry(t *testing.T) { + t.Parallel() + adapters := map[string]providerAdapter{ + "acme": newFakeAdapter("acme"), + "local": newFakeAdapter("local"), + } + cfg := &Config{ + Providers: map[string]*ProviderConfig{ + "acme": {Discovery: true}, + "local": {Models: []string{"current"}}, + }, + ModelAliases: map[string]string{"shared-id": "local/current"}, + ModelMeta: map[string]*ModelMetaConfig{ + "local/current": {ContextTokens: 1048576, Free: true}, + }, + } + reg, err := newModelRegistry(adapters, cfg, func(string, ...any) {}) + if err != nil { + t.Fatalf("newModelRegistry: %v", err) + } + // acme's discovery finds a model literally named "shared-id" — the same + // string as the configured alias, discovered only after construction, so + // validateModelAliases never saw it. + reg.states["acme"].finishRefresh(reg.now(), []string{"shared-id"}, nil) + + got := reg.listFor(allowAllGroup()) + byID := make(map[string]map[string]any, len(got)) + bareCount := 0 + for _, entry := range got { + id, _ := entry["id"].(string) + byID[id] = entry + if id == "shared-id" { + bareCount++ + } + } + if bareCount != 1 { + t.Fatalf("listFor has %d entries for id %q, want exactly 1: %v", bareCount, "shared-id", got) + } + + bare := byID["shared-id"] + if bare["owned_by"] != "local" { + t.Errorf("owned_by = %v, want %q (the alias's own target provider, matching resolve)", bare["owned_by"], "local") + } + if bare["context_window"] != 1048576 { + t.Errorf("context_window = %v, want 1048576 (the alias target's metadata)", bare["context_window"]) + } + if _, ok := byID["acme/shared-id"]; !ok { + t.Errorf("listFor = %v, want the shadowed discovered model listed as %q", got, "acme/shared-id") + } +}