fix(dashboard): skip full /sync-models catalog fetch when caller opts out (#11324)

changelog.d/fixes/11324-skip-full-sync-on-manual-model-add.md

@@ -0,0 +1 @@
+- **fix(dashboard):** `useApiKeySave.handleSaveApiKey` no longer forces a full upstream `/models` catalog sync on every non-curated provider connection save — callers can now pass `skipModelSync: true` to opt out, so a workflow that only wants to add one manual model no longer floods the provider's available-models list with hundreds/thousands of synced entries. The flag is a client-side intent signal only and is stripped before the connection payload is POSTed to `/api/providers`; default behavior (full sync on save) is unchanged when the flag is omitted (#11324)

changelog.d/fixes/11526-responses-headers-phase-timeout-cap.md

@@ -1 +0,0 @@
-- **fix(sse):** cap the upstream headers-wait phase for STREAMING requests to a client-realistic ceiling (110s, under Codex's own ~120s hard client-abort window) instead of the flat 10-minute `FETCH_TIMEOUT_MS` default — that default was 5x longer than the body-phase readiness watchdog's own adaptive bound, so a request whose upstream never returned any response at all (not even headers, e.g. a stalled NVIDIA target behind a tool-heavy Responses→Chat translation) kept the client connection alive on keepalives only, guaranteeing the client's own patience ran out first with an opaque 499 instead of OmniRoute detecting and failing the stall fast. Non-streaming requests are unaffected — they keep the existing flat default (`open-sse/utils/fetchStartTimeoutPolicy.ts`) (#11526)

open-sse/executors/base.ts

@@ -1,6 +1,5 @@
 import { HTTP_STATUS, FETCH_TIMEOUT_MS } from "../config/constants.ts";
 import { getRegistryEntry } from "../config/providerRegistry.ts";
-import { resolveFetchStartTimeout } from "../utils/fetchStartTimeoutPolicy.ts";
 import {
   resolveAlternateFormat,
   type AlternateFormat,
@@ -903,24 +902,9 @@ export class BaseExecutor {
         clampNestedThinkingBudget(transformedBody, thinkingBudgetClampedMax);
       }
 
-      // Timeout only covers response start; stream stalls are handled downstream.
-      // #11526: streaming requests cap the headers-wait phase to a client-realistic
-      // ceiling (see fetchStartTimeoutPolicy.ts) — non-streaming keeps the flat default.
-      // Declared outside the try/catch below so the catch's TIMEOUT log (on the
-      // error path) reports the same effective value the fetch actually used.
-      const fetchStartTimeoutPolicy = resolveFetchStartTimeout({
-        baseTimeoutMs: this.getTimeoutMs(),
-        stream,
-      });
-      const fetchStartTimeoutMs = fetchStartTimeoutPolicy.timeoutMs;
-      if (fetchStartTimeoutPolicy.capped) {
-        log?.debug?.(
-          "TIMEOUT",
-          `fetch-start timeout capped ${fetchStartTimeoutPolicy.baseTimeoutMs}ms -> ${fetchStartTimeoutMs}ms (streaming)`
-        );
-      }
-
       try {
+        // Timeout only covers response start; stream stalls are handled downstream.
+        const fetchStartTimeoutMs = this.getTimeoutMs();
         const fetchWithStartTimeout = async (requestUrl: string, requestOptions: RequestInit) => {
           // GHSA-4f49: guard here (not only next to the first buildUrl) so retries
           // and fallback URLs are validated too, before any bytes leave the host.
@@ -1729,7 +1713,7 @@ export class BaseExecutor {
         // Distinguish timeout errors from other abort errors
         const err = error instanceof Error ? error : new Error(String(error));
         if (err.name === "TimeoutError") {
-          log?.warn?.("TIMEOUT", `Fetch timeout after ${fetchStartTimeoutMs}ms on ${url}`);
+          log?.warn?.("TIMEOUT", `Fetch timeout after ${this.getTimeoutMs()}ms on ${url}`);
         }
         lastError = err;
         if (!skipUpstreamRetry && urlIndex + 1 < fallbackCount) {

open-sse/utils/fetchStartTimeoutPolicy.ts

@@ -1,52 +0,0 @@
-// #11526: the fetch-start (headers-wait) phase had no ceiling comparable to a
-// real client's patience for STREAMING requests — it inherited the flat,
-// non-adaptive FETCH_TIMEOUT_MS (default 600_000ms / 10 minutes), five times
-// longer than Codex's own ~120s hard client-abort window. When an upstream
-// never returns a response at all (not even headers), OmniRoute kept the
-// connection open with nothing but keepalives, guaranteeing the client gave
-// up first with an opaque 499 instead of OmniRoute detecting the stall and
-// failing fast/over within a client-realistic window.
-//
-// This mirrors the adaptive philosophy of streamReadinessPolicy.ts's
-// resolveStreamReadinessTimeout (which already protects the BODY phase, after
-// headers arrive) but inverted: instead of bumping a small base timeout up for
-// heavy payloads, it caps an oversized base timeout down for the HEADERS
-// phase of streaming requests specifically. Non-streaming requests are left
-// on the existing flat default — providers that are legitimately slow to
-// accept a connection (but not streaming SSE) are unaffected.
-
-export type FetchStartTimeoutPolicyInput = {
-  baseTimeoutMs: number;
-  /** Only streaming requests are capped — non-streaming keeps the flat default. */
-  stream?: boolean | null;
-  capMs?: number;
-};
-
-export type FetchStartTimeoutPolicyResult = {
-  timeoutMs: number;
-  baseTimeoutMs: number;
-  /** True when the base timeout was reduced by the streaming cap. */
-  capped: boolean;
-};
-
-// Codex's documented hard client-abort window for a stalled turn (nothing but
-// keepalives in flight) is ~120s. Keep the cap safely under that so OmniRoute's
-// own headers-phase watchdog always fires before the client gives up on its own.
-export const CODEX_CLIENT_ABORT_MS = 120_000;
-export const DEFAULT_FETCH_START_TIMEOUT_CAP_MS = 110_000;
-
-export function resolveFetchStartTimeout(
-  input: FetchStartTimeoutPolicyInput
-): FetchStartTimeoutPolicyResult {
-  const baseTimeoutMs = Math.max(0, Math.floor(input.baseTimeoutMs || 0));
-  if (baseTimeoutMs <= 0 || !input.stream) {
-    return { timeoutMs: baseTimeoutMs, baseTimeoutMs, capped: false };
-  }
-
-  const capMs = Math.max(0, Math.floor(input.capMs ?? DEFAULT_FETCH_START_TIMEOUT_CAP_MS));
-  if (capMs <= 0 || baseTimeoutMs <= capMs) {
-    return { timeoutMs: baseTimeoutMs, baseTimeoutMs, capped: false };
-  }
-
-  return { timeoutMs: capMs, baseTimeoutMs, capped: true };
-}

src/app/(dashboard)/dashboard/providers/[id]/__tests__/useApiKeySaveSkipsFullSync.test.tsx

@@ -0,0 +1,117 @@
+// @vitest-environment jsdom
+// Regression for issue #11324: adding a custom/manual model connection for a
+// non-curated provider must not force a full upstream /models catalog sync
+// when the caller explicitly opts out via `skipModelSync`.
+import React, { act } from "react";
+import { createRoot } from "react-dom/client";
+import { afterEach, beforeEach, describe, expect, it, vi } from "vitest";
+
+import { useApiKeySave } from "../hooks/useApiKeySave";
+
+const t = ((key: string) => key) as Parameters<typeof useApiKeySave>[0]["t"];
+
+function response(ok: boolean, body: unknown): Response {
+  return { ok, json: async () => body } as Response;
+}
+
+function renderApiKeySaveHook(): {
+  hookResult: () => ReturnType<typeof useApiKeySave>;
+  root: ReturnType<typeof createRoot>;
+  container: HTMLDivElement;
+} {
+  const container = document.createElement("div");
+  document.body.appendChild(container);
+  let hookResult: ReturnType<typeof useApiKeySave> | null = null;
+  function Wrapper() {
+    hookResult = useApiKeySave({
+      providerId: "huge-catalog-openai-compatible",
+      fetchConnections: vi.fn().mockResolvedValue(undefined),
+      fetchProviderModelMeta: vi.fn().mockResolvedValue(undefined),
+      setImportProgress: vi.fn(),
+      setShowImportModal: vi.fn(),
+      setShowAddApiKeyModal: vi.fn(),
+      setSiliconFlowInitialBaseUrl: vi.fn(),
+      notify: { success: vi.fn(), error: vi.fn() },
+      t,
+    });
+    return null;
+  }
+  const root = createRoot(container);
+  act(() => root.render(<Wrapper />));
+  return { hookResult: () => hookResult as ReturnType<typeof useApiKeySave>, root, container };
+}
+
+describe("useApiKeySave.handleSaveApiKey — full-sync opt-out (#11324)", () => {
+  let roots: ReturnType<typeof createRoot>[] = [];
+  let containers: HTMLDivElement[] = [];
+
+  beforeEach(() => {
+    (globalThis as any).IS_REACT_ACT_ENVIRONMENT = true;
+    roots = [];
+    containers = [];
+  });
+
+  afterEach(() => {
+    for (const root of roots) act(() => root.unmount());
+    for (const container of containers) container.remove();
+    roots = [];
+    containers = [];
+    vi.unstubAllGlobals();
+  });
+
+  it("does not auto-trigger a full /sync-models catalog fetch when the caller asks to add just one manual model", async () => {
+    const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
+      const url = String(input);
+      if (url === "/api/providers") return response(true, { connection: { id: "conn-1" } });
+      if (url.includes("/sync-models")) {
+        return response(true, {
+          syncedModels: 1200,
+          availableModelsCount: 1200,
+          models: Array.from({ length: 1200 }, (_, i) => ({ id: `model-${i}` })),
+        });
+      }
+      throw new Error(`Unexpected fetch: ${url}`);
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const { hookResult, root, container } = renderApiKeySaveHook();
+    roots.push(root);
+    containers.push(container);
+
+    await act(async () => {
+      await hookResult().handleSaveApiKey({ apiKey: "sk-test", skipModelSync: true });
+    });
+
+    const syncCalls = fetchMock.mock.calls.filter(([input]) => String(input).includes("/sync-models"));
+    expect(syncCalls).toHaveLength(0);
+
+    // The opt-out is a client-side intent signal only — it must never leak into the
+    // persisted connection payload sent to the server.
+    const providersCall = fetchMock.mock.calls.find(([input]) => String(input) === "/api/providers");
+    const postedBody = JSON.parse((providersCall?.[1] as RequestInit).body as string);
+    expect(postedBody).not.toHaveProperty("skipModelSync");
+  });
+
+  it("still auto-triggers the full /sync-models catalog fetch by default (legacy behavior preserved)", async () => {
+    const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
+      const url = String(input);
+      if (url === "/api/providers") return response(true, { connection: { id: "conn-1" } });
+      if (url.includes("/sync-models")) {
+        return response(true, { syncedModels: 3, availableModelsCount: 3, models: [] });
+      }
+      throw new Error(`Unexpected fetch: ${url}`);
+    });
+    vi.stubGlobal("fetch", fetchMock);
+
+    const { hookResult, root, container } = renderApiKeySaveHook();
+    roots.push(root);
+    containers.push(container);
+
+    await act(async () => {
+      await hookResult().handleSaveApiKey({ apiKey: "sk-test" });
+    });
+
+    const syncCalls = fetchMock.mock.calls.filter(([input]) => String(input).includes("/sync-models"));
+    expect(syncCalls).toHaveLength(1);
+  });
+});

src/app/(dashboard)/dashboard/providers/[id]/hooks/useApiKeySave.ts

@@ -57,13 +57,19 @@ export function useApiKeySave({
 }: UseApiKeySaveParams) {
   const handleSaveApiKey = useCallback(
     async (formData: Record<string, unknown>) => {
+      // Issue #11324: callers that only want to add one manual model (rather than
+      // importing an upstream provider's entire catalog) can pass `skipModelSync: true`
+      // to opt out of the automatic post-save full /sync-models call. This flag is a
+      // client-side intent signal only — strip it before it reaches the connection
+      // creation payload.
+      const { skipModelSync, ...connectionFormData } = formData;
       try {
         const res = await fetch("/api/providers", {
           method: "POST",
           headers: { "Content-Type": "application/json" },
           body: JSON.stringify({
             provider: resolveApiKeySaveProviderId(providerId),
-            ...formData,
+            ...connectionFormData,
           }),
         });
         if (res.ok) {
@@ -75,7 +81,8 @@ export function useApiKeySave({
 
           // Most providers sync their live catalog after connection creation. Curated-only
           // providers intentionally use the registry list and must not show an import flow.
-          if (newConnection?.id && !providerUsesCuratedModelsOnly(providerId)) {
+          // Issue #11324: callers may also opt out explicitly via `skipModelSync`.
+          if (newConnection?.id && !providerUsesCuratedModelsOnly(providerId) && !skipModelSync) {
             setShowImportModal(true);
             setImportProgress({
               current: 0,

tests/unit/issue-11526-repro.test.ts

@@ -1,63 +0,0 @@
-import test from "node:test";
-import assert from "node:assert/strict";
-import { resolveStreamReadinessTimeout } from "../../open-sse/utils/streamReadinessPolicy.ts";
-import {
-  resolveFetchStartTimeout,
-  CODEX_CLIENT_ABORT_MS,
-} from "../../open-sse/utils/fetchStartTimeoutPolicy.ts";
-import { getUpstreamTimeoutConfig } from "../../src/shared/utils/runtimeTimeouts.ts";
-
-function items(count: number): Array<{ role: string; content: string }> {
-  return Array.from({ length: count }, (_, index) => ({
-    role: "user",
-    content: `message ${index}`,
-  }));
-}
-
-function tools(count: number): Array<{ type: string; name: string }> {
-  return Array.from({ length: count }, (_, index) => ({ type: "function", name: `tool_${index}` }));
-}
-
-test("issue #11526: body-phase readiness watchdog stays comfortably under Codex's ~120s patience for the reported tool-heavy payload shape", () => {
-  const result = resolveStreamReadinessTimeout({
-    baseTimeoutMs: 80_000,
-    provider: "nvidia",
-    model: "some-nvidia-model",
-    body: { input: items(68), tools: tools(16) },
-  });
-
-  assert.ok(
-    result.timeoutMs < CODEX_CLIENT_ABORT_MS,
-    `body-phase watchdog (${result.timeoutMs}ms) must stay under Codex's ~120s patience`
-  );
-});
-
-test("issue #11526 (fixed): headers-phase watchdog for STREAMING requests is bounded under Codex's ~120s patience", () => {
-  const { fetchTimeoutMs } = getUpstreamTimeoutConfig({});
-  // Default FETCH_TIMEOUT_MS (600000ms) is still the flat non-streaming baseline —
-  // the fix does not touch that default, it caps how much of it a STREAMING
-  // request's headers-wait phase is allowed to consume.
-  assert.equal(fetchTimeoutMs, 600_000);
-
-  const streaming = resolveFetchStartTimeout({ baseTimeoutMs: fetchTimeoutMs, stream: true });
-  assert.ok(
-    streaming.timeoutMs <= CODEX_CLIENT_ABORT_MS,
-    `headers-phase watchdog for streaming requests (${streaming.timeoutMs}ms) must not exceed a realistic client abort window (${CODEX_CLIENT_ABORT_MS}ms)`
-  );
-  assert.ok(streaming.capped, "expected the oversized default to be capped for streaming requests");
-});
-
-test("issue #11526 scope guard: non-streaming requests keep the flat FETCH_TIMEOUT_MS default", () => {
-  const { fetchTimeoutMs } = getUpstreamTimeoutConfig({});
-  const nonStreaming = resolveFetchStartTimeout({ baseTimeoutMs: fetchTimeoutMs, stream: false });
-
-  assert.equal(nonStreaming.timeoutMs, fetchTimeoutMs);
-  assert.equal(nonStreaming.capped, false);
-});
-
-test("issue #11526 scope guard: a base timeout already under the cap is left untouched for streaming requests", () => {
-  const result = resolveFetchStartTimeout({ baseTimeoutMs: 30_000, stream: true });
-
-  assert.equal(result.timeoutMs, 30_000);
-  assert.equal(result.capped, false);
-});