From 072887599c423913b36594c75d224dca5489862a Mon Sep 17 00:00:00 2001 From: elianiva Date: Thu, 2 Oct 2025 19:30:15 +0700 Subject: [PATCH 1/3] fix(export): exclude max tokens field for models that don't support it --- src/core/config/ProviderSettingsManager.ts | 20 ++ .../config/__tests__/importExport.spec.ts | 261 ++++++++++++++++++ 2 files changed, 281 insertions(+) diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 357a04b33a..7ee694c6f7 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -16,6 +16,7 @@ import { import { TelemetryService } from "@roo-code/telemetry" import { Mode, modes } from "../../shared/modes" +import { buildApiHandler } from "../../api" // Type-safe model migrations mapping type ModelMigrations = { @@ -528,6 +529,25 @@ export class ProviderSettingsManager { for (const name in configs) { // Avoid leaking properties from other providers. configs[name] = discriminatedProviderSettingsWithIdSchema.parse(configs[name]) + + // If it has no apiProvider, skip filtering + if (!configs[name].apiProvider) { + continue + } + + // Try to build an API handler to get model information + const apiHandler = buildApiHandler(configs[name]) + const modelInfo = apiHandler.getModel().info + + // Check if the model supports reasoning budgets + const supportsThinkingBudget = + modelInfo.supportsReasoningBudget || modelInfo.requiredReasoningBudget + + // If the model doesn't support reasoning budgets, remove the token fields + if (!supportsThinkingBudget) { + delete configs[name].modelMaxTokens + delete configs[name].modelMaxThinkingTokens + } } return profiles }) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 361d6b23b0..f7c39eaf12 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -17,6 +17,11 @@ import { safeWriteJson } from "../../../utils/safeWriteJson" import type { Mock } from "vitest" vi.mock("vscode", () => ({ + workspace: { + getConfiguration: vi.fn().mockReturnValue({ + get: vi.fn(), + }), + }, window: { showOpenDialog: vi.fn(), showSaveDialog: vi.fn(), @@ -58,6 +63,45 @@ vi.mock("os", () => ({ vi.mock("../../../utils/safeWriteJson") +// Mock buildApiHandler to avoid issues with provider instantiation in tests +vi.mock("../../../api", () => ({ + buildApiHandler: vi.fn().mockImplementation((config) => { + // Return different model info based on the provider and model + const getModelInfo = () => { + if (config.apiProvider === "claude-code") { + return { + id: config.apiModelId || "claude-sonnet-4-5", + info: { + supportsReasoningBudget: false, + requiredReasoningBudget: false, + }, + } + } + if (config.apiProvider === "anthropic" && config.apiModelId === "claude-3-5-sonnet-20241022") { + return { + id: "claude-3-5-sonnet-20241022", + info: { + supportsReasoningBudget: true, + requiredReasoningBudget: true, + }, + } + } + // Default fallback + return { + id: config.apiModelId || "claude-sonnet-4-5", + info: { + supportsReasoningBudget: false, + requiredReasoningBudget: false, + }, + } + } + + return { + getModel: vi.fn().mockReturnValue(getModelInfo()), + } + }), +})) + describe("importExport", () => { let mockProviderSettingsManager: ReturnType> let mockContextProxy: ReturnType> @@ -436,6 +480,71 @@ describe("importExport", () => { showErrorMessageSpy.mockRestore() }) + + it("should handle import when reasoning budget fields are missing from config", async () => { + // This test verifies that import works correctly when reasoning budget fields are not present + // Using claude-code provider which doesn't support reasoning budgets + + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "claude-code-provider", + apiConfigs: { + "claude-code-provider": { + apiProvider: "claude-code" as ProviderName, + apiModelId: "claude-3-5-sonnet-20241022", + id: "claude-code-id", + apiKey: "test-key", + // No modelMaxTokens or modelMaxThinkingTokens fields + }, + }, + }, + globalSettings: { mode: "code", autoApprovalEnabled: true }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + const previousProviderProfiles = { + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + } + + mockProviderSettingsManager.export.mockResolvedValue(previousProviderProfiles) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "claude-code-provider", id: "claude-code-id", apiProvider: "claude-code" as ProviderName }, + { name: "default", id: "default-id", apiProvider: "anthropic" as ProviderName }, + ]) + + mockContextProxy.export.mockResolvedValue({ mode: "code" }) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + expect(result.success).toBe(true) + expect(fs.readFile).toHaveBeenCalledWith("/mock/path/settings.json", "utf-8") + expect(mockProviderSettingsManager.export).toHaveBeenCalled() + + expect(mockProviderSettingsManager.import).toHaveBeenCalledWith({ + currentApiConfigName: "claude-code-provider", + apiConfigs: { + default: { apiProvider: "anthropic" as ProviderName, id: "default-id" }, + "claude-code-provider": { + apiProvider: "claude-code" as ProviderName, + apiModelId: "claude-3-5-sonnet-20241022", + apiKey: "test-key", + id: "claude-code-id", + }, + }, + modeApiConfigs: {}, + }) + + expect(mockContextProxy.setValues).toHaveBeenCalledWith({ mode: "code", autoApprovalEnabled: true }) + expect(mockContextProxy.setValue).toHaveBeenCalledWith("currentApiConfigName", "claude-code-provider") + }) }) describe("exportSettings", () => { @@ -1608,5 +1717,157 @@ describe("importExport", () => { "https://custom-api.example.com/v1", ) }) + + it("should exclude modelMaxTokens and modelMaxThinkingTokens when supportsReasoningBudget is false", async () => { + // This test verifies that token fields are excluded when model doesn't support reasoning budget + // Using claude-code provider which has supportsReasoningBudget: false + + ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ + fsPath: "/mock/path/roo-code-settings.json", + }) + + // Use a real ProviderSettingsManager instance to test the actual filtering logic + const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) + + // Wait for initialization to complete + await realProviderSettingsManager.initialize() + + // Save a claude-code provider config with token fields + await realProviderSettingsManager.saveConfig("claude-code-provider", { + apiProvider: "claude-code" as ProviderName, + apiModelId: "claude-sonnet-4-5", + id: "claude-code-id", + modelMaxTokens: 4096, // This should be removed during export + modelMaxThinkingTokens: 2048, // This should be removed during export + }) + + // Set this as the current provider + await realProviderSettingsManager.activateProfile({ name: "claude-code-provider" }) + + const mockGlobalSettings = { + mode: "code", + autoApprovalEnabled: true, + } + + mockContextProxy.export.mockResolvedValue(mockGlobalSettings) + ;(fs.mkdir as Mock).mockResolvedValue(undefined) + + await exportSettings({ + providerSettingsManager: realProviderSettingsManager, + contextProxy: mockContextProxy, + }) + + // Get the exported data + const exportedData = (safeWriteJson as Mock).mock.calls[0][1] + + // Verify that token fields were excluded because supportsReasoningBudget is false + const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider"] + expect(provider).toBeDefined() + expect(provider.apiModelId).toBe("claude-sonnet-4-5") + expect("modelMaxTokens" in provider).toBe(false) // Should be excluded + expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded + }) + + it("should exclude modelMaxTokens and modelMaxThinkingTokens when requiredReasoningBudget is false", async () => { + // This test verifies that token fields are excluded when model doesn't require reasoning budget + // Using claude-code provider which has requiredReasoningBudget: false + + ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ + fsPath: "/mock/path/roo-code-settings.json", + }) + + // Use a real ProviderSettingsManager instance to test the actual filtering logic + const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) + + // Wait for initialization to complete + await realProviderSettingsManager.initialize() + + // Save a claude-code provider config with token fields + await realProviderSettingsManager.saveConfig("claude-code-provider-2", { + apiProvider: "claude-code" as ProviderName, + apiModelId: "claude-sonnet-4-5", + id: "claude-code-id-2", + apiKey: "test-key", + modelMaxTokens: 4096, // This should be removed during export + modelMaxThinkingTokens: 2048, // This should be removed during export + }) + + // Set this as the current provider + await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-2" }) + + const mockGlobalSettings = { + mode: "code", + autoApprovalEnabled: true, + } + + mockContextProxy.export.mockResolvedValue(mockGlobalSettings) + ;(fs.mkdir as Mock).mockResolvedValue(undefined) + + await exportSettings({ + providerSettingsManager: realProviderSettingsManager, + contextProxy: mockContextProxy, + }) + + // Get the exported data + const exportedData = (safeWriteJson as Mock).mock.calls[0][1] + + // Verify that token fields were excluded because requiredReasoningBudget is false + const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-2"] + expect(provider).toBeDefined() + expect(provider.apiModelId).toBe("claude-sonnet-4-5") + expect("modelMaxTokens" in provider).toBe(false) // Should be excluded + expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded + }) + + it("should exclude modelMaxTokens and modelMaxThinkingTokens when both supportsReasoningBudget and requiredReasoningBudget are false", async () => { + // This test verifies that token fields are excluded when model has both reasoning budget flags set to false + // Using claude-code provider which has both flags set to false + + ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ + fsPath: "/mock/path/roo-code-settings.json", + }) + + // Use a real ProviderSettingsManager instance to test the actual filtering logic + const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) + + // Wait for initialization to complete + await realProviderSettingsManager.initialize() + + // Save a claude-code provider config with token fields + await realProviderSettingsManager.saveConfig("claude-code-provider-3", { + apiProvider: "claude-code" as ProviderName, + apiModelId: "claude-3-5-haiku-20241022", // Use a different model ID + id: "claude-code-id-3", + apiKey: "test-key", + modelMaxTokens: 4096, // This should be removed during export + modelMaxThinkingTokens: 2048, // This should be removed during export + }) + + // Set this as the current provider + await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-3" }) + + const mockGlobalSettings = { + mode: "code", + autoApprovalEnabled: true, + } + + mockContextProxy.export.mockResolvedValue(mockGlobalSettings) + ;(fs.mkdir as Mock).mockResolvedValue(undefined) + + await exportSettings({ + providerSettingsManager: realProviderSettingsManager, + contextProxy: mockContextProxy, + }) + + // Get the exported data + const exportedData = (safeWriteJson as Mock).mock.calls[0][1] + + // Verify that token fields were excluded because both reasoning budget flags are false + const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-3"] + expect(provider).toBeDefined() + expect(provider.apiModelId).toBe("claude-3-5-haiku-20241022") + expect("modelMaxTokens" in provider).toBe(false) // Should be excluded + expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded + }) }) }) From 902e1fcbb6c17a8c80ab00e45ac4ea07a4fc6f5f Mon Sep 17 00:00:00 2001 From: elianiva Date: Thu, 2 Oct 2025 20:02:56 +0700 Subject: [PATCH 2/3] refactor(export): wrap export in try catch --- src/core/config/ProviderSettingsManager.ts | 28 +++++++++++++--------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/core/config/ProviderSettingsManager.ts b/src/core/config/ProviderSettingsManager.ts index 7ee694c6f7..81a5521b62 100644 --- a/src/core/config/ProviderSettingsManager.ts +++ b/src/core/config/ProviderSettingsManager.ts @@ -536,17 +536,23 @@ export class ProviderSettingsManager { } // Try to build an API handler to get model information - const apiHandler = buildApiHandler(configs[name]) - const modelInfo = apiHandler.getModel().info - - // Check if the model supports reasoning budgets - const supportsThinkingBudget = - modelInfo.supportsReasoningBudget || modelInfo.requiredReasoningBudget - - // If the model doesn't support reasoning budgets, remove the token fields - if (!supportsThinkingBudget) { - delete configs[name].modelMaxTokens - delete configs[name].modelMaxThinkingTokens + try { + const apiHandler = buildApiHandler(configs[name]) + const modelInfo = apiHandler.getModel().info + + // Check if the model supports reasoning budgets + const supportsReasoningBudget = + modelInfo.supportsReasoningBudget || modelInfo.requiredReasoningBudget + + // If the model doesn't support reasoning budgets, remove the token fields + if (!supportsReasoningBudget) { + delete configs[name].modelMaxTokens + delete configs[name].modelMaxThinkingTokens + } + } catch (error) { + // If we can't build the API handler or get model info, skip filtering + // to avoid accidental data loss from incomplete configurations + console.warn(`Skipping token field filtering for config '${name}': ${error}`) } } return profiles From e3ed0f7905ffb3419da15da9d677c06840b1ddb2 Mon Sep 17 00:00:00 2001 From: elianiva Date: Thu, 2 Oct 2025 20:03:14 +0700 Subject: [PATCH 3/3] test(export): use parameterized test --- .../config/__tests__/importExport.spec.ts | 203 ++++++------------ 1 file changed, 62 insertions(+), 141 deletions(-) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index f7c39eaf12..3d5329f377 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -1718,156 +1718,77 @@ describe("importExport", () => { ) }) - it("should exclude modelMaxTokens and modelMaxThinkingTokens when supportsReasoningBudget is false", async () => { - // This test verifies that token fields are excluded when model doesn't support reasoning budget - // Using claude-code provider which has supportsReasoningBudget: false - - ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ - fsPath: "/mock/path/roo-code-settings.json", - }) - - // Use a real ProviderSettingsManager instance to test the actual filtering logic - const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) - - // Wait for initialization to complete - await realProviderSettingsManager.initialize() - - // Save a claude-code provider config with token fields - await realProviderSettingsManager.saveConfig("claude-code-provider", { - apiProvider: "claude-code" as ProviderName, - apiModelId: "claude-sonnet-4-5", - id: "claude-code-id", - modelMaxTokens: 4096, // This should be removed during export - modelMaxThinkingTokens: 2048, // This should be removed during export - }) - - // Set this as the current provider - await realProviderSettingsManager.activateProfile({ name: "claude-code-provider" }) - - const mockGlobalSettings = { - mode: "code", - autoApprovalEnabled: true, - } - - mockContextProxy.export.mockResolvedValue(mockGlobalSettings) - ;(fs.mkdir as Mock).mockResolvedValue(undefined) - - await exportSettings({ - providerSettingsManager: realProviderSettingsManager, - contextProxy: mockContextProxy, - }) - - // Get the exported data - const exportedData = (safeWriteJson as Mock).mock.calls[0][1] - - // Verify that token fields were excluded because supportsReasoningBudget is false - const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider"] - expect(provider).toBeDefined() - expect(provider.apiModelId).toBe("claude-sonnet-4-5") - expect("modelMaxTokens" in provider).toBe(false) // Should be excluded - expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded - }) - - it("should exclude modelMaxTokens and modelMaxThinkingTokens when requiredReasoningBudget is false", async () => { - // This test verifies that token fields are excluded when model doesn't require reasoning budget - // Using claude-code provider which has requiredReasoningBudget: false - - ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ - fsPath: "/mock/path/roo-code-settings.json", - }) - - // Use a real ProviderSettingsManager instance to test the actual filtering logic - const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) - - // Wait for initialization to complete - await realProviderSettingsManager.initialize() - - // Save a claude-code provider config with token fields - await realProviderSettingsManager.saveConfig("claude-code-provider-2", { - apiProvider: "claude-code" as ProviderName, - apiModelId: "claude-sonnet-4-5", - id: "claude-code-id-2", - apiKey: "test-key", - modelMaxTokens: 4096, // This should be removed during export - modelMaxThinkingTokens: 2048, // This should be removed during export - }) - - // Set this as the current provider - await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-2" }) - - const mockGlobalSettings = { - mode: "code", - autoApprovalEnabled: true, - } - - mockContextProxy.export.mockResolvedValue(mockGlobalSettings) - ;(fs.mkdir as Mock).mockResolvedValue(undefined) - - await exportSettings({ - providerSettingsManager: realProviderSettingsManager, - contextProxy: mockContextProxy, - }) - - // Get the exported data - const exportedData = (safeWriteJson as Mock).mock.calls[0][1] - - // Verify that token fields were excluded because requiredReasoningBudget is false - const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-2"] - expect(provider).toBeDefined() - expect(provider.apiModelId).toBe("claude-sonnet-4-5") - expect("modelMaxTokens" in provider).toBe(false) // Should be excluded - expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded - }) - - it("should exclude modelMaxTokens and modelMaxThinkingTokens when both supportsReasoningBudget and requiredReasoningBudget are false", async () => { - // This test verifies that token fields are excluded when model has both reasoning budget flags set to false - // Using claude-code provider which has both flags set to false + it.each([ + { + testCase: "supportsReasoningBudget is false", + providerName: "claude-code-provider", + modelId: "claude-sonnet-4-5", + providerId: "claude-code-id", + }, + { + testCase: "requiredReasoningBudget is false", + providerName: "claude-code-provider-2", + modelId: "claude-sonnet-4-5", + providerId: "claude-code-id-2", + }, + { + testCase: "both supportsReasoningBudget and requiredReasoningBudget are false", + providerName: "claude-code-provider-3", + modelId: "claude-3-5-haiku-20241022", + providerId: "claude-code-id-3", + }, + ])( + "should exclude modelMaxTokens and modelMaxThinkingTokens when $testCase", + async ({ providerName, modelId, providerId }) => { + // This test verifies that token fields are excluded when model doesn't support reasoning budget + // Using claude-code provider which has supportsReasoningBudget: false and requiredReasoningBudget: false - ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ - fsPath: "/mock/path/roo-code-settings.json", - }) + ;(vscode.window.showSaveDialog as Mock).mockResolvedValue({ + fsPath: "/mock/path/roo-code-settings.json", + }) - // Use a real ProviderSettingsManager instance to test the actual filtering logic - const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) + // Use a real ProviderSettingsManager instance to test the actual filtering logic + const realProviderSettingsManager = new ProviderSettingsManager(mockExtensionContext) - // Wait for initialization to complete - await realProviderSettingsManager.initialize() + // Wait for initialization to complete + await realProviderSettingsManager.initialize() - // Save a claude-code provider config with token fields - await realProviderSettingsManager.saveConfig("claude-code-provider-3", { - apiProvider: "claude-code" as ProviderName, - apiModelId: "claude-3-5-haiku-20241022", // Use a different model ID - id: "claude-code-id-3", - apiKey: "test-key", - modelMaxTokens: 4096, // This should be removed during export - modelMaxThinkingTokens: 2048, // This should be removed during export - }) + // Save a claude-code provider config with token fields + await realProviderSettingsManager.saveConfig(providerName, { + apiProvider: "claude-code" as ProviderName, + apiModelId: modelId, + id: providerId, + apiKey: "test-key", + modelMaxTokens: 4096, // This should be removed during export + modelMaxThinkingTokens: 2048, // This should be removed during export + }) - // Set this as the current provider - await realProviderSettingsManager.activateProfile({ name: "claude-code-provider-3" }) + // Set this as the current provider + await realProviderSettingsManager.activateProfile({ name: providerName }) - const mockGlobalSettings = { - mode: "code", - autoApprovalEnabled: true, - } + const mockGlobalSettings = { + mode: "code", + autoApprovalEnabled: true, + } - mockContextProxy.export.mockResolvedValue(mockGlobalSettings) - ;(fs.mkdir as Mock).mockResolvedValue(undefined) + mockContextProxy.export.mockResolvedValue(mockGlobalSettings) + ;(fs.mkdir as Mock).mockResolvedValue(undefined) - await exportSettings({ - providerSettingsManager: realProviderSettingsManager, - contextProxy: mockContextProxy, - }) + await exportSettings({ + providerSettingsManager: realProviderSettingsManager, + contextProxy: mockContextProxy, + }) - // Get the exported data - const exportedData = (safeWriteJson as Mock).mock.calls[0][1] + // Get the exported data + const exportedData = (safeWriteJson as Mock).mock.calls[0][1] - // Verify that token fields were excluded because both reasoning budget flags are false - const provider = exportedData.providerProfiles.apiConfigs["claude-code-provider-3"] - expect(provider).toBeDefined() - expect(provider.apiModelId).toBe("claude-3-5-haiku-20241022") - expect("modelMaxTokens" in provider).toBe(false) // Should be excluded - expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded - }) + // Verify that token fields were excluded because reasoning budget is not supported/required + const provider = exportedData.providerProfiles.apiConfigs[providerName] + expect(provider).toBeDefined() + expect(provider.apiModelId).toBe(modelId) + expect("modelMaxTokens" in provider).toBe(false) // Should be excluded + expect("modelMaxThinkingTokens" in provider).toBe(false) // Should be excluded + }, + ) }) })