diff --git a/changelog.d/features/12473-github-live-catalog-filter.md b/changelog.d/features/12473-github-live-catalog-filter.md new file mode 100644 index 00000000000..8eed69412e4 --- /dev/null +++ b/changelog.d/features/12473-github-live-catalog-filter.md @@ -0,0 +1 @@ +- **feat(providers):** skip GitHub combo members missing from the live synced catalog, and drop Copilot models that are policy-disabled or hidden from the model picker ([#12473](https://github.com/diegosouzapw/OmniRoute/pull/12473)) — thanks @RaviTharuma diff --git a/open-sse/services/githubCopilotModels.ts b/open-sse/services/githubCopilotModels.ts index b7a87ffbf22..39d112f014b 100644 --- a/open-sse/services/githubCopilotModels.ts +++ b/open-sse/services/githubCopilotModels.ts @@ -92,8 +92,15 @@ function toNonEmptyString(value: unknown): string | null { // (rename-robust) rather than an id allowlist: any model the account is entitled // to whose capabilities.type is "chat" (or that carries a chat-shaped // supported_endpoints) is kept, so a newly-entitled model shows up with no code -// change. Only explicitly non-chat rows (embeddings / completion) are dropped. +// change. Also filters out rows when policy.state is set and != "enabled", or +// when model_picker_enabled=false. Explicitly non-chat rows (embeddings / +// completion) are dropped as well. function isRoutableChatModel(item: RawRecord): boolean { + const policy = asRecord(item.policy); + const policyState = toNonEmptyString(policy.state); + if (policyState && policyState !== "enabled") return false; + if (item.model_picker_enabled === false) return false; + const capabilities = asRecord(item.capabilities); const capType = toNonEmptyString(capabilities.type); if (capType) return capType === "chat"; diff --git a/src/lib/db/models/activeSyncedCatalog.ts b/src/lib/db/models/activeSyncedCatalog.ts index 18ade8260b9..d20e3616954 100644 --- a/src/lib/db/models/activeSyncedCatalog.ts +++ b/src/lib/db/models/activeSyncedCatalog.ts @@ -13,6 +13,24 @@ export type ActiveSyncedCatalog = { models: SyncedAvailableModel[]; }; +/** + * Fail-open membership check for explicit combo members against a live catalog. + * `null` means no authoritative catalog is synced yet (unchanged behavior). + */ +export function catalogContainsModel( + catalog: ActiveSyncedCatalog, + modelId: string +): boolean | null { + if (!catalog.authoritative) return null; + const trimmed = modelId.trim(); + if (!trimmed) return false; + const ids = new Set(catalog.models.map((model) => model.id)); + if (ids.has(trimmed)) return true; + const slash = trimmed.indexOf("/"); + if (slash > 0 && ids.has(trimmed.slice(slash + 1))) return true; + return false; +} + export type ProviderCatalogReconciliation = { providers: string[]; excludedProviders: string[]; diff --git a/src/sse/handlers/chat.ts b/src/sse/handlers/chat.ts index 7ad2131076b..f72b074b31b 100644 --- a/src/sse/handlers/chat.ts +++ b/src/sse/handlers/chat.ts @@ -78,6 +78,7 @@ import { } from "@/lib/db/sessionAccountAffinity"; import { dispatchChatWithAffinityEviction } from "./chatDispatch"; import { getCachedSettings, getCombosCacheVersion } from "@/lib/db/readCache"; +import { comboCheckProvider, ghComboGate } from "./chat/githubLiveCatalogFilter.ts"; import { getCombos } from "@/lib/db/combos"; import { resolveModelLockoutSettings } from "@/lib/resilience/modelLockoutSettings"; import { @@ -1025,18 +1026,10 @@ async function handleChatImplementation( if (isCommonChatGptWebRetirementError(error)) return false; throw error; } - // Apply the same prefix-override guard as handleSingleModelChat: - // if providerId is just the prefix already in the model string, use - // the fully-resolved modelInfo.provider for a precise credential check. - const provider = (() => { - if (!target?.providerId) return modelInfo.provider; - if (target.providerId === modelInfo.provider) return modelInfo.provider; - if (modelString.startsWith(target.providerId + "/")) return modelInfo.provider; - return target.providerId; - })(); - if (!provider) return true; // can't determine provider, let it try - + const provider = comboCheckProvider(modelString, modelInfo, target?.providerId); const resolvedModel = modelInfo.model || modelString; + const githubGate = await ghComboGate(comboPreselectedCredentials, provider, resolvedModel); + if (githubGate !== null) return githubGate; const hasForcedConnection = typeof target?.connectionId === "string" && target.connectionId.trim().length > 0; let allowedConnections = intersectAllowedConnectionIds( diff --git a/src/sse/handlers/chat/githubLiveCatalogFilter.ts b/src/sse/handlers/chat/githubLiveCatalogFilter.ts new file mode 100644 index 00000000000..4112e815fcc --- /dev/null +++ b/src/sse/handlers/chat/githubLiveCatalogFilter.ts @@ -0,0 +1,71 @@ +/** + * GitHub live-catalog combo gate (#12137). + * + * Extracted from chat.ts so the frozen handler does not grow. Explicit combo + * members missing from an authoritative GitHub catalog are skipped; a catalog + * that is not synced yet fails open (same pattern as providerWildcard). + * + * The catalog promise is memoized per request-scope object so combo candidates + * share one getActiveSyncedCatalog fetch instead of re-hitting the DB. + */ +import { + catalogContainsModel, + getActiveSyncedCatalog, + type ActiveSyncedCatalog, +} from "@/lib/db/models/activeSyncedCatalog"; + +const catalogByScope = new WeakMap>>(); + +/** Prefix-override guard used by combo pre-check (same as handleSingleModelChat). */ +export function comboCheckProvider( + modelString: string, + modelInfo: { provider?: string }, + providerId?: string | null +): string | undefined { + if (!providerId) return modelInfo.provider; + if (providerId === modelInfo.provider) return modelInfo.provider; + if (modelString.startsWith(providerId + "/")) return modelInfo.provider; + return providerId; +} + +function loadGithubLiveCatalog( + scope: object, + providerId: string, + loadCatalog: (id: string) => Promise = getActiveSyncedCatalog +): Promise { + let byProvider = catalogByScope.get(scope); + if (!byProvider) { + byProvider = new Map(); + catalogByScope.set(scope, byProvider); + } + let pending = byProvider.get(providerId); + if (!pending) { + pending = loadCatalog(providerId); + byProvider.set(providerId, pending); + } + return pending; +} + +/** + * Combo pre-check for GitHub live-catalog membership. + * + * Returns: + * - `true` — allow immediately (provider could not be determined) + * - `false` — skip this combo member + * - `null` — not a GitHub skip; continue the remaining credential checks + */ +export async function ghComboGate( + scope: object, + provider: string | null | undefined, + resolvedModel: string, + loadCatalog?: (id: string) => Promise +): Promise { + if (!provider) return true; + if (provider !== "github" && provider !== "gh") return null; + const inLiveCatalog = catalogContainsModel( + await loadGithubLiveCatalog(scope, provider, loadCatalog), + resolvedModel + ); + if (inLiveCatalog === false) return false; + return null; +} diff --git a/tests/unit/github-copilot-model-discovery.test.ts b/tests/unit/github-copilot-model-discovery.test.ts index 77c5b11a916..83ff7ee24e9 100644 --- a/tests/unit/github-copilot-model-discovery.test.ts +++ b/tests/unit/github-copilot-model-discovery.test.ts @@ -38,6 +38,20 @@ const MOCK_COPILOT_MODELS_RESPONSE = { policy: { state: "enabled" }, capabilities: { type: "chat", limits: { max_context_window_tokens: 128000 } }, }, + { + id: "disabled-by-policy", + name: "Disabled by policy", + model_picker_enabled: true, + policy: { state: "disabled" }, + capabilities: { type: "chat" }, + }, + { + id: "hidden-from-picker", + name: "Hidden from picker", + model_picker_enabled: false, + policy: { state: "enabled" }, + capabilities: { type: "chat" }, + }, { id: "claude-sonnet-4.5", name: "Claude Sonnet 4.5", @@ -80,6 +94,8 @@ test("#3120 parseGitHubCopilotModels keeps every entitled CHAT model (capability assert.equal(gpt.owned_by, "github"); assert.ok(!ids.includes("text-embedding-3-small"), "embeddings models are skipped"); assert.ok(!ids.includes("gpt-41-copilot"), "completion utility models are skipped"); + assert.ok(!ids.includes("disabled-by-policy"), "policy.state=disabled is not routable"); + assert.ok(!ids.includes("hidden-from-picker"), "model_picker_enabled=false is not routable"); }); test("#3121 a model NOT in the live response is not advertised (entitlement filtering)", () => { diff --git a/tests/unit/github-live-catalog-combo-12137.test.ts b/tests/unit/github-live-catalog-combo-12137.test.ts new file mode 100644 index 00000000000..ccfd1054e42 --- /dev/null +++ b/tests/unit/github-live-catalog-combo-12137.test.ts @@ -0,0 +1,86 @@ +/** + * #12137 — explicit GitHub combo members vs live synced catalog. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import { catalogContainsModel } from "../../src/lib/db/models/activeSyncedCatalog.ts"; +import { + comboCheckProvider, + ghComboGate, +} from "../../src/sse/handlers/chat/githubLiveCatalogFilter.ts"; + +test("fail-open when the GitHub catalog is not authoritative yet", () => { + assert.equal( + catalogContainsModel( + { + authoritative: false, + models: [{ id: "claude-sonnet-5", name: "Claude Sonnet 5", source: "imported" }], + }, + "claude-sonnet-5" + ), + null + ); +}); + +test("rejects explicit members missing from an authoritative GitHub catalog", () => { + assert.equal( + catalogContainsModel( + { + authoritative: true, + models: [{ id: "claude-sonnet-5", name: "Claude Sonnet 5", source: "imported" }], + }, + "github/claude-fable-5" + ), + false + ); +}); + +test("accepts prefixed and bare ids that are in the live catalog", () => { + const catalog = { + authoritative: true, + models: [{ id: "claude-sonnet-5", name: "Claude Sonnet 5", source: "imported" }], + }; + assert.equal(catalogContainsModel(catalog, "claude-sonnet-5"), true); + assert.equal(catalogContainsModel(catalog, "github/claude-sonnet-5"), true); +}); + +test("comboCheckProvider applies the prefix-override guard", () => { + assert.equal(comboCheckProvider("github/claude-sonnet-5", { provider: "github" }), "github"); + assert.equal( + comboCheckProvider("github/claude-sonnet-5", { provider: "github" }, "github"), + "github" + ); + assert.equal( + comboCheckProvider("xiaomi/mimo-v2-flash", { provider: "xiaomi" }, "opengate"), + "opengate" + ); + assert.equal(comboCheckProvider("gh/claude-sonnet-5", { provider: "github" }, "gh"), "github"); +}); + +test("ghComboGate allows undetermined providers and fail-opens unsynced catalogs", async () => { + const scope = {}; + const unsynced = async () => ({ + authoritative: false, + models: [{ id: "claude-sonnet-5", name: "Claude Sonnet 5", source: "imported" as const }], + }); + assert.equal(await ghComboGate(scope, "", "claude-sonnet-5", unsynced), true); + assert.equal(await ghComboGate(scope, "openai", "gpt-4", unsynced), null); + assert.equal(await ghComboGate(scope, "github", "claude-sonnet-5", unsynced), null); +}); + +test("ghComboGate skips GitHub members missing from an authoritative catalog", async () => { + const scope = {}; + let loads = 0; + const load = async () => { + loads += 1; + return { + authoritative: true, + models: [{ id: "claude-sonnet-5", name: "Claude Sonnet 5", source: "imported" as const }], + }; + }; + assert.equal(await ghComboGate(scope, "github", "claude-fable-5", load), false); + assert.equal(await ghComboGate(scope, "github", "claude-sonnet-5", load), null); + assert.equal(await ghComboGate(scope, "github", "github/claude-sonnet-5", load), null); + assert.equal(loads, 1, "catalog fetch is memoized per request scope"); + assert.equal(await ghComboGate({}, "gh", "claude-fable-5", load), false); +});