diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 169e4a0efa..17a061be14 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -5,6 +5,7 @@ vi.mock("@roo-code/telemetry", () => ({ TelemetryService: { instance: { captureEvent: vi.fn(), + isTelemetryEnabled: vi.fn().mockReturnValue(true), }, }, })) @@ -45,6 +46,7 @@ vi.mock("../openrouter") vi.mock("../requesty") vi.mock("../kenari") vi.mock("../moonshot") +vi.mock("../zoo-gateway") // Mock ContextProxy with a simple static instance vi.mock("../../../core/config/ContextProxy", () => ({ @@ -62,18 +64,21 @@ import type { Mock, Mocked } from "vitest" import { providerIdentifiers } from "@roo-code/types" import * as fsSync from "fs" import NodeCache from "node-cache" +import { TelemetryService } from "@roo-code/telemetry" import { getModels, getModelsFromCache } from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" import { getRequestyModels } from "../requesty" import { getKenariModels } from "../kenari" import { getMoonshotModels } from "../moonshot" +import { getZooGatewayModels } from "../zoo-gateway" const mockGetLiteLLMModels = getLiteLLMModels as Mock const mockGetOpenRouterModels = getOpenRouterModels as Mock const mockGetRequestyModels = getRequestyModels as Mock const mockGetKenariModels = getKenariModels as Mock const mockGetMoonshotModels = getMoonshotModels as Mock +const mockGetZooGatewayModels = getZooGatewayModels as Mock const DUMMY_REQUESTY_KEY = "requesty-key-for-testing" @@ -395,6 +400,162 @@ describe("empty cache protection", () => { expect(result).toEqual(mockModels) expect(mockSet).toHaveBeenCalledWith("openrouter", mockModels) }) + + it("reuses an in-flight fetch for concurrent getModels() calls to the same provider", async () => { + const mockModels = { + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + } + + let resolvePromise: (value: typeof mockModels) => void + const delayedPromise = new Promise((resolve) => { + resolvePromise = resolve + }) + mockGetOpenRouterModels.mockReturnValue(delayedPromise) + mockGet.mockReturnValue(undefined) + + const promise1 = getModels({ provider: providerIdentifiers.openrouter }) + const promise2 = getModels({ provider: providerIdentifiers.openrouter }) + + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + resolvePromise!(mockModels) + + const [result1, result2] = await Promise.all([promise1, promise2]) + expect(result1).toEqual(mockModels) + expect(result2).toEqual(mockModels) + }) + + it("shares a single in-flight fetch between getModels() and refreshModels() for the same key", async () => { + // Both entry points converge on the same coordinator so a getModels() cache miss + // racing a concurrent refreshModels() call can't produce two unordered cache writes. + const mockModels = { + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + } + + let resolvePromise: (value: typeof mockModels) => void + const delayedPromise = new Promise((resolve) => { + resolvePromise = resolve + }) + mockGetOpenRouterModels.mockReturnValue(delayedPromise) + mockGet.mockReturnValue(undefined) + + const { refreshModels } = await import("../modelCache") + + const getPromise = getModels({ provider: providerIdentifiers.openrouter }) + const refreshPromise = refreshModels({ provider: providerIdentifiers.openrouter }) + + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + resolvePromise!(mockModels) + + const [getResult, refreshResult] = await Promise.all([getPromise, refreshPromise]) + expect(getResult).toEqual(mockModels) + expect(refreshResult).toEqual(mockModels) + }) + + it("preserves each entry point's own failure contract when joining a shared in-flight fetch", async () => { + // getModels() and refreshModels() share the same underlying provider fetch + // (dedupedFetch), but must not share its resolution/rejection wholesale: getModels() + // always re-throws on failure, while refreshModels() always degrades to cache/{}. + // Whichever call happens to start the shared fetch must not impose its own contract + // on the other caller that joined it. + const fetchError = new Error("provider unreachable") + + let rejectPromise: (error: Error) => void + const delayedRejection = new Promise((_resolve, reject) => { + rejectPromise = reject + }) + mockGetOpenRouterModels.mockReturnValue(delayedRejection) + mockGet.mockReturnValue(undefined) + + const { refreshModels } = await import("../modelCache") + + // refreshModels() starts (and registers) the shared fetch; getModels() joins it. + const refreshPromise = refreshModels({ provider: providerIdentifiers.openrouter }) + const getPromise = getModels({ provider: providerIdentifiers.openrouter }) + + expect(mockGetOpenRouterModels).toHaveBeenCalledTimes(1) + + rejectPromise!(fetchError) + + // refreshModels() degrades gracefully (no existing cache -> {}); getModels() still + // re-throws the original error instead of silently returning refreshModels()'s {}. + await expect(refreshPromise).resolves.toEqual({}) + await expect(getPromise).rejects.toThrow("provider unreachable") + }) + + it("does not share an in-flight fetch between different endpoints/keys", async () => { + const mockModelsA = { + "litellm/model-a": { + maxTokens: 4096, + contextWindow: 64000, + supportsPromptCache: false, + description: "Server A model", + }, + } + const mockModelsB = { + "litellm/model-b": { + maxTokens: 4096, + contextWindow: 64000, + supportsPromptCache: false, + description: "Server B model", + }, + } + mockGetLiteLLMModels.mockResolvedValueOnce(mockModelsA).mockResolvedValueOnce(mockModelsB) + mockGet.mockReturnValue(undefined) + + const [resultA, resultB] = await Promise.all([ + getModels({ provider: providerIdentifiers.litellm, apiKey: "key-a", baseUrl: "http://server-a:4000" }), + getModels({ provider: providerIdentifiers.litellm, apiKey: "key-b", baseUrl: "http://server-b:4000" }), + ]) + + expect(mockGetLiteLLMModels).toHaveBeenCalledTimes(2) + expect(resultA).toEqual(mockModelsA) + expect(resultB).toEqual(mockModelsB) + }) + + it("re-arms the empty-response throttle after a non-empty response from an auth-scoped provider", async () => { + // zoo-gateway is auth-scoped and skips caching entirely, but a non-empty response + // must still clear the throttle so a later empty response is reported again. + mockGetZooGatewayModels.mockResolvedValueOnce({}) + + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) + + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + const mockModels = { + "zoo-gateway/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "Zoo Gateway model", + }, + } + mockGetZooGatewayModels.mockResolvedValueOnce(mockModels) + + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) + + // Auth-scoped providers never populate the cache. + expect(mockSet).not.toHaveBeenCalled() + + mockGetZooGatewayModels.mockResolvedValueOnce({}) + + await getModels({ provider: providerIdentifiers.zooGateway, apiKey: "test-key" }) + + // The throttle should have been re-armed by the non-empty response above, so this + // second empty response is reported again instead of being suppressed. + expect(TelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) }) describe("refreshModels", () => { @@ -581,6 +742,226 @@ describe("empty cache protection", () => { }) }) +describe("MODEL_CACHE_EMPTY_RESPONSE throttling", () => { + type ModelCacheModule = typeof import("../modelCache") + + let freshGetModels: ModelCacheModule["getModels"] + let freshRefreshModels: ModelCacheModule["refreshModels"] + let freshMockGetOpenRouterModels: Mock + let freshMockGetLiteLLMModels: Mock + let freshMockGetZooGatewayModels: Mock + + beforeEach(async () => { + // The empty-response throttle is deliberately module-level, persistent state (once per + // cache key per session). Reset modules per test so each test starts with a clean gate. + vi.resetModules() + vi.clearAllMocks() + + const modelCacheModule: ModelCacheModule = await import("../modelCache") + const openRouterModule = await import("../openrouter") + const liteLLMModule = await import("../litellm") + const zooGatewayModule = await import("../zoo-gateway") + + freshGetModels = modelCacheModule.getModels + freshRefreshModels = modelCacheModule.refreshModels + freshMockGetOpenRouterModels = openRouterModule.getOpenRouterModels as Mock + freshMockGetLiteLLMModels = liteLLMModule.getLiteLLMModels as Mock + freshMockGetZooGatewayModels = zooGatewayModule.getZooGatewayModels as Mock + + const NodeCacheModule = await import("node-cache") + const MockedNodeCache = vi.mocked(NodeCacheModule.default) + const mockCache = vi.mocked(new MockedNodeCache()) + mockCache.get.mockReturnValue(undefined) + }) + + it("fires MODEL_CACHE_EMPTY_RESPONSE only once for repeated empty getModels responses from the same provider", async () => { + freshMockGetOpenRouterModels.mockResolvedValue({}) + + await freshGetModels({ provider: providerIdentifiers.openrouter }) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ provider: providerIdentifiers.openrouter, context: "getModels" }), + ) + }) + + it("fires again after a non-empty response resets the throttle", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + freshMockGetOpenRouterModels.mockResolvedValue({ + "openrouter/model": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: false, + description: "OpenRouter model", + }, + }) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + + freshMockGetOpenRouterModels.mockResolvedValue({}) + await freshGetModels({ provider: providerIdentifiers.openrouter }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles independently per provider", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + freshMockGetLiteLLMModels.mockResolvedValue({}) + + await freshGetModels({ provider: providerIdentifiers.openrouter }) + await freshGetModels({ provider: providerIdentifiers.litellm, apiKey: "key", baseUrl: "http://localhost:4000" }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles empty responses from refreshModels using the same per-key gate", async () => { + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetOpenRouterModels.mockResolvedValue({}) + + await freshRefreshModels({ provider: providerIdentifiers.openrouter }) + await freshRefreshModels({ provider: providerIdentifiers.openrouter }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledWith( + "Model Cache Empty Response", + expect.objectContaining({ + provider: providerIdentifiers.openrouter, + context: "refreshModels", + hasExistingCache: false, + existingCacheSize: 0, + }), + ) + }) + + it("throttles independently per distinct endpoint, not just per provider name", async () => { + // Two different LiteLLM servers share the "litellm" provider name but are a different + // cache identity (see getCacheKey) -- an empty response from one must not suppress the + // signal for the other. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetLiteLLMModels.mockResolvedValue({}) + + await freshGetModels({ + provider: providerIdentifiers.litellm, + apiKey: "key-a", + baseUrl: "http://server-a:4000", + }) + await freshGetModels({ + provider: providerIdentifiers.litellm, + apiKey: "key-a", + baseUrl: "http://server-a:4000", + }) + await freshGetModels({ + provider: providerIdentifiers.litellm, + apiKey: "key-b", + baseUrl: "http://server-b:4000", + }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles zoo-gateway independently per session token, even though caching itself is skipped", async () => { + // zoo-gateway is auth-scoped (see AUTH_SCOPED_PROVIDERS) and never persists to the + // memory/disk cache, but the empty-response throttle must still discriminate by + // identity: a sign-out/sign-in cycle to a different account carries a different + // session token (apiKey) on the same gateway URL, and must not have its empty-response + // signal suppressed by the previous account's throttle entry. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetZooGatewayModels.mockResolvedValue({}) + + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(1) + + await freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("throttles zoo-gateway independently per gateway baseUrl", async () => { + // Same session token, different gateway endpoint (e.g. staging vs. production) -- + // must also be treated as a distinct identity for throttle purposes. + const { TelemetryService: FreshTelemetryService } = await import("@roo-code/telemetry") + + freshMockGetZooGatewayModels.mockResolvedValue({}) + + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "token", + baseUrl: "https://gateway-a.example.com", + }) + await freshGetModels({ + provider: providerIdentifiers.zooGateway, + apiKey: "token", + baseUrl: "https://gateway-b.example.com", + }) + + expect(FreshTelemetryService.instance.captureEvent).toHaveBeenCalledTimes(2) + }) + + it("never shares results across different zoo-gateway credentials (auth isolation)", async () => { + // The in-flight fetch map must key on the full compound identity for auth-scoped + // providers too, so a slow fetch for one account's session token can never resolve + // into a concurrent call carrying a different account's token. + const accountAModels = { + "zoo-gateway/account-a-model": { + maxTokens: 4096, + contextWindow: 64000, + supportsPromptCache: false, + description: "Account A model", + }, + } + const accountBModels = { + "zoo-gateway/account-b-model": { + maxTokens: 4096, + contextWindow: 64000, + supportsPromptCache: false, + description: "Account B model", + }, + } + + let resolveA: (value: typeof accountAModels) => void + let resolveB: (value: typeof accountBModels) => void + freshMockGetZooGatewayModels + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveA = resolve + }), + ) + .mockImplementationOnce( + () => + new Promise((resolve) => { + resolveB = resolve + }), + ) + + const promiseA = freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-a-token" }) + const promiseB = freshGetModels({ provider: providerIdentifiers.zooGateway, apiKey: "account-b-token" }) + + expect(freshMockGetZooGatewayModels).toHaveBeenCalledTimes(2) + + resolveB!(accountBModels) + resolveA!(accountAModels) + + const [resultA, resultB] = await Promise.all([promiseA, promiseB]) + expect(resultA).toEqual(accountAModels) + expect(resultB).toEqual(accountBModels) + }) +}) + describe("key-scoped cache key derivation", () => { // Exercises the per-API-key cache discriminator that all KEY_SCOPED_PROVIDERS share. // Requesty is used only because it is a key-scoped provider with a mocked fetcher; the diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index af956548ef..d43efa6ac2 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -43,9 +43,37 @@ const modelRecordSchema = z.record(z.string(), modelInfoSchema) // deduplicate each other's in-flight refreshes. const inFlightRefresh = new Map>() +// 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 +// event on every cache refresh; gate it to at most once per distinct provider+server+key identity +// until a non-empty response is seen -- the same identity dimensions the model cache itself uses, +// so two different endpoints for the same provider can never suppress each other's signal. +const reportedEmptyModelResponse = new Set() + +function captureModelCacheEmptyResponseOnce( + provider: RouterName, + cacheKey: string, + properties: Record, +): void { + if (reportedEmptyModelResponse.has(cacheKey)) { + return + } + + if (!TelemetryService.instance.isTelemetryEnabled()) { + return + } + + reportedEmptyModelResponse.add(cacheKey) + TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { provider, ...properties }) +} + // Providers whose model list is determined by the server URL, not just by the provider name. // Each unique baseUrl must be cached independently so that switching endpoints never serves -// stale results from a previously-cached server. +// stale results from a previously-cached server. zoo-gateway is included too: although it's +// auth-scoped and never actually persisted (see shouldSkipCache), getCacheKey() also keys the +// empty-response throttle (reportedEmptyModelResponse) and the in-flight fetch map, both of +// which must still discriminate by endpoint (e.g. staging vs. production gateway) even when +// caching itself is skipped. const URL_SCOPED_PROVIDERS: ReadonlySet = new Set([ providerIdentifiers.litellm, providerIdentifiers.poe, @@ -54,16 +82,23 @@ const URL_SCOPED_PROVIDERS: ReadonlySet = new Set([ providerIdentifiers.ollama, providerIdentifiers.lmstudio, providerIdentifiers.requesty, + providerIdentifiers.zooGateway, ]) // Providers where the API key itself determines which models are visible (e.g. per-key // allowlists). For these the cache key also includes a short hash of // the API key so that two different keys on the same server never share a cache entry. +// zoo-gateway and kimi-code are included so a sign-out/sign-in cycle to a different account +// (same server, different session token) doesn't collapse into the same throttle/in-flight +// identity -- see the URL_SCOPED_PROVIDERS comment above for why this matters despite caching +// being skipped for both. const KEY_SCOPED_PROVIDERS: ReadonlySet = new Set([ providerIdentifiers.litellm, // Per-key model allowlists are a first-class LiteLLM proxy feature providerIdentifiers.poe, // Per-account model availability providerIdentifiers.requesty, // Per-account custom model policies providerIdentifiers.moonshot, // Per-key model visibility (api.moonshot.ai vs api.moonshot.cn) + providerIdentifiers.zooGateway, // Per-session-token account identity + providerIdentifiers.kimiCode, // Per-session-token account identity ]) // Providers whose model lists are scoped to the signed-in user (e.g. per-account @@ -264,33 +299,52 @@ export const getModels = async (options: GetModelsOptions): Promise const shouldSkipCache = isAuthScopedProvider(provider) - let models = shouldSkipCache ? undefined : getModelsFromCache(options) + const models = shouldSkipCache ? undefined : getModelsFromCache(options) if (models) { return models } + // Route the cache-miss fetch through dedupedFetch(), the same single-flight coordinator + // refreshModels() uses, keyed on the same compound cacheKey. Without this, concurrent + // getModels() calls for the same key each independently miss the cache and fire their own + // redundant provider fetch, and a getModels() fetch racing a refreshModels() fetch for the + // same key has no ordering guarantee -- whichever call's memoryCache.set() lands last wins, + // even if it started (and thus reflects) an earlier, staler request. Sharing dedupedFetch() + // means every caller for a given key -- get or refresh -- converges on one underlying + // provider fetch. Each entry point still applies its own success/failure contract on top + // (see below) rather than returning the shared promise directly, so a fetch failure that + // 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 { - models = await fetchModelsFromProvider(options) - const modelCount = Object.keys(models).length + const fetched = await sharedFetch + const modelCount = Object.keys(fetched).length // Only cache non-empty results so a failed API response doesn't get persisted // as if the provider had no models. Auth-scoped providers skip caching entirely. - if (modelCount > 0 && !shouldSkipCache) { - memoryCache.set(cacheKey, models) - - await writeModels(cacheKey, models).catch((err) => - console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), - ) - } else if (modelCount === 0) { - TelemetryService.instance.captureEvent(TelemetryEventName.MODEL_CACHE_EMPTY_RESPONSE, { - provider, + if (modelCount > 0) { + // Clear the empty-response throttle for any non-empty response, including from + // auth-scoped providers that skip caching, so a later empty response is reported again. + reportedEmptyModelResponse.delete(cacheKey) + + if (!shouldSkipCache) { + memoryCache.set(cacheKey, fetched) + + await writeModels(cacheKey, fetched).catch((err) => + console.error(`[MODEL_CACHE] Error writing ${cacheKey} models to file cache:`, err), + ) + } + } else { + captureModelCacheEmptyResponseOnce(provider, cacheKey, { context: "getModels", hasExistingCache: false, }) } - return models + return fetched } catch (error) { // Log the error and re-throw it so the caller can handle it (e.g., show a UI message). console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error) @@ -299,6 +353,27 @@ export const getModels = async (options: GetModelsOptions): Promise } } +/** + * Single-flight the raw provider fetch for a cache key across getModels() and refreshModels(). + * Callers apply their own caching/degradation/telemetry behavior on top of the resolved value + * or rejection -- this only ensures at most one fetchModelsFromProvider() call is in flight per + * cache key at a time. + */ +function dedupedFetch(cacheKey: string, options: GetModelsOptions): Promise { + const existingRequest = inFlightRefresh.get(cacheKey) + if (existingRequest) { + return existingRequest + } + + const fetchPromise = fetchModelsFromProvider(options).finally(() => { + inFlightRefresh.delete(cacheKey) + }) + + inFlightRefresh.set(cacheKey, fetchPromise) + + return fetchPromise +} + /** * Force-refresh models from API, bypassing cache. * Uses atomic writes so cache remains available during refresh. @@ -314,30 +389,20 @@ export const refreshModels = async (options: GetModelsOptions): Promise => { try { // Force fresh API fetch - skip getModelsFromCache() check - const models = await fetchModelsFromProvider(options) + const models = await sharedFetch const modelCount = Object.keys(models).length // Get existing cached data for comparison @@ -345,8 +410,7 @@ export const refreshModels = async (options: GetModelsOptions): Promise 0, existingCacheSize: existingCount, @@ -358,6 +422,8 @@ export const refreshModels = async (options: GetModelsOptions): Promise