fix(aliases): deterministic integration gate, warn-first fast path, deduped listing

Review fix, task 6 (1 Important + 3 folded):

1. TestModelAliases now polls GET /v1/models (waitForModelAlias) until
   "aliased/mock" appears, bounded 30s, before asserting - removes the
   stale-dynamic-config propagation window a plain healthy-200 check
   cannot rule out. Confirmed config-propagation, not alias-logic, per
   reviewer's config+error-text analysis.
2. registry.go resolve(): check the warned-set (aliasShadowWarned) before
   paying bareWinner's O(providerNames) scan on every aliased request -
   only the first request for a colliding alias pays that cost now.
3. listFor: skip an alias entry whose id already exists in owners (a real
   model already owns it) - previously produced duplicate ids when a
   discovered model collided with a configured alias. resolve's own
   alias-wins precedence is unchanged; only the listing is deduped. New
   test: TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision.
4. newAliasTestRegistry doc comment: "grp's given aliases" -> "the given
   aliases" (no grp parameter exists).

integration/integration_test.go

@@ -139,6 +139,37 @@ func waitForHealthy(t *testing.T, url, apiKey string, timeout time.Duration) {
 	t.Fatalf("%s did not become healthy within %s (last status=%d, last error=%v)", url, timeout, lastStatus, lastErr)
 }
 
+// waitForModelAlias polls GET /v1/models on baseURL with apiKey until an
+// entry with id wantID appears, or timeout elapses. Task 6 review fix: a
+// plain 200 from /v1/models (waitForHealthy's own check) only proves
+// Traefik's file provider has loaded SOME generation of dynamic.yml — not
+// necessarily the one carrying modelAliases. A request landing in that
+// narrow propagation window would see a config generation from before the
+// alias applied and get a spurious "unknown model" 404, which is exactly
+// what one cold-start run of this suite hit. Polling the actual listing
+// removes that window deterministically instead of guessing at a fixed
+// sleep.
+func waitForModelAlias(t *testing.T, baseURL, apiKey, wantID string, timeout time.Duration) {
+	t.Helper()
+	deadline := time.Now().Add(timeout)
+	var lastBody map[string]any
+	for time.Now().Before(deadline) {
+		resp, body := doJSON(t, http.MethodGet, baseURL+"/v1/models", apiKey, nil)
+		if resp.StatusCode == http.StatusOK {
+			lastBody = body
+			if data, ok := body["data"].([]any); ok {
+				for _, entry := range data {
+					if m, ok := entry.(map[string]any); ok && m["id"] == wantID {
+						return
+					}
+				}
+			}
+		}
+		time.Sleep(500 * time.Millisecond)
+	}
+	t.Fatalf("GET %s/v1/models never listed alias %q within %s, last body=%#v", baseURL, wantID, timeout, lastBody)
+}
+
 // checkNoPluginErrors greps docker compose's captured logs for service,
 // failing if no plugin-loading message appears at all, or if any
 // plugin-related line is logged at error level.
@@ -248,6 +279,11 @@ func TestUnifiedChatAllProviders(t *testing.T) {
 // registry_test.go's Gateway-level equivalents for the same assertion
 // under go test).
 func TestModelAliases(t *testing.T) {
+	// Review fix: wait for the alias to actually appear in /v1/models
+	// before asserting anything below — see waitForModelAlias's doc
+	// comment for why a plain healthy-200 is not enough on its own.
+	waitForModelAlias(t, traefik1URL, aliceKey, "aliased/mock", 30*time.Second)
+
 	// openai-type: the upstream response is forwarded verbatim
 	// (provider_openai.go), so its "model" field carries whatever the
 	// mock echoes back — the resolved upstream id "gpt-mock", not the

registry.go

@@ -466,8 +466,15 @@ func (m *modelRegistry) bareWinner(id string) (string, bool) {
 // errModelUnknown for a model no configured provider knows at all.
 func (m *modelRegistry) resolve(id string, grp *group) (providerAdapter, string, string, error) {
 	if target, isAlias := m.aliases[id]; isAlias {
-		if _, discoveredCollision := m.bareWinner(id); discoveredCollision {
+		// Warned-set checked FIRST (review fix): bareWinner is an
-			m.warnAliasShadowsDiscoveredOnce(id)
+		// O(providerNames) scan, and every request for an already-warned
+		// 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.
+		if !m.aliasShadowWarned(id) {
+			if _, discoveredCollision := m.bareWinner(id); discoveredCollision {
+				m.warnAliasShadowsDiscoveredOnce(id)
+			}
 		}
 		return m.resolveAliasTarget(id, target, grp)
 	}
@@ -529,6 +536,27 @@ func (m *modelRegistry) resolveAliasTarget(alias, target string, grp *group) (pr
 	return m.resolveAgainst(providerName, target, target, alias, grp, notFound)
 }
 
+// aliasShadowWarnKey builds the m.warned key for alias's discovered-
+// collision warning — shared by aliasShadowWarned and
+// warnAliasShadowsDiscoveredOnce so the two can never drift apart into
+// mismatched keys. Prefixed with "alias-shadow:" so it can never collide
+// with warnCollisionOnce's own plain-id keys in the same m.warned map.
+func aliasShadowWarnKey(alias string) string {
+	return "alias-shadow:" + alias
+}
+
+// aliasShadowWarned reports whether warnAliasShadowsDiscoveredOnce has
+// already logged for alias, with no other side effect — resolve's fast
+// path (its own doc comment) checks this BEFORE paying bareWinner's
+// O(providerNames) scan, so a registry with many providers does not pay
+// that cost on every request for an alias once its one-time warning has
+// already fired.
+func (m *modelRegistry) aliasShadowWarned(alias string) bool {
+	m.warnedMu.Lock()
+	defer m.warnedMu.Unlock()
+	return m.warned[aliasShadowWarnKey(alias)]
+}
+
 // warnAliasShadowsDiscoveredOnce logs, at most once per colliding alias id
 // for this registry's lifetime, that alias also names a model some
 // provider's discovery fetch found after construction — resolve's
@@ -536,13 +564,11 @@ func (m *modelRegistry) resolveAliasTarget(alias, target string, grp *group) (pr
 // wins, since only an EXPLICIT collision is rejected at construction
 // (validateModelAliases); discovery is dynamic and runs after that check,
 // so this collision can only be caught, and only warned about, here at
-// request time. Keyed with an "alias-shadow:" prefix in the same m.warned
+// request time.
-// map warnCollisionOnce uses, so the two unrelated warning kinds can never
-// share a key and suppress each other.
 func (m *modelRegistry) warnAliasShadowsDiscoveredOnce(alias string) {
 	m.warnedMu.Lock()
 	defer m.warnedMu.Unlock()
-	key := "alias-shadow:" + alias
+	key := aliasShadowWarnKey(alias)
 	if m.warned[key] {
 		return
 	}
@@ -668,15 +694,22 @@ func (m *modelRegistry) listFor(grp *group) []map[string]any {
 	// 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
+	// itself contain one (configNamePattern, providers.go).
-	// that also happens to collide with a since-discovered model's own
+	//
-	// bare id (impossible for an EXPLICIT model — validateModelAliases
+	// An alias id that also happens to collide with a since-discovered
-	// already rejects that at construction) is not deduped against that
+	// model's own bare id (impossible for an EXPLICIT model —
-	// model's own entry above: the same documented, alias-wins
+	// validateModelAliases already rejects that at construction) is
-	// precedence edge resolve's warnAliasShadowsDiscoveredOnce logs
+	// SKIPPED here (review fix), not appended a second time: owners
-	// about, accepted here too as rare enough not to warrant extra
+	// already holds a real-model entry for that id from the loop above,
-	// bookkeeping.
+	// and adding another with the same id 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 _, collides := owners[alias]; collides {
+			continue
+		}
 		_, _, canonical, err := m.resolveAliasTarget(alias, target, grp)
 		if err != nil {
 			continue

registry_test.go

@@ -899,7 +899,7 @@ func TestNewGateway_WarmFill_PopulatesDiscoveredModels_BeforeAnyRequest(t *testi
 // --- model aliases (spec §5, v0.2) ---
 
 // newAliasTestRegistry builds a registry with one "openai" provider
-// (explicit model "gpt-test") and grp's given aliases, failing the test on
+// (explicit model "gpt-test") and the given aliases, failing the test on
 // any construction error. log defaults to a no-op when nil.
 func newAliasTestRegistry(t *testing.T, aliases map[string]string, log func(string, ...any)) *modelRegistry {
 	t.Helper()
@@ -1243,3 +1243,60 @@ func TestModelRegistry_ListFor_Alias_NotAuthorized_Omitted(t *testing.T) {
 		}
 	}
 }
+
+// TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision is the
+// review fix for listFor's alias-listing loop: when a provider's
+// discovery fetch finds a model whose bare id is identical to a
+// configured alias id (impossible for an EXPLICIT model —
+// validateModelAliases already rejects that at construction, but
+// discovery is dynamic and runs after that check), the listing must show
+// exactly ONE entry for that id — not one from the real discovered model
+// and a second, duplicate one from the alias loop. resolve's own
+// precedence (registry.go's resolve doc comment) must still have the
+// alias win at request time regardless of which entry the listing shows.
+func TestModelRegistry_ListFor_Alias_DedupedAgainstDiscoveredCollision(t *testing.T) {
+	t.Parallel()
+	adapters := map[string]providerAdapter{
+		"openai": newFakeAdapter("openai"),
+		"acme":   newFakeAdapter("acme"),
+	}
+	cfg := &Config{
+		Providers: map[string]*ProviderConfig{
+			"openai": {Models: []string{"gpt-test"}},
+			"acme":   {Discovery: true},
+		},
+		ModelAliases: map[string]string{"shared-id": "openai/gpt-test"},
+	}
+	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())
+	count := 0
+	for _, entry := range got {
+		if entry["id"] == "shared-id" {
+			count++
+		}
+	}
+	if count != 1 {
+		t.Errorf("listFor has %d entries for id %q, want exactly 1 (deduped)", count, "shared-id")
+	}
+
+	// resolve must still prefer the alias, per its documented precedence,
+	// regardless of which single entry the listing showed above.
+	adapter, _, canonical, err := reg.resolve("shared-id", allowAllGroup())
+	if err != nil {
+		t.Fatalf("resolve: %v", err)
+	}
+	if adapter != adapters["openai"] {
+		t.Errorf("adapter = %v, want openai (the alias, not acme's discovered model, must still win resolution)", adapter)
+	}
+	if canonical != "openai/gpt-test" {
+		t.Errorf("canonical = %q, want %q", canonical, "openai/gpt-test")
+	}
+}