From e653dd817bafea0fb061a0237fcf0dfe7fbfd733 Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Sat, 19 Sep 2026 20:52:08 +0200 Subject: [PATCH 1/3] fix(model-cache): propagate caller cancellation through catalog fetchers Thread an optional AbortSignal through the model-cache single-flight so caller cancellation reaches the fetcher layer. Each flight owns a refcounted AbortController: the shared network request is aborted only when the last waiter leaves, the in-flight entry is released synchronously at last-waiter abort, and a joiner arriving after the release starts a fresh fetch. Every fetch routed through the single-flight carries a bounded 15 s timeout that manifests as an abort. Fetchers whose HTTP client natively supports cancellation (axios signal, fetch signal) cancel the network request on the last-waiter abort; SDK-bound fetchers (poe, LM Studio client calls) honor cancellation at their await boundaries by releasing the shared entry and stopping their waiters. Fixes #1615 --- .../fetchers/__tests__/deepseek.spec.ts | 31 ++ .../fetchers/__tests__/kenari.spec.ts | 31 +- .../fetchers/__tests__/kimi-code.spec.ts | 14 + .../fetchers/__tests__/litellm.spec.ts | 59 ++- .../fetchers/__tests__/lmstudio.test.ts | 46 +- .../fetchers/__tests__/modelCache.spec.ts | 497 +++++++++++++++++- .../fetchers/__tests__/moonshot.spec.ts | 30 ++ .../fetchers/__tests__/nanogpt.spec.ts | 31 +- .../fetchers/__tests__/ollama.test.ts | 77 +++ .../fetchers/__tests__/opencode-go.spec.ts | 29 +- .../fetchers/__tests__/openrouter.spec.ts | 44 ++ .../providers/fetchers/__tests__/poe.spec.ts | 22 + .../fetchers/__tests__/requesty.spec.ts | 32 ++ .../fetchers/__tests__/unbound.spec.ts | 40 ++ .../__tests__/vercel-ai-gateway.spec.ts | 35 +- .../fetchers/__tests__/zoo-gateway.spec.ts | 40 ++ src/api/providers/fetchers/deepseek.ts | 133 ++--- src/api/providers/fetchers/kenari.ts | 14 +- src/api/providers/fetchers/kimi-code.ts | 11 +- src/api/providers/fetchers/litellm.ts | 16 +- src/api/providers/fetchers/lmstudio.ts | 16 +- src/api/providers/fetchers/modelCache.ts | 222 ++++++-- src/api/providers/fetchers/moonshot.ts | 95 ++-- src/api/providers/fetchers/nanogpt.ts | 10 +- src/api/providers/fetchers/ollama.ts | 12 +- src/api/providers/fetchers/opencode-go.ts | 14 +- src/api/providers/fetchers/openrouter.ts | 13 +- src/api/providers/fetchers/poe.ts | 17 +- src/api/providers/fetchers/requesty.ts | 14 +- src/api/providers/fetchers/unbound.ts | 13 +- .../providers/fetchers/vercel-ai-gateway.ts | 13 +- src/api/providers/fetchers/zoo-gateway.ts | 16 +- src/shared/api.ts | 6 +- 33 files changed, 1486 insertions(+), 207 deletions(-) create mode 100644 src/api/providers/fetchers/__tests__/unbound.spec.ts diff --git a/src/api/providers/fetchers/__tests__/deepseek.spec.ts b/src/api/providers/fetchers/__tests__/deepseek.spec.ts index 7856874329..3af27ab4d7 100644 --- a/src/api/providers/fetchers/__tests__/deepseek.spec.ts +++ b/src/api/providers/fetchers/__tests__/deepseek.spec.ts @@ -59,4 +59,35 @@ describe("getDeepSeekModels", () => { await expect(getDeepSeekModels("http://127.0.0.1:43123/v1", "mock-key")).rejects.toThrow("HTTP 404: Not Found") }) + + it("passes the caller's abort signal to the request", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response(JSON.stringify({ data: [] }), { status: 200 })) + const controller = new AbortController() + + await getDeepSeekModels(undefined, "test-key", { signal: controller.signal }) + + expect(fetchSpy).toHaveBeenCalledWith( + "https://api.deepseek.com/models", + expect.objectContaining({ signal: controller.signal }), + ) + }) + + it("rejects instead of serving the static fallback when the signal is aborted during an error response", async () => { + process.env.E2E_MOCK_MODEL_LIST_FALLBACK = "true" + const controller = new AbortController() + controller.abort() + + vi.spyOn(globalThis, "fetch").mockResolvedValue( + new Response('{"error":{"message":"Not found","type":"not_found"}}', { + status: 404, + statusText: "Not Found", + }), + ) + + await expect( + getDeepSeekModels("http://127.0.0.1:43123/v1", "mock-key", { signal: controller.signal }), + ).rejects.toMatchObject({ name: "AbortError" }) + }) }) diff --git a/src/api/providers/fetchers/__tests__/kenari.spec.ts b/src/api/providers/fetchers/__tests__/kenari.spec.ts index 41a56fa87a..1995d73db6 100644 --- a/src/api/providers/fetchers/__tests__/kenari.spec.ts +++ b/src/api/providers/fetchers/__tests__/kenari.spec.ts @@ -38,7 +38,7 @@ describe("Kenari Fetchers", () => { expect(mockedAxios.get).toHaveBeenCalledWith("https://kenari.id/v1/models", { headers: { Authorization: "Bearer test-key" }, - timeout: 10_000, + signal: undefined, }) expect(Object.keys(models).sort()).toEqual(["claude-sonnet-5", "glm-5-2"]) @@ -100,6 +100,33 @@ describe("Kenari Fetchers", () => { warnSpy.mockRestore() }) + + it("forwards the caller's abort signal to the request", async () => { + mockedAxios.get.mockResolvedValue({ data: { data: [] } }) + const controller = new AbortController() + + await getKenariModels("test-key", { signal: controller.signal }) + + expect(mockedAxios.get).toHaveBeenCalledWith("https://kenari.id/v1/models", { + headers: { Authorization: "Bearer test-key" }, + signal: controller.signal, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation((_url: string, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getKenariModels("k", { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) }) describe("parseKenariModel", () => { @@ -110,7 +137,7 @@ describe("Kenari Fetchers", () => { expect(mockedAxios.get).toHaveBeenCalledWith("https://kenari.id/v1/models", { headers: undefined, - timeout: 10_000, + signal: undefined, }) }) diff --git a/src/api/providers/fetchers/__tests__/kimi-code.spec.ts b/src/api/providers/fetchers/__tests__/kimi-code.spec.ts index a96e760253..8378f7d8d0 100644 --- a/src/api/providers/fetchers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/fetchers/__tests__/kimi-code.spec.ts @@ -98,6 +98,20 @@ describe("Kimi Code model discovery", () => { expect(vi.getTimerCount()).toBe(0) }) + it("rejects when the caller aborts a pending discovery request", async () => { + vi.spyOn(globalThis, "fetch").mockImplementation((_input, init) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(init.signal?.reason), { once: true }) + }) + }) + const controller = new AbortController() + + const result = getKimiCodeModels("token", { signal: controller.signal }) + controller.abort() + + await expect(result).rejects.toMatchObject({ name: "AbortError" }) + }) + it("overrides maxTokens from server max_tokens in mapKimiCodeModel", () => { const mapped = mapKimiCodeModel({ id: "kimi-for-coding", diff --git a/src/api/providers/fetchers/__tests__/litellm.spec.ts b/src/api/providers/fetchers/__tests__/litellm.spec.ts index a13a930b84..0d8798c6eb 100644 --- a/src/api/providers/fetchers/__tests__/litellm.spec.ts +++ b/src/api/providers/fetchers/__tests__/litellm.spec.ts @@ -35,7 +35,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -56,7 +55,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -77,7 +75,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -98,7 +95,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -119,7 +115,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -140,7 +135,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -161,7 +155,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -213,7 +206,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) expect(result).toEqual({ @@ -334,7 +326,6 @@ describe("getLiteLLMModels", () => { "Content-Type": "application/json", ...DEFAULT_HEADERS, }, - timeout: 5000, }) }) @@ -456,18 +447,54 @@ describe("getLiteLLMModels", () => { ) }) - it("handles timeout parameter correctly", async () => { + it("forwards the caller signal and sends no per-call timeout", async () => { + const mockResponse = { data: { data: [] } } + mockedAxios.get.mockResolvedValue(mockResponse) + const controller = new AbortController() + + await getLiteLLMModels("test-api-key", "http://localhost:4000", { signal: controller.signal }) + + // Exact-shape assertion: an added per-call option (e.g. a reintroduced + // timeout) would fail this match, keeping the request bound single-sourced. + expect(mockedAxios.get).toHaveBeenCalledWith("http://localhost:4000/v1/model/info", { + headers: { + Authorization: "Bearer test-api-key", + "Content-Type": "application/json", + ...DEFAULT_HEADERS, + }, + signal: controller.signal, + }) + }) + + it("passes an undefined signal through when no options are provided", async () => { const mockResponse = { data: { data: [] } } mockedAxios.get.mockResolvedValue(mockResponse) await getLiteLLMModels("test-api-key", "http://localhost:4000") - expect(mockedAxios.get).toHaveBeenCalledWith( - "http://localhost:4000/v1/model/info", - expect.objectContaining({ - timeout: 5000, - }), - ) + expect(mockedAxios.get).toHaveBeenCalledWith("http://localhost:4000/v1/model/info", { + headers: { + Authorization: "Bearer test-api-key", + "Content-Type": "application/json", + ...DEFAULT_HEADERS, + }, + signal: undefined, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation((_url: string, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getLiteLLMModels("test-api-key", "http://localhost:4000", { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) }) it("returns empty object when data array is empty", async () => { diff --git a/src/api/providers/fetchers/__tests__/lmstudio.test.ts b/src/api/providers/fetchers/__tests__/lmstudio.test.ts index 789b57096f..0de4f22573 100644 --- a/src/api/providers/fetchers/__tests__/lmstudio.test.ts +++ b/src/api/providers/fetchers/__tests__/lmstudio.test.ts @@ -148,7 +148,7 @@ describe("LMStudio Fetcher", () => { const result = await getLMStudioModels(baseUrl) expect(mockedAxios.get).toHaveBeenCalledTimes(1) - expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).toHaveBeenCalledTimes(1) expect(MockedLMStudioClientConstructor).toHaveBeenCalledWith({ baseUrl: lmsUrl }) expect(mockListDownloadedModels).toHaveBeenCalledTimes(1) @@ -168,7 +168,7 @@ describe("LMStudio Fetcher", () => { const result = await getLMStudioModels(baseUrl) expect(mockedAxios.get).toHaveBeenCalledTimes(1) - expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).toHaveBeenCalledTimes(1) expect(MockedLMStudioClientConstructor).toHaveBeenCalledWith({ baseUrl: lmsUrl }) expect(mockListDownloadedModels).toHaveBeenCalledTimes(1) @@ -408,7 +408,7 @@ describe("LMStudio Fetcher", () => { await getLMStudioModels("") - expect(mockedAxios.get).toHaveBeenCalledWith(`${defaultBaseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${defaultBaseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).toHaveBeenCalledWith({ baseUrl: defaultLmsUrl }) }) @@ -420,7 +420,7 @@ describe("LMStudio Fetcher", () => { await getLMStudioModels(httpsBaseUrl) - expect(mockedAxios.get).toHaveBeenCalledWith(`${httpsBaseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${httpsBaseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).toHaveBeenCalledWith({ baseUrl: wssLmsUrl }) }) @@ -442,7 +442,7 @@ describe("LMStudio Fetcher", () => { const result = await getLMStudioModels(baseUrl) expect(mockedAxios.get).toHaveBeenCalledTimes(1) - expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).not.toHaveBeenCalled() expect(mockListLoaded).not.toHaveBeenCalled() expect(consoleErrorSpy).toHaveBeenCalledWith( @@ -461,7 +461,7 @@ describe("LMStudio Fetcher", () => { const result = await getLMStudioModels(baseUrl) expect(mockedAxios.get).toHaveBeenCalledTimes(1) - expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`) + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`, { signal: undefined }) expect(MockedLMStudioClientConstructor).not.toHaveBeenCalled() expect(mockListLoaded).not.toHaveBeenCalled() expect(consoleInfoSpy).toHaveBeenCalledWith(`Error connecting to LMStudio at ${baseUrl}`) @@ -488,5 +488,39 @@ describe("LMStudio Fetcher", () => { expect(result).toEqual({}) consoleErrorSpy.mockRestore() }) + + it("should pass the caller's abort signal to the connection probe", async () => { + const controller = new AbortController() + mockedAxios.get.mockResolvedValueOnce({ data: { status: "ok" } }) + mockListDownloadedModels.mockResolvedValueOnce([]) + mockListLoaded.mockResolvedValueOnce([]) + + await getLMStudioModels(baseUrl, { signal: controller.signal }) + + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/v1/models`, { signal: controller.signal }) + }) + + it("should reject with an AbortError when the signal aborts the probe, without calling the SDK", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation((_url: string, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a request rejects when its signal fires, + // including when the signal was already aborted when the request started. + return new Promise((_resolve, reject) => { + if (config?.signal?.aborted) { + reject(new Error("canceled")) + return + } + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getLMStudioModels(baseUrl, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + expect(MockedLMStudioClientConstructor).not.toHaveBeenCalled() + expect(mockListDownloadedModels).not.toHaveBeenCalled() + expect(mockListLoaded).not.toHaveBeenCalled() + }) }) }) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 108aa1827b..b8c9b95398 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -61,7 +61,9 @@ vi.mock("../../../core/config/ContextProxy", () => ({ })) // Then imports +import { getEventListeners } from "events" import type { Mock, Mocked } from "vitest" +import type { ModelRecord } from "@roo-code/types" import { providerIdentifiers } from "@roo-code/types" import * as fsSync from "fs" import NodeCache from "node-cache" @@ -107,7 +109,10 @@ describe("getModels with new GetModelsOptions", () => { baseUrl: "http://localhost:4000", }) - expect(mockGetLiteLLMModels).toHaveBeenCalledWith("test-api-key", "http://localhost:4000") + // Every single-flight fetch carries the flight's bound/abort signal. + expect(mockGetLiteLLMModels).toHaveBeenCalledWith("test-api-key", "http://localhost:4000", { + signal: expect.any(AbortSignal), + }) expect(result).toEqual(mockModels) }) @@ -158,7 +163,9 @@ describe("getModels with new GetModelsOptions", () => { const result = await getModels({ provider: providerIdentifiers.requesty, apiKey: DUMMY_REQUESTY_KEY }) - expect(mockGetRequestyModels).toHaveBeenCalledWith(undefined, DUMMY_REQUESTY_KEY) + expect(mockGetRequestyModels).toHaveBeenCalledWith(undefined, DUMMY_REQUESTY_KEY, { + signal: expect.any(AbortSignal), + }) expect(result).toEqual(mockModels) }) @@ -179,7 +186,9 @@ describe("getModels with new GetModelsOptions", () => { baseUrl: "https://router.requesty.ai/v1", }) - expect(mockGetRequestyModels).toHaveBeenCalledWith("https://router.requesty.ai/v1", DUMMY_REQUESTY_KEY) + expect(mockGetRequestyModels).toHaveBeenCalledWith("https://router.requesty.ai/v1", DUMMY_REQUESTY_KEY, { + signal: expect.any(AbortSignal), + }) expect(result).toEqual(mockModels) }) @@ -196,7 +205,9 @@ describe("getModels with new GetModelsOptions", () => { const result = await getModels({ provider: providerIdentifiers.kenari, apiKey: "kenari-key-for-testing" }) - expect(mockGetKenariModels).toHaveBeenCalledWith("kenari-key-for-testing") + expect(mockGetKenariModels).toHaveBeenCalledWith("kenari-key-for-testing", { + signal: expect.any(AbortSignal), + }) expect(result).toEqual(mockModels) }) @@ -212,7 +223,7 @@ describe("getModels with new GetModelsOptions", () => { const result = await getModels({ provider: providerIdentifiers.nanogpt, apiKey: "nanogpt-key" }) - expect(mockGetNanoGptModels).toHaveBeenCalledWith("nanogpt-key") + expect(mockGetNanoGptModels).toHaveBeenCalledWith("nanogpt-key", { signal: expect.any(AbortSignal) }) expect(result).toEqual(mockModels) }) @@ -246,7 +257,9 @@ describe("getModels with new GetModelsOptions", () => { baseUrl: "https://api.moonshot.ai/v1", }) - expect(mockGetMoonshotModels).toHaveBeenCalledWith("https://api.moonshot.ai/v1", "test-key") + expect(mockGetMoonshotModels).toHaveBeenCalledWith("https://api.moonshot.ai/v1", "test-key", { + signal: expect.any(AbortSignal), + }) expect(result).toEqual(mockModels) }) @@ -1204,3 +1217,475 @@ describe("compound cache key derivation across scoping dimensions", () => { expect(cacheKey).toBe("openrouter") }) }) + +// Single-flight cancellation. Each test below exercises one interleaving of caller aborts, +// the per-flight fetch bound, and settlement; the mocks hold the HTTP layer pending so a +// rejection that only arrives at abort time (rather than at fetch settle time) is provable. + +const cancelledModels = { + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + }, +} + +const cancelledModelsB = { + "openrouter/other": { + maxTokens: 4096, + contextWindow: 64000, + supportsPromptCache: false, + }, +} + +// Drains the full microtask queue: a setImmediate callback runs only after every pending +// promise reaction has settled, so all settle-path bookkeeping has definitively happened. +const drainMicrotasks = (): Promise => new Promise((resolve) => setImmediate(resolve)) + +// Standard per-test setup: clear call state, force a memory-cache miss, and force a disk miss. +const setupCancellationMocks = () => { + vi.clearAllMocks() + const mockCache = vi.mocked(new (vi.mocked(NodeCache))()) + mockCache.get.mockReturnValue(undefined) + vi.mocked(fsSync.existsSync).mockReturnValue(false) +} + +// A fetcher double that never settles on its own and reports the signal the dispatcher gave it. +const neverSettlingFetcher = (capture: (signal: AbortSignal | undefined) => void) => + mockGetOpenRouterModels.mockImplementation((_options, opts) => { + capture(opts?.signal) + return new Promise(() => {}) + }) + +it("threads a cancellation signal into the dispatched fetcher from both entry points", async () => { + setupCancellationMocks() + mockGetOpenRouterModels.mockResolvedValue(cancelledModels) + + const controller = new AbortController() + await getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + + const getCall = mockGetOpenRouterModels.mock.calls[mockGetOpenRouterModels.mock.calls.length - 1] + expect(getCall[1]?.signal).toBeInstanceOf(AbortSignal) + + const { refreshModels } = await import("../modelCache") + await refreshModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + + const refreshCall = mockGetOpenRouterModels.mock.calls[mockGetOpenRouterModels.mock.calls.length - 1] + expect(refreshCall[1]?.signal).toBeInstanceOf(AbortSignal) +}) + +it("rejects a pre-aborted caller without creating or starting any flight", async () => { + setupCancellationMocks() + const preAborted = AbortSignal.abort() + + await expect(getModels({ provider: providerIdentifiers.openrouter, signal: preAborted })).rejects.toMatchObject({ + name: "AbortError", + }) + expect(mockGetOpenRouterModels).not.toHaveBeenCalled() + + // refreshModels() keeps its graceful-degradation contract even for a pre-aborted caller. + const { refreshModels } = await import("../modelCache") + await expect(refreshModels({ provider: providerIdentifiers.openrouter, signal: preAborted })).resolves.toEqual({}) + expect(mockGetOpenRouterModels).not.toHaveBeenCalled() +}) + +it("rejects the last waiter at the abort event and releases the entry synchronously", async () => { + setupCancellationMocks() + let flightSignal: AbortSignal | undefined + neverSettlingFetcher((signal) => { + flightSignal = signal + }) + + const controller = new AbortController() + const waitPromise = getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + + controller.abort() + + // The rejection was produced synchronously during abort(): the fetch has never settled, + // so if the wait only ended at fetch settle time this promise would hang until timeout. + await expect(waitPromise).rejects.toMatchObject({ name: "AbortError" }) + expect(flightSignal?.aborted).toBe(true) + + // Entry already gone at the very next synchronous observation: a fresh call starts a fresh + // fetch instead of joining the doomed one. + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModelsB) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) +}) + +it("serves a joiner that arrives after the last-waiter release with a fresh request", async () => { + setupCancellationMocks() + let resolveSecond: ((models: ModelRecord) => void) | undefined + let fetchCalls = 0 + mockGetOpenRouterModels.mockImplementation((_options, _opts) => { + fetchCalls++ + if (fetchCalls === 1) { + return new Promise(() => {}) + } + return new Promise((resolve) => { + resolveSecond = resolve + }) + }) + + const controller = new AbortController() + const aborted = getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + controller.abort() + await expect(aborted).rejects.toMatchObject({ name: "AbortError" }) + + // Joined strictly after the release but while the doomed fetch's rejection is still + // outstanding: must be a fresh flight, not the doomed promise. + const fresh = getModels({ provider: providerIdentifiers.openrouter }) + expect(fetchCalls).toBe(2) + resolveSecond!(cancelledModelsB) + await expect(fresh).resolves.toEqual(cancelledModelsB) +}) + +it("keeps entry bookkeeping consistent for both settle-vs-abort race orders", async () => { + const raceModels = { + "openrouter/raced": { maxTokens: 8192, contextWindow: 128000, supportsPromptCache: false }, + } + + // Order 1: the last-waiter abort wins and the doomed fetch settles only afterwards, while + // a NEWER flight already occupies the map slot. The settle-time identity-guarded delete + // must miss, leaving the newer flight's entry (and its joiners) intact. + setupCancellationMocks() + let resolveRaced: ((models: ModelRecord) => void) | undefined + let resolveSecond: ((models: ModelRecord) => void) | undefined + mockGetOpenRouterModels.mockImplementation(() => { + return new Promise((resolve) => { + resolveRaced = resolve + }) + }) + + const controllerA = new AbortController() + const controllerB = new AbortController() + const racedA = getModels({ provider: providerIdentifiers.openrouter, signal: controllerA.signal }) + const racedB = getModels({ provider: providerIdentifiers.openrouter, signal: controllerB.signal }) + + controllerA.abort() + controllerB.abort() + await expect(racedA).rejects.toMatchObject({ name: "AbortError" }) + await expect(racedB).rejects.toMatchObject({ name: "AbortError" }) + + // A fresh flight takes the slot while the doomed fetch is still pending... + mockGetOpenRouterModels.mockImplementation(() => { + return new Promise((resolve) => { + resolveSecond = resolve + }) + }) + const successor = getModels({ provider: providerIdentifiers.openrouter }) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) + // ...and only NOW the doomed fetch settles, queueing its guarded delete. + resolveRaced!(raceModels) + await drainMicrotasks() + + // The newer flight survived the stale delete: another caller still joins it (no 3rd fetch). + const lateJoiner = getModels({ provider: providerIdentifiers.openrouter }) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) + resolveSecond!(cancelledModelsB) + await expect(successor).resolves.toEqual(cancelledModelsB) + await expect(lateJoiner).resolves.toEqual(cancelledModelsB) + + // Order 2: settlement wins. The waiter resolves through the fetch; an abort dispatched + // afterwards finds the waiter already detached and changes nothing. + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + const lateController = new AbortController() + const settleFirst = getModels({ provider: providerIdentifiers.openrouter, signal: lateController.signal }) + await drainMicrotasks() + lateController.abort() + await expect(settleFirst).resolves.toEqual(cancelledModelsB) +}) + +it("leaves the fetch and the entry intact when one of two waiters aborts", async () => { + setupCancellationMocks() + let flightSignal: AbortSignal | undefined + let resolvePending: ((models: typeof cancelledModels) => void) | undefined + mockGetOpenRouterModels.mockImplementation((_options, opts) => { + flightSignal = opts?.signal + return new Promise((resolve) => { + resolvePending = resolve + }) + }) + + const controller = new AbortController() + const aborting = getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + const staying = getModels({ provider: providerIdentifiers.openrouter }) + + controller.abort() + await expect(aborting).rejects.toMatchObject({ name: "AbortError" }) + + // One waiter left: the shared fetch must neither be aborted nor evicted from the map. + expect(flightSignal?.aborted).toBe(false) + + // A third concurrent caller joins the SAME flight (not a fresh one) while two waiters remain. + const joining = getModels({ provider: providerIdentifiers.openrouter }) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + resolvePending!(cancelledModels) + await expect(staying).resolves.toEqual(cancelledModels) + await expect(joining).resolves.toEqual(cancelledModels) +}) + +it("aborts the shared fetch when the last waiter leaves", async () => { + setupCancellationMocks() + let flightSignal: AbortSignal | undefined + neverSettlingFetcher((signal) => { + flightSignal = signal + }) + + const first = new AbortController() + const second = new AbortController() + const waitA = getModels({ provider: providerIdentifiers.openrouter, signal: first.signal }) + const waitB = getModels({ provider: providerIdentifiers.openrouter, signal: second.signal }) + + first.abort() + expect(flightSignal?.aborted).toBe(false) + second.abort() + expect(flightSignal?.aborted).toBe(true) + + await Promise.all([ + expect(waitA).rejects.toMatchObject({ name: "AbortError" }), + expect(waitB).rejects.toMatchObject({ name: "AbortError" }), + ]) +}) + +it("keeps a sibling waiter running when a joiner aborts its own signal", async () => { + setupCancellationMocks() + let resolvePending: ((models: typeof cancelledModels) => void) | undefined + mockGetOpenRouterModels.mockReturnValue( + new Promise((resolve) => { + resolvePending = resolve + }), + ) + + const joiningController = new AbortController() + const siblingController = new AbortController() + const joiner = getModels({ provider: providerIdentifiers.openrouter, signal: joiningController.signal }) + const sibling = getModels({ provider: providerIdentifiers.openrouter, signal: siblingController.signal }) + + let siblingSettled = false + const tracked = sibling.then( + () => { + siblingSettled = true + }, + () => { + siblingSettled = true + }, + ) + + joiningController.abort() + await expect(joiner).rejects.toMatchObject({ name: "AbortError" }) + await drainMicrotasks() + expect(siblingSettled).toBe(false) + + resolvePending!(cancelledModels) + await tracked + expect(siblingSettled).toBe(true) +}) + +it("rejects every waiter and releases the entry when the flight timeout fires", async () => { + setupCancellationMocks() + const timeoutControllers: AbortController[] = [] + const timeoutSpy = vi.spyOn(AbortSignal, "timeout").mockImplementation(() => { + const timeoutController = new AbortController() + timeoutControllers.push(timeoutController) + return timeoutController.signal + }) + try { + let flightSignal: AbortSignal | undefined + neverSettlingFetcher((signal) => { + flightSignal = signal + }) + + const firstController = new AbortController() + const waitA = getModels({ provider: providerIdentifiers.openrouter, signal: firstController.signal }) + const waitB = getModels({ provider: providerIdentifiers.openrouter }) + + // The bound is owned by the single-flight entry, armed once at flight creation. + expect(timeoutSpy).toHaveBeenCalledTimes(1) + expect(timeoutSpy).toHaveBeenCalledWith(15_000) + + timeoutControllers[0].abort() + + await Promise.all([ + expect(waitA).rejects.toMatchObject({ name: "AbortError" }), + expect(waitB).rejects.toMatchObject({ name: "AbortError" }), + ]) + expect(flightSignal?.aborted).toBe(true) + + // Entry released on timeout: the next caller starts a fresh flight with its own timer. + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModelsB) + expect(timeoutSpy).toHaveBeenCalledTimes(2) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) + } finally { + timeoutSpy.mockRestore() + } +}) + +it("detaches the timeout and waiter listeners when the flight settles", async () => { + setupCancellationMocks() + const timeoutSignals: AbortSignal[] = [] + const timeoutSpy = vi.spyOn(AbortSignal, "timeout").mockImplementation(() => { + const timeoutController = new AbortController() + timeoutSignals.push(timeoutController.signal) + return timeoutController.signal + }) + try { + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModels) + // No caller signal: the waiter's abort view IS the timeout signal, so both the + // flight's timeout listener and the waiter's listener live on that one signal. + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModels) + await drainMicrotasks() + + // Nothing may keep observing the flight's bound after settlement: every abort listener + // registered on it was explicitly removed. + expect(getEventListeners(timeoutSignals[0], "abort")).toHaveLength(0) + } finally { + timeoutSpy.mockRestore() + } +}) + +it("treats a timeout and a caller abort firing back-to-back as idempotent detaches", async () => { + setupCancellationMocks() + const timeoutControllers: AbortController[] = [] + const timeoutSpy = vi.spyOn(AbortSignal, "timeout").mockImplementation(() => { + const timeoutController = new AbortController() + timeoutControllers.push(timeoutController) + return timeoutController.signal + }) + try { + neverSettlingFetcher(() => {}) + let rejectionsA = 0 + let rejectionsB = 0 + + const controllerA = new AbortController() + const controllerB = new AbortController() + const waitA = getModels({ provider: providerIdentifiers.openrouter, signal: controllerA.signal }) + const waitB = getModels({ provider: providerIdentifiers.openrouter, signal: controllerB.signal }) + waitA.catch(() => { + rejectionsA++ + }) + waitB.catch(() => { + rejectionsB++ + }) + + // Abort one waiter, then fire the flight's bound: the second event must find each + // waiter's detach already done (or idempotent) and never double-decrement the count. + controllerA.abort() + timeoutControllers[0].abort() + await drainMicrotasks() + + expect(rejectionsA).toBe(1) + expect(rejectionsB).toBe(1) + + // Entry state stayed consistent through both events: the next caller gets a fresh flight. + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModelsB) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) + } finally { + timeoutSpy.mockRestore() + } +}) + +it("makes an abort that arrives after settlement inert", async () => { + setupCancellationMocks() + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModels) + + const controller = new AbortController() + const waitPromise = getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + const result = await waitPromise + expect(result).toEqual(cancelledModels) + + controller.abort() + await drainMicrotasks() + + // The settled flight already removed its entry; the late abort must not resurrect or abort + // anything, and a later caller still starts a fresh fetch. + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModelsB) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) +}) + +it("rejects getModels but degrades refreshModels promptly when their fetch is aborted", async () => { + setupCancellationMocks() + neverSettlingFetcher(() => {}) + + // getModels() surfaces the abort as a rejection at abort time even though the fetch never + // settles on its own. + const getController = new AbortController() + const getPromise = getModels({ provider: providerIdentifiers.openrouter, signal: getController.signal }) + getController.abort() + await expect(getPromise).rejects.toMatchObject({ name: "AbortError" }) + + // refreshModels() keeps its graceful-degradation contract, but arrives at it promptly at + // abort time rather than waiting on the hung fetch. + const mockCache = vi.mocked(new (vi.mocked(NodeCache))()) + mockCache.get.mockReturnValue(cancelledModels) + const { refreshModels } = await import("../modelCache") + const refreshController = new AbortController() + const refreshPromise = refreshModels({ provider: providerIdentifiers.openrouter, signal: refreshController.signal }) + refreshController.abort() + await expect(refreshPromise).resolves.toEqual(cancelledModels) +}) + +it("releases the entry for a fetcher double that honors no cancellation at all", async () => { + setupCancellationMocks() + // Release-only double: ignores the forwarded signal entirely and never settles. + mockGetOpenRouterModels.mockImplementation(() => new Promise(() => {})) + + const controller = new AbortController() + const waiting = getModels({ provider: providerIdentifiers.openrouter, signal: controller.signal }) + controller.abort() + + // Waiter stopped and entry released despite the client exposing no cancellation surface. + await expect(waiting).rejects.toMatchObject({ name: "AbortError" }) + mockGetOpenRouterModels.mockResolvedValueOnce(cancelledModelsB) + await expect(getModels({ provider: providerIdentifiers.openrouter })).resolves.toEqual(cancelledModelsB) + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) +}) + +it("passes the caller signal straight through the auth-scoped bypass without entering the flight map", async () => { + setupCancellationMocks() + // The single-flight arms its per-flight fetch bound whenever it creates a flight; the + // auth-scoped bypass must never touch that machinery. + const boundSpy = vi.spyOn(AbortSignal, "timeout") + try { + mockGetZooGatewayModels + .mockImplementationOnce( + (_options, opts) => + new Promise((_resolve, reject) => { + opts?.signal?.addEventListener( + "abort", + () => { + const abortError = new Error("This operation was aborted") + abortError.name = "AbortError" + reject(abortError) + }, + { once: true }, + ) + }), + ) + .mockResolvedValueOnce(cancelledModelsB) + + const controller = new AbortController() + const first = getModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "token-a", + signal: controller.signal, + }) + // Auth isolation requires no dedup even for an identical provider+token: each call fires + // its own fetch, so the bypass never shares (or poisons) a flight with anything. + const second = getModels({ provider: providerIdentifiers.zooGateway, apiKey: "token-a" }) + expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) + expect(mockGetZooGatewayModels.mock.calls[0][1]?.signal).toBe(controller.signal) + expect(mockGetZooGatewayModels.mock.calls[1][1]).toBeUndefined() + + controller.abort() + await expect(first).rejects.toMatchObject({ name: "AbortError" }) + await expect(second).resolves.toEqual(cancelledModelsB) + expect(boundSpy).not.toHaveBeenCalled() + } finally { + boundSpy.mockRestore() + } +}) diff --git a/src/api/providers/fetchers/__tests__/moonshot.spec.ts b/src/api/providers/fetchers/__tests__/moonshot.spec.ts index 786c6dfbdf..a2b2bd704e 100644 --- a/src/api/providers/fetchers/__tests__/moonshot.spec.ts +++ b/src/api/providers/fetchers/__tests__/moonshot.spec.ts @@ -177,4 +177,34 @@ describe("getMoonshotModels", () => { "HTTP 500: Internal Server Error", ) }) + + it("passes the caller's abort signal to the request", async () => { + const fetchSpy = vi + .spyOn(globalThis, "fetch") + .mockResolvedValue(new Response(JSON.stringify({ data: [] }), { status: 200 })) + const controller = new AbortController() + + await getMoonshotModels("https://api.moonshot.ai/v1", "mock-key", { signal: controller.signal }) + + expect(fetchSpy).toHaveBeenCalledWith( + "https://api.moonshot.ai/v1/models", + expect.objectContaining({ signal: controller.signal }), + ) + }) + + it("rejects with the abort reason when the caller aborts a pending request", async () => { + const controller = new AbortController() + vi.spyOn(globalThis, "fetch").mockImplementation((_input, init) => { + // Mirror the HTTP client: a pending request rejects with the signal's + // abort reason when the signal fires. + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener("abort", () => reject(init.signal?.reason), { once: true }) + }) + }) + + const fetchPromise = getMoonshotModels("https://api.moonshot.ai/v1", "mock-key", { signal: controller.signal }) + controller.abort(new Error("caller canceled")) + + await expect(fetchPromise).rejects.toThrow("caller canceled") + }) }) diff --git a/src/api/providers/fetchers/__tests__/nanogpt.spec.ts b/src/api/providers/fetchers/__tests__/nanogpt.spec.ts index edbe8d1690..7985028d4d 100644 --- a/src/api/providers/fetchers/__tests__/nanogpt.spec.ts +++ b/src/api/providers/fetchers/__tests__/nanogpt.spec.ts @@ -14,7 +14,7 @@ describe("NanoGPT model fetcher", () => { await getNanoGptModels("key-a") expect(axios.get).toHaveBeenCalledWith(`${NANOGPT_BASE_URL}/models?detailed=true`, { headers: { Authorization: "Bearer key-a" }, - timeout: 10_000, + signal: undefined, }) }) @@ -23,10 +23,37 @@ describe("NanoGPT model fetcher", () => { await getNanoGptModels() expect(axios.get).toHaveBeenCalledWith(`${NANOGPT_BASE_URL}/models?detailed=true`, { headers: undefined, - timeout: 10_000, + signal: undefined, }) }) + it("forwards the caller's abort signal to the request", async () => { + vi.mocked(axios.get).mockResolvedValue({ data: { data: [] } }) + const controller = new AbortController() + + await getNanoGptModels("key-a", { signal: controller.signal }) + + expect(axios.get).toHaveBeenCalledWith(`${NANOGPT_BASE_URL}/models?detailed=true`, { + headers: { Authorization: "Bearer key-a" }, + signal: controller.signal, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + vi.mocked(axios.get).mockImplementation((_url, config) => { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getNanoGptModels("key-a", { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) + it("maps detailed metadata and exact per-million pricing for multiple models", async () => { vi.mocked(axios.get).mockResolvedValue({ data: { diff --git a/src/api/providers/fetchers/__tests__/ollama.test.ts b/src/api/providers/fetchers/__tests__/ollama.test.ts index 9c0b547e88..49be678f1e 100644 --- a/src/api/providers/fetchers/__tests__/ollama.test.ts +++ b/src/api/providers/fetchers/__tests__/ollama.test.ts @@ -485,5 +485,82 @@ describe("Ollama Fetcher", () => { consoleErrorSpy.mockRestore() }) + + it("should pass the caller's abort signal to both the list and per-model requests", async () => { + const baseUrl = "http://localhost:11434" + const controller = new AbortController() + + mockedAxios.get.mockResolvedValueOnce({ + data: { + models: [ + { + name: "test-model:latest", + model: "test-model:latest", + details: { family: "llama", parameter_size: "7B" }, + }, + ], + }, + }) + mockedAxios.post.mockResolvedValueOnce({ data: { details: { family: "llama" }, model_info: {} } }) + + await getOllamaModels(baseUrl, undefined, { signal: controller.signal }) + + expect(mockedAxios.get).toHaveBeenCalledWith(`${baseUrl}/api/tags`, { + headers: {}, + signal: controller.signal, + }) + expect(mockedAxios.post).toHaveBeenCalledWith( + `${baseUrl}/api/show`, + { model: "test-model:latest" }, + { headers: {}, signal: controller.signal }, + ) + }) + + it("should reject with an AbortError when the signal aborts the fan-out requests", async () => { + const baseUrl = "http://localhost:11434" + const controller = new AbortController() + + mockedAxios.get.mockResolvedValueOnce({ + data: { + models: [ + { + name: "model-a:latest", + model: "model-a:latest", + details: { family: "llama", parameter_size: "7B" }, + }, + { + name: "model-b:latest", + model: "model-b:latest", + details: { family: "llama", parameter_size: "7B" }, + }, + ], + }, + }) + mockedAxios.post.mockImplementation((_url: string, _body: unknown, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a request rejects when its signal fires, + // including when the signal was already aborted when the request started. + return new Promise((_resolve, reject) => { + if (config?.signal?.aborted) { + reject(new Error("canceled")) + return + } + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const consoleErrorSpy = vi.spyOn(console, "error").mockImplementation(function () {}) + + // The abort lands while the model list is still awaiting, so every + // per-model request is created already-aborted; the fan-out's per-model + // catch swallows those rejections, and the function-level abort guard + // must still turn the pending cancellation into a rejection rather + // than an (empty) success. + const fetchPromise = getOllamaModels(baseUrl, undefined, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + + consoleErrorSpy.mockRestore() + }) }) }) diff --git a/src/api/providers/fetchers/__tests__/opencode-go.spec.ts b/src/api/providers/fetchers/__tests__/opencode-go.spec.ts index 6b5b9ba37a..123e912f7f 100644 --- a/src/api/providers/fetchers/__tests__/opencode-go.spec.ts +++ b/src/api/providers/fetchers/__tests__/opencode-go.spec.ts @@ -35,7 +35,7 @@ describe("Opencode Go Fetchers", () => { expect(mockedAxios.get).toHaveBeenCalledWith("https://opencode.ai/zen/go/v1/models", { headers: { Authorization: "Bearer test-key" }, - timeout: 10_000, + signal: undefined, }) expect(Object.keys(models).sort()).toEqual(["deepseek-v4-pro", "glm-5.1"]) @@ -150,6 +150,33 @@ describe("Opencode Go Fetchers", () => { warnSpy.mockRestore() }) + + it("forwards the caller's abort signal to the request", async () => { + mockedAxios.get.mockResolvedValue({ data: { data: [] } }) + const controller = new AbortController() + + await getOpencodeGoModels("test-key", { signal: controller.signal }) + + expect(mockedAxios.get).toHaveBeenCalledWith("https://opencode.ai/zen/go/v1/models", { + headers: { Authorization: "Bearer test-key" }, + signal: controller.signal, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation((_url: string, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getOpencodeGoModels("k", { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) }) describe("parseOpencodeGoModel", () => { diff --git a/src/api/providers/fetchers/__tests__/openrouter.spec.ts b/src/api/providers/fetchers/__tests__/openrouter.spec.ts index 5d1f03bb0b..982d880a65 100644 --- a/src/api/providers/fetchers/__tests__/openrouter.spec.ts +++ b/src/api/providers/fetchers/__tests__/openrouter.spec.ts @@ -76,6 +76,50 @@ describe("OpenRouter API", () => { nockDone() }) + + it("passes the caller's abort signal to the catalog request", async () => { + const controller = new AbortController() + + const axios = await import("axios") + const getSpy = vi.spyOn(axios.default, "get").mockResolvedValue({ data: { data: [] } }) + + await getOpenRouterModels(undefined, { signal: controller.signal }) + + expect(getSpy).toHaveBeenCalledWith("https://openrouter.ai/api/v1/models", { signal: controller.signal }) + + getSpy.mockRestore() + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + + const axios = await import("axios") + const getSpy = vi.spyOn(axios.default, "get").mockImplementation((_url, config) => { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getOpenRouterModels(undefined, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + + getSpy.mockRestore() + }) + + it("requests the catalog without a signal when none is provided", async () => { + const axios = await import("axios") + const getSpy = vi.spyOn(axios.default, "get").mockResolvedValue({ data: { data: [] } }) + + const models = await getOpenRouterModels() + + expect(getSpy).toHaveBeenCalledWith("https://openrouter.ai/api/v1/models", { signal: undefined }) + expect(models).toEqual({}) + + getSpy.mockRestore() + }) }) describe("getOpenRouterModelEndpoints", () => { diff --git a/src/api/providers/fetchers/__tests__/poe.spec.ts b/src/api/providers/fetchers/__tests__/poe.spec.ts index 070caf0cc6..ae81cb8d6c 100644 --- a/src/api/providers/fetchers/__tests__/poe.spec.ts +++ b/src/api/providers/fetchers/__tests__/poe.spec.ts @@ -110,6 +110,28 @@ describe("getPoeModels", () => { }) }) + it("accepts an abort signal without changing results (SDK exposes no cancellation surface)", async () => { + mockFetchPoeModels.mockResolvedValue([]) + mockGetModels.mockReturnValue([ + { + id: "some-model", + rawId: "some-model", + contextWindow: 4096, + maxOutputTokens: 1024, + supportsImages: false, + supportsPromptCache: false, + }, + ]) + const controller = new AbortController() + + const models = await getPoeModels("key", undefined, { signal: controller.signal }) + + // The signal is accepted for interface parity; the Poe SDK call is + // issued unchanged because the client exposes no cancellation option. + expect(mockFetchPoeModels).toHaveBeenCalledWith({ apiKey: "key", baseURL: undefined }) + expect(models["some-model"]).toBeDefined() + }) + it("maps supportsReasoningEffort when present", async () => { mockFetchPoeModels.mockResolvedValue([]) mockGetModels.mockReturnValue([ diff --git a/src/api/providers/fetchers/__tests__/requesty.spec.ts b/src/api/providers/fetchers/__tests__/requesty.spec.ts index 53891273a9..8d8e47bf9b 100644 --- a/src/api/providers/fetchers/__tests__/requesty.spec.ts +++ b/src/api/providers/fetchers/__tests__/requesty.spec.ts @@ -140,4 +140,36 @@ describe("getRequestyModels", () => { expect(sonnet.supportsReasoningBinary).toBeUndefined() expect(sonnet.supportsTemperature).toBeUndefined() }) + + it("passes the caller's abort signal to the catalog request", async () => { + const controller = new AbortController() + mockAxiosGet.mockResolvedValueOnce({ data: { data: [] } }) + + await getRequestyModels(undefined, undefined, { signal: controller.signal }) + + expect(mockAxiosGet).toHaveBeenCalledWith("https://router.requesty.ai/v1/models", { + headers: {}, + signal: controller.signal, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockAxiosGet.mockImplementation((_url, config) => { + // Mirror the HTTP client: a request rejects when its signal fires, + // including when the signal was already aborted when the request started. + return new Promise((_resolve, reject) => { + if (config?.signal?.aborted) { + reject(new Error("canceled")) + return + } + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getRequestyModels(undefined, undefined, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) }) diff --git a/src/api/providers/fetchers/__tests__/unbound.spec.ts b/src/api/providers/fetchers/__tests__/unbound.spec.ts new file mode 100644 index 0000000000..442673990d --- /dev/null +++ b/src/api/providers/fetchers/__tests__/unbound.spec.ts @@ -0,0 +1,40 @@ +// npx vitest run api/providers/fetchers/__tests__/unbound.spec.ts + +import axios from "axios" + +import { getUnboundModels } from "../unbound" + +vi.mock("axios") +const mockAxiosGet = vi.mocked(axios.get) + +it("passes the caller's abort signal to the catalog request", async () => { + const controller = new AbortController() + mockAxiosGet.mockResolvedValueOnce({ data: [] }) + + await getUnboundModels("test-api-key", { signal: controller.signal }) + + expect(mockAxiosGet).toHaveBeenCalledWith("https://api.getunbound.ai/models", { + headers: { Authorization: "Bearer test-api-key" }, + signal: controller.signal, + }) +}) + +it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockAxiosGet.mockImplementation((_url, config) => { + // Mirror the HTTP client: a request rejects when its signal fires, + // including when the signal was already aborted when the request started. + return new Promise((_resolve, reject) => { + if (config?.signal?.aborted) { + reject(new Error("canceled")) + return + } + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getUnboundModels(undefined, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) +}) diff --git a/src/api/providers/fetchers/__tests__/vercel-ai-gateway.spec.ts b/src/api/providers/fetchers/__tests__/vercel-ai-gateway.spec.ts index 6815cdd7eb..51a4a5000b 100644 --- a/src/api/providers/fetchers/__tests__/vercel-ai-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/vercel-ai-gateway.spec.ts @@ -77,7 +77,9 @@ describe("Vercel AI Gateway Fetchers", () => { const models = await getVercelAiGatewayModels() - expect(mockedAxios.get).toHaveBeenCalledWith("https://ai-gateway.vercel.sh/v1/models") + expect(mockedAxios.get).toHaveBeenCalledWith("https://ai-gateway.vercel.sh/v1/models", { + signal: undefined, + }) expect(Object.keys(models)).toHaveLength(2) // Only language models expect(models["anthropic/claude-sonnet-4"]).toBeDefined() expect(models["anthropic/claude-3.5-haiku"]).toBeDefined() @@ -112,6 +114,37 @@ describe("Vercel AI Gateway Fetchers", () => { consoleErrorSpy.mockRestore() }) + it("passes the caller's abort signal to the catalog request", async () => { + const controller = new AbortController() + mockedAxios.get.mockResolvedValueOnce({ data: { object: "list", data: [] } }) + + await getVercelAiGatewayModels(undefined, { signal: controller.signal }) + + expect(mockedAxios.get).toHaveBeenCalledWith("https://ai-gateway.vercel.sh/v1/models", { + signal: controller.signal, + }) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation((_url: string, config?: { signal?: AbortSignal }) => { + // Mirror the HTTP client: a request rejects when its signal fires, + // including when the signal was already aborted when the request started. + return new Promise((_resolve, reject) => { + if (config?.signal?.aborted) { + reject(new Error("canceled")) + return + } + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getVercelAiGatewayModels(undefined, { signal: controller.signal }) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) + it("continues processing with partially valid schema", async () => { const consoleErrorSpy = vitest.spyOn(console, "error").mockImplementation(function () {}) const invalidResponse = { diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index ae9bdcc4b1..e7c766fb52 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -156,6 +156,46 @@ describe("Zoo Gateway Fetchers", () => { expect(consoleErrorSpy).toHaveBeenCalled() consoleErrorSpy.mockRestore() }) + + it("forwards the caller's abort signal alongside the retained timeout", async () => { + mockedAxios.get.mockResolvedValueOnce(mockResponse) + const controller = new AbortController() + + await getZooGatewayModels( + { zooGatewayBaseUrl: baseUrl, zooSessionToken: token }, + { + signal: controller.signal, + }, + ) + + expect(mockedAxios.get).toHaveBeenCalledWith( + `${baseUrl}/models`, + expect.objectContaining({ + timeout: expect.any(Number), + signal: controller.signal, + }), + ) + }) + + it("rejects with an AbortError when the signal aborts the pending request", async () => { + const controller = new AbortController() + mockedAxios.get.mockImplementation(function (_url: string, config?: { signal?: AbortSignal }) { + // Mirror the HTTP client: a pending request rejects when its signal fires. + return new Promise((_resolve, reject) => { + config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) + }) + }) + + const fetchPromise = getZooGatewayModels( + { zooGatewayBaseUrl: baseUrl, zooSessionToken: token }, + { + signal: controller.signal, + }, + ) + controller.abort() + + await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) + }) }) describe("parseZooGatewayModel", () => { diff --git a/src/api/providers/fetchers/deepseek.ts b/src/api/providers/fetchers/deepseek.ts index 106d77969a..1b7722e033 100644 --- a/src/api/providers/fetchers/deepseek.ts +++ b/src/api/providers/fetchers/deepseek.ts @@ -2,6 +2,7 @@ import type { ModelRecord } from "@roo-code/types" import { deepSeekModels, DEEP_SEEK_DEFAULT_TEMPERATURE } from "@roo-code/types" import { DEFAULT_HEADERS } from "../constants" +import { throwIfAborted } from "../utils/abort-signal" /** * Fetches available models from the DeepSeek API and merges them with known specs. @@ -10,7 +11,11 @@ import { DEFAULT_HEADERS } from "../constants" * or context window info, so we merge the API response with the static * `deepSeekModels` map for known models. Unknown models get sensible defaults. */ -export async function getDeepSeekModels(baseUrl?: string, apiKey?: string): Promise { +export async function getDeepSeekModels( + baseUrl?: string, + apiKey?: string, + opts?: { signal?: AbortSignal }, +): Promise { const normalizedBase = (baseUrl || "https://api.deepseek.com").replace(/\/?v1\/?$/, "") const url = `${normalizedBase}/models` const allowModelListFallback = process.env.E2E_MOCK_MODEL_LIST_FALLBACK === "true" @@ -24,79 +29,77 @@ export async function getDeepSeekModels(baseUrl?: string, apiKey?: string): Prom headers["Authorization"] = `Bearer ${apiKey}` } - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), 10000) + const response = await fetch(url, { + headers, + signal: opts?.signal, + }) + + if (!response.ok) { + // An aborted request must reject instead of taking the static-models + // fallback below: that would present a cancelled fetch to callers as a + // usable catalog. + throwIfAborted(opts?.signal) + + let errorBody = "" + try { + errorBody = await response.text() + } catch { + errorBody = "(unable to read response body)" + } - try { - const response = await fetch(url, { - headers, - signal: controller.signal, + console.error(`[getDeepSeekModels] HTTP error:`, { + status: response.status, + statusText: response.statusText, + url, + body: errorBody, }) - if (!response.ok) { - let errorBody = "" - try { - errorBody = await response.text() - } catch { - errorBody = "(unable to read response body)" - } - - console.error(`[getDeepSeekModels] HTTP error:`, { - status: response.status, - statusText: response.statusText, - url, - body: errorBody, - }) - - // In mocked e2e environments, /models may be intentionally unimplemented. - // Allow an explicit test-only fallback to static DeepSeek model metadata. - if (allowModelListFallback && response.status === 404) { - const models: ModelRecord = Object.create(null) - for (const [modelId, modelInfo] of Object.entries(deepSeekModels)) { - models[modelId] = { ...modelInfo } - } - return models + // In mocked e2e environments, /models may be intentionally unimplemented. + // Allow an explicit test-only fallback to static DeepSeek model metadata. + if (allowModelListFallback && response.status === 404) { + const models: ModelRecord = Object.create(null) + for (const [modelId, modelInfo] of Object.entries(deepSeekModels)) { + models[modelId] = { ...modelInfo } } - - throw new Error(`HTTP ${response.status}: ${response.statusText}`) + return models } - const data = await response.json() + throw new Error(`HTTP ${response.status}: ${response.statusText}`) + } + + const data = await response.json() - if (!data?.data || !Array.isArray(data.data)) { - console.error("[getDeepSeekModels] Unexpected response format:", data) - throw new Error("Failed to fetch DeepSeek models: Unexpected response format.") - } + if (!data?.data || !Array.isArray(data.data)) { + console.error("[getDeepSeekModels] Unexpected response format:", data) + throw new Error("Failed to fetch DeepSeek models: Unexpected response format.") + } - // Use null-prototype object to prevent prototype pollution - const models: ModelRecord = Object.create(null) - - for (const model of data.data) { - const modelId = typeof model.id === "string" && model.id ? model.id : null - if (!modelId) continue - - const knownSpecs = deepSeekModels[modelId as keyof typeof deepSeekModels] - - if (knownSpecs) { - models[modelId] = { ...knownSpecs } - } else { - models[modelId] = { - maxTokens: 8192, - contextWindow: 128_000, - supportsImages: false, - supportsPromptCache: true, - inputPrice: 0.28, - outputPrice: 0.42, - cacheWritesPrice: 0.28, - cacheReadsPrice: 0.028, - defaultTemperature: DEEP_SEEK_DEFAULT_TEMPERATURE, - description: `DeepSeek model: ${modelId}`, - } + // Use null-prototype object to prevent prototype pollution + const models: ModelRecord = Object.create(null) + + for (const model of data.data) { + const modelId = typeof model.id === "string" && model.id ? model.id : null + if (!modelId) continue + + const knownSpecs = deepSeekModels[modelId as keyof typeof deepSeekModels] + + if (knownSpecs) { + models[modelId] = { ...knownSpecs } + } else { + models[modelId] = { + maxTokens: 8192, + contextWindow: 128_000, + supportsImages: false, + supportsPromptCache: true, + inputPrice: 0.28, + outputPrice: 0.42, + cacheWritesPrice: 0.28, + cacheReadsPrice: 0.028, + defaultTemperature: DEEP_SEEK_DEFAULT_TEMPERATURE, + description: `DeepSeek model: ${modelId}`, } } - - return models - } finally { - clearTimeout(timeoutId) } + + return models } diff --git a/src/api/providers/fetchers/kenari.ts b/src/api/providers/fetchers/kenari.ts index b0c69c4efa..c73469dd97 100644 --- a/src/api/providers/fetchers/kenari.ts +++ b/src/api/providers/fetchers/kenari.ts @@ -4,6 +4,8 @@ import { z } from "zod" import type { ModelInfo } from "@roo-code/types" import { kenariDefaultModelInfo, KENARI_BASE_URL } from "@roo-code/types" +import { throwIfAborted } from "../utils/abort-signal" + // The Kenari `/models` endpoint follows the OpenAI `/models` shape and is // public (no key required). The `id` is the only guaranteed field; metadata is // optional and best-effort, so the schema is intentionally permissive. @@ -61,15 +63,19 @@ export const parseKenariModel = (model: KenariModel): ModelInfo => ({ * * @param apiKey - Optional Bearer token; the endpoint is public but the key is * sent when available. + * @param opts - Optional per-request controls; `signal` cancels the in-flight request. * @returns A record mapping model IDs to their normalised {@link ModelInfo}. */ -export async function getKenariModels(apiKey?: string): Promise> { +export async function getKenariModels( + apiKey?: string, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} try { const response = await axios.get(`${KENARI_BASE_URL}/models`, { headers: apiKey ? { Authorization: `Bearer ${apiKey}` } : undefined, - timeout: 10_000, + signal: opts?.signal, }) const result = kenariModelsResponseSchema.safeParse(response.data) @@ -91,6 +97,10 @@ export async function getKenariModels(apiKey?: string): Promise): Mo } } -export async function getKimiCodeModels(apiKey?: string): Promise { +export async function getKimiCodeModels(apiKey?: string, opts?: { signal?: AbortSignal }): Promise { if (!apiKey) throw new Error("Kimi Code authentication is required to fetch models") + // This auth-scoped fetch bypasses the model-cache single-flight, so the + // entry-level timeout never covers it; the bound stays local. The deadline + // fires through an explicit controller (not AbortSignal.timeout) so the + // rejection keeps this exact Error identity; the caller's signal, when + // present, aborts alongside it via AbortSignal.any. const controller = new AbortController() const timeout = setTimeout( () => controller.abort(new Error("Kimi Code models request timed out")), @@ -47,7 +54,7 @@ export async function getKimiCodeModels(apiKey?: string): Promise { try { const response = await fetch(`${KIMI_CODE_BASE_URL}/models`, { headers: { Accept: "application/json", Authorization: `Bearer ${apiKey}` }, - signal: controller.signal, + signal: mergeAbortSignals(controller.signal, opts?.signal), }) if (!response.ok) { const error = new Error(`Kimi Code models request failed: ${response.status} ${response.statusText}`) diff --git a/src/api/providers/fetchers/litellm.ts b/src/api/providers/fetchers/litellm.ts index 7ebb754ed6..c98c36c914 100644 --- a/src/api/providers/fetchers/litellm.ts +++ b/src/api/providers/fetchers/litellm.ts @@ -4,15 +4,21 @@ import type { ModelRecord } from "@roo-code/types" import { isLiteLLMPreserveReasoningModel } from "@roo-code/types" import { DEFAULT_HEADERS } from "../constants" +import { throwIfAborted } from "../utils/abort-signal" /** * Fetches available models from a LiteLLM server * * @param apiKey The API key for the LiteLLM server * @param baseUrl The base URL of the LiteLLM server + * @param opts Optional per-request controls; `signal` cancels the in-flight request. * @returns A promise that resolves to a record of model IDs to model info * @throws Will throw an error if the request fails or the response is not as expected. */ -export async function getLiteLLMModels(apiKey: string, baseUrl: string): Promise { +export async function getLiteLLMModels( + apiKey: string, + baseUrl: string, + opts?: { signal?: AbortSignal }, +): Promise { try { const headers: Record = { "Content-Type": "application/json", @@ -28,8 +34,7 @@ export async function getLiteLLMModels(apiKey: string, baseUrl: string): Promise // Normalize the pathname by removing trailing slashes and multiple slashes urlObj.pathname = urlObj.pathname.replace(/\/+$/, "").replace(/\/+/g, "/") + "/v1/model/info" const url = urlObj.href - // Added timeout to prevent indefinite hanging - const response = await axios.get(url, { headers, timeout: 5000 }) + const response = await axios.get(url, { headers, signal: opts?.signal }) const models: ModelRecord = {} // Process the model info from the response @@ -90,6 +95,11 @@ export async function getLiteLLMModels(apiKey: string, baseUrl: string): Promise return models } catch (error: any) { + // Surface cancellation as a plain AbortError: wrapping it in the + // "Failed to fetch" messages below would hide the abort from callers + // that discriminate on the error name. + throwIfAborted(opts?.signal) + console.error("Error fetching LiteLLM models:", error.message ? error.message : error) if (axios.isAxiosError(error) && error.response) { throw new Error( diff --git a/src/api/providers/fetchers/lmstudio.ts b/src/api/providers/fetchers/lmstudio.ts index 842fe9d08d..32387e2f08 100644 --- a/src/api/providers/fetchers/lmstudio.ts +++ b/src/api/providers/fetchers/lmstudio.ts @@ -3,6 +3,8 @@ import { LLM, LLMInfo, LLMInstanceInfo, LMStudioClient } from "@lmstudio/sdk" import { type ModelInfo, lMStudioDefaultModelInfo, providerIdentifiers } from "@roo-code/types" +import { throwIfAborted } from "../utils/abort-signal" + import { flushModels, getModels } from "./modelCache" const modelsWithLoadedDetails = new Set() @@ -49,7 +51,10 @@ export const parseLMStudioModel = (rawModel: LLMInstanceInfo | LLMInfo): ModelIn return modelInfo } -export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Promise> { +export async function getLMStudioModels( + baseUrl = "http://localhost:1234", + opts?: { signal?: AbortSignal }, +): Promise> { // clear the set of models that have full details loaded modelsWithLoadedDetails.clear() // clearing the input can leave an empty string; use the default in that case @@ -66,8 +71,11 @@ export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Prom // test the connection to LM Studio first // errors will be caught further down - await axios.get(`${baseUrl}/v1/models`) + await axios.get(`${baseUrl}/v1/models`, { signal: opts?.signal }) + // The SDK's model-list calls expose no cancellation option, so an abort + // during them cannot reach the network; releasing the shared cache entry + // and stopping the waiters happens at the model-cache layer. const client = new LMStudioClient({ baseUrl: lmsUrl }) // First, try to get all downloaded models @@ -116,6 +124,10 @@ export async function getLMStudioModels(baseUrl = "http://localhost:1234"): Prom modelsWithLoadedDetails.add(lmstudioModel.modelKey) } } catch (error) { + // An aborted connection probe must reject instead of falling through to + // the empty-catalog handling below. + throwIfAborted(opts?.signal) + if (error.code === "ECONNREFUSED") { console.warn(`Error connecting to LMStudio at ${baseUrl}`) } else { diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 50dbe12f6e..b29011c818 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -17,6 +17,8 @@ import { getCacheDirectoryPath } from "../../../utils/storage" import type { RouterName } from "../../../shared/api" import { fileExistsAtPath } from "../../../utils/fs" +import { mergeAbortSignals, throwIfAborted } from "../utils/abort-signal" + import { getOpenRouterModels } from "./openrouter" import { getVercelAiGatewayModels } from "./vercel-ai-gateway" import { getOpencodeGoModels } from "./opencode-go" @@ -42,7 +44,34 @@ const modelRecordSchema = z.record(z.string(), modelInfoSchema) // Track in-flight refresh requests to prevent concurrent API calls for the same provider+url. // Keyed on the compound cache key (see getCacheKey) so that two different URL-scoped servers never // deduplicate each other's in-flight refreshes. -const inFlightRefresh = new Map>() +const inFlightRefresh = new Map() + +// Upper bound for any fetch started through the single-flight, so a hung endpoint can never keep +// an in-flight entry pending indefinitely. The value is the maximum of the 5–15 s bounds the +// individual fetchers it subsumes used to apply, relaxing rather than tightening endpoints that +// already had a bound. +const MODEL_CATALOG_FETCH_TIMEOUT_MS = 15_000 + +/** + * State of one shared (single-flight) provider fetch. + * + * Cancellation invariants this record upholds: + * - The internal AbortController is the only object that may cancel the flight's network + * request; caller signals are only ever merged into a per-waiter abort view, so one waiter + * aborting can never cancel the fetch out from under the others. + * - When the last waiter detaches while the fetch is still pending, the flight is aborted and + * its map entry removed synchronously, so a caller arriving immediately afterwards starts a + * fresh fetch instead of joining a doomed one. + * - Settlement never stores data in the map: it only removes the entry it created, guarded by + * flight identity so a late-settling stale flight can never evict a newer one. + */ +type FlightRecord = { + promise: Promise + controller: AbortController + timeoutSignal: AbortSignal + waiters: number + pending: boolean +} // Cache keys (see getCacheKey) for which we've already reported an empty model response this // session. A persistently-empty endpoint (e.g. misconfigured server) would otherwise re-fire this @@ -219,59 +248,70 @@ async function readModels(cacheKey: string): Promise { * Extracted to avoid duplication between getModels() and refreshModels(). * * @param options - Provider options for fetching models + * @param signal - Cancellation signal for this fetch: the shared flight's internal controller + * signal when routed through dedupedFetch(), or the caller's own signal on the auth-scoped + * direct path (which never enters the single-flight). * @returns Fresh models from the provider API */ -async function fetchModelsFromProvider(options: GetModelsOptions): Promise { +async function fetchModelsFromProvider(options: GetModelsOptions, signal?: AbortSignal): Promise { const { provider } = options + // Fetchers read the carrier through `opts?.signal`. Spread (rather than passing a possibly + // undefined positional argument) so a signal-less call keeps exactly its old arity: fetchers + // and their tests can distinguish "no third argument" from "third argument undefined". + const fetchOpts: [] | [{ signal: AbortSignal }] = signal ? [{ signal }] : [] + let models: ModelRecord switch (provider) { case providerIdentifiers.openrouter: - models = await getOpenRouterModels() + models = await getOpenRouterModels(undefined, ...fetchOpts) break case providerIdentifiers.requesty: // Requesty models endpoint requires an API key for per-user custom policies. - models = await getRequestyModels(options.baseUrl, options.apiKey) + models = await getRequestyModels(options.baseUrl, options.apiKey, ...fetchOpts) break case providerIdentifiers.unbound: - models = await getUnboundModels(options.apiKey) + models = await getUnboundModels(options.apiKey, ...fetchOpts) break case providerIdentifiers.litellm: - models = await getLiteLLMModels(options.apiKey ?? "", options.baseUrl) + models = await getLiteLLMModels(options.apiKey ?? "", options.baseUrl, ...fetchOpts) break case providerIdentifiers.ollama: - models = await getOllamaModels(options.baseUrl, options.apiKey) + models = await getOllamaModels(options.baseUrl, options.apiKey, ...fetchOpts) break case providerIdentifiers.lmstudio: - models = await getLMStudioModels(options.baseUrl) + models = await getLMStudioModels(options.baseUrl, ...fetchOpts) break case providerIdentifiers.vercelAiGateway: - models = await getVercelAiGatewayModels() + models = await getVercelAiGatewayModels(undefined, ...fetchOpts) break case providerIdentifiers.opencodeGo: - models = await getOpencodeGoModels(options.apiKey) + models = await getOpencodeGoModels(options.apiKey, ...fetchOpts) break case providerIdentifiers.kenari: - models = await getKenariModels(options.apiKey) + models = await getKenariModels(options.apiKey, ...fetchOpts) break case providerIdentifiers.nanogpt: - models = await getNanoGptModels(options.apiKey) + models = await getNanoGptModels(options.apiKey, ...fetchOpts) break case providerIdentifiers.poe: - models = await getPoeModels(options.apiKey, options.baseUrl) + models = await getPoeModels(options.apiKey, options.baseUrl, ...fetchOpts) break case providerIdentifiers.deepseek: - models = await getDeepSeekModels(options.baseUrl, options.apiKey) + models = await getDeepSeekModels(options.baseUrl, options.apiKey, ...fetchOpts) break case providerIdentifiers.moonshot: - models = await getMoonshotModels(options.baseUrl, options.apiKey) + models = await getMoonshotModels(options.baseUrl, options.apiKey, ...fetchOpts) break case providerIdentifiers.zooGateway: - models = await getZooGatewayModels({ zooSessionToken: options.apiKey, zooGatewayBaseUrl: options.baseUrl }) + models = await getZooGatewayModels( + { zooSessionToken: options.apiKey, zooGatewayBaseUrl: options.baseUrl }, + ...fetchOpts, + ) break case providerIdentifiers.kimiCode: - models = await getKimiCodeModels(options.apiKey) + models = await getKimiCodeModels(options.apiKey, ...fetchOpts) break default: { // Ensures router is exhaustively checked if RouterName is a strict union. @@ -318,9 +358,11 @@ export const getModels = async (options: GetModelsOptions): Promise // refreshModels() degrades to cached data doesn't surface as a silent stale result to // getModels(), and a fetch failure joined from refreshModels() still re-throws for // getModels() callers. - const sharedFetch = shouldSkipCache ? fetchModelsFromProvider(options) : dedupedFetch(cacheKey, options) - try { + const sharedFetch = shouldSkipCache + ? fetchModelsFromProvider(options, options.signal) + : dedupedFetch(cacheKey, options) + const fetched = await sharedFetch const modelCount = Object.keys(fetched).length @@ -361,22 +403,134 @@ export const getModels = async (options: GetModelsOptions): Promise * cache key at a time. */ function dedupedFetch(cacheKey: string, options: GetModelsOptions): Promise { - const existingRequest = inFlightRefresh.get(cacheKey) - if (existingRequest) { - return existingRequest + // A pre-aborted caller fails fast before any flight is created or joined: an aborted call + // must never start (or extend) a shared fetch. + throwIfAborted(options.signal) + + const existingRecord = inFlightRefresh.get(cacheKey) + if (existingRecord) { + return joinFlight(cacheKey, existingRecord, options.signal) } - const fetchPromise = fetchModelsFromProvider(options).finally(() => { - inFlightRefresh.delete(cacheKey) - }) + const controller = new AbortController() + const timeoutSignal = AbortSignal.timeout(MODEL_CATALOG_FETCH_TIMEOUT_MS) + const onTimeout = () => controller.abort(timeoutSignal.reason) + timeoutSignal.addEventListener("abort", onTimeout, { once: true }) + const removeTimeoutListener = () => timeoutSignal.removeEventListener("abort", onTimeout) + + // Settlement and a last-waiter abort may happen in either order; both paths are idempotent, + // and the identity guard makes the two interleavings equivalent. + let settled = false + const guardedDelete = () => { + // Identity guard: only remove this flight's own entry. A late-settling flight that lost + // its slot must never evict the fresh flight that replaced it. + if (inFlightRefresh.get(cacheKey) === record) { + inFlightRefresh.delete(cacheKey) + } + } + + const promise: Promise = fetchModelsFromProvider(options, controller.signal) + .then((models) => { + // Settlement never writes data into the map -- fetched data reaches callers only + // through the promise they awaited -- it only removes this flight's entry. + settled = true + removeTimeoutListener() + guardedDelete() + return models + }) + .catch((error: unknown) => { + settled = true + removeTimeoutListener() + guardedDelete() + // Re-throw so every still-joined waiter's awaited chain rejects. Waiters attach + // handlers to this promise at join time (and a settling flight detaches them), so a + // rejection here always has an observer and can never surface unhandled. + throw error + }) + + // A released flight (all waiters gone) can still reject later when its fetch observes the + // cancellation. This terminal observer keeps that rejection from surfacing as an unhandled + // rejection; joined waiters observe the identical rejection through their own race. + void promise.catch(() => {}) + + const record: FlightRecord = { + promise, + controller, + timeoutSignal, + waiters: 0, + get pending() { + return !settled + }, + } - // The finally cleanup above can only run after this function's current synchronous run -- + // The settle reactions above can only run after this function's current synchronous run -- // including the set() below -- completes, since that's the earliest a promise reaction can - // fire. So the entry is always registered before finally can delete it, even if - // fetchModelsFromProvider() resolves immediately. - inFlightRefresh.set(cacheKey, fetchPromise) + // fire. So the entry is always registered before any settle handler can delete it, even if + // fetchModelsFromProvider() settles immediately. + inFlightRefresh.set(cacheKey, record) - return fetchPromise + return joinFlight(cacheKey, record, options.signal) +} + +/** + * Attach one waiter to an existing flight. Each waiter counts itself in record.waiters and + * carries a view signal that fires at the earlier of the flight's bound or its own caller's + * abort -- the view governs only this waiter's wait, never the network. When the last waiter + * detaches while the fetch is still pending, the flight is aborted and released synchronously, + * so a caller arriving afterwards provably starts a fresh flight. + */ +function joinFlight(cacheKey: string, record: FlightRecord, callerSignal?: AbortSignal): Promise { + throwIfAborted(callerSignal) + + record.waiters++ + let detached = false + let removeViewListener: (() => void) | undefined + + const detach = () => { + if (detached) { + return + } + detached = true + removeViewListener?.() + record.waiters-- + if (record.waiters === 0 && record.pending) { + // Synchronous release: abort the shared fetch and drop the entry before any further + // await point runs, so a late joiner never sees a doomed flight. + record.controller.abort() + if (inFlightRefresh.get(cacheKey) === record) { + inFlightRefresh.delete(cacheKey) + } + } + } + + // Per-waiter abort view: fires at the earlier of the flight's timeout bound and this + // caller's own signal. It governs only this waiter's detach — never the network — so one + // waiter aborting cannot cancel the shared fetch or its siblings' waits. + const view = mergeAbortSignals(record.timeoutSignal, callerSignal) + + const cancelled = new Promise((_resolve, reject) => { + const rejectAbort = () => { + // Detach inside the abort event itself, not in a later microtask: the last waiter's + // abort must release the entry synchronously, so a caller issuing a new fetch right + // after abort() provably observes a fresh flight, not the doomed one. + detach() + const abortError = new Error("This operation was aborted") + abortError.name = "AbortError" + reject(abortError) + } + if (view.aborted) { + rejectAbort() + return + } + view.addEventListener("abort", rejectAbort, { once: true }) + removeViewListener = () => view.removeEventListener("abort", rejectAbort) + }) + + // The settle hook detaches this waiter once the flight settles, so an abort arriving after + // settlement is inert and no listener survives the flight. + void record.promise.then(detach, detach) + + return Promise.race([record.promise, cancelled]).finally(detach) } /** @@ -402,9 +556,13 @@ export const refreshModels = async (options: GetModelsOptions): Promise { +export async function getMoonshotModels( + baseUrl?: string, + apiKey?: string, + opts?: { signal?: AbortSignal }, +): Promise { // Moonshot API uses OpenAI-compatible /v1/models endpoint. // The base URL from settings already includes /v1 (e.g. https://api.moonshot.ai/v1), // so we keep it as-is and append /models directly. @@ -26,64 +30,57 @@ export async function getMoonshotModels(baseUrl?: string, apiKey?: string): Prom headers["Authorization"] = `Bearer ${apiKey}` } - const controller = new AbortController() - const timeoutId = setTimeout(() => controller.abort(), 10000) + const response = await fetch(url, { + headers, + signal: opts?.signal, + }) + + if (!response.ok) { + let errorBody = "" + try { + errorBody = await response.text() + } catch { + errorBody = "(unable to read response body)" + } - try { - const response = await fetch(url, { - headers, - signal: controller.signal, + console.error(`[getMoonshotModels] HTTP error:`, { + status: response.status, + statusText: response.statusText, + url, + body: errorBody, }) - if (!response.ok) { - let errorBody = "" - try { - errorBody = await response.text() - } catch { - errorBody = "(unable to read response body)" - } - - console.error(`[getMoonshotModels] HTTP error:`, { - status: response.status, - statusText: response.statusText, - url, - body: errorBody, - }) - - throw new Error(`HTTP ${response.status}: ${response.statusText}`) - } + throw new Error(`HTTP ${response.status}: ${response.statusText}`) + } - const data = await response.json() + const data = await response.json() - if (!data?.data || !Array.isArray(data.data)) { - console.error("[getMoonshotModels] Unexpected response format:", data) - throw new Error("Failed to fetch Moonshot models: Unexpected response format.") - } + if (!data?.data || !Array.isArray(data.data)) { + console.error("[getMoonshotModels] Unexpected response format:", data) + throw new Error("Failed to fetch Moonshot models: Unexpected response format.") + } - // Use null-prototype object to prevent prototype pollution - const models: ModelRecord = Object.create(null) + // Use null-prototype object to prevent prototype pollution + const models: ModelRecord = Object.create(null) - for (const model of data.data) { - const modelId = typeof model.id === "string" && model.id ? model.id : null - if (!modelId) continue + for (const model of data.data) { + const modelId = typeof model.id === "string" && model.id ? model.id : null + if (!modelId) continue - const knownSpecs = moonshotModels[modelId as keyof typeof moonshotModels] + const knownSpecs = moonshotModels[modelId as keyof typeof moonshotModels] - if (knownSpecs) { - models[modelId] = { ...knownSpecs } - } else { - models[modelId] = { - maxTokens: 16_000, - contextWindow: 262_144, - supportsImages: false, - supportsPromptCache: true, - description: `Moonshot model: ${modelId}`, - } + if (knownSpecs) { + models[modelId] = { ...knownSpecs } + } else { + models[modelId] = { + maxTokens: 16_000, + contextWindow: 262_144, + supportsImages: false, + supportsPromptCache: true, + description: `Moonshot model: ${modelId}`, } } - - return models - } finally { - clearTimeout(timeoutId) } + + return models } diff --git a/src/api/providers/fetchers/nanogpt.ts b/src/api/providers/fetchers/nanogpt.ts index 1098039072..42d07b2036 100644 --- a/src/api/providers/fetchers/nanogpt.ts +++ b/src/api/providers/fetchers/nanogpt.ts @@ -3,6 +3,8 @@ import { z } from "zod" import { NANOGPT_BASE_URL, nanoGptDefaultModelInfo, type ModelInfo, type ModelRecord } from "@roo-code/types" +import { throwIfAborted } from "../utils/abort-signal" + const nanoGptReasoningEfforts: NonNullable = ["low", "medium", "high"] const nanoGptReasoningEffortSchema = z.enum(["none", "minimal", "low", "medium", "high", "xhigh", "max"]) const nanoGptAstraModelIds = new Set(["openai/gpt-6-astra", "openai/gpt-6-astra-pro"]) @@ -75,11 +77,11 @@ export const parseNanoGptModel = (model: NanoGptModel): ModelInfo => ({ }) /** Fetches NanoGPT's public detailed catalog, optionally scoped by a Bearer key. */ -export async function getNanoGptModels(apiKey?: string): Promise { +export async function getNanoGptModels(apiKey?: string, opts?: { signal?: AbortSignal }): Promise { try { const response = await axios.get(`${NANOGPT_BASE_URL}/models?detailed=true`, { headers: apiKey ? { Authorization: `Bearer ${apiKey}` } : undefined, - timeout: 10_000, + signal: opts?.signal, }) const responseResult = nanoGptModelsResponseSchema.safeParse(response.data) if (!responseResult.success) { @@ -106,6 +108,10 @@ export async function getNanoGptModels(apiKey?: string): Promise { return models } catch (error) { + // Surface cancellation as a rejection: logging and returning here would + // present an aborted fetch to callers as a successful (empty) catalog. + throwIfAborted(opts?.signal) + console.error(`Error fetching NanoGPT models: ${getSafeErrorMessage(error, apiKey)}`) return {} } diff --git a/src/api/providers/fetchers/ollama.ts b/src/api/providers/fetchers/ollama.ts index 9f88d75327..d16899dcce 100644 --- a/src/api/providers/fetchers/ollama.ts +++ b/src/api/providers/fetchers/ollama.ts @@ -2,6 +2,8 @@ import axios from "axios" import { ModelInfo, ollamaDefaultModelInfo } from "@roo-code/types" import { z } from "zod" +import { throwIfAborted } from "../utils/abort-signal" + const OllamaModelDetailsSchema = z.object({ family: z.string(), families: z.array(z.string()).nullable().optional(), @@ -65,6 +67,7 @@ export const parseOllamaModel = (rawModel: OllamaModelInfoResponse): ModelInfo | export async function getOllamaModels( baseUrl = "http://localhost:11434", apiKey?: string, + opts?: { signal?: AbortSignal }, ): Promise> { const models: Record = {} @@ -82,7 +85,7 @@ export async function getOllamaModels( headers["Authorization"] = `Bearer ${apiKey}` } - const response = await axios.get(`${baseUrl}/api/tags`, { headers }) + const response = await axios.get(`${baseUrl}/api/tags`, { headers, signal: opts?.signal }) const parsedResponse = OllamaModelsResponseSchema.safeParse(response.data) const modelInfoPromises = [] @@ -95,7 +98,7 @@ export async function getOllamaModels( { model: ollamaModel.model, }, - { headers }, + { headers, signal: opts?.signal }, ) .then((ollamaModelInfo) => { const modelInfo = parseOllamaModel(ollamaModelInfo.data) @@ -128,5 +131,10 @@ export async function getOllamaModels( } } + // The per-model fan-out tolerates individual request failures, so an abort that + // fires mid-fan-out surfaces through those swallowed rejections; without this + // guard the caller would receive a partial catalog as a successful result. + throwIfAborted(opts?.signal) + return models } diff --git a/src/api/providers/fetchers/opencode-go.ts b/src/api/providers/fetchers/opencode-go.ts index 6b2ad361b2..68ac3d2d45 100644 --- a/src/api/providers/fetchers/opencode-go.ts +++ b/src/api/providers/fetchers/opencode-go.ts @@ -4,6 +4,8 @@ import { z } from "zod" import type { ModelInfo } from "@roo-code/types" import { opencodeGoDefaultModelInfo, getOpencodeGoModelInfo } from "@roo-code/types" +import { throwIfAborted } from "../utils/abort-signal" + const OPENCODE_GO_BASE_URL = "https://opencode.ai/zen/go/v1" // The Opencode Go `/models` endpoint follows the OpenAI `/models` shape. The @@ -87,15 +89,19 @@ export const parseOpencodeGoModel = (model: OpencodeGoModel): ModelInfo => { * with a console warning rather than propagated to the UI. * * @param apiKey - Optional Bearer token for authenticated requests. + * @param opts - Optional per-request controls; `signal` cancels the in-flight request. * @returns A record mapping model IDs to their normalised {@link ModelInfo}. */ -export async function getOpencodeGoModels(apiKey?: string): Promise> { +export async function getOpencodeGoModels( + apiKey?: string, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} try { const response = await axios.get(`${OPENCODE_GO_BASE_URL}/models`, { headers: apiKey ? { Authorization: `Bearer ${apiKey}` } : undefined, - timeout: 10_000, + signal: opts?.signal, }) const result = opencodeGoModelsResponseSchema.safeParse(response.data) @@ -117,6 +123,10 @@ export async function getOpencodeGoModels(apiKey?: string): Promise> { +export async function getOpenRouterModels( + options?: ApiHandlerOptions, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} const baseURL = options?.openRouterBaseUrl || "https://openrouter.ai/api/v1" try { - const response = await axios.get(`${baseURL}/models`) + const response = await axios.get(`${baseURL}/models`, { signal: opts?.signal }) const result = openRouterModelsResponseSchema.safeParse(response.data) const data = result.success ? result.data.data : response.data.data @@ -127,6 +132,10 @@ export async function getOpenRouterModels(options?: ApiHandlerOptions): Promise< models[id] = parsedModel } } catch (error) { + // Surface cancellation as a rejection: logging and returning here would + // present an aborted fetch to callers as a successful (partial) catalog. + throwIfAborted(opts?.signal) + console.error( `Error fetching OpenRouter models: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`, ) diff --git a/src/api/providers/fetchers/poe.ts b/src/api/providers/fetchers/poe.ts index b9ca30d0f4..a78ddd352c 100644 --- a/src/api/providers/fetchers/poe.ts +++ b/src/api/providers/fetchers/poe.ts @@ -1,11 +1,22 @@ import type { ModelInfo, ModelRecord } from "@roo-code/types" import { fetchPoeModels, getModels } from "ai-sdk-provider-poe/code" -export async function getPoeModels(apiKey?: string, baseURL?: string): Promise { +import { throwIfAborted } from "../utils/abort-signal" + +export async function getPoeModels( + apiKey?: string, + baseURL?: string, + opts?: { signal?: AbortSignal }, +): Promise { try { // fetchPoeModels populates the internal model store, then getModels() // returns only code-capable models with camelCase fields. + // The Poe SDK exposes no cancellation option, so the caller's signal + // cannot reach the network here; an abort still releases the shared + // cache entry and stops the waiters at the model-cache layer. + throwIfAborted(opts?.signal) await fetchPoeModels({ apiKey, baseURL }) + throwIfAborted(opts?.signal) const poeModels = getModels() const models: ModelRecord = {} @@ -36,6 +47,10 @@ export async function getPoeModels(apiKey?: string, baseURL?: string): Promise> { +import { throwIfAborted } from "../utils/abort-signal" + +export async function getRequestyModels( + baseUrl?: string, + apiKey?: string, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} try { @@ -18,7 +24,7 @@ export async function getRequestyModels(baseUrl?: string, apiKey?: string): Prom const resolvedBaseUrl = toRequestyServiceUrl(baseUrl) const modelsUrl = new URL("v1/models", resolvedBaseUrl) - const response = await axios.get(modelsUrl.toString(), { headers }) + const response = await axios.get(modelsUrl.toString(), { headers, signal: opts?.signal }) const rawModels = response.data.data for (const rawModel of rawModels) { @@ -66,6 +72,10 @@ export async function getRequestyModels(baseUrl?: string, apiKey?: string): Prom models[rawModel.id] = modelInfo } } catch (error) { + // Surface cancellation as a rejection: logging and returning here would + // present an aborted fetch to callers as a successful (partial) catalog. + throwIfAborted(opts?.signal) + console.error(`Error fetching Requesty models: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`) } diff --git a/src/api/providers/fetchers/unbound.ts b/src/api/providers/fetchers/unbound.ts index 3841011809..c006ce564f 100644 --- a/src/api/providers/fetchers/unbound.ts +++ b/src/api/providers/fetchers/unbound.ts @@ -4,7 +4,12 @@ import type { ModelInfo } from "@roo-code/types" import { parseApiPrice } from "../../../shared/cost" -export async function getUnboundModels(apiKey?: string | null): Promise> { +import { throwIfAborted } from "../utils/abort-signal" + +export async function getUnboundModels( + apiKey?: string | null, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} try { @@ -14,7 +19,7 @@ export async function getUnboundModels(apiKey?: string | null): Promise> { +export async function getVercelAiGatewayModels( + options?: ApiHandlerOptions, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} const baseURL = "https://ai-gateway.vercel.sh/v1" try { - const response = await axios.get(`${baseURL}/models`) + const response = await axios.get(`${baseURL}/models`, { signal: opts?.signal }) const result = vercelAiGatewayModelsResponseSchema.safeParse(response.data) const data = result.success ? result.data.data : response.data.data @@ -80,6 +85,10 @@ export async function getVercelAiGatewayModels(options?: ApiHandlerOptions): Pro models[id] = parseVercelAiGatewayModel({ id, model }) } } catch (error) { + // Surface cancellation as a rejection: logging and returning here would + // present an aborted fetch to callers as a successful (partial) catalog. + throwIfAborted(opts?.signal) + console.error( `Error fetching Vercel AI Gateway models: ${JSON.stringify(error, Object.getOwnPropertyNames(error), 2)}`, ) diff --git a/src/api/providers/fetchers/zoo-gateway.ts b/src/api/providers/fetchers/zoo-gateway.ts index 074ec1dfae..51e31f003e 100644 --- a/src/api/providers/fetchers/zoo-gateway.ts +++ b/src/api/providers/fetchers/zoo-gateway.ts @@ -5,13 +5,17 @@ import type { ModelInfo } from "@roo-code/types" import type { ApiHandlerOptions } from "../../../shared/api" import { getZooCodeBaseUrl, resolveZooGatewaySessionToken } from "../../../services/zoo-code-auth" +import { throwIfAborted } from "../utils/abort-signal" + import { type VercelAiGatewayModel, parseVercelAiGatewayModel, vercelAiGatewayModelsResponseSchema, } from "./vercel-ai-gateway" -// Bound model discovery so a network stall can't hang provider initialization paths. +// Bound model discovery so a network stall can't hang provider initialization +// paths. Auth-scoped fetchers bypass the model-cache single-flight, so this +// per-call timeout is the only bound on the request. const MODEL_DISCOVERY_TIMEOUT_MS = 15_000 /** @@ -20,7 +24,10 @@ const MODEL_DISCOVERY_TIMEOUT_MS = 15_000 * Fetches models from the Zoo Gateway API. Requires authentication via the zoo_ext_ token. */ -export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise> { +export async function getZooGatewayModels( + options?: ApiHandlerOptions, + opts?: { signal?: AbortSignal }, +): Promise> { const models: Record = {} const baseURL = options?.zooGatewayBaseUrl ?? `${getZooCodeBaseUrl()}/api/gateway/v1` @@ -37,6 +44,7 @@ export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise< const response = await axios.get(`${baseURL}/models`, { headers, timeout: MODEL_DISCOVERY_TIMEOUT_MS, + signal: opts?.signal, }) const result = vercelAiGatewayModelsResponseSchema.safeParse(response.data) @@ -57,6 +65,10 @@ export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise< models[id] = parseZooGatewayModel({ id, model }) } } catch (error) { + // Surface cancellation as a rejection: logging and returning here would + // present an aborted fetch to callers as a successful (partial) catalog. + throwIfAborted(opts?.signal) + // Log only safe fields; never serialize the full error object because it // includes request config/headers which carry the bearer session token. const err = error as { diff --git a/src/shared/api.ts b/src/shared/api.ts index d1cc21cad0..d301ee68b9 100644 --- a/src/shared/api.ts +++ b/src/shared/api.ts @@ -166,11 +166,15 @@ export const getModelMaxOutputTokens = ({ // GetModelsOptions -// Allow callers to always pass apiKey/baseUrl without excess property errors, +// Allow callers to always pass apiKey/baseUrl/signal without excess property errors, // while still enforcing required fields per provider where applicable. type CommonFetchParams = { apiKey?: string baseUrl?: string + // Optional cancellation for model-catalog fetches (getModels/refreshModels): the signal is + // threaded through the model-cache single-flight so an aborted caller stops waiting at the + // moment of abort. Every provider arm inherits it via the intersection below. + signal?: AbortSignal } // Exhaustive, value-level map for all dynamic providers. From 649a11d5dab3939c990d2c3136e87ac9a7e63a49 Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Sat, 19 Sep 2026 21:30:17 +0200 Subject: [PATCH 2/3] test(fetchers): add poe abort negative-path tests for guard sites Cover the three cancellation branches in getPoeModels: a pre-aborted signal rejects with AbortError before the SDK call, an abort observed while the SDK call is pending rejects instead of resolving a catalog, and an SDK rejection after caller cancellation rethrows AbortError instead of returning an empty catalog. Addresses the pre-merge Regression Evidence check asking for focused negative-path tests for the changed Poe cancellation behavior. --- .../providers/fetchers/__tests__/poe.spec.ts | 51 +++++++++++++++++++ 1 file changed, 51 insertions(+) diff --git a/src/api/providers/fetchers/__tests__/poe.spec.ts b/src/api/providers/fetchers/__tests__/poe.spec.ts index ae81cb8d6c..c91f93b7de 100644 --- a/src/api/providers/fetchers/__tests__/poe.spec.ts +++ b/src/api/providers/fetchers/__tests__/poe.spec.ts @@ -150,4 +150,55 @@ describe("getPoeModels", () => { expect(models["openai/o3"].supportsReasoningEffort).toEqual(["low", "medium", "high"]) }) + + it("rejects with an AbortError and skips the SDK call when the signal is already aborted", async () => { + const controller = new AbortController() + controller.abort() + + await expect(getPoeModels("key", undefined, { signal: controller.signal })).rejects.toMatchObject({ + name: "AbortError", + }) + + expect(mockFetchPoeModels).not.toHaveBeenCalled() + }) + + it("rejects with an AbortError when the caller aborts while the SDK call is pending", async () => { + // The double cannot observe the signal itself (the Poe SDK exposes no + // cancellation surface), so the test settles it explicitly after the abort. + let settleSdk: () => void = () => {} + mockFetchPoeModels.mockImplementation( + () => + new Promise((resolve) => { + settleSdk = () => resolve() + }), + ) + + const controller = new AbortController() + const result = getPoeModels("key", undefined, { signal: controller.signal }) + controller.abort() + settleSdk() + + await expect(result).rejects.toMatchObject({ name: "AbortError" }) + expect(mockGetModels).not.toHaveBeenCalled() + }) + + it("rejects with an AbortError instead of an empty catalog when the SDK fails after the caller aborts", async () => { + let failSdk: (error: Error) => void = () => {} + mockFetchPoeModels.mockImplementation( + () => + new Promise((_resolve, reject) => { + failSdk = (error) => reject(error) + }), + ) + + const controller = new AbortController() + const result = getPoeModels("key", undefined, { signal: controller.signal }) + controller.abort() + failSdk(new Error("network failure")) + + // A swallowed SDK failure would resolve to an empty catalog, presenting a + // cancelled fetch to callers as a successful one. + await expect(result).rejects.toMatchObject({ name: "AbortError" }) + expect(mockGetModels).not.toHaveBeenCalled() + }) }) From 6a37f3f2642200458ec0994288dc1b3a5a5cb0c6 Mon Sep 17 00:00:00 2001 From: Franz Daubner Date: Sun, 20 Sep 2026 09:53:30 +0200 Subject: [PATCH 3/3] fix(model-cache): drop caller-signal acceptance on auth-scoped fetch paths The zooGateway and kimiCode auth-scoped paths bypass the inFlightRefresh single-flight entirely and are outside the scope of #1615. Revert these two fetchers, their specs, and the modelCache dispatch/entry points to the main-branch form: no caller signal is forwarded there, and each fetcher keeps its own request bound, so behavior on those paths is unchanged vs main. The auth-scoped branches keep regression tests asserting the signal is ignored and the flight machinery is not entered. --- .../fetchers/__tests__/kimi-code.spec.ts | 14 ------- .../fetchers/__tests__/modelCache.spec.ts | 29 +++++--------- .../fetchers/__tests__/zoo-gateway.spec.ts | 40 ------------------- src/api/providers/fetchers/kimi-code.ts | 11 +---- src/api/providers/fetchers/modelCache.ts | 27 ++++++------- src/api/providers/fetchers/zoo-gateway.ts | 16 +------- 6 files changed, 26 insertions(+), 111 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/kimi-code.spec.ts b/src/api/providers/fetchers/__tests__/kimi-code.spec.ts index 8378f7d8d0..a96e760253 100644 --- a/src/api/providers/fetchers/__tests__/kimi-code.spec.ts +++ b/src/api/providers/fetchers/__tests__/kimi-code.spec.ts @@ -98,20 +98,6 @@ describe("Kimi Code model discovery", () => { expect(vi.getTimerCount()).toBe(0) }) - it("rejects when the caller aborts a pending discovery request", async () => { - vi.spyOn(globalThis, "fetch").mockImplementation((_input, init) => { - return new Promise((_resolve, reject) => { - init?.signal?.addEventListener("abort", () => reject(init.signal?.reason), { once: true }) - }) - }) - const controller = new AbortController() - - const result = getKimiCodeModels("token", { signal: controller.signal }) - controller.abort() - - await expect(result).rejects.toMatchObject({ name: "AbortError" }) - }) - it("overrides maxTokens from server max_tokens in mapKimiCodeModel", () => { const mapped = mapKimiCodeModel({ id: "kimi-for-coding", diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index b8c9b95398..3b95f32234 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -1645,28 +1645,13 @@ it("releases the entry for a fetcher double that honors no cancellation at all", expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(2) }) -it("passes the caller signal straight through the auth-scoped bypass without entering the flight map", async () => { +it("ignores the caller signal on the auth-scoped bypass without entering the flight map", async () => { setupCancellationMocks() // The single-flight arms its per-flight fetch bound whenever it creates a flight; the // auth-scoped bypass must never touch that machinery. const boundSpy = vi.spyOn(AbortSignal, "timeout") try { - mockGetZooGatewayModels - .mockImplementationOnce( - (_options, opts) => - new Promise((_resolve, reject) => { - opts?.signal?.addEventListener( - "abort", - () => { - const abortError = new Error("This operation was aborted") - abortError.name = "AbortError" - reject(abortError) - }, - { once: true }, - ) - }), - ) - .mockResolvedValueOnce(cancelledModelsB) + mockGetZooGatewayModels.mockResolvedValue(cancelledModelsB) const controller = new AbortController() const first = getModels({ @@ -1678,11 +1663,15 @@ it("passes the caller signal straight through the auth-scoped bypass without ent // its own fetch, so the bypass never shares (or poisons) a flight with anything. const second = getModels({ provider: providerIdentifiers.zooGateway, apiKey: "token-a" }) expect(mockGetZooGatewayModels).toHaveBeenCalledTimes(2) - expect(mockGetZooGatewayModels.mock.calls[0][1]?.signal).toBe(controller.signal) - expect(mockGetZooGatewayModels.mock.calls[1][1]).toBeUndefined() + // The bypass carries no cancellation: the fetcher receives exactly its own options + // argument, so the caller's bound is never threaded to this path. + expect(mockGetZooGatewayModels.mock.calls[0]).toHaveLength(1) + expect(mockGetZooGatewayModels.mock.calls[1]).toHaveLength(1) + // The caller's signal is ignored on this path: aborting changes nothing for a fetch the + // single-flight never owns, and the fetcher's own request bound remains the stop mechanism. controller.abort() - await expect(first).rejects.toMatchObject({ name: "AbortError" }) + await expect(first).resolves.toEqual(cancelledModelsB) await expect(second).resolves.toEqual(cancelledModelsB) expect(boundSpy).not.toHaveBeenCalled() } finally { diff --git a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts index e7c766fb52..ae9bdcc4b1 100644 --- a/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts +++ b/src/api/providers/fetchers/__tests__/zoo-gateway.spec.ts @@ -156,46 +156,6 @@ describe("Zoo Gateway Fetchers", () => { expect(consoleErrorSpy).toHaveBeenCalled() consoleErrorSpy.mockRestore() }) - - it("forwards the caller's abort signal alongside the retained timeout", async () => { - mockedAxios.get.mockResolvedValueOnce(mockResponse) - const controller = new AbortController() - - await getZooGatewayModels( - { zooGatewayBaseUrl: baseUrl, zooSessionToken: token }, - { - signal: controller.signal, - }, - ) - - expect(mockedAxios.get).toHaveBeenCalledWith( - `${baseUrl}/models`, - expect.objectContaining({ - timeout: expect.any(Number), - signal: controller.signal, - }), - ) - }) - - it("rejects with an AbortError when the signal aborts the pending request", async () => { - const controller = new AbortController() - mockedAxios.get.mockImplementation(function (_url: string, config?: { signal?: AbortSignal }) { - // Mirror the HTTP client: a pending request rejects when its signal fires. - return new Promise((_resolve, reject) => { - config?.signal?.addEventListener?.("abort", () => reject(new Error("canceled")), { once: true }) - }) - }) - - const fetchPromise = getZooGatewayModels( - { zooGatewayBaseUrl: baseUrl, zooSessionToken: token }, - { - signal: controller.signal, - }, - ) - controller.abort() - - await expect(fetchPromise).rejects.toMatchObject({ name: "AbortError" }) - }) }) describe("parseZooGatewayModel", () => { diff --git a/src/api/providers/fetchers/kimi-code.ts b/src/api/providers/fetchers/kimi-code.ts index 14a23f7972..02bf9180f7 100644 --- a/src/api/providers/fetchers/kimi-code.ts +++ b/src/api/providers/fetchers/kimi-code.ts @@ -9,8 +9,6 @@ import { type ModelRecord, } from "@roo-code/types" -import { mergeAbortSignals } from "../utils/abort-signal" - export const kimiCodeModelSchema = z.object({ id: z.string().min(1), context_length: z.number().positive().optional(), @@ -39,13 +37,8 @@ export function mapKimiCodeModel(model: z.infer): Mo } } -export async function getKimiCodeModels(apiKey?: string, opts?: { signal?: AbortSignal }): Promise { +export async function getKimiCodeModels(apiKey?: string): Promise { if (!apiKey) throw new Error("Kimi Code authentication is required to fetch models") - // This auth-scoped fetch bypasses the model-cache single-flight, so the - // entry-level timeout never covers it; the bound stays local. The deadline - // fires through an explicit controller (not AbortSignal.timeout) so the - // rejection keeps this exact Error identity; the caller's signal, when - // present, aborts alongside it via AbortSignal.any. const controller = new AbortController() const timeout = setTimeout( () => controller.abort(new Error("Kimi Code models request timed out")), @@ -54,7 +47,7 @@ export async function getKimiCodeModels(apiKey?: string, opts?: { signal?: Abort try { const response = await fetch(`${KIMI_CODE_BASE_URL}/models`, { headers: { Accept: "application/json", Authorization: `Bearer ${apiKey}` }, - signal: mergeAbortSignals(controller.signal, opts?.signal), + signal: controller.signal, }) if (!response.ok) { const error = new Error(`Kimi Code models request failed: ${response.status} ${response.statusText}`) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index b29011c818..6f0898c71b 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -248,9 +248,9 @@ async function readModels(cacheKey: string): Promise { * Extracted to avoid duplication between getModels() and refreshModels(). * * @param options - Provider options for fetching models - * @param signal - Cancellation signal for this fetch: the shared flight's internal controller - * signal when routed through dedupedFetch(), or the caller's own signal on the auth-scoped - * direct path (which never enters the single-flight). + * @param signal - Cancellation signal forwarded to the dispatched fetcher. The single-flight + * (dedupedFetch) passes its internal controller's signal; the auth-scoped direct path passes + * none, so those fetchers keep their own bounds. * @returns Fresh models from the provider API */ async function fetchModelsFromProvider(options: GetModelsOptions, signal?: AbortSignal): Promise { @@ -305,13 +305,10 @@ async function fetchModelsFromProvider(options: GetModelsOptions, signal?: Abort models = await getMoonshotModels(options.baseUrl, options.apiKey, ...fetchOpts) break case providerIdentifiers.zooGateway: - models = await getZooGatewayModels( - { zooSessionToken: options.apiKey, zooGatewayBaseUrl: options.baseUrl }, - ...fetchOpts, - ) + models = await getZooGatewayModels({ zooSessionToken: options.apiKey, zooGatewayBaseUrl: options.baseUrl }) break case providerIdentifiers.kimiCode: - models = await getKimiCodeModels(options.apiKey, ...fetchOpts) + models = await getKimiCodeModels(options.apiKey) break default: { // Ensures router is exhaustively checked if RouterName is a strict union. @@ -359,9 +356,10 @@ export const getModels = async (options: GetModelsOptions): Promise // getModels(), and a fetch failure joined from refreshModels() still re-throws for // getModels() callers. try { - const sharedFetch = shouldSkipCache - ? fetchModelsFromProvider(options, options.signal) - : dedupedFetch(cacheKey, options) + // The auth-scoped fetch bypasses the single-flight entirely, so options.signal is + // deliberately not forwarded there: there is no shared entry to release on abort, and + // these fetchers bound their own requests. + const sharedFetch = shouldSkipCache ? fetchModelsFromProvider(options) : dedupedFetch(cacheKey, options) const fetched = await sharedFetch const modelCount = Object.keys(fetched).length @@ -559,9 +557,10 @@ export const refreshModels = async (options: GetModelsOptions): Promise> { +export async function getZooGatewayModels(options?: ApiHandlerOptions): Promise> { const models: Record = {} const baseURL = options?.zooGatewayBaseUrl ?? `${getZooCodeBaseUrl()}/api/gateway/v1` @@ -44,7 +37,6 @@ export async function getZooGatewayModels( const response = await axios.get(`${baseURL}/models`, { headers, timeout: MODEL_DISCOVERY_TIMEOUT_MS, - signal: opts?.signal, }) const result = vercelAiGatewayModelsResponseSchema.safeParse(response.data) @@ -65,10 +57,6 @@ export async function getZooGatewayModels( models[id] = parseZooGatewayModel({ id, model }) } } catch (error) { - // Surface cancellation as a rejection: logging and returning here would - // present an aborted fetch to callers as a successful (partial) catalog. - throwIfAborted(opts?.signal) - // Log only safe fields; never serialize the full error object because it // includes request config/headers which carry the bearer session token. const err = error as {