From e7add83720f9f000f9b4914e03587fb138c7c071 Mon Sep 17 00:00:00 2001 From: Nguyen Thanh Dat Date: Mon, 24 Aug 2026 17:09:20 +0700 Subject: [PATCH 1/2] fix(catalog): match selectedModels the way the canonical resolver matches it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `filterCatalogVisibleModels` built the per-provider allowlist as a plain `Set(selectedModels)` and tested it with `allow.has(m.id)` — the native model id, exactly. The canonical resolution of the same list keys it through the slug equivalence: sync.ts:819-821 new Set([...models].map(m => slugEquivalenceKey(routedSlug(provider, m)))) sync.ts:1039 selected === undefined || selected.has(slugEquivalenceKey(slug)) so the two disagree for any provider whose native ids contain a slash: stored "moonshotai/kimi-k3-free" sync accepts=true catalog filter accepts=true stored "moonshotai-kimi-k3-free" sync accepts=true catalog filter accepts=false The second is the Codex-facing slug `routedSlug()` produces and the picker displays, and `ocx models remove` already accepts it (tests/cli-models.test.ts:332, "models remove accepts raw and encoded slash selectors"). An allowlist written from what the user sees therefore blanked the provider's catalog silently, while `routeModel` decoded the same string back and served the model happily. Affects providers with slash-bearing native ids: openrouter, zenmux, nvidia, together, fireworks. The `disabledModels` loop three lines above is already tolerant of both forms via `slugEquals`, and slug-codec.ts:20-21 states the rule this restores: "Config comparisons are tolerant … so legacy raw values keep working regardless of which form was stored." --- src/codex/catalog/provider-fetch.ts | 12 ++++++--- tests/selected-models.test.ts | 39 +++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+), 3 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index 565f6057a43..a862e018b1e 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,13 @@ 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 hid every model it was meant to keep. + 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 +1578,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..b8efc8d56dc 100644 --- a/tests/selected-models.test.ts +++ b/tests/selected-models.test.ts @@ -47,3 +47,42 @@ describe("filterCatalogVisibleModels — per-provider allowlist", () => { expect(out.map(x => x.id).sort()).toEqual(["model-1999", "model-7"]); }); }); + +describe("filterCatalogVisibleModels — slash-bearing ids", () => { + // `sync.ts` keys this same list through `slugEquivalenceKey(routedSlug(...))`, so + // the native id and the encoded slug the Codex picker displays are one entry there. + // A bare `Set(selectedModels)` here 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. + 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"); + }); +}); From 185b6bcfd2185d66b2dd0adb2a2e38da4b6d74ff Mon Sep 17 00:00:00 2001 From: bitkyc08-arch Date: Tue, 25 Aug 2026 00:12:43 +0900 Subject: [PATCH 2/2] test(catalog): pin the lossy collision and record the rejected alternative MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The key comparison is right for the reported bug, but it is lossy in a way worth writing down: "a/b" and "a-b" collapse to one equivalence key, so a provider publishing both spellings has them selected together. That behavior now has tests asserting what the code actually does, rather than being left for someone to discover from a support thread. It also has a rejected alternative recorded next to it. Resolving each selection against the provider's current rows looks stricter and is not: the roster is an incomplete dictionary, so when live discovery omits "a-b" but returns "a/b", an exact "a-b" selection resolves onto "a/b" and reproduces the same over-grant. It would additionally make fresh filtering disagree with the equivalence relation sync.ts applies when merging the persisted catalog — two catalog stages with different rules is the bug class this change removes. The real fix is one selection resolver shared by filtering, persisted sync, CLI removal, and routing, evaluated against a complete known-id set, with a single ambiguity policy. That is an architecture change and does not belong in a bugfix-only release; tracked as #2491. Tests: 242 pass across selected-models, codex-catalog, slug-codec, and cli-models. tsc --noEmit clean. --- src/codex/catalog/provider-fetch.ts | 13 ++++++++- tests/selected-models.test.ts | 45 +++++++++++++++++++++++++---- 2 files changed, 52 insertions(+), 6 deletions(-) diff --git a/src/codex/catalog/provider-fetch.ts b/src/codex/catalog/provider-fetch.ts index a862e018b1e..e0e70009fe3 100644 --- a/src/codex/catalog/provider-fetch.ts +++ b/src/codex/catalog/provider-fetch.ts @@ -1563,7 +1563,18 @@ export function filterCatalogVisibleModels( // 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 hid every model it was meant to keep. + // 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))))); } diff --git a/tests/selected-models.test.ts b/tests/selected-models.test.ts index b8efc8d56dc..755060410cc 100644 --- a/tests/selected-models.test.ts +++ b/tests/selected-models.test.ts @@ -49,11 +49,10 @@ describe("filterCatalogVisibleModels — per-provider allowlist", () => { }); describe("filterCatalogVisibleModels — slash-bearing ids", () => { - // `sync.ts` keys this same list through `slugEquivalenceKey(routedSlug(...))`, so - // the native id and the encoded slug the Codex picker displays are one entry there. - // A bare `Set(selectedModels)` here 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 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")]; @@ -85,4 +84,40 @@ describe("filterCatalogVisibleModels — slash-bearing ids", () => { })); 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"]); + }); });