fix(api): coalesce concurrent GET /v1/models to one builder run (#6408) (#6440)

coalesce concurrent GET /v1/models (#6408, 3/3). Integrated into release/v3.8.46.

CHANGELOG.md

@@ -21,6 +21,7 @@
 
 ### 🐛 Bug Fixes
 
+- **fix(api):** concurrent `GET /v1/models` requests are coalesced into a single catalog build ([#6408](https://github.com/diegosouzapw/OmniRoute/issues/6408)). Regression guard: `tests/unit/v1-models-concurrent-6408.test.ts`. (thanks @chirag127)
 - **fix(api):** `/v1/completions` now echoes the requested `body.model` in its JSON + streamed responses. Regression guard: `tests/unit/completions-body-model-echo.test.ts`. (thanks @chirag127)
 - **fix(api):** env-var master keys now see the full `/v1/models` catalog ([#6406](https://github.com/diegosouzapw/OmniRoute/issues/6406)). Regression guard: `tests/unit/models-catalog-envkey-6406.test.ts`. (thanks @chirag127)
 - **fix(api):** non-streaming `/v1/completions` responses now echo `body.model` aligned with the `X-OmniRoute-Model` header ([#6426](https://github.com/diegosouzapw/OmniRoute/issues/6426)). Regression guard: `tests/unit/v1-completions-model-header-match-6426.test.ts`. (thanks @chirag127)

src/app/api/v1/models/catalog.ts

@@ -82,6 +82,42 @@ import { getModelCatalogAuthRejection, isCodexModelCatalogClient } from "./catal
 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<string, string>; expiresAt: number };
+const CATALOG_CACHE_TTL_MS = 1500; // ~one request-latency window; safe vs SDK bursts
+const catalogCache = new Map<string, CachedCatalog>();
+const catalogInFlight = new Map<string, Promise<CachedCatalog>>();
+
+// 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();
+}
+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";
+  return `${prefix}|${isCodex}|${apiKey}`;
+}
+
 /**
  * Build unified OpenAI-compatible model catalog response.
  * Reused by `/api/v1/models` and `/api/v1` to avoid semantic drift (T09).
@@ -91,6 +127,88 @@ export async function getUnifiedModelsResponse(
   corsHeaders: Record<string, string> = {}
 ) {
   const diagnosticHeaders = getCatalogDiagnosticsHeaders({ request });
+
+  // #6408 fast path: reject unauthorized callers first (auth state is per-request
+  // and MUST NOT be cached), then coalesce identical concurrent requests + short-
+  // TTL memoize the serialized JSON body.
+  try {
+    let settingsForAuth: Record<string, any> = {};
+    try {
+      settingsForAuth = await getSettings();
+    } catch {}
+    const authRejection = await getModelCatalogAuthRejection(request, settingsForAuth, {
+      ...corsHeaders,
+      ...diagnosticHeaders,
+    });
+    if (authRejection) return authRejection;
+  } catch {
+    // Fall through to full builder on auth-check failure; core handles errors.
+  }
+
+  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 },
+    });
+  }
+
+  let inflight = catalogInFlight.get(cacheKey);
+  if (!inflight) {
+    inflight = buildCatalogPayload(request).then((payload) => {
+      catalogCache.set(cacheKey, {
+        body: payload.body,
+        headers: payload.headers,
+        expiresAt: Date.now() + CATALOG_CACHE_TTL_MS,
+      });
+      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, {
+      headers: { ...corsHeaders, ...diagnosticHeaders, ...payload.headers },
+    });
+  } catch (err) {
+    return Response.json(
+      {
+        error: {
+          message: err instanceof Error ? err.message : String(err),
+          type: "server_error",
+          code: INTERNAL_PROXY_ERROR,
+        },
+      },
+      { status: 500, headers: { ...corsHeaders, ...diagnosticHeaders } }
+    );
+  }
+}
+
+async function buildCatalogPayload(
+  request: Request
+): Promise<{ body: string; headers: Record<string, string> }> {
+  _catalogBuilderRuns++;
+  const built = await buildUnifiedModelsResponseCore(request);
+  const body = await built.text();
+  const headers: Record<string, string> = {};
+  built.headers.forEach((value, key) => {
+    headers[key] = value;
+  });
+  return { body, headers };
+}
+
+/**
+ * Original catalog builder. Runs once per unique cache key per TTL window.
+ */
+async function buildUnifiedModelsResponseCore(
+  request: Request,
+  corsHeaders: Record<string, string> = {}
+) {
+  const diagnosticHeaders = getCatalogDiagnosticsHeaders({ request });
   try {
     let settings: Record<string, any> = {};
     try {

tests/unit/v1-models-concurrent-6408.test.ts

@@ -0,0 +1,102 @@
+// Regression guard for #6408 — GET /v1/models serializes concurrent requests
+// (~1.2 s per request under any concurrency).
+//
+// The catalog builder walks 8 model registries + hits SQLite for connections,
+// combos, custom models, and aliases on every call. Under Next.js App Router
+// (single-threaded per-instance), N concurrent GETs run back-to-back so the
+// 10th request completes ~12 s after the 1st (linear staircase reproduced in
+// the issue).
+//
+// Fix: coalesce identical concurrent requests onto a single in-flight promise
+// and memoize the serialized JSON body for ~1.5 s. This test uses the
+// __getCatalogBuilderRunsForTest hook to assert that N concurrent GETs
+// collapse to exactly ONE builder execution (not N).
+
+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-6408-"));
+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 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("#6408 — 10 concurrent identical GET /v1/models calls collapse to ONE builder run", async () => {
+  const N = 10;
+  const requests = Array.from({ length: N }, () => new Request("http://localhost/v1/models"));
+  const responses = await Promise.all(
+    requests.map((req) => v1ModelsCatalog.getUnifiedModelsResponse(req))
+  );
+
+  for (const res of responses) assert.equal(res.status, 200);
+  const bodies = await Promise.all(responses.map((r) => r.text()));
+  for (let i = 1; i < bodies.length; i++) {
+    assert.equal(bodies[i], bodies[0], "concurrent responses must be byte-identical");
+  }
+
+  const runs = v1ModelsCatalog.__getCatalogBuilderRunsForTest();
+  assert.equal(
+    runs,
+    1,
+    `builder ran ${runs} times for ${N} concurrent requests — expected exactly 1 (in-flight coalescing)`
+  );
+});
+
+test("#6408 — a second call within the TTL window is served from cache without re-running the builder", async () => {
+  const res1 = await v1ModelsCatalog.getUnifiedModelsResponse(
+    new Request("http://localhost/v1/models")
+  );
+  assert.equal(res1.status, 200);
+  assert.equal(v1ModelsCatalog.__getCatalogBuilderRunsForTest(), 1);
+
+  const res2 = await v1ModelsCatalog.getUnifiedModelsResponse(
+    new Request("http://localhost/v1/models")
+  );
+  assert.equal(res2.status, 200);
+  assert.equal(
+    v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
+    1,
+    "second call within TTL should reuse cache — builder must not run again"
+  );
+
+  assert.equal(await res2.text(), await res1.text());
+});
+
+test("#6408 — requests with different cache keys (prefix param) run the builder independently", async () => {
+  const resAlias = await v1ModelsCatalog.getUnifiedModelsResponse(
+    new Request("http://localhost/v1/models?prefix=alias")
+  );
+  const resCanon = await v1ModelsCatalog.getUnifiedModelsResponse(
+    new Request("http://localhost/v1/models?prefix=canonical")
+  );
+  assert.equal(resAlias.status, 200);
+  assert.equal(resCanon.status, 200);
+  assert.equal(
+    v1ModelsCatalog.__getCatalogBuilderRunsForTest(),
+    2,
+    "distinct cache keys must not collapse into each other"
+  );
+});