diff --git a/src/app/api/v1/models/catalog.ts b/src/app/api/v1/models/catalog.ts index af0192a998..eac1dbdf46 100644 --- a/src/app/api/v1/models/catalog.ts +++ b/src/app/api/v1/models/catalog.ts @@ -95,6 +95,7 @@ import { import { getModelCatalogAuthRejection, isCodexModelCatalogClient } from "./catalogRequest"; import { isFreeModel, providerHasFreeModels } from "@/shared/utils/freeModels"; import { isCodexDiscoveryModelExcluded } from "@/shared/services/codexDiscoveryPolicy"; +import { buildErrorBody } from "@omniroute/open-sse/utils/error"; // Public API of this module is preserved after the catalog helper extraction: // `isVisionModelId` (vision-detection-consistency.test.ts) and @@ -103,87 +104,22 @@ import { isCodexDiscoveryModelExcluded } from "@/shared/services/codexDiscoveryP export { isVisionModelId } from "@/shared/constants/visionModels"; export { getCustomVisionCapabilityFields }; -// #6408 — Concurrent GET /v1/models requests serialized (~1.2s each × N). The -// per-request builder walks 8 registries + hits SQLite for connections, combos, -// custom models, and aliases; under Next.js single-threaded App Router request -// handling, N concurrent calls execute back-to-back and the Nth completes -// N × single-request latency (linear staircase reproduced in the issue). -// -// Fix: coalesce identical concurrent requests onto a single in-flight promise, -// 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; - status: number; - expiresAt: number; -}; -const CATALOG_CACHE_TTL_MS_DEFAULT = 1500; // fallback; overridden by settings -const catalogCache = new Map(); -const catalogInFlight = new Map>(); +// The response cache (coalescing, short-TTL memoization and stale-while-revalidate) +// lives in ./catalogCache. Re-exported here because the existing tests import the +// hooks from this module, and CATALOG_STALE_WHILE_REVALIDATE_MS is part of the +// documented behavior of this endpoint. +import { CATALOG_CACHE_TTL_MS_DEFAULT, resolveCachedCatalogResponse } from "./catalogCache"; -// Test hook — increments each time the full catalog builder runs. Used by -// tests/unit/v1-models-concurrent-6408.test.ts to prove concurrent requests -// share one execution. Not part of the public API; do not read from app code. -let _catalogBuilderRuns = 0; -export function __resetCatalogBuilderRunsForTest(): void { - _catalogBuilderRuns = 0; - catalogCache.clear(); - catalogInFlight.clear(); - lastSeenCatalogCacheVersion = getModelCatalogCacheVersion(); -} -export function __getCatalogBuilderRunsForTest(): number { - return _catalogBuilderRuns; -} - -function buildCatalogCacheKey(request: Request): string { - const url = new URL(request.url); - const prefix = url.searchParams.get("prefix") || ""; - const apiKey = extractApiKey(request) || ""; - const isCodex = isCodexModelCatalogClient(request) ? "1" : "0"; - const configuredOnly = url.searchParams.get("configuredOnly") === "true" ? "1" : "0"; - return `${prefix}|${isCodex}|${apiKey}|${configuredOnly}`; -} - -// 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; -} +export { + CATALOG_STALE_WHILE_REVALIDATE_MS, + __resetCatalogBuilderRunsForTest, + __getCatalogBuilderRunsForTest, + __expireCatalogCacheForTest, + __setCatalogCacheEntryForTest, + __flushCatalogBackgroundRefreshForTest, + __forceCatalogInFlightRejectionForTest, +} from "./catalogCache"; +export type { CachedCatalog } from "./catalogCache"; /** * Build unified OpenAI-compatible model catalog response. @@ -212,47 +148,22 @@ 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, { - status: cached.status, - headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders), - }); - } - let inflight = catalogInFlight.get(cacheKey); - if (!inflight) { - inflight = buildCatalogPayload(request).then((payload) => { - catalogCache.set(cacheKey, { - body: payload.body, - headers: payload.headers, - status: payload.status, - expiresAt: Date.now() + payload.cacheTTL, - }); - return payload; - }); - catalogInFlight.set(cacheKey, inflight); - inflight.finally(() => { - if (catalogInFlight.get(cacheKey) === inflight) catalogInFlight.delete(cacheKey); - }); - } - try { - const payload = await inflight; - return new Response(payload.body, { - status: payload.status, - headers: mergeCatalogHeaders(corsHeaders, payload.headers, diagnosticHeaders), - }); + return await resolveCachedCatalogResponse( + request, + { corsHeaders, diagnosticHeaders }, + buildCatalogPayload + ); } catch (err) { + // Hard rule #12: never put a raw err.message/err.stack in a response body. + // Route it through the shared sanitizer instead — same status/type/code as + // before, minus the stack-trace/path leak. + const message = err instanceof Error ? err.message : String(err); return Response.json( - { - error: { - message: err instanceof Error ? err.message : String(err), - type: "server_error", - code: INTERNAL_PROXY_ERROR, - }, - }, + buildErrorBody(500, message, undefined, { + type: "server_error", + code: INTERNAL_PROXY_ERROR, + }), { status: 500, headers: { ...corsHeaders, ...diagnosticHeaders } } ); } @@ -261,7 +172,6 @@ export async function getUnifiedModelsResponse( async function buildCatalogPayload( request: Request ): Promise<{ body: string; headers: Record; status: number; cacheTTL: number }> { - _catalogBuilderRuns++; const built = await buildUnifiedModelsResponseCore(request); const body = await built.text(); const headers: Record = {}; @@ -1583,14 +1493,14 @@ async function buildUnifiedModelsResponseCore( }); } catch (error) { console.log("Error fetching models:", error); + // Hard rule #12 — this is the realistically reachable 500 for the endpoint + // (the wrapper's catch only fires on an in-flight rejection), so it must go + // through the shared sanitizer too. Same status/type/code as before. return Response.json( - { - error: { - message: error instanceof Error ? error.message : String(error), - type: "server_error", - code: INTERNAL_PROXY_ERROR, - }, - }, + buildErrorBody(500, error instanceof Error ? error.message : String(error), undefined, { + type: "server_error", + code: INTERNAL_PROXY_ERROR, + }), { status: 500, headers: { diff --git a/src/app/api/v1/models/catalogCache.ts b/src/app/api/v1/models/catalogCache.ts new file mode 100644 index 0000000000..7ae78f1806 --- /dev/null +++ b/src/app/api/v1/models/catalogCache.ts @@ -0,0 +1,284 @@ +/** + * Response cache for `GET /v1/models`, extracted from catalog.ts. + * + * #6408 — concurrent catalog requests used to serialize (~1.2 s each × N). The + * builder walks 8 registries and hits SQLite for connections, combos, custom + * models and aliases; under Next.js's single-threaded App Router request + * handling, N concurrent calls execute back-to-back and the Nth completes at + * N × single-request latency. So identical concurrent requests are coalesced + * onto one in-flight promise and the serialized body is memoized for a short + * window. + * + * Auth rejection is NOT handled here and must stay in the caller: it depends on + * live per-request state (dashboard cookie, API key) and must never be cached. + */ +import { getModelCatalogCacheVersion } from "@/lib/db/readCache"; +import { extractApiKey } from "@/sse/services/auth"; + +import { isCodexModelCatalogClient } from "./catalogRequest"; + +export type CachedCatalog = { + body: string; + headers: Record; + status: number; + expiresAt: number; +}; + +/** Payload shape returned by the builder the caller injects. */ +export type CatalogPayload = { + body: string; + headers: Record; + status: number; + cacheTTL: number; +}; + +/** + * A client with a short discovery timeout (Claude Code allows 3 s) must never + * wait on a full rebuild. Once a cached 200 expires it is still served + * immediately for up to this long while a background refresh repopulates it. + * Bounded so a refresh that keeps failing cannot pin an old catalog forever — + * past this window callers fall back to waiting, same as a cold cache. + */ +export const CATALOG_STALE_WHILE_REVALIDATE_MS = 30_000; + +/** Fallback memoization window; overridden by `settings.cache.modelCatalogCacheTtlMs`. */ +export const CATALOG_CACHE_TTL_MS_DEFAULT = 1500; + +const catalogCache = new Map(); +const catalogInFlight = new Map>(); + +let _catalogBuilderRuns = 0; + +function buildCatalogCacheKey(request: Request): string { + const url = new URL(request.url); + const prefix = url.searchParams.get("prefix") || ""; + const apiKey = extractApiKey(request) || ""; + const isCodex = isCodexModelCatalogClient(request) ? "1" : "0"; + const configuredOnly = url.searchParams.get("configuredOnly") === "true" ? "1" : "0"; + return `${prefix}|${isCodex}|${apiKey}|${configuredOnly}`; +} + +// 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 mix Title-Case keys (diagnostic/cors headers built by app code) with +// lower-case ones (payload headers captured via the Fetch `Headers` iterator). A plain +// object spread keeps both casings as distinct keys, and the `Response` constructor +// then *appends* rather than overwrites them, producing comma-joined duplicates (e.g. +// request-id echoing "foo, foo"). Merge through a real `Headers` so `.set()` overwrites +// case-insensitively. Earlier sources are the base; the caller passes diagnostics last +// so per-request fields reflect the current request, not whichever one filled the cache. +export 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; +} + +function storePayload(cacheKey: string, payload: CatalogPayload): CachedCatalog { + const entry: CachedCatalog = { + body: payload.body, + headers: payload.headers, + status: payload.status, + expiresAt: Date.now() + payload.cacheTTL, + }; + catalogCache.set(cacheKey, entry); + return entry; +} + +/** + * Kick off a background rebuild so an expired-but-stale-eligible entry can be + * refreshed without the current request waiting on it. Reuses catalogInFlight — + * no second coalescing mechanism — so a concurrent cold/stale request for the + * same key joins this refresh instead of starting another. + * + * The builder runs one macrotask later so the stale response that triggered this + * call is handed back before the builder's synchronous prologue runs; the whole + * point of this path is that the caller does not pay for the rebuild. + * + * The tracked promise **rejects** on failure. catalogInFlight is shared with the + * cold path: a caller whose entry aged past the stale window skips the stale + * branch and awaits whatever promise it finds here, and resolving with the stale + * entry would hand it a body it was no longer entitled to while disguising a + * build failure as a 200. The rejection is pre-handled so this path can never + * raise an unhandledRejection; a failed refresh simply never overwrites the entry. + */ +function scheduleBackgroundRefresh( + cacheKey: string, + request: Request, + buildPayload: (request: Request) => Promise +): void { + if (catalogInFlight.has(cacheKey)) return; // a refresh for this key is already running + + const refreshPromise: Promise = new Promise((resolve, reject) => { + setTimeout(() => { + runBuilder(buildPayload, request) + .then((payload) => resolve(storePayload(cacheKey, payload))) + .catch((err) => { + console.error( + `[catalog] Background stale-while-revalidate refresh failed for key "${cacheKey}":`, + err + ); + reject(err); + }); + }, 0); + }); + + // Nobody on the stale path awaits this, so pre-handle the rejection; a cold-path + // caller that joins it via catalogInFlight attaches its own handler and still + // observes the failure. + refreshPromise.catch(() => {}); + + catalogInFlight.set(cacheKey, refreshPromise); + refreshPromise + .catch(() => {}) + .finally(() => { + if (catalogInFlight.get(cacheKey) === refreshPromise) catalogInFlight.delete(cacheKey); + }); +} + +function runBuilder( + buildPayload: (request: Request) => Promise, + request: Request +): Promise { + _catalogBuilderRuns++; + return buildPayload(request); +} + +/** + * Resolve the cached catalog response for `request`, building it through + * `buildPayload` when there is nothing fresh to serve. + * + * Returns `null` when the caller must build and handle errors itself — i.e. the + * in-flight build rejected — so the error-response shape stays in the caller. + */ +export async function resolveCachedCatalogResponse( + request: Request, + headerSources: { corsHeaders: Record; diagnosticHeaders: Record }, + buildPayload: (request: Request) => Promise +): Promise { + const { corsHeaders, diagnosticHeaders } = headerSources; + dropCatalogCacheIfStateChanged(); + + const cacheKey = buildCatalogCacheKey(request); + const now = Date.now(); + const cached = catalogCache.get(cacheKey); + + if (cached && cached.expiresAt > now) { + return new Response(cached.body, { + status: cached.status, + headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders), + }); + } + + // Stale-while-revalidate: an expired entry is still served immediately as long as + // (a) it was a successful build — a cached error replayed as "stale" would mask an + // intermittent failure behind a fake success forever — and (b) it is within the + // staleness window, so a refresh that keeps failing eventually falls through to the + // cold-path wait instead of pinning ancient data. + if ( + cached && + cached.status === 200 && + now - cached.expiresAt <= CATALOG_STALE_WHILE_REVALIDATE_MS + ) { + scheduleBackgroundRefresh(cacheKey, request, buildPayload); + return new Response(cached.body, { + status: cached.status, + headers: mergeCatalogHeaders(corsHeaders, cached.headers, diagnosticHeaders), + }); + } + + let inflight = catalogInFlight.get(cacheKey); + if (!inflight) { + inflight = runBuilder(buildPayload, request).then((payload) => storePayload(cacheKey, payload)); + catalogInFlight.set(cacheKey, inflight); + inflight.finally(() => { + if (catalogInFlight.get(cacheKey) === inflight) catalogInFlight.delete(cacheKey); + }); + } + + const payload = await inflight; + return new Response(payload.body, { + status: payload.status, + headers: mergeCatalogHeaders(corsHeaders, payload.headers, diagnosticHeaders), + }); +} + +// ── Test hooks ─────────────────────────────────────────────────────────────── +// Not part of the public API; do not read from app code. + +/** Resets the builder counter and every cached/in-flight entry. */ +export function __resetCatalogBuilderRunsForTest(): void { + _catalogBuilderRuns = 0; + catalogCache.clear(); + catalogInFlight.clear(); + lastSeenCatalogCacheVersion = getModelCatalogCacheVersion(); +} + +/** Counts full builder executions — proves concurrent requests share one run (#6408). */ +export function __getCatalogBuilderRunsForTest(): number { + return _catalogBuilderRuns; +} + +/** + * Marks every cached entry as expired `msAgo` milliseconds ago instead of sleeping + * out the real TTL. Pass more than CATALOG_STALE_WHILE_REVALIDATE_MS to simulate an + * entry that has aged past the stale-serving window. + */ +export function __expireCatalogCacheForTest(msAgo = 1): void { + const expiresAt = Date.now() - msAgo; + for (const [key, entry] of catalogCache.entries()) { + catalogCache.set(key, { ...entry, expiresAt }); + } +} + +/** + * Seeds the entry a given request would read, for status/staleness combinations the + * intentionally exception-resistant builder cannot be made to produce (e.g. a cached + * non-200). Takes the Request so the cache-key format stays private to this module. + */ +export function __setCatalogCacheEntryForTest(request: Request, entry: CachedCatalog): void { + catalogCache.set(buildCatalogCacheKey(request), entry); +} + +/** Awaits any background refresh in flight, instead of guessing at a real-time sleep. */ +export async function __flushCatalogBackgroundRefreshForTest(): Promise { + await Promise.all([...catalogInFlight.values()].map((p) => p.catch(() => {}))); +} + +/** + * Injects a synthetic in-flight rejection so the caller's catch branch (sanitized + * error body) can be exercised deterministically — the builder core try/catches every + * registry and DB read individually, so it is not a practical error-injection point. + * + * Deliberately does not self-clean the way production entries do: this promise is + * already rejected at creation, so a cleanup callback would delete the map entry + * within a microtask or two — before the caller's several-await auth check finishes — + * silently swapping in a fresh cold build instead of the intended failure. The next + * __resetCatalogBuilderRunsForTest() clears it. + */ +export function __forceCatalogInFlightRejectionForTest(request: Request, error: unknown): void { + const rejected: Promise = Promise.reject(error); + rejected.catch(() => {}); // mark as handled — avoids an unhandledRejection warning + catalogInFlight.set(buildCatalogCacheKey(request), rejected); +} diff --git a/tests/unit/v1-models-discovery-conformance.test.ts b/tests/unit/v1-models-discovery-conformance.test.ts new file mode 100644 index 0000000000..e1ab778003 --- /dev/null +++ b/tests/unit/v1-models-discovery-conformance.test.ts @@ -0,0 +1,206 @@ +// Task D1 — GET /v1/models conformance for Claude Code's gateway model discovery. +// +// With CLAUDE_CODE_ENABLE_GATEWAY_MODEL_DISCOVERY=1, Claude Code issues +// `GET /v1/models?limit=1000` with a 3s timeout and `redirect: "fail"`, reading +// only `id` + `display_name` from each entry. Three failure modes empty the +// picker with no error surfaced to the user: the endpoint taking longer than +// 3s, redirecting, or returning an entry without a string `id`. This suite +// covers all three, plus the stale-while-revalidate cache behavior that makes +// the timeout fix possible and the error-sanitization fix for the builder's +// catch block (hard rule #12). +// +// Harness modeled on tests/unit/v1-models-concurrent-6408.test.ts: temp +// DATA_DIR, core.resetDbInstance()/apiKeysDb.resetApiKeyState() cleanup so the +// native test runner does not hang on an open SQLite handle. + +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-d1-discovery-")); +process.env.DATA_DIR = TEST_DATA_DIR; +process.env.API_KEY_SECRET = process.env.API_KEY_SECRET || "catalog-test-secret"; + +const core = await import("../../src/lib/db/core.ts"); +const apiKeysDb = await import("../../src/lib/db/apiKeys.ts"); +const readCache = await import("../../src/lib/db/readCache.ts"); +const v1ModelsCatalog = await import("../../src/app/api/v1/models/catalog.ts"); + +async function resetStorage() { + core.resetDbInstance(); + apiKeysDb.resetApiKeyState(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); + fs.mkdirSync(TEST_DATA_DIR, { recursive: true }); + v1ModelsCatalog.__resetCatalogBuilderRunsForTest(); +} + +test.beforeEach(async () => { + await resetStorage(); +}); + +test.after(async () => { + core.resetDbInstance(); + apiKeysDb.resetApiKeyState(); + fs.rmSync(TEST_DATA_DIR, { recursive: true, force: true }); +}); + +test("1. GET /v1/models never returns a 3xx redirect status (regression guard)", async () => { + const res = await v1ModelsCatalog.getUnifiedModelsResponse( + new Request("http://localhost/v1/models") + ); + assert.ok( + res.status < 300 || res.status >= 400, + `expected a non-redirect status, got ${res.status}` + ); +}); + +test("2. every catalog entry has a non-empty string id, and display_name (if present) is a string", async () => { + const res = await v1ModelsCatalog.getUnifiedModelsResponse( + new Request("http://localhost/v1/models") + ); + assert.equal(res.status, 200); + const body = await res.json(); + assert.ok(Array.isArray(body.data), "response must carry a data array"); + assert.ok(body.data.length > 0, "catalog must not be empty"); + for (const entry of body.data) { + assert.equal(typeof entry.id, "string", `entry.id must be a string, got ${typeof entry.id}`); + assert.ok(entry.id.length > 0, "entry.id must not be an empty string"); + if (Object.prototype.hasOwnProperty.call(entry, "display_name")) { + assert.equal( + typeof entry.display_name, + "string", + "display_name, when present, must be a string" + ); + } + } +}); + +test("3. stale-first: an expired 200 entry within the staleness window is served immediately, then refreshed in the background", async () => { + const makeRequest = () => new Request("http://localhost/v1/models"); + + const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res1.status, 200); + const body1 = await res1.text(); + const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest(); + assert.equal(runsAfterFirst, 1, "the first request must have run the builder exactly once"); + + // Deterministically expire the entry (just-expired — well inside the stale window) + // instead of sleeping out the real TTL. + v1ModelsCatalog.__expireCatalogCacheForTest(); + + const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res2.status, 200); + const body2 = await res2.text(); + assert.equal(body2, body1, "the stale response must be the cached body, served unchanged"); + assert.equal( + v1ModelsCatalog.__getCatalogBuilderRunsForTest(), + runsAfterFirst, + "the builder must NOT have run yet at response time — the stale response must not wait for a rebuild" + ); + + await v1ModelsCatalog.__flushCatalogBackgroundRefreshForTest(); + assert.equal( + v1ModelsCatalog.__getCatalogBuilderRunsForTest(), + runsAfterFirst + 1, + "the background refresh must have run once flushed" + ); +}); + +test("4. beyond the staleness window, the response waits for a fresh build again", async () => { + const makeRequest = () => new Request("http://localhost/v1/models"); + + const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res1.status, 200); + const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest(); + assert.equal(runsAfterFirst, 1); + + // Push the entry's age past CATALOG_STALE_WHILE_REVALIDATE_MS. + v1ModelsCatalog.__expireCatalogCacheForTest( + v1ModelsCatalog.CATALOG_STALE_WHILE_REVALIDATE_MS + 5_000 + ); + + const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res2.status, 200); + assert.equal( + v1ModelsCatalog.__getCatalogBuilderRunsForTest(), + runsAfterFirst + 1, + "past the staleness window, the builder must run again BEFORE the response is returned " + + "(a refresh that keeps failing must not pin a stale catalog forever)" + ); +}); + +test("5. a cached non-200 entry is never served as stale", async () => { + const request = new Request("http://localhost/v1/models"); + v1ModelsCatalog.__setCatalogCacheEntryForTest(request, { + body: JSON.stringify({ error: { message: "boom", type: "server_error", code: "X" } }), + headers: {}, + status: 500, + // "Just expired" — this age would be well within the stale window if the + // cached status were 200. + expiresAt: Date.now() - 1, + }); + const runsBefore = v1ModelsCatalog.__getCatalogBuilderRunsForTest(); + assert.equal(runsBefore, 0); + + const res = await v1ModelsCatalog.getUnifiedModelsResponse( + new Request("http://localhost/v1/models") + ); + + assert.equal( + v1ModelsCatalog.__getCatalogBuilderRunsForTest(), + runsBefore + 1, + "a cached error entry must never be served as stale — the builder must run instead" + ); + assert.equal(res.status, 200, "the fresh rebuild replaces the cached error response"); +}); + +test("6. a DB-state change drops the cache outright — the next response is a fresh build, never the stale pre-change body", async () => { + const makeRequest = () => new Request("http://localhost/v1/models"); + + const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res1.status, 200); + const runsAfterFirst = v1ModelsCatalog.__getCatalogBuilderRunsForTest(); + assert.equal(runsAfterFirst, 1); + + // Any settings/connections/combos/pricing write bumps this version; catalog.ts + // drops its entire cache map the next time it is read, independent of TTL/staleness. + readCache.invalidateDbCache(); + + const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(makeRequest()); + assert.equal(res2.status, 200); + assert.equal( + v1ModelsCatalog.__getCatalogBuilderRunsForTest(), + runsAfterFirst + 1, + "a state change must force a fresh build — the (now-superseded) cached entry must never be " + + "served, stale or otherwise" + ); +}); + +test("7. the 500 error path is sanitized — no stack trace or absolute source path leaks into the body", async () => { + const request = new Request("http://localhost/v1/models?prefix=alias&__d1_err_test=1"); + const rawMessage = + "Query failed at /home/diegosouzapw/dev/proxys/OmniRoute-Enterprise/secret/catalog.ts:42:1"; + const err = new Error(rawMessage); + v1ModelsCatalog.__forceCatalogInFlightRejectionForTest(request, err); + + const res = await v1ModelsCatalog.getUnifiedModelsResponse(request); + assert.equal(res.status, 500); + const body = await res.json(); + + assert.ok( + !body.error.message.includes("at /"), + "error body must not leak a stack-trace-like path" + ); + assert.ok( + !body.error.message.includes("catalog.ts"), + "error body must not leak the source file name" + ); + assert.ok( + !body.error.message.includes("diegosouzapw"), + "error body must not leak the local username/path" + ); + assert.equal(body.error.type, "server_error"); + assert.ok(typeof body.error.code === "string" && body.error.code.length > 0); +});