From 1f900cdcb16ebdb83de96478e308731ad4699a3a Mon Sep 17 00:00:00 2001 From: Roo Code Date: Wed, 26 Nov 2025 06:11:03 +0000 Subject: [PATCH] fix: improve OpenRouter model cache persistence to prevent model disappearance - Add validation to check if API response contains enough models - Implement intelligent model merging to preserve user-selected models - Preserve models specified in preserveModelIds even if not in API response - Add fallback to disk cache when API returns incomplete data - Update tests to verify cache validation and merging logic This fixes the issue where newer OpenRouter models like gpt-5.1, gemini-3-pro-preview, and grok-4.1-fast would disappear after the 5-minute cache expiration when the API returned incomplete or temporarily missing model data. Fixes #9597 --- .../fetchers/__tests__/modelCache.spec.ts | 162 +++++++++++++++++- src/api/providers/fetchers/modelCache.ts | 113 ++++++++++-- src/api/providers/openrouter.ts | 18 +- 3 files changed, 272 insertions(+), 21 deletions(-) diff --git a/src/api/providers/fetchers/__tests__/modelCache.spec.ts b/src/api/providers/fetchers/__tests__/modelCache.spec.ts index 530ea8de97..44699c5fee 100644 --- a/src/api/providers/fetchers/__tests__/modelCache.spec.ts +++ b/src/api/providers/fetchers/__tests__/modelCache.spec.ts @@ -28,6 +28,16 @@ vi.mock("fs", () => ({ readFileSync: vi.fn().mockReturnValue("{}"), })) +// Mock safeWriteJson +vi.mock("../../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn().mockResolvedValue(undefined), +})) + +// Mock fileExistsAtPath +vi.mock("../../../../utils/fs", () => ({ + fileExistsAtPath: vi.fn().mockResolvedValue(false), +})) + // Mock all the model fetchers vi.mock("../litellm") vi.mock("../openrouter") @@ -35,9 +45,16 @@ vi.mock("../requesty") vi.mock("../glama") vi.mock("../unbound") vi.mock("../io-intelligence") +vi.mock("../ollama") +vi.mock("../lmstudio") +vi.mock("../vercel-ai-gateway") +vi.mock("../deepinfra") +vi.mock("../huggingface") +vi.mock("../roo") +vi.mock("../chutes") // Mock ContextProxy with a simple static instance -vi.mock("../../../core/config/ContextProxy", () => ({ +vi.mock("../../../../core/config/ContextProxy", () => ({ ContextProxy: { instance: { globalStorageUri: { @@ -47,17 +64,24 @@ vi.mock("../../../core/config/ContextProxy", () => ({ }, })) +// Mock getCacheDirectoryPath +vi.mock("../../../../utils/storage", () => ({ + getCacheDirectoryPath: vi.fn().mockResolvedValue("/mock/storage/path/cache"), +})) + // Then imports import type { Mock } from "vitest" import * as fsSync from "fs" +import * as fs from "fs/promises" import NodeCache from "node-cache" -import { getModels, getModelsFromCache } from "../modelCache" +import { getModels, getModelsFromCache, refreshModels } from "../modelCache" import { getLiteLLMModels } from "../litellm" import { getOpenRouterModels } from "../openrouter" import { getRequestyModels } from "../requesty" import { getGlamaModels } from "../glama" import { getUnboundModels } from "../unbound" import { getIOIntelligenceModels } from "../io-intelligence" +import { safeWriteJson } from "../../../../utils/safeWriteJson" const mockGetLiteLLMModels = getLiteLLMModels as Mock const mockGetOpenRouterModels = getOpenRouterModels as Mock @@ -266,9 +290,8 @@ describe("getModelsFromCache disk fallback", () => { const result = getModelsFromCache("openrouter") - // In the test environment, ContextProxy.instance may not be fully initialized, - // so getCacheDirectoryPathSync returns undefined and disk cache is not attempted - expect(result).toBeUndefined() + // Now that getCacheDirectoryPathSync returns a path, disk cache should work + expect(result).toEqual(diskModels) }) it("handles disk read errors gracefully", () => { @@ -301,3 +324,132 @@ describe("getModelsFromCache disk fallback", () => { consoleErrorSpy.mockRestore() }) }) + +describe("OpenRouter model cache validation and merging", () => { + let mockCache: any + + beforeEach(() => { + vi.clearAllMocks() + // Get the mock cache instance + const MockedNodeCache = vi.mocked(NodeCache) + mockCache = new MockedNodeCache() + // Reset memory cache to always miss + mockCache.get.mockReturnValue(undefined) + // Mock safeWriteJson to avoid file system operations + vi.mocked(safeWriteJson).mockResolvedValue(undefined) + }) + + it("uses full API response when it contains enough models", async () => { + // API returns complete response with many models + const completeApiResponse = { + ...Array.from({ length: 120 }, (_, i) => ({ + [`model-${i}`]: { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: `Model ${i}`, + }, + })).reduce((acc, curr) => ({ ...acc, ...curr }), {}), + } + + mockGetOpenRouterModels.mockResolvedValue(completeApiResponse) + + const result = await getModels({ + provider: "openrouter", + }) + + // Should use the full API response + expect(Object.keys(result).length).toBe(120) + expect(result["model-0"]).toBeDefined() + expect(result["model-119"]).toBeDefined() + }) + + it("refreshModels preserves models even during refresh", async () => { + // Set up existing cache in memory + const existingModels = { + "openai/gpt-5.1": { + maxTokens: 8192, + contextWindow: 128000, + supportsPromptCache: true, + description: "GPT-5.1 model", + }, + "model-1": { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: "Model 1", + }, + } + + // Configure memory cache to return existing models + mockCache.get.mockReturnValue(existingModels) + + // API returns incomplete response + const incompleteApiResponse = { + "model-1": { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: "Model 1 updated", + }, + // Missing gpt-5.1 + } + + mockGetOpenRouterModels.mockResolvedValue(incompleteApiResponse) + + const result = await refreshModels({ + provider: "openrouter", + preserveModelIds: new Set(["openai/gpt-5.1"]), + }) + + // Should preserve gpt-5.1 even though it's not in API response + expect(result["openai/gpt-5.1"]).toEqual(existingModels["openai/gpt-5.1"]) + // Should update model-1 with new data + expect(result["model-1"].description).toBe("Model 1 updated") + }) + + it("validates model count threshold for OpenRouter", async () => { + const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + const consoleDebugSpy = vi.spyOn(console, "debug").mockImplementation(() => {}) + + // Existing cache with many models + const existingModels = { + ...Array.from({ length: 100 }, (_, i) => ({ + [`existing-model-${i}`]: { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: `Existing Model ${i}`, + }, + })).reduce((acc, curr) => ({ ...acc, ...curr }), {}), + } + + vi.mocked(fs.readFile).mockResolvedValue(JSON.stringify(existingModels)) + + // API returns too few models (below threshold) + const tooFewModels = { + "model-1": { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: "Model 1", + }, + "model-2": { + maxTokens: 4096, + contextWindow: 8192, + supportsPromptCache: false, + description: "Model 2", + }, + } + + mockGetOpenRouterModels.mockResolvedValue(tooFewModels) + + await getModels({ provider: "openrouter" }) + + // Should log warning about incomplete response + expect(consoleWarnSpy).toHaveBeenCalledWith(expect.stringContaining("OpenRouter returned only 2 models")) + + consoleWarnSpy.mockRestore() + consoleDebugSpy.mockRestore() + }) +}) diff --git a/src/api/providers/fetchers/modelCache.ts b/src/api/providers/fetchers/modelCache.ts index 50edbf274a..4f6ba9c640 100644 --- a/src/api/providers/fetchers/modelCache.ts +++ b/src/api/providers/fetchers/modelCache.ts @@ -117,6 +117,49 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise): ModelRecord { + const merged = { ...newModels } + + // Preserve specific models if they're in the preserve list + if (preserveKeys && preserveKeys.size > 0) { + for (const key of preserveKeys) { + if (existingModels[key] && !merged[key]) { + // Keep the existing model if it's not in the new set + merged[key] = existingModels[key] + console.debug(`[MODEL_CACHE] Preserved model ${key} from existing cache`) + } + } + } + + return merged +} + /** * Get models from the cache or fetch them from the provider and cache them. * There are two caches: @@ -126,10 +169,13 @@ async function fetchModelsFromProvider(options: GetModelsOptions): Promise => { - const { provider } = options +export const getModels = async ( + options: GetModelsOptions & { preserveModelIds?: Set }, +): Promise => { + const { provider, preserveModelIds } = options let models = getModelsFromCache(provider) @@ -138,26 +184,45 @@ export const getModels = async (options: GetModelsOptions): Promise } try { - models = await fetchModelsFromProvider(options) + const fetchedModels = await fetchModelsFromProvider(options) - // Cache the fetched models (even if empty, to signify a successful fetch with no models). + // Get existing cache for merging + const existingModels = await readModels(provider).catch(() => undefined) + + // Validate the fetched models + const isValid = validateFetchedModels(provider, fetchedModels) + + if (!isValid && existingModels) { + // If validation fails and we have existing models, merge carefully + models = mergeModels(existingModels, fetchedModels, preserveModelIds) + } else if (existingModels && preserveModelIds && preserveModelIds.size > 0) { + // Even if valid, preserve specific models if requested + models = mergeModels(existingModels, fetchedModels, preserveModelIds) + } else { + models = fetchedModels + } + + // Cache the merged models memoryCache.set(provider, models) await writeModels(provider, models).catch((err) => console.error(`[MODEL_CACHE] Error writing ${provider} models to file cache:`, err), ) - try { - models = await readModels(provider) - } catch (error) { - console.error(`[getModels] error reading ${provider} models from file cache`, error) - } return models || {} } catch (error) { - // Log the error and re-throw it so the caller can handle it (e.g., show a UI message). + // On error, try to use existing cache as fallback console.error(`[getModels] Failed to fetch models in modelCache for ${provider}:`, error) - throw error // Re-throw the original error to be handled by the caller. + // Try to load from disk cache as fallback + const diskCache = await readModels(provider).catch(() => undefined) + if (diskCache) { + console.debug(`[MODEL_CACHE] Using disk cache as fallback for ${provider}`) + memoryCache.set(provider, diskCache) + return diskCache + } + + throw error // Re-throw the original error if no fallback available } } @@ -168,12 +233,32 @@ export const getModels = async (options: GetModelsOptions): Promise * @param options - Provider options for fetching models * @returns Fresh models from API */ -export const refreshModels = async (options: GetModelsOptions): Promise => { - const { provider } = options +export const refreshModels = async ( + options: GetModelsOptions & { preserveModelIds?: Set }, +): Promise => { + const { provider, preserveModelIds } = options try { // Force fresh API fetch - skip getModelsFromCache() check - const models = await fetchModelsFromProvider(options) + const fetchedModels = await fetchModelsFromProvider(options) + + // Get existing models for intelligent merging + const existingModels = getModelsFromCache(provider) + + // Validate the fetched models + const isValid = validateFetchedModels(provider, fetchedModels) + + let models: ModelRecord + if (!isValid && existingModels) { + // If validation fails, merge with existing to preserve user models + models = mergeModels(existingModels, fetchedModels, preserveModelIds) + console.debug(`[refreshModels] Merged ${provider} models due to incomplete API response`) + } else if (existingModels && preserveModelIds && preserveModelIds.size > 0) { + // Preserve specific models even in valid responses + models = mergeModels(existingModels, fetchedModels, preserveModelIds) + } else { + models = fetchedModels + } // Update memory cache first memoryCache.set(provider, models) diff --git a/src/api/providers/openrouter.ts b/src/api/providers/openrouter.ts index 3db61087ef..28dd131c0b 100644 --- a/src/api/providers/openrouter.ts +++ b/src/api/providers/openrouter.ts @@ -82,8 +82,16 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH private async loadDynamicModels(): Promise { try { + // Preserve the current model ID even if it's not in the API response + const preserveModelIds = this.options.openRouterModelId + ? new Set([this.options.openRouterModelId]) + : undefined + const [models, endpoints] = await Promise.all([ - getModels({ provider: "openrouter" }), + getModels({ + provider: "openrouter", + preserveModelIds, + }), getModelEndpoints({ router: "openrouter", modelId: this.options.openRouterModelId, @@ -359,8 +367,14 @@ export class OpenRouterHandler extends BaseProvider implements SingleCompletionH } public async fetchModel() { + // Preserve the current model ID even if it's not in the API response + const preserveModelIds = this.options.openRouterModelId ? new Set([this.options.openRouterModelId]) : undefined + const [models, endpoints] = await Promise.all([ - getModels({ provider: "openrouter" }), + getModels({ + provider: "openrouter", + preserveModelIds, + }), getModelEndpoints({ router: "openrouter", modelId: this.options.openRouterModelId,