diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 565f6057a43..e0e70009fe3 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -41,7 +41,7 @@ import type { FastPolicyAuthority } from "../../providers/fastwire"; import { effectiveGoogleMode, getProviderRegistryEntry, providerMatchesRegistryTransport } from "../../providers/registry"; import { parseAntigravityAvailableModels, registerAntigravityDiscoveredWireModels } from "../../providers/antigravity-models"; import { applyProviderContextCap, providerContextCap, resolveUnknownRoutedContextWindow } from "../../providers/context-cap"; -import { routedSlug, slugEquals, slugsEquivalent } from "../../providers/slug-codec"; +import { routedSlug, slugEquals, slugEquivalenceKey, slugsEquivalent } from "../../providers/slug-codec"; import { CODEX_GPT5_IDENTITY_LINE } from "../../adapters/identity"; import { filterCursorConfiguredModelsByLiveDiscovery } from "../../adapters/cursor/discovery"; import { fetchCursorUsableModels } from "../../adapters/cursor/live-models"; @@ -1560,7 +1560,24 @@ export function filterCatalogVisibleModels( const allowByProvider = new Map>(); for (const [name, prov] of Object.entries(config.providers)) { const sel = prov.selectedModels; - if (Array.isArray(sel) && sel.length > 0) allowByProvider.set(name, new Set(sel)); + // Keyed the way `sync.ts` keys the same list, so a slash-bearing native id and + // the encoded slug the Codex picker displays are one entry rather than two. A + // bare `Set(sel)` matched only the native form, so an allowlist written from the + // displayed slug — which `ocx models remove` also accepts — hid every model it + // was meant to keep. + // + // The key is deliberately lossy: `p/a/b` and `p/a-b` collapse to one entry, so a + // provider publishing both spellings has them selected together. That is a real + // limitation, pinned by the tests below and tracked as a follow-up; it is NOT + // fixed here. Resolving selections against the current roster instead was tried + // and rejected — the roster is an incomplete dictionary (live discovery can omit + // a published id), so it produces the same over-grant while additionally + // disagreeing with the `slugEquivalenceKey` contract `sync.ts` uses at merge time. + // Two catalog stages with different equivalence relations is the exact bug class + // this change exists to remove. + if (Array.isArray(sel) && sel.length > 0) { + allowByProvider.set(name, new Set(sel.map(model => slugEquivalenceKey(routedSlug(name, model))))); + } } return models.filter(m => { const nativeAlias = m.provider === COMBO_NAMESPACE && m.nativeAlias === true; @@ -1572,7 +1589,7 @@ export function filterCatalogVisibleModels( if (slugEquals(stored, m.provider, m.id)) return false; } const allow = allowByProvider.get(m.provider); - return !allow || allow.has(m.id); + return !allow || allow.has(slugEquivalenceKey(routedSlug(m.provider, m.id))); }); } diff --git a/tests/selected-models.test.ts b/tests/selected-models.test.ts index 1e28df11026..755060410cc 100644 --- a/tests/selected-models.test.ts +++ b/tests/selected-models.test.ts @@ -47,3 +47,77 @@ describe("filterCatalogVisibleModels — per-provider allowlist", () => { expect(out.map(x => x.id).sort()).toEqual(["model-1999", "model-7"]); }); }); + +describe("filterCatalogVisibleModels — slash-bearing ids", () => { + // The Codex picker displays a slash-bearing native id in its ENCODED form, and + // `ocx models remove` accepts that form too, so an allowlist is routinely written + // with slugs the provider never published. A bare `Set(selectedModels)` matched + // only the native spelling and hid every model it was meant to keep. + const native = "moonshotai/kimi-k3-free"; + const encoded = "moonshotai-kimi-k3-free"; + const rows = [m("zenmux", native), m("zenmux", "openai/gpt-5.5")]; + + test("an allowlist written with the encoded slug keeps the model", () => { + const visible = filterCatalogVisibleModels(rows, cfg({ + zenmux: { selectedModels: [encoded] }, + })); + expect(visible.map(v => v.id)).toEqual([native]); + }); + + test("the native form keeps working", () => { + const visible = filterCatalogVisibleModels(rows, cfg({ + zenmux: { selectedModels: [native] }, + })); + expect(visible.map(v => v.id)).toEqual([native]); + }); + + test("a mixed allowlist keeps both, without duplicating either", () => { + const visible = filterCatalogVisibleModels(rows, cfg({ + zenmux: { selectedModels: [encoded, "openai/gpt-5.5"] }, + })); + expect(visible.map(v => v.id).sort()).toEqual([native, "openai/gpt-5.5"].sort()); + }); + + test("a model outside the allowlist is still hidden", () => { + const visible = filterCatalogVisibleModels(rows, cfg({ + zenmux: { selectedModels: [encoded] }, + })); + expect(visible.map(v => v.id)).not.toContain("openai/gpt-5.5"); + }); + + // The encoding is lossy: `a/b` and `a-b` share one encoded form, so a provider that + // publishes both spellings has them selected TOGETHER. These tests pin that as known + // behavior rather than leaving it undiscovered. + // + // It is not fixed here, and the obvious alternative does not work. Resolving each + // selection against the provider's current rows was tried: the roster is an + // incomplete dictionary — live discovery can omit a published id — so an exact + // `a-b` selection still resolves onto `a/b` whenever `a-b` is missing from that + // snapshot, producing the same over-grant. It would additionally disagree with the + // `slugEquivalenceKey` relation `sync.ts` applies when merging the persisted + // catalog, and two catalog stages using different equivalence rules is the bug + // class this change removes. A real fix needs ONE selection resolver shared by + // filtering, persisted sync, and routing, against a complete known-id set. + describe("a provider publishing both spellings (known lossy case)", () => { + const both = [m("p", "a/b"), m("p", "a-b")]; + + test("selecting either spelling selects both", () => { + const viaSlash = filterCatalogVisibleModels(both, cfg({ p: { selectedModels: ["a/b"] } })); + expect(viaSlash.map(v => v.id).sort()).toEqual(["a-b", "a/b"]); + const viaDash = filterCatalogVisibleModels(both, cfg({ p: { selectedModels: ["a-b"] } })); + expect(viaDash.map(v => v.id).sort()).toEqual(["a-b", "a/b"]); + }); + + test("an unrelated model is still hidden, so the allowlist has not collapsed", () => { + const rows = [...both, m("p", "unrelated")]; + const visible = filterCatalogVisibleModels(rows, cfg({ p: { selectedModels: ["a-b"] } })); + expect(visible.map(v => v.id)).not.toContain("unrelated"); + }); + }); + + test("a nested-slash id is kept by its fully encoded slug", () => { + const nested = [m("p", "x/y/z"), m("p", "other")]; + const visible = filterCatalogVisibleModels(nested, cfg({ p: { selectedModels: ["x-y-z"] } })); + expect(visible.map(v => v.id)).toEqual(["x/y/z"]); + }); +});