From ed2bf5dfe9049bb3b9cee1188c7f53ce8029df8a Mon Sep 17 00:00:00 2001 From: diegosouzapw <8016841+diegosouzapw@users.noreply.github.com> Date: Thu, 10 Sep 2026 14:26:31 -0300 Subject: [PATCH] fix(nvidia): fail open when a synced model catalog goes stale (#12849) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A connection's synced model catalog (populated via Import Models, or opt-in autoFetchModels/autoSync) was treated as authoritative forever once populated. lookupModelMeta (src/sse/services/model.ts) rejects any model absent from an authoritative synced catalog, and nothing ever refreshed it automatically (modelSyncScheduler only re-syncs autoSync:true connections, off by default). A NVIDIA connection synced once therefore had routing permanently pinned to that moment's catalog: live upstream models added afterwards (even ones present in the current static registry, e.g. moonshotai/kimi-k3) were rejected with 'not available in the active live catalog' indefinitely. Add a per-connection synced_models_at timestamp (provider_connections, migration 176), stamped by replaceSyncedAvailableModelsForConnection on every sync. getActiveSyncedCatalog now only treats a provider's synced catalog as authoritative while at least one active connection was synced within OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS (default 30 days); once every connection is stale — or was never synced, e.g. pre-migration rows — it fails open the same way an unsynced provider already does, instead of gating on a frozen point-in-time snapshot forever. Root cause confirmed via a TDD repro proven RED against src/lib/db/models.ts, src/lib/db/models/activeSyncedCatalog.ts and src/lib/db/providers.ts as they stood on release/v3.8.51 (all 5 new assertions failed — the DB did not even have the synced_models_at column yet), then GREEN after the fix (tests/unit/nvidia-stale-synced-catalog-12849.test.ts). The narrower reporter-blamed cause (a stale hand-maintained open-sse/config/nvidiaHostedModels.snapshot.json allowlist) was already fixed by #12538 and is not read by any runtime routing path. Refs #12849 --- .../12849-nvidia-stale-synced-catalog.md | 1 + ...6_provider_connection_synced_models_at.sql | 6 + src/lib/db/models.ts | 6 +- src/lib/db/models/activeSyncedCatalog.ts | 63 ++++++-- src/lib/db/providers.ts | 24 +++ .../nvidia-stale-synced-catalog-12849.test.ts | 147 ++++++++++++++++++ 6 files changed, 237 insertions(+), 10 deletions(-) create mode 100644 changelog.d/fixes/12849-nvidia-stale-synced-catalog.md create mode 100644 src/lib/db/migrations/176_provider_connection_synced_models_at.sql create mode 100644 tests/unit/nvidia-stale-synced-catalog-12849.test.ts diff --git a/changelog.d/fixes/12849-nvidia-stale-synced-catalog.md b/changelog.d/fixes/12849-nvidia-stale-synced-catalog.md new file mode 100644 index 0000000000..9987f0a391 --- /dev/null +++ b/changelog.d/fixes/12849-nvidia-stale-synced-catalog.md @@ -0,0 +1 @@ +- fix(nvidia): fail open when a synced model catalog goes stale instead of gating forever (#12849) diff --git a/src/lib/db/migrations/176_provider_connection_synced_models_at.sql b/src/lib/db/migrations/176_provider_connection_synced_models_at.sql new file mode 100644 index 0000000000..f6323011d5 --- /dev/null +++ b/src/lib/db/migrations/176_provider_connection_synced_models_at.sql @@ -0,0 +1,6 @@ +-- #12849: track when a connection's synced model catalog was last written so +-- getActiveSyncedCatalog can stop treating it as authoritative forever. Plain +-- TEXT column (ISO timestamp) — rowToCamel passes it through as-is; +-- NULL = never synced (pre-existing rows fail open, same as today's no-sync +-- state, rather than staying pinned to a frozen snapshot indefinitely). +ALTER TABLE provider_connections ADD COLUMN synced_models_at TEXT; diff --git a/src/lib/db/models.ts b/src/lib/db/models.ts index b716d88173..af3fd2de7c 100644 --- a/src/lib/db/models.ts +++ b/src/lib/db/models.ts @@ -8,7 +8,7 @@ import { isRetiredGitHubCopilotModelId } from "@omniroute/open-sse/config/provid import type { SqliteAdapter } from "./adapters/types"; import { getDbInstance } from "./core"; -import { getProviderConnectionsCount } from "./providers"; +import { getProviderConnectionsCount, touchConnectionSyncedModelsAt } from "./providers"; import { type JsonRecord, getKeyValue } from "./models/shared"; import { normalizeSyncedAvailableModels, @@ -615,6 +615,10 @@ export async function replaceSyncedAvailableModelsForConnection( const key = `${providerId}:${connectionId}`; const normalizedModels = normalizeSyncedAvailableModels(models, providerId); persistCanonicalSyncedAvailableModels(key, normalizedModels, normalizeSyncedAvailableModels); + // #12849: stamp the sync time on every successful sync — even a re-sync that + // returns an unchanged list proves the catalog is still current, so staleness + // gating in getActiveSyncedCatalog must not treat it as aging regardless. + if (connectionId) await touchConnectionSyncedModelsAt(connectionId); // Return the full unioned list for the provider return getSyncedAvailableModels(providerId); } diff --git a/src/lib/db/models/activeSyncedCatalog.ts b/src/lib/db/models/activeSyncedCatalog.ts index 4e68a17d2b..5bcc29ed5b 100644 --- a/src/lib/db/models/activeSyncedCatalog.ts +++ b/src/lib/db/models/activeSyncedCatalog.ts @@ -41,8 +41,30 @@ export type ProviderCatalogReconciliation = { type ProviderConnectionRef = { id: string; provider: string; + syncedModelsAt: string | null; }; +// #12849: a connection synced once and never refreshed must not pin routing to +// that point-in-time snapshot forever — a live model the provider has since +// added would be rejected as "unavailable" indefinitely. Once the synced +// catalog exceeds this age (or was never timestamped — pre-migration rows), +// getActiveSyncedCatalog stops treating it as authoritative and fails open, +// matching the existing no-sync-yet behavior. Overridable for ops/testing. +const DEFAULT_SYNCED_CATALOG_STALE_AFTER_MS = 30 * 24 * 60 * 60 * 1000; // 30 days + +function getSyncedCatalogStaleAfterMs(): number { + const raw = process.env.OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS; + const parsed = raw !== undefined ? Number(raw) : NaN; + return Number.isFinite(parsed) && parsed > 0 ? parsed : DEFAULT_SYNCED_CATALOG_STALE_AFTER_MS; +} + +function isSyncedAtFresh(syncedModelsAt: string | null): boolean { + if (!syncedModelsAt) return false; + const syncedAtMs = Date.parse(syncedModelsAt); + if (Number.isNaN(syncedAtMs)) return false; + return Date.now() - syncedAtMs <= getSyncedCatalogStaleAfterMs(); +} + function resolveStoredProviderId(aliasOrId: string): string { const normalized = aliasOrId.trim(); if (!normalized) return ""; @@ -92,6 +114,7 @@ function readConnectionRef(connection: unknown): ProviderConnectionRef | null { const record = connection as { id?: unknown; provider?: unknown; + syncedModelsAt?: unknown; }; if ( @@ -106,6 +129,7 @@ function readConnectionRef(connection: unknown): ProviderConnectionRef | null { return { id: record.id, provider: record.provider, + syncedModelsAt: typeof record.syncedModelsAt === "string" ? record.syncedModelsAt : null, }; } @@ -179,24 +203,38 @@ async function unionCustomModels( * Return the unioned synced catalog belonging only to active connections. * * A provider is authoritative only when at least one active connection has a - * non-empty usable catalog. Missing, empty, malformed, or unavailable state - * fails open to the static registry. + * non-empty usable catalog that was synced recently enough (#12849). Missing, + * empty, malformed, stale, or unavailable state fails open to the static + * registry instead of gating on a frozen point-in-time snapshot forever. */ -async function loadConnectionCatalog(storedProviderId: string): Promise { +type ConnectionCatalog = { + models: SyncedAvailableModel[]; + hasFreshConnection: boolean; +}; + +async function loadConnectionCatalog(storedProviderId: string): Promise { const [connections, modelsByConnection] = await Promise.all([ getRawProviderConnections({ provider: storedProviderId, isActive: true }, undefined, undefined, [ "id", "provider", + "synced_models_at", ]), getSyncedAvailableModelsByConnection(storedProviderId), ]); - const activeConnectionIds = connections + const activeConnections = connections .map(readConnectionRef) - .filter((connection): connection is ProviderConnectionRef => connection !== null) - .map((connection) => connection.id); + .filter((connection): connection is ProviderConnectionRef => connection !== null); - return collectModelsForConnections(modelsByConnection, activeConnectionIds); + return { + models: collectModelsForConnections( + modelsByConnection, + activeConnections.map((connection) => connection.id) + ), + hasFreshConnection: activeConnections.some((connection) => + isSyncedAtFresh(connection.syncedModelsAt) + ), + }; } export async function getActiveSyncedCatalog(providerId: string): Promise { @@ -212,11 +250,18 @@ export async function getActiveSyncedCatalog(providerId: string): Promise catalog.models)) + ) ); if (models.length > 0) { + // #12849: only gate on this catalog while at least one sibling connection + // was synced recently — otherwise a one-time historical sync would keep + // rejecting live models forever with no way to self-recover. + const hasFreshConnection = siblingCatalogs.some((catalog) => catalog.hasFreshConnection); return { - authoritative: providerUsesAuthoritativeLiveCatalog(providerId), + authoritative: providerUsesAuthoritativeLiveCatalog(providerId) && hasFreshConnection, models, }; } diff --git a/src/lib/db/providers.ts b/src/lib/db/providers.ts index 1cb3f5a73a..cb59c012a6 100644 --- a/src/lib/db/providers.ts +++ b/src/lib/db/providers.ts @@ -229,6 +229,7 @@ export const PROVIDER_CONNECTIONS_COLUMNS = new Set([ "rate_limit_overrides_json", "created_at", "updated_at", + "synced_models_at", ]); // ──────────────── Provider Connections ──────────────── @@ -1063,6 +1064,29 @@ export async function touchConnectionLastUsed( }); } +/** + * #12849: stamp when a connection's synced model catalog was last written. + * getActiveSyncedCatalog reads this to stop treating a synced catalog as + * authoritative forever — a connection synced once and never refreshed + * silently pinned routing to that point-in-time snapshot with no staleness + * check. Lightweight targeted UPDATE, mirrors touchConnectionLastUsed. + */ +export async function touchConnectionSyncedModelsAt(id: string): Promise { + if (!id) return; + const db = getDbInstance() as unknown as DbLike; + const now = new Date().toISOString(); + db.prepare( + `UPDATE provider_connections SET + synced_models_at = @syncedModelsAt, + updated_at = @updatedAt + WHERE id = @id` + ).run({ + syncedModelsAt: now, + updatedAt: now, + id, + }); +} + /** * Lightweight backoff reset — runs a targeted UPDATE without SELECT or re-encrypt. * Follows the `clearConnectionErrorIfUnchanged` pattern but without the CAS check, diff --git a/tests/unit/nvidia-stale-synced-catalog-12849.test.ts b/tests/unit/nvidia-stale-synced-catalog-12849.test.ts new file mode 100644 index 0000000000..008d2c3efc --- /dev/null +++ b/tests/unit/nvidia-stale-synced-catalog-12849.test.ts @@ -0,0 +1,147 @@ +/** + * #12849: NVIDIA (and every other authoritative-live-catalog provider) treated a + * connection's *synced* model catalog as authoritative forever once populated — + * no staleness check, no default periodic refresh. A model that is live upstream + * and present in the current static registry was rejected as "not available in + * the active live catalog" indefinitely once any historical sync existed. + * + * getActiveSyncedCatalog now fails open once a connection's synced catalog + * exceeds a staleness threshold (default 30 days; overridable via + * OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS), instead of gating on a frozen + * point-in-time snapshot forever. + */ +import test from "node:test"; +import assert from "node:assert/strict"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +const TEST_DATA_DIR = fs.mkdtempSync(path.join(os.tmpdir(), "omniroute-nvidia-stale-12849-")); + +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "nvidia-stale-12849-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const { replaceSyncedAvailableModelsForConnection } = await import("../../src/lib/db/models.ts"); +const { getModelInfo } = await import("../../src/sse/services/model.ts"); +const { nvidiaProvider } = await import( + "../../open-sse/config/providers/registry/nvidia/index.ts" +); + +const PROVIDER = "nvidia"; +const CONNECTION_ID = "nvidia-stale-catalog-12849"; +// Live upstream + present in the current static registry (asserted below), but +// deliberately absent from the small "historical sync" catalog seeded here. +const LIVE_MODEL = "moonshotai/kimi-k3"; +const STALE_SYNC_ONLY_MODEL = "some-retired-model-that-no-longer-exists"; + +function connectionRow(): { syncedModelsAt: string | null } { + const db = core.getDbInstance(); + const row = db + .prepare("SELECT synced_models_at AS syncedModelsAt FROM provider_connections WHERE id = ?") + .get(CONNECTION_ID) as { syncedModelsAt: string | null } | undefined; + if (!row) throw new Error(`connection ${CONNECTION_ID} not found`); + return row; +} + +function ageConnectionSync(daysAgo: number): void { + const db = core.getDbInstance(); + const agedTimestamp = new Date(Date.now() - daysAgo * 24 * 60 * 60 * 1000).toISOString(); + db.prepare("UPDATE provider_connections SET synced_models_at = ? WHERE id = ?").run( + agedTimestamp, + CONNECTION_ID + ); +} + +async function seedHistoricalSync(): Promise { + const db = core.getDbInstance(); + const now = new Date().toISOString(); + db.prepare( + `INSERT OR REPLACE INTO provider_connections (id, provider, is_active, created_at, updated_at) + VALUES (?, ?, 1, ?, ?)` + ).run(CONNECTION_ID, PROVIDER, now, now); + + await replaceSyncedAvailableModelsForConnection(PROVIDER, CONNECTION_ID, [ + { id: STALE_SYNC_ONLY_MODEL, name: STALE_SYNC_ONLY_MODEL, source: "imported" }, + ]); +} + +test.beforeEach(async () => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + + assert.ok( + nvidiaProvider.models.some((model) => model.id === LIVE_MODEL), + `precondition: ${LIVE_MODEL} must exist in the current NVIDIA static registry` + ); + + await seedHistoricalSync(); +}); + +test.after(() => { + core.resetDbInstance(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true, maxRetries: 5, retryDelay: 100 }); +}); + +test("#12849: a fresh synced catalog still gates — a model missing from it is rejected", async () => { + // Sanity: touchConnectionSyncedModelsAt stamped this sync as fresh already. + const { syncedModelsAt } = connectionRow(); + assert.ok(syncedModelsAt, "replaceSyncedAvailableModelsForConnection must stamp synced_models_at"); + + const resolved = await getModelInfo(`${PROVIDER}/${LIVE_MODEL}`); + + assert.equal(resolved.provider, null); + assert.equal(resolved.errorType, "model_not_found"); + assert.match(resolved.errorMessage, /active live catalog/i); +}); + +test("#12849: a stale synced catalog fails open — a live+registry model is no longer rejected", async () => { + ageConnectionSync(45); // past the 30-day default staleness threshold + + const resolved = await getModelInfo(`${PROVIDER}/${LIVE_MODEL}`); + + assert.equal( + resolved.provider, + PROVIDER, + `expected the stale catalog to fail open, got errorMessage=${resolved.errorMessage}` + ); + assert.equal(resolved.model, LIVE_MODEL); +}); + +test("#12849: a stale synced catalog is treated as non-authoritative in getActiveSyncedCatalog", async () => { + const { getActiveSyncedCatalog } = await import("../../src/lib/db/models/activeSyncedCatalog.ts"); + + ageConnectionSync(45); + + const catalog = await getActiveSyncedCatalog(PROVIDER); + + assert.equal(catalog.authoritative, false); +}); + +test("#12849: a connection never synced (no timestamp) is non-authoritative, not gated forever", async () => { + const db = core.getDbInstance(); + db.prepare("UPDATE provider_connections SET synced_models_at = NULL WHERE id = ?").run( + CONNECTION_ID + ); + + const resolved = await getModelInfo(`${PROVIDER}/${LIVE_MODEL}`); + + assert.equal(resolved.provider, PROVIDER); + assert.equal(resolved.model, LIVE_MODEL); +}); + +test("#12849: OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS overrides the default threshold", async () => { + const previous = process.env.OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS; + process.env.OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS = String(60 * 60 * 1000); // 1 hour + try { + ageConnectionSync(1); // 1 day old — stale under the 1-hour override, fresh under the 30-day default + + const resolved = await getModelInfo(`${PROVIDER}/${LIVE_MODEL}`); + + assert.equal(resolved.provider, PROVIDER); + } finally { + if (previous === undefined) delete process.env.OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS; + else process.env.OMNIROUTE_SYNCED_CATALOG_STALE_AFTER_MS = previous; + } +});