diff --git a/src/core/config/ContextProxy.ts b/src/core/config/ContextProxy.ts index c3b602ea74d..87ce79a3251 100644 --- a/src/core/config/ContextProxy.ts +++ b/src/core/config/ContextProxy.ts @@ -96,6 +96,9 @@ export class ContextProxy { // Migration: Move legacy customCondensingPrompt to customSupportPrompts await this.migrateLegacyCondensingPrompt() + // Migration: Clear old default condensing prompt so users get the improved v2 default + await this.migrateOldDefaultCondensingPrompt() + this._isInitialized = true } @@ -138,6 +141,87 @@ export class ContextProxy { } } + /** + * Clears the old v1 default condensing prompt from customSupportPrompts.CONDENSE if present. + * + * Before PR #10873 "Intelligent Context Condensation v2", the default condensing prompt was + * a simpler 6-section format. Users who had this old default saved in their settings would + * be stuck with it instead of getting the improved v2 default (which includes analysis tags, + * error tracking, all user messages, and better task continuity). + * + * This migration uses fingerprinting to detect the old v1 default - checking for key + * identifying phrases unique to v1 and absence of v2-specific features. This is more + * lenient than exact matching and handles whitespace variations. + */ + private async migrateOldDefaultCondensingPrompt() { + try { + const currentSupportPrompts = + this.originalContext.globalState.get>("customSupportPrompts") || {} + + const savedCondensePrompt = currentSupportPrompts.CONDENSE + + if (savedCondensePrompt && this.isOldV1DefaultCondensePrompt(savedCondensePrompt)) { + logger.info( + "Clearing old v1 default condensing prompt from customSupportPrompts.CONDENSE - user will now get the improved v2 default", + ) + + // Remove the CONDENSE key from customSupportPrompts + const { CONDENSE: _, ...remainingPrompts } = currentSupportPrompts + const updatedPrompts = Object.keys(remainingPrompts).length > 0 ? remainingPrompts : undefined + + await this.originalContext.globalState.update("customSupportPrompts", updatedPrompts) + this.stateCache.customSupportPrompts = updatedPrompts + } + } catch (error) { + logger.error( + `Error during old default condensing prompt migration: ${error instanceof Error ? error.message : String(error)}`, + ) + } + } + + /** + * Detects if a prompt is the old v1 default condensing prompt using fingerprinting. + * This is more lenient than exact matching - it checks for key identifying phrases + * unique to v1 and absence of v2-specific features. + * + * V1 characteristics: + * - Exactly 6 numbered sections (1-6) + * - Contains specific section headers like "Previous Conversation", "Current Work", etc. + * - Does NOT contain v2-specific features like "", "SYSTEM OPERATION", etc. + */ + private isOldV1DefaultCondensePrompt(prompt: string): boolean { + // Key phrases unique to the v1 default (must ALL be present) + const v1RequiredPhrases = [ + "Your task is to create a detailed summary of the conversation so far", + "1. Previous Conversation:", + "2. Current Work:", + "3. Key Technical Concepts:", + "4. Relevant Files and Code:", + "5. Problem Solving:", + "6. Pending Tasks and Next Steps:", + "Output only the summary of the conversation so far", + ] + + // V2-specific features (if ANY are present, this is NOT v1 default) + const v2Features = [ + "", + "SYSTEM OPERATION", + "Errors and fixes", + "All user messages", + "7.", // v2 has more than 6 sections + "8.", + "9.", + ] + + // Check that all v1 required phrases are present + const hasAllV1Phrases = v1RequiredPhrases.every((phrase) => prompt.toLowerCase().includes(phrase.toLowerCase())) + + // Check that no v2 features are present + const hasNoV2Features = v2Features.every((feature) => !prompt.toLowerCase().includes(feature.toLowerCase())) + + return hasAllV1Phrases && hasNoV2Features + } + /** * Migrates invalid/removed apiProvider values by clearing them from storage. * This handles cases where a user had a provider selected that was later removed diff --git a/src/core/config/__tests__/ContextProxy.spec.ts b/src/core/config/__tests__/ContextProxy.spec.ts index bfdbd1619f5..2060260c6ca 100644 --- a/src/core/config/__tests__/ContextProxy.spec.ts +++ b/src/core/config/__tests__/ContextProxy.spec.ts @@ -70,16 +70,18 @@ describe("ContextProxy", () => { describe("constructor", () => { it("should initialize state cache with all global state keys", () => { - // +2 for the migration checks: + // +3 for the migration checks: // 1. openRouterImageGenerationSettings // 2. customCondensingPrompt - expect(mockGlobalState.get).toHaveBeenCalledTimes(GLOBAL_STATE_KEYS.length + 2) + // 3. customSupportPrompts (for migrateOldDefaultCondensingPrompt) + expect(mockGlobalState.get).toHaveBeenCalledTimes(GLOBAL_STATE_KEYS.length + 3) for (const key of GLOBAL_STATE_KEYS) { expect(mockGlobalState.get).toHaveBeenCalledWith(key) } // Also check for migration calls expect(mockGlobalState.get).toHaveBeenCalledWith("openRouterImageGenerationSettings") expect(mockGlobalState.get).toHaveBeenCalledWith("customCondensingPrompt") + expect(mockGlobalState.get).toHaveBeenCalledWith("customSupportPrompts") }) it("should initialize secret cache with all secret keys", () => { @@ -102,8 +104,8 @@ describe("ContextProxy", () => { const result = proxy.getGlobalState("apiProvider") expect(result).toBe("deepseek") - // Original context should be called once during updateGlobalState (+2 for migration checks) - expect(mockGlobalState.get).toHaveBeenCalledTimes(GLOBAL_STATE_KEYS.length + 2) // From initialization + migration checks + // Original context should be called once during updateGlobalState (+3 for migration checks) + expect(mockGlobalState.get).toHaveBeenCalledTimes(GLOBAL_STATE_KEYS.length + 3) // From initialization + migration checks }) it("should handle default values correctly", async () => { @@ -506,4 +508,123 @@ describe("ContextProxy", () => { expect(settings.apiProvider).toBeUndefined() }) }) + + describe("old default condensing prompt migration", () => { + // The old v1 default condensing prompt from before PR #10873 + const OLD_V1_DEFAULT_CONDENSE_PROMPT = `Your task is to create a detailed summary of the conversation so far, paying close attention to the user's explicit requests and your previous actions. +This summary should be thorough in capturing technical details, code patterns, and architectural decisions that would be essential for continuing with the conversation and supporting any continuing tasks. + +Your summary should be structured as follows: +Context: The context to continue the conversation with. If applicable based on the current task, this should include: + 1. Previous Conversation: High level details about what was discussed throughout the entire conversation with the user. This should be written to allow someone to be able to follow the general overarching conversation flow. + 2. Current Work: Describe in detail what was being worked on prior to this request to summarize the conversation. Pay special attention to the more recent messages in the conversation. + 3. Key Technical Concepts: List all important technical concepts, technologies, coding conventions, and frameworks discussed, which might be relevant for continuing with this work. + 4. Relevant Files and Code: If applicable, enumerate specific files and code sections examined, modified, or created for the task continuation. Pay special attention to the most recent messages and changes. + 5. Problem Solving: Document problems solved thus far and any ongoing troubleshooting efforts. + 6. Pending Tasks and Next Steps: Outline all pending tasks that you have explicitly been asked to work on, as well as list the next steps you will take for all outstanding work, if applicable. Include code snippets where they add clarity. For any next steps, include direct quotes from the most recent conversation showing exactly what task you were working on and where you left off. This should be verbatim to ensure there's no information loss in context between tasks. + +Example summary structure: +1. Previous Conversation: + [Detailed description] +2. Current Work: + [Detailed description] +3. Key Technical Concepts: + - [Concept 1] + - [Concept 2] + - [...] +4. Relevant Files and Code: + - [File Name 1] + - [Summary of why this file is important] + - [Summary of the changes made to this file, if any] + - [Important Code Snippet] + - [File Name 2] + - [Important Code Snippet] + - [...] +5. Problem Solving: + [Detailed description] +6. Pending Tasks and Next Steps: + - [Task 1 details & next steps] + - [Task 2 details & next steps] + - [...] + +Output only the summary of the conversation so far, without any additional commentary or explanation.` + + it("should clear old v1 default condensing prompt from customSupportPrompts during initialization", async () => { + // Reset and create a new proxy with old v1 default prompt in customSupportPrompts + vi.clearAllMocks() + mockGlobalState.get.mockImplementation((key: string) => { + if (key === "customSupportPrompts") { + return { CONDENSE: OLD_V1_DEFAULT_CONDENSE_PROMPT } + } + return undefined + }) + + const proxyWithOldDefault = new ContextProxy(mockContext) + await proxyWithOldDefault.initialize() + + // Should have cleared the old default by updating customSupportPrompts to undefined + // (since CONDENSE was the only key) + expect(mockGlobalState.update).toHaveBeenCalledWith("customSupportPrompts", undefined) + }) + + it("should preserve other custom prompts when clearing old v1 default", async () => { + // Reset and create a new proxy with old v1 default plus other custom prompts + vi.clearAllMocks() + mockGlobalState.get.mockImplementation((key: string) => { + if (key === "customSupportPrompts") { + return { + CONDENSE: OLD_V1_DEFAULT_CONDENSE_PROMPT, + EXPLAIN: "Custom explain prompt", + } + } + return undefined + }) + + const proxyWithOldDefault = new ContextProxy(mockContext) + await proxyWithOldDefault.initialize() + + // Should have updated customSupportPrompts to keep EXPLAIN but remove CONDENSE + expect(mockGlobalState.update).toHaveBeenCalledWith("customSupportPrompts", { + EXPLAIN: "Custom explain prompt", + }) + }) + + it("should not clear truly customized condensing prompts", async () => { + // Reset and create a new proxy with a truly customized condensing prompt + vi.clearAllMocks() + const customPrompt = "My custom condensing instructions" + mockGlobalState.get.mockImplementation((key: string) => { + if (key === "customSupportPrompts") { + return { CONDENSE: customPrompt } + } + return undefined + }) + + const proxyWithCustomPrompt = new ContextProxy(mockContext) + await proxyWithCustomPrompt.initialize() + + // Should NOT have called update for customSupportPrompts (custom prompt should be preserved) + const updateCalls = mockGlobalState.update.mock.calls + const customSupportPromptsUpdateCalls = updateCalls.filter( + (call: any[]) => call[0] === "customSupportPrompts", + ) + expect(customSupportPromptsUpdateCalls.length).toBe(0) + }) + + it("should not fail when customSupportPrompts is undefined", async () => { + // Reset and create a new proxy with no customSupportPrompts + vi.clearAllMocks() + mockGlobalState.get.mockReturnValue(undefined) + + const proxyWithNoPrompts = new ContextProxy(mockContext) + await proxyWithNoPrompts.initialize() + + // Should not have called update for customSupportPrompts + const updateCalls = mockGlobalState.update.mock.calls + const customSupportPromptsUpdateCalls = updateCalls.filter( + (call: any[]) => call[0] === "customSupportPrompts", + ) + expect(customSupportPromptsUpdateCalls.length).toBe(0) + }) + }) }) diff --git a/src/core/config/__tests__/importExport.spec.ts b/src/core/config/__tests__/importExport.spec.ts index 9aee8693c67..9873ffde945 100644 --- a/src/core/config/__tests__/importExport.spec.ts +++ b/src/core/config/__tests__/importExport.spec.ts @@ -27,6 +27,7 @@ vi.mock("vscode", () => ({ showSaveDialog: vi.fn(), showErrorMessage: vi.fn(), showInformationMessage: vi.fn(), + showWarningMessage: vi.fn(), }, Uri: { file: vi.fn((filePath) => ({ fsPath: filePath })), @@ -149,6 +150,7 @@ describe("importExport", () => { expect(vscode.window.showOpenDialog).toHaveBeenCalledWith({ filters: { JSON: ["json"] }, canSelectMany: false, + defaultUri: expect.anything(), // Defaults to Downloads or last export path }) expect(fs.readFile).not.toHaveBeenCalled() @@ -537,6 +539,487 @@ describe("importExport", () => { expect(mockContextProxy.setValues).toHaveBeenCalledWith({ mode: "code", autoApprovalEnabled: true }) expect(mockContextProxy.setValue).toHaveBeenCalledWith("currentApiConfigName", "openai-provider") }) + + describe("lenient import with invalid providers", () => { + it("should sanitize profiles with invalid apiProvider and return warnings", async () => { + // Test importing a profile with a removed/invalid provider like "claude-code" + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "valid-profile", + apiConfigs: { + "valid-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "valid-id", + }, + "invalid-profile": { + apiProvider: "claude-code", // Invalid/removed provider + apiKey: "some-key", + id: "invalid-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, + { name: "default", id: "default-id", apiProvider: "anthropic" as ProviderName }, + ]) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should succeed + expect(result.success).toBe(true) + + // Should have warnings about the sanitized profile + expect(result).toHaveProperty("warnings") + expect((result as { warnings?: string[] }).warnings).toBeDefined() + expect((result as { warnings?: string[] }).warnings!.length).toBeGreaterThan(0) + expect((result as { warnings?: string[] }).warnings![0]).toContain("invalid-profile") + expect((result as { warnings?: string[] }).warnings![0]).toContain("claude-code") + + // The valid profile should be imported + expect(mockProviderSettingsManager.import).toHaveBeenCalled() + const importedProfiles = mockProviderSettingsManager.import.mock.calls[0][0] + expect(importedProfiles.apiConfigs["valid-profile"]).toBeDefined() + expect(importedProfiles.apiConfigs["valid-profile"].apiProvider).toBe("openai") + + // The invalid profile should still be imported but without apiProvider + expect(importedProfiles.apiConfigs["invalid-profile"]).toBeDefined() + expect(importedProfiles.apiConfigs["invalid-profile"].apiProvider).toBeUndefined() + }) + + it("should skip completely invalid profiles and return warnings", async () => { + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "valid-profile", + apiConfigs: { + "valid-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "valid-id", + }, + "type-invalid": { + // Invalid type - modelTemperature should be a number, not a string + modelTemperature: "not-a-number", + id: "type-invalid-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, + ]) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should succeed (valid profile was imported) + expect(result.success).toBe(true) + + // Should have warnings about the skipped profile + expect((result as { warnings?: string[] }).warnings).toBeDefined() + expect((result as { warnings?: string[] }).warnings!.some((w) => w.includes("type-invalid"))).toBe(true) + expect((result as { warnings?: string[] }).warnings!.some((w) => w.includes("skipped"))).toBe(true) + + // The valid profile should be imported + const importedProfiles = mockProviderSettingsManager.import.mock.calls[0][0] + expect(importedProfiles.apiConfigs["valid-profile"]).toBeDefined() + + // The type-invalid profile should NOT be imported + expect(importedProfiles.apiConfigs["type-invalid"]).toBeUndefined() + }) + + it("should fail when NO valid profiles can be imported", async () => { + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "invalid-profile", + apiConfigs: { + "invalid-profile-1": { + // Invalid type - rateLimitSeconds should be number + rateLimitSeconds: "not-a-number", + id: "invalid-1", + }, + "invalid-profile-2": { + // Invalid type - modelTemperature should be number + modelTemperature: { invalid: "object" }, + id: "invalid-2", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should fail since all profiles have schema validation errors + expect(result.success).toBe(false) + expect(result.error).toContain("No valid profiles could be imported") + + // Should NOT have called import since there were no valid profiles + expect(mockProviderSettingsManager.import).not.toHaveBeenCalled() + }) + + it("should show warning notification when importing with warnings via importSettingsWithFeedback", async () => { + const filePath = "/mock/path/settings.json" + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "valid-profile", + apiConfigs: { + "valid-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "valid-id", + }, + "problematic-profile": { + apiProvider: "removed-provider", // Invalid provider + apiKey: "some-key", + id: "problematic-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + ;(fs.access as Mock).mockResolvedValue(undefined) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, + ]) + + const mockProvider = { + settingsImportedAt: 0, + postStateToWebview: vi.fn().mockResolvedValue(undefined), + } + + const showWarningMessageSpy = vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined) + const showInfoMessageSpy = vi + .spyOn(vscode.window, "showInformationMessage") + .mockResolvedValue(undefined) + const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + await importSettingsWithFeedback( + { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + provider: mockProvider, + }, + filePath, + ) + + // Should show warning message with short summary (not full details) + expect(showWarningMessageSpy).toHaveBeenCalledWith( + expect.stringContaining("1 profile had issues during import."), + ) + expect(showWarningMessageSpy).toHaveBeenCalledWith( + expect.stringContaining("See Developer Tools console for details."), + ) + // Should log full details to console + expect(consoleWarnSpy).toHaveBeenCalledWith( + "Settings import completed with warnings:", + expect.arrayContaining([expect.stringContaining("problematic-profile")]), + ) + expect(showInfoMessageSpy).not.toHaveBeenCalled() + + // Provider state should still be updated + expect(mockProvider.settingsImportedAt).toBeGreaterThan(0) + expect(mockProvider.postStateToWebview).toHaveBeenCalled() + + showWarningMessageSpy.mockRestore() + showInfoMessageSpy.mockRestore() + consoleWarnSpy.mockRestore() + }) + + it("should handle multiple profiles with mixed valid and invalid providers", async () => { + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "anthropic-profile", + apiConfigs: { + "anthropic-profile": { + apiProvider: "anthropic" as ProviderName, + anthropicApiKey: "key-1", + id: "anthropic-id", + }, + "openai-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "key-2", + id: "openai-id", + }, + "old-claude-profile": { + apiProvider: "claude-code", // Removed provider + apiKey: "key-3", + id: "claude-id", + }, + "another-invalid": { + apiProvider: "some-old-provider", // Another removed provider + apiKey: "key-4", + id: "another-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "anthropic-profile", id: "anthropic-id", apiProvider: "anthropic" as ProviderName }, + { name: "openai-profile", id: "openai-id", apiProvider: "openai" as ProviderName }, + ]) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should succeed + expect(result.success).toBe(true) + + // Should have multiple warnings + const warnings = (result as { warnings?: string[] }).warnings! + expect(warnings.length).toBe(2) // Two profiles had invalid providers + expect(warnings.some((w) => w.includes("old-claude-profile"))).toBe(true) + expect(warnings.some((w) => w.includes("another-invalid"))).toBe(true) + + // Valid profiles should be imported correctly + const importedProfiles = mockProviderSettingsManager.import.mock.calls[0][0] + expect(importedProfiles.apiConfigs["anthropic-profile"].apiProvider).toBe("anthropic") + expect(importedProfiles.apiConfigs["openai-profile"].apiProvider).toBe("openai") + + // Invalid provider profiles should have apiProvider removed + expect(importedProfiles.apiConfigs["old-claude-profile"].apiProvider).toBeUndefined() + expect(importedProfiles.apiConfigs["another-invalid"].apiProvider).toBeUndefined() + }) + + it("should fallback currentApiConfigName when the imported current profile was skipped", async () => { + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + // Import file where currentApiConfigName points to an invalid profile that gets skipped + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "invalid-current-profile", // This profile is completely invalid + apiConfigs: { + "invalid-current-profile": { + // Invalid type - rateLimitSeconds should be number + rateLimitSeconds: "not-a-number", + id: "invalid-current-id", + }, + "valid-fallback-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "fallback-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-fallback-profile", id: "fallback-id", apiProvider: "openai" as ProviderName }, + ]) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should succeed + expect(result.success).toBe(true) + + // Should have warnings about the skipped profile AND the fallback + const warnings = (result as { warnings?: string[] }).warnings! + expect(warnings).toBeDefined() + expect(warnings.some((w) => w.includes("invalid-current-profile") && w.includes("skipped"))).toBe(true) + expect( + warnings.some( + (w) => + w.includes("invalid-current-profile") && + w.includes("not available") && + w.includes("valid-fallback-profile"), + ), + ).toBe(true) + + // The currentApiConfigName should be set to the valid fallback profile, not the invalid one + const importedProfiles = mockProviderSettingsManager.import.mock.calls[0][0] + expect(importedProfiles.currentApiConfigName).toBe("valid-fallback-profile") + + // contextProxy should also be set with the fallback profile name + expect(mockContextProxy.setValue).toHaveBeenCalledWith("currentApiConfigName", "valid-fallback-profile") + + // The invalid profile should NOT be imported + expect(importedProfiles.apiConfigs["invalid-current-profile"]).toBeUndefined() + // The valid fallback profile should be imported + expect(importedProfiles.apiConfigs["valid-fallback-profile"]).toBeDefined() + }) + + it("should keep previous currentApiConfigName when all imported profiles are invalid", async () => { + ;(vscode.window.showOpenDialog as Mock).mockResolvedValue([{ fsPath: "/mock/path/settings.json" }]) + + // All profiles in the import are invalid, but we have existing profiles + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "invalid-profile", + apiConfigs: { + "invalid-profile": { + rateLimitSeconds: "not-a-number", + id: "invalid-id", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "existing-profile", + apiConfigs: { + "existing-profile": { apiProvider: "anthropic" as ProviderName, id: "existing-id" }, + }, + }) + + const result = await importSettings({ + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + }) + + // Import should fail because no valid profiles could be imported + expect(result.success).toBe(false) + expect(result.error).toContain("No valid profiles could be imported") + }) + + it("should show plural summary for multiple profile warnings via importSettingsWithFeedback", async () => { + const filePath = "/mock/path/settings.json" + const mockFileContent = JSON.stringify({ + providerProfiles: { + currentApiConfigName: "valid-profile", + apiConfigs: { + "valid-profile": { + apiProvider: "openai" as ProviderName, + apiKey: "test-key", + id: "valid-id", + }, + "problematic-profile-1": { + apiProvider: "removed-provider-1", + apiKey: "key-1", + id: "problematic-id-1", + }, + "problematic-profile-2": { + apiProvider: "removed-provider-2", + apiKey: "key-2", + id: "problematic-id-2", + }, + }, + }, + globalSettings: { mode: "code" }, + }) + + ;(fs.readFile as Mock).mockResolvedValue(mockFileContent) + ;(fs.access as Mock).mockResolvedValue(undefined) + + mockProviderSettingsManager.export.mockResolvedValue({ + currentApiConfigName: "default", + apiConfigs: { default: { apiProvider: "anthropic" as ProviderName, id: "default-id" } }, + }) + mockProviderSettingsManager.listConfig.mockResolvedValue([ + { name: "valid-profile", id: "valid-id", apiProvider: "openai" as ProviderName }, + ]) + + const mockProvider = { + settingsImportedAt: 0, + postStateToWebview: vi.fn().mockResolvedValue(undefined), + } + + const showWarningMessageSpy = vi.spyOn(vscode.window, "showWarningMessage").mockResolvedValue(undefined) + const consoleWarnSpy = vi.spyOn(console, "warn").mockImplementation(() => {}) + + await importSettingsWithFeedback( + { + providerSettingsManager: mockProviderSettingsManager, + contextProxy: mockContextProxy, + customModesManager: mockCustomModesManager, + provider: mockProvider, + }, + filePath, + ) + + // Should show warning message with plural summary for multiple warnings + expect(showWarningMessageSpy).toHaveBeenCalledWith( + expect.stringContaining("2 profiles had issues during import."), + ) + // Should log full details to console + expect(consoleWarnSpy).toHaveBeenCalledWith( + "Settings import completed with warnings:", + expect.arrayContaining([ + expect.stringContaining("problematic-profile-1"), + expect.stringContaining("problematic-profile-2"), + ]), + ) + + showWarningMessageSpy.mockRestore() + consoleWarnSpy.mockRestore() + }) + }) }) describe("exportSettings", () => { diff --git a/src/core/config/importExport.ts b/src/core/config/importExport.ts index de3119e0c90..542f5b07430 100644 --- a/src/core/config/importExport.ts +++ b/src/core/config/importExport.ts @@ -6,7 +6,12 @@ import fs from "fs/promises" import * as vscode from "vscode" import { z, ZodError } from "zod" -import { globalSettingsSchema } from "@roo-code/types" +import { + globalSettingsSchema, + providerSettingsWithIdSchema, + isProviderName, + type ProviderSettingsWithId, +} from "@roo-code/types" import { TelemetryService } from "@roo-code/telemetry" import { ProviderSettingsManager, providerProfilesSchema } from "./ProviderSettingsManager" @@ -32,36 +37,119 @@ type ImportWithProviderOptions = ImportOptions & { } } +/** + * Sanitizes a provider config by resetting invalid/removed apiProvider values. + * Returns the sanitized config and a warning message if the provider was invalid. + */ +function sanitizeProviderConfig(configName: string, apiConfig: unknown): { config: unknown; warning?: string } { + if (typeof apiConfig !== "object" || apiConfig === null) { + return { config: apiConfig } + } + + const config = apiConfig as Record + + // Check if apiProvider is set and if it's still valid + if (config.apiProvider !== undefined && !isProviderName(config.apiProvider)) { + const invalidProvider = config.apiProvider + // Return a new config object without the invalid apiProvider + const { apiProvider, ...restConfig } = config + return { + config: restConfig, + warning: `Profile "${configName}": Invalid provider "${invalidProvider}" was removed. Please reconfigure this profile.`, + } + } + + return { config: apiConfig } +} + /** * Imports configuration from a specific file path * Shares base functionality for import settings for both the manual - * and automatic settings importing + * and automatic settings importing. + * + * Uses lenient parsing to handle invalid/removed providers gracefully: + * - Invalid apiProvider values are removed (profile is kept but needs reconfiguration) + * - Completely invalid profiles are skipped + * - Warnings are returned for any issues encountered */ export async function importSettingsFromPath( filePath: string, { providerSettingsManager, contextProxy, customModesManager }: ImportOptions, ) { - const schema = z.object({ - providerProfiles: providerProfilesSchema, + // Use a lenient schema that accepts any apiConfigs, then validate each individually + const lenientProviderProfilesSchema = providerProfilesSchema.extend({ + apiConfigs: z.record(z.string(), z.any()), + }) + + const lenientSchema = z.object({ + providerProfiles: lenientProviderProfilesSchema, globalSettings: globalSettingsSchema.optional(), }) try { const previousProviderProfiles = await providerSettingsManager.export() - const { providerProfiles: newProviderProfiles, globalSettings = {} } = schema.parse( - JSON.parse(await fs.readFile(filePath, "utf-8")), - ) + const rawData = JSON.parse(await fs.readFile(filePath, "utf-8")) + const { providerProfiles: rawProviderProfiles, globalSettings = {} } = lenientSchema.parse(rawData) + + // Track warnings for profiles that had issues + const warnings: string[] = [] + const validApiConfigs: Record = {} + + // Process each apiConfig individually with sanitization + for (const [configName, rawConfig] of Object.entries(rawProviderProfiles.apiConfigs)) { + // First sanitize to handle invalid apiProvider values + const { config: sanitizedConfig, warning } = sanitizeProviderConfig(configName, rawConfig) + if (warning) { + warnings.push(warning) + } + + // Then validate the sanitized config + const result = providerSettingsWithIdSchema.safeParse(sanitizedConfig) + if (result.success) { + validApiConfigs[configName] = result.data + } else { + // Profile is completely invalid - skip it + const issues = result.error.issues.map((i) => `${i.path.join(".")}: ${i.message}`).join(", ") + warnings.push(`Profile "${configName}" was skipped: ${issues}`) + } + } + + // If no valid configs were imported and there were issues, report them + if (Object.keys(validApiConfigs).length === 0 && warnings.length > 0) { + return { + success: false, + error: `No valid profiles could be imported:\n${warnings.join("\n")}`, + } + } + + // Determine the currentApiConfigName: + // 1. If the imported currentApiConfigName exists in validApiConfigs, use it + // 2. Otherwise, fall back to the first valid imported profile + // 3. If no valid profiles were imported, keep the previous currentApiConfigName + let currentApiConfigName = rawProviderProfiles.currentApiConfigName + const validProfileNames = Object.keys(validApiConfigs) + if (!validApiConfigs[currentApiConfigName]) { + if (validProfileNames.length > 0) { + currentApiConfigName = validProfileNames[0] + warnings.push( + `Profile "${rawProviderProfiles.currentApiConfigName}" was not available; defaulting to "${currentApiConfigName}".`, + ) + } else { + // No valid imported profiles; keep the existing currentApiConfigName + currentApiConfigName = previousProviderProfiles.currentApiConfigName + } + } const providerProfiles = { - currentApiConfigName: newProviderProfiles.currentApiConfigName, + currentApiConfigName, apiConfigs: { ...previousProviderProfiles.apiConfigs, - ...newProviderProfiles.apiConfigs, + ...validApiConfigs, }, modeApiConfigs: { ...previousProviderProfiles.modeApiConfigs, - ...newProviderProfiles.modeApiConfigs, + ...rawProviderProfiles.modeApiConfigs, }, } @@ -89,7 +177,12 @@ export async function importSettingsFromPath( contextProxy.setValue("listApiConfigMeta", await providerSettingsManager.listConfig()) - return { providerProfiles, globalSettings, success: true } + return { + providerProfiles, + globalSettings, + success: true, + warnings: warnings.length > 0 ? warnings : undefined, + } } catch (e) { let error = "Unknown error" @@ -110,9 +203,16 @@ export async function importSettingsFromPath( * @returns Promise resolving to import result */ export const importSettings = async ({ providerSettingsManager, contextProxy, customModesManager }: ImportOptions) => { + // Use the last export path as a sensible default, falling back to Downloads + const defaultUri = resolveDefaultSaveUri(contextProxy, "lastSettingsExportPath", "roo-code-settings.json", { + useWorkspace: false, + fallbackDir: path.join(os.homedir(), "Downloads"), + }) + const uris = await vscode.window.showOpenDialog({ filters: { JSON: ["json"] }, canSelectMany: false, + defaultUri, }) if (!uris) { @@ -219,7 +319,22 @@ export const importSettingsWithFeedback = async ( if (result.success) { provider.settingsImportedAt = Date.now() await provider.postStateToWebview() - await vscode.window.showInformationMessage(t("common:info.settings_imported")) + + // Show warnings if any profiles had issues but were still imported (with modifications) + if (result.warnings && result.warnings.length > 0) { + // Log full details to the console for debugging + console.warn("Settings import completed with warnings:", result.warnings) + + // Show a short summary in the toast notification + const count = result.warnings.length + const summary = + count === 1 ? `1 profile had issues during import.` : `${count} profiles had issues during import.` + await vscode.window.showWarningMessage( + `${t("common:info.settings_imported")} ${summary} See Developer Tools console for details.`, + ) + } else { + await vscode.window.showInformationMessage(t("common:info.settings_imported")) + } } else if (result.error) { await vscode.window.showErrorMessage(t("common:errors.settings_import_failed", { error: result.error })) }