From d776a42324598bb058db4f3bc0bec5dbb132ed62 Mon Sep 17 00:00:00 2001 From: Diego Rodrigues de Sa e Souza Date: Tue, 7 Jul 2026 07:41:20 -0300 Subject: [PATCH] fix(api): invalidate /v1/models + specialty catalogs on DB writes; fix duplicated headers and 500 replayed as 200 (#6408) The #6408 request-shape-keyed TTL cache around getUnifiedModelsResponse was not keyed by DB/settings state, so a write followed by a read within the ~1.5s TTL replayed the pre-write catalog (dropping newly-eligible models like codex/gpt-5.5 and every specialty catalog routed through it since #6303). Fold a modelCatalogCacheVersion (bumped by invalidateDbCache, already called on every settings/connection/combo/pricing write) into the cache so a state change forces an immediate miss; merge response headers through a real Headers instance (fixes X-Request-Id duplication); carry and replay status end-to-end (a mid-build 500 was replayed as 200). Tests call the existing __resetCatalogBuilderRunsForTest hook in setup, matching v1-models-concurrent-6408. --- src/app/api/v1/models/catalog.ts | 66 +++++++++++++++++-- src/lib/db/readCache.ts | 30 ++++++++- tests/unit/image-generation-route.test.ts | 7 ++ tests/unit/models-catalog-route.test.ts | 5 ++ .../specialty-model-catalog-routes.test.ts | 8 +++ 5 files changed, 108 insertions(+), 8 deletions(-) diff --git a/src/app/api/v1/models/catalog.ts b/src/app/api/v1/models/catalog.ts index c45109def8..ab6fe78b79 100644 --- a/src/app/api/v1/models/catalog.ts +++ b/src/app/api/v1/models/catalog.ts @@ -28,6 +28,7 @@ import { createBuiltinAutoCombo, } from "@omniroute/open-sse/services/autoCombo/builtinCatalog"; import { getAllSyncedAvailableModels, type SyncedAvailableModel } from "@/lib/db/models"; +import { getModelCatalogCacheVersion } from "@/lib/db/readCache"; import { getCompatibleFallbackModels } from "@/lib/providers/managedAvailableModels"; import { getOpenRouterCatalog } from "@/lib/catalog/openrouterCatalog"; import { hasEligibleConnectionForModel } from "@/domain/connectionModelRules"; @@ -94,7 +95,12 @@ export { getCustomVisionCapabilityFields }; // then memoize the serialized body for a short window so a burst (SDK startup, // multi-tab dashboard poll) returns from cache. Auth-rejection paths are NOT // cached (they depend on live session state — dashboard cookies, API key). -type CachedCatalog = { body: string; headers: Record; expiresAt: number }; +type CachedCatalog = { + body: string; + headers: Record; + status: number; + expiresAt: number; +}; const CATALOG_CACHE_TTL_MS = 1500; // ~one request-latency window; safe vs SDK bursts const catalogCache = new Map(); const catalogInFlight = new Map>(); @@ -107,6 +113,7 @@ export function __resetCatalogBuilderRunsForTest(): void { _catalogBuilderRuns = 0; catalogCache.clear(); catalogInFlight.clear(); + lastSeenCatalogCacheVersion = getModelCatalogCacheVersion(); } export function __getCatalogBuilderRunsForTest(): number { return _catalogBuilderRuns; @@ -120,6 +127,45 @@ function buildCatalogCacheKey(request: Request): string { return `${prefix}|${isCodex}|${apiKey}`; } +// Tracks the model-catalog cache version (src/lib/db/readCache.ts) as of the last +// cache access. invalidateDbCache() bumps that version on every settings/connections/ +// combos/pricing write; when it moves on, every memoized entry here was built from +// state that no longer holds, so drop them all rather than keying by version (which +// would leak one Map entry per version forever instead of ever pruning old ones). +let lastSeenCatalogCacheVersion = getModelCatalogCacheVersion(); +function dropCatalogCacheIfStateChanged(): void { + const currentVersion = getModelCatalogCacheVersion(); + if (currentVersion === lastSeenCatalogCacheVersion) return; + lastSeenCatalogCacheVersion = currentVersion; + catalogCache.clear(); + // Deliberately NOT clearing catalogInFlight: an in-flight build already reads live + // DB/settings state as of when it started, so letting it finish and populate the + // (now-current) cache entry is correct — clearing it would just force a redundant + // second builder run for requests that arrive mid-flight. +} + +// Header sources here mix Title-Case keys (diagnosticHeaders, corsHeaders — plain +// objects built by app code) with lower-case keys (payload/cached.headers — captured +// via the Fetch `Headers` iterator, which always yields lower-cased names). Merging +// those with a plain object spread leaves both casings present as distinct object +// keys; the `Response` constructor then treats them as the same case-insensitive +// header and *appends* rather than overwrites, producing a comma-joined duplicate +// (e.g. request-id echoing "foo, foo"). Merge through a real `Headers` instance +// instead so `.set()` overwrites case-insensitively. Sources listed earlier are the +// base (cached/freshly-built payload headers); `diagnosticHeaders` is applied last so +// per-request fields (e.g. X-Request-Id) always reflect the *current* request rather +// than whichever request happened to populate the cache entry. +function mergeCatalogHeaders(...sources: Array | undefined>): Headers { + const merged = new Headers(); + for (const source of sources) { + if (!source) continue; + for (const [key, value] of Object.entries(source)) { + merged.set(key, value); + } + } + return merged; +} + /** * Build unified OpenAI-compatible model catalog response. * Reused by `/api/v1/models` and `/api/v1` to avoid semantic drift (T09). @@ -147,11 +193,13 @@ export async function getUnifiedModelsResponse( // Fall through to full builder on auth-check failure; core handles errors. } + dropCatalogCacheIfStateChanged(); const cacheKey = buildCatalogCacheKey(request); const cached = catalogCache.get(cacheKey); if (cached && cached.expiresAt > Date.now()) { return new Response(cached.body, { - headers: { ...corsHeaders, ...diagnosticHeaders, ...cached.headers }, + status: cached.status, + headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders), }); } @@ -161,6 +209,7 @@ export async function getUnifiedModelsResponse( catalogCache.set(cacheKey, { body: payload.body, headers: payload.headers, + status: payload.status, expiresAt: Date.now() + CATALOG_CACHE_TTL_MS, }); return payload; @@ -174,7 +223,8 @@ export async function getUnifiedModelsResponse( try { const payload = await inflight; return new Response(payload.body, { - headers: { ...corsHeaders, ...diagnosticHeaders, ...payload.headers }, + status: payload.status, + headers: mergeCatalogHeaders(corsHeaders, payload.headers, diagnosticHeaders), }); } catch (err) { return Response.json( @@ -192,7 +242,7 @@ export async function getUnifiedModelsResponse( async function buildCatalogPayload( request: Request -): Promise<{ body: string; headers: Record }> { +): Promise<{ body: string; headers: Record; status: number }> { _catalogBuilderRuns++; const built = await buildUnifiedModelsResponseCore(request); const body = await built.text(); @@ -200,7 +250,13 @@ async function buildCatalogPayload( built.headers.forEach((value, key) => { headers[key] = value; }); - return { body, headers }; + // buildUnifiedModelsResponseCore() itself returns a real error Response (status 500) + // when the builder crashes (e.g. a DB read throws) instead of throwing — status must + // be captured and replayed through the cache/coalescing wrapper above, otherwise the + // caller-facing Response (built with a fresh `new Response(...)`, defaulting to 200) + // silently downgrades a genuine server error into an HTTP 200 with an `error`-shaped + // JSON body. + return { body, headers, status: built.status }; } /** diff --git a/src/lib/db/readCache.ts b/src/lib/db/readCache.ts index 7c8b75521d..8ce10f1460 100644 --- a/src/lib/db/readCache.ts +++ b/src/lib/db/readCache.ts @@ -164,15 +164,39 @@ export function getCombosCacheVersion(): number { return combosCacheVersion; } +// ──────────────── Model Catalog Cache Invalidation Signal ──────────────── +// +// #6408 added a request-shape-keyed (prefix/isCodex/apiKey) TTL cache around the +// unified /v1/models builder (src/app/api/v1/models/catalog.ts) to coalesce +// concurrent/bursty GETs. That cache key does not vary with the underlying DB +// state the builder reads (connections, settings, combos), so a write followed by +// a read within the ~1.5s TTL replayed the pre-write response. Same import-cycle +// constraint as combosCacheVersion above (a db module must not import the route +// module) — catalog.ts instead compares this version on every access and drops its +// whole cache the moment it moves, so any write that calls invalidateDbCache() makes +// the next read miss immediately instead of waiting out the TTL. +let modelCatalogCacheVersion = 0; + +/** + * Current model-catalog-cache version. `getUnifiedModelsResponse()` folds this + * into its response cache key; a change means settings/connections/combos were + * written since the cache was populated and the cached body is stale. + */ +export function getModelCatalogCacheVersion(): number { + return modelCatalogCacheVersion; +} + /** * Invalidate all caches (call after writes to any of: settings, pricing, * connections, combos). */ -export function invalidateDbCache( - scope?: "settings" | "pricing" | "connections" | "combos" -): void { +export function invalidateDbCache(scope?: "settings" | "pricing" | "connections" | "combos"): void { if (!scope || scope === "settings") settingsCache.invalidate(); if (!scope || scope === "pricing") pricingCache.invalidate(); if (!scope || scope === "connections") connectionsCache.invalidate(); if (!scope || scope === "combos") combosCacheVersion++; + // Settings/connections/combos all feed the unified model catalog builder + // (blockedProviders + hidePaidModels, provider connections + excludedModels, + // combo definitions, respectively) — pricing does too, via isFreeModel(). + modelCatalogCacheVersion++; } diff --git a/tests/unit/image-generation-route.test.ts b/tests/unit/image-generation-route.test.ts index 05b1a48697..b00c159255 100644 --- a/tests/unit/image-generation-route.test.ts +++ b/tests/unit/image-generation-route.test.ts @@ -14,6 +14,7 @@ const apiKeysDb = await import("../../src/lib/db/apiKeys.ts"); const settingsDb = await import("../../src/lib/db/settings.ts"); const imageRoute = await import("../../src/app/api/v1/images/generations/route.ts"); const imageEditRoute = await import("../../src/app/api/v1/images/edits/route.ts"); +const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts"); const originalFetch = globalThis.fetch; @@ -23,6 +24,12 @@ async function resetStorage() { core.resetDbInstance(); fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + // #6303 moved this route onto the shared unified catalog (getUnifiedModelsResponse), + // which #6408 wrapped in a 1.5s TTL response cache keyed only by (prefix, isCodex + // client, apiKey) — NOT by DB state. Without clearing it between test cases, a test + // running within the TTL window of a previous one gets served the previous test's + // stale serialized catalog instead of a fresh build reflecting this test's DB state. + v1ModelsCatalog.__resetCatalogBuilderRunsForTest(); } async function seedConnection(provider: string, overrides: { apiKey?: string | null } = {}) { diff --git a/tests/unit/models-catalog-route.test.ts b/tests/unit/models-catalog-route.test.ts index c6d8a1c9af..0a7b1f897e 100644 --- a/tests/unit/models-catalog-route.test.ts +++ b/tests/unit/models-catalog-route.test.ts @@ -23,6 +23,11 @@ async function resetStorage() { apiKeysDb.resetApiKeyState(); fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + // #6408 added a 1.5s TTL response cache to getUnifiedModelsResponse keyed only by + // (prefix, isCodex client, apiKey) — NOT by DB/settings state. Without clearing it + // between test cases, a test running within the TTL window of a previous one gets + // served the previous test's stale serialized catalog instead of a fresh build. + v1ModelsCatalog.__resetCatalogBuilderRunsForTest(); } async function seedConnection(provider: string, overrides: Record = {}) { diff --git a/tests/unit/specialty-model-catalog-routes.test.ts b/tests/unit/specialty-model-catalog-routes.test.ts index fc415c3e05..790592ab20 100644 --- a/tests/unit/specialty-model-catalog-routes.test.ts +++ b/tests/unit/specialty-model-catalog-routes.test.ts @@ -14,11 +14,19 @@ const imageRoute = await import("../../src/app/api/v1/images/generations/route.t const embeddingsRoute = await import("../../src/app/api/v1/embeddings/route.ts"); const videoRoute = await import("../../src/app/api/v1/videos/generations/route.ts"); const musicRoute = await import("../../src/app/api/v1/music/generations/route.ts"); +const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts"); async function resetStorage() { core.resetDbInstance(); fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + // These routes all derive from the shared unified catalog (getUnifiedModelsResponse), + // which #6408 wrapped in a 1.5s TTL response cache keyed only by (prefix, isCodex + // client, apiKey) — NOT by DB state. Every test in this file hits that same cache key, + // so without clearing it between test cases a test running within the TTL window of + // a previous one gets served the previous test's stale catalog instead of a fresh + // build reflecting this test's own seeded connections. + v1ModelsCatalog.__resetCatalogBuilderRunsForTest(); } async function seedConnection(provider: string) {