From 8c89e47a5111af06936d494d50364c732b3d1e69 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 10:29:08 -0400 Subject: [PATCH 1/7] Write custom mode files as YAML instead of JSON --- src/core/config/CustomModesManager.ts | 12 ++-- .../__tests__/CustomModesManager.test.ts | 67 +++++++++---------- 2 files changed, 38 insertions(+), 41 deletions(-) diff --git a/src/core/config/CustomModesManager.ts b/src/core/config/CustomModesManager.ts index 5b9ba84b366..743f96c00eb 100644 --- a/src/core/config/CustomModesManager.ts +++ b/src/core/config/CustomModesManager.ts @@ -120,7 +120,7 @@ export class CustomModesManager { const fileExists = await fileExistsAtPath(filePath) if (!fileExists) { - await this.queueWrite(() => fs.writeFile(filePath, JSON.stringify({ customModes: [] }, null, 2))) + await this.queueWrite(() => fs.writeFile(filePath, yaml.stringify({ customModes: [] }))) } return filePath @@ -136,7 +136,7 @@ export class CustomModesManager { const content = await fs.readFile(settingsPath, "utf-8") const errorMessage = - "Invalid custom modes format. Please ensure your settings follow the correct JSON format." + "Invalid custom modes format. Please ensure your settings follow the correct YAML format." let config: any @@ -291,7 +291,7 @@ export class CustomModesManager { content = await fs.readFile(filePath, "utf-8") } catch (error) { // File might not exist yet. - content = JSON.stringify({ customModes: [] }) + content = yaml.stringify({ customModes: [] }) } let settings @@ -299,12 +299,12 @@ export class CustomModesManager { try { settings = yaml.parse(content) } catch (error) { - console.error(`[CustomModesManager] Failed to parse JSON from ${filePath}:`, error) + console.error(`[CustomModesManager] Failed to parse YAML from ${filePath}:`, error) settings = { customModes: [] } } settings.customModes = operation(settings.customModes || []) - await fs.writeFile(filePath, JSON.stringify(settings, null, 2), "utf-8") + await fs.writeFile(filePath, yaml.stringify(settings), "utf-8") } private async refreshMergedState(): Promise { @@ -369,7 +369,7 @@ export class CustomModesManager { public async resetCustomModes(): Promise { try { const filePath = await this.getCustomModesFilePath() - await fs.writeFile(filePath, JSON.stringify({ customModes: [] }, null, 2)) + await fs.writeFile(filePath, yaml.stringify({ customModes: [] })) await this.context.globalState.update("customModes", []) this.clearCache() await this.onUpdate() diff --git a/src/core/config/__tests__/CustomModesManager.test.ts b/src/core/config/__tests__/CustomModesManager.test.ts index 1f1fea18829..15bf2447268 100644 --- a/src/core/config/__tests__/CustomModesManager.test.ts +++ b/src/core/config/__tests__/CustomModesManager.test.ts @@ -48,7 +48,7 @@ describe("CustomModesManager", () => { ;(fs.mkdir as jest.Mock).mockResolvedValue(undefined) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: [] }) + return yaml.stringify({ customModes: [] }) } throw new Error("File not found") }) @@ -68,7 +68,7 @@ describe("CustomModesManager", () => { ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } if (path === mockRoomodes) { return yaml.stringify({ customModes: roomodesModes }) @@ -94,10 +94,10 @@ describe("CustomModesManager", () => { ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } if (path === mockRoomodes) { - return JSON.stringify({ customModes: roomodesModes }) + return yaml.stringify({ customModes: roomodesModes }) } throw new Error("File not found") }) @@ -122,7 +122,7 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -133,15 +133,15 @@ describe("CustomModesManager", () => { expect(modes[0].slug).toBe("mode1") }) - it("should handle invalid JSON in .roomodes", async () => { + it("should handle invalid YAML in .roomodes", async () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } if (path === mockRoomodes) { - return "invalid json" + return "invalid yaml content" } throw new Error("File not found") }) @@ -158,7 +158,7 @@ describe("CustomModesManager", () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -180,7 +180,7 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -200,7 +200,7 @@ describe("CustomModesManager", () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -225,7 +225,7 @@ describe("CustomModesManager", () => { const updatedSettingsModes = [updatedMode] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: updatedSettingsModes }) + return yaml.stringify({ customModes: updatedSettingsModes }) } throw new Error("File not found") }) @@ -246,7 +246,7 @@ describe("CustomModesManager", () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -264,7 +264,7 @@ describe("CustomModesManager", () => { // Mock the updated file content (empty) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: [] }) + return yaml.stringify({ customModes: [] }) } throw new Error("File not found") }) @@ -282,7 +282,7 @@ describe("CustomModesManager", () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -310,7 +310,7 @@ describe("CustomModesManager", () => { const updatedSettingsModes = [updatedMode] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: updatedSettingsModes }) + return yaml.stringify({ customModes: updatedSettingsModes }) } throw new Error("File not found") }) @@ -328,7 +328,7 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: updatedSettingsModes }) + return yaml.stringify({ customModes: updatedSettingsModes }) } throw new Error("File not found") }) @@ -343,7 +343,7 @@ describe("CustomModesManager", () => { const settingsModes = [{ slug: "mode1", name: "Mode 1", roleDefinition: "Role 1", groups: ["read"] }] ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -369,7 +369,7 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -390,7 +390,7 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: settingsModes }) + return yaml.stringify({ customModes: settingsModes }) } throw new Error("File not found") }) @@ -434,10 +434,10 @@ describe("CustomModesManager", () => { ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockRoomodes) { - return JSON.stringify(roomodesContent) + return yaml.stringify(roomodesContent) } if (path === mockSettingsPath) { - return JSON.stringify(settingsContent) + return yaml.stringify(settingsContent) } throw new Error("File not found") }) @@ -502,13 +502,13 @@ describe("CustomModesManager", () => { }) ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify({ customModes: [] }) + return yaml.stringify({ customModes: [] }) } if (path === mockRoomodes) { if (!roomodesContent) { throw new Error("File not found") } - return JSON.stringify(roomodesContent) + return yaml.stringify(roomodesContent) } throw new Error("File not found") }) @@ -564,7 +564,7 @@ describe("CustomModesManager", () => { let settingsContent = { customModes: [] } ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify(settingsContent) + return yaml.stringify(settingsContent) } throw new Error("File not found") }) @@ -629,16 +629,13 @@ describe("CustomModesManager", () => { await manager.getCustomModesFilePath() - expect(fs.writeFile).toHaveBeenCalledWith( - settingsPath, - expect.stringMatching(/^\{\s+"customModes":\s+\[\s*\]\s*\}$/), - ) + expect(fs.writeFile).toHaveBeenCalledWith(settingsPath, expect.stringMatching(/^customModes: \[\]/)) }) it("watches file for changes", async () => { const configPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes) - ;(fs.readFile as jest.Mock).mockResolvedValue(JSON.stringify({ customModes: [] })) + ;(fs.readFile as jest.Mock).mockResolvedValue(yaml.stringify({ customModes: [] })) ;(arePathsEqual as jest.Mock).mockImplementation((path1: string, path2: string) => { return path.normalize(path1) === path.normalize(path2) }) @@ -673,7 +670,7 @@ describe("CustomModesManager", () => { let settingsContent = { customModes: [existingMode] } ;(fs.readFile as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsPath) { - return JSON.stringify(settingsContent) + return yaml.stringify(settingsContent) } throw new Error("File not found") }) @@ -718,9 +715,9 @@ describe("CustomModesManager", () => { }) describe("updateModesInFile", () => { - it("handles corrupted JSON content gracefully", async () => { - const corruptedJson = "{ invalid json content" - ;(fs.readFile as jest.Mock).mockResolvedValue(corruptedJson) + it("handles corrupted YAML content gracefully", async () => { + const corruptedYaml = "customModes: [invalid yaml content" + ;(fs.readFile as jest.Mock).mockResolvedValue(corruptedYaml) const newMode: ModeConfig = { slug: "test-mode", @@ -732,7 +729,7 @@ describe("CustomModesManager", () => { await manager.updateCustomMode("test-mode", newMode) - // Verify that a valid JSON structure was written + // Verify that a valid YAML structure was written const writeCall = (fs.writeFile as jest.Mock).mock.calls[0] const writtenContent = yaml.parse(writeCall[1]) expect(writtenContent).toEqual({ From 5d7036a8fce7135bcb3eefdb56d0fd2c6d616e7d Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 10:29:51 -0400 Subject: [PATCH 2/7] Migrate custom_modes.json to custom_modes.yaml --- src/__tests__/migrateSettings.test.ts | 203 ++++++++++++++++++++++---- src/shared/globalFileNames.ts | 2 +- src/utils/migrateSettings.ts | 98 ++++++++++--- 3 files changed, 258 insertions(+), 45 deletions(-) diff --git a/src/__tests__/migrateSettings.test.ts b/src/__tests__/migrateSettings.test.ts index 9bea4aa9b9a..e3b55eee7ac 100644 --- a/src/__tests__/migrateSettings.test.ts +++ b/src/__tests__/migrateSettings.test.ts @@ -7,9 +7,16 @@ import { migrateSettings } from "../utils/migrateSettings" // Mock dependencies jest.mock("vscode") -jest.mock("fs/promises") +jest.mock("fs/promises", () => ({ + mkdir: jest.fn().mockResolvedValue(undefined), + readFile: jest.fn(), + writeFile: jest.fn().mockResolvedValue(undefined), + rename: jest.fn().mockResolvedValue(undefined), + unlink: jest.fn().mockResolvedValue(undefined), +})) jest.mock("fs") jest.mock("../utils/fs") +jest.mock("yaml") describe("Settings Migration", () => { let mockContext: vscode.ExtensionContext @@ -18,11 +25,12 @@ describe("Settings Migration", () => { const mockSettingsDir = path.join(mockStoragePath, "settings") // Legacy file names - const legacyCustomModesPath = path.join(mockSettingsDir, "cline_custom_modes.json") + const legacyCustomModesJson = path.join(mockSettingsDir, "custom_modes.json") + const legacyClineCustomModesPath = path.join(mockSettingsDir, "cline_custom_modes.json") const legacyMcpSettingsPath = path.join(mockSettingsDir, "cline_mcp_settings.json") // New file names - const newCustomModesPath = path.join(mockSettingsDir, GlobalFileNames.customModes) + const newCustomModesYaml = path.join(mockSettingsDir, GlobalFileNames.customModes) const newMcpSettingsPath = path.join(mockSettingsDir, GlobalFileNames.mcpSettings) beforeEach(() => { @@ -43,66 +51,82 @@ describe("Settings Migration", () => { globalStorageUri: { fsPath: mockStoragePath }, } as unknown as vscode.ExtensionContext - // The fs/promises mock is already set up in src/__mocks__/fs/promises.ts - // We don't need to manually mock these methods - // Set global outputChannel for all tests ;(global as any).outputChannel = mockOutputChannel }) it("should migrate custom modes file if old file exists and new file doesn't", async () => { - // Mock file existence checks + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + // Setup mock for rename function + const mockRename = (fs.rename as jest.Mock).mockResolvedValue(undefined) + + // Mock file existence checks - only return true for paths we want to exist ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsDir) return true - if (path === legacyCustomModesPath) return true - if (path === newCustomModesPath) return false - return false + if (path === legacyClineCustomModesPath) return true + return false // All other paths don't exist, including destination files }) + // Run the migration await migrateSettings(mockContext, mockOutputChannel) - // Verify file was renamed - expect(fs.rename).toHaveBeenCalledWith(legacyCustomModesPath, newCustomModesPath) + // Verify expected rename call + expect(mockRename).toHaveBeenCalledWith(legacyClineCustomModesPath, newMcpSettingsPath) }) it("should migrate MCP settings file if old file exists and new file doesn't", async () => { - // Mock file existence checks + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + // Setup mock for rename function + const mockRename = (fs.rename as jest.Mock).mockResolvedValue(undefined) + + // Ensure the other files don't interfere with this test ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsDir) return true if (path === legacyMcpSettingsPath) return true - if (path === newMcpSettingsPath) return false - return false + if (path === legacyClineCustomModesPath) return false // Ensure this file doesn't exist + if (path === legacyCustomModesJson) return false // Ensure this file doesn't exist + return false // All other paths don't exist, including destination files }) + // Run the migration await migrateSettings(mockContext, mockOutputChannel) - // Verify file was renamed - expect(fs.rename).toHaveBeenCalledWith(legacyMcpSettingsPath, newMcpSettingsPath) + // Verify expected rename call + expect(mockRename).toHaveBeenCalledWith(legacyMcpSettingsPath, newMcpSettingsPath) }) it("should not migrate if new file already exists", async () => { - // Mock file existence checks + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + // Setup mock for rename function + const mockRename = (fs.rename as jest.Mock).mockResolvedValue(undefined) + + // Mock file existence checks - both source and destination exist ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsDir) return true - if (path === legacyCustomModesPath) return true - if (path === newCustomModesPath) return true + if (path === legacyClineCustomModesPath) return true + if (path === newMcpSettingsPath) return true // Destination already exists if (path === legacyMcpSettingsPath) return true - if (path === newMcpSettingsPath) return true return false }) await migrateSettings(mockContext, mockOutputChannel) - // Verify no files were renamed - expect(fs.rename).not.toHaveBeenCalled() + // Verify rename was not called since destination file exists + expect(mockRename).not.toHaveBeenCalled() }) it("should handle errors gracefully", async () => { - // Mock file existence checks to throw an error - ;(fileExistsAtPath as jest.Mock).mockRejectedValue(new Error("Test error")) + // Clear mocks + jest.clearAllMocks() - // Set the global outputChannel for the test - ;(global as any).outputChannel = mockOutputChannel + // Mock file existence to throw error + ;(fileExistsAtPath as jest.Mock).mockRejectedValue(new Error("Test error")) await migrateSettings(mockContext, mockOutputChannel) @@ -111,4 +135,129 @@ describe("Settings Migration", () => { expect.stringContaining("Error migrating settings files"), ) }) + + it("should convert custom_modes.json to YAML format", async () => { + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + const testJsonContent = JSON.stringify({ customModes: [{ slug: "test-mode", name: "Test Mode" }] }) + const testYamlContent = "customModes:\n - slug: test-mode\n name: Test Mode" + + // Mock yaml library + const yamlMock = require("yaml") + yamlMock.stringify.mockReturnValue(testYamlContent) + + // Setup mock functions + const mockWrite = (fs.writeFile as jest.Mock).mockResolvedValue(undefined) + const mockUnlink = (fs.unlink as jest.Mock).mockResolvedValue(undefined) + + // Mock file read to return JSON content + ;(fs.readFile as jest.Mock).mockImplementation(async (path: any) => { + if (path === legacyCustomModesJson) { + return testJsonContent + } + throw new Error("File not found: " + path) + }) + + // Isolate this test by making sure only the specific JSON file exists + ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { + if (path === mockSettingsDir) return true + if (path === legacyCustomModesJson) return true + if (path === legacyClineCustomModesPath) return false + if (path === legacyMcpSettingsPath) return false + return false + }) + + await migrateSettings(mockContext, mockOutputChannel) + + // Verify file operations + expect(mockWrite).toHaveBeenCalledWith(newCustomModesYaml, testYamlContent, "utf-8") + expect(mockUnlink).toHaveBeenCalledWith(legacyCustomModesJson) + }) + + it("should handle corrupt JSON gracefully", async () => { + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + // Setup mock functions + const mockWrite = (fs.writeFile as jest.Mock).mockResolvedValue(undefined) + const mockUnlink = (fs.unlink as jest.Mock).mockResolvedValue(undefined) + + // Mock file read to return corrupt JSON + ;(fs.readFile as jest.Mock).mockImplementation(async (path: any) => { + if (path === legacyCustomModesJson) { + return "{ invalid json content" + } + throw new Error("File not found: " + path) + }) + + // Isolate this test + ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { + if (path === mockSettingsDir) return true + if (path === legacyCustomModesJson) return true + if (path === legacyClineCustomModesPath) return false + if (path === legacyMcpSettingsPath) return false + return false + }) + + // Create a corrupted JSON situation by making parse throw + const originalJSONParse = JSON.parse + JSON.parse = jest.fn().mockImplementation(() => { + throw new Error("Invalid JSON") + }) + + try { + await migrateSettings(mockContext, mockOutputChannel) + + // Verify error was logged + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("Error parsing custom_modes.json"), + ) + + // Verify no write/unlink operations were performed + expect(mockWrite).not.toHaveBeenCalled() + expect(mockUnlink).not.toHaveBeenCalled() + } finally { + // Restore original JSON.parse + JSON.parse = originalJSONParse + } + }) + + it("should skip migration when YAML file already exists", async () => { + // Clear all previous mocks to ensure clean test environment + jest.clearAllMocks() + + // Setup mock functions + const mockWrite = (fs.writeFile as jest.Mock).mockResolvedValue(undefined) + const mockUnlink = (fs.unlink as jest.Mock).mockResolvedValue(undefined) + + // Mock file read + ;(fs.readFile as jest.Mock).mockImplementation(async (path: any) => { + if (path === legacyCustomModesJson) { + return JSON.stringify({ customModes: [] }) + } + throw new Error("File not found: " + path) + }) + + // Mock file existence checks - both source and yaml destination exist + ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { + if (path === mockSettingsDir) return true + if (path === legacyCustomModesJson) return true + if (path === newCustomModesYaml) return true // YAML already exists + if (path === legacyClineCustomModesPath) return false + if (path === legacyMcpSettingsPath) return false + return false + }) + + await migrateSettings(mockContext, mockOutputChannel) + + // Verify skip message was logged + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + "custom_modes.yaml already exists, skipping migration", + ) + + // Verify no file operations occurred + expect(mockWrite).not.toHaveBeenCalled() + expect(mockUnlink).not.toHaveBeenCalled() + }) }) diff --git a/src/shared/globalFileNames.ts b/src/shared/globalFileNames.ts index b82ea9b00ef..98b48485f06 100644 --- a/src/shared/globalFileNames.ts +++ b/src/shared/globalFileNames.ts @@ -2,6 +2,6 @@ export const GlobalFileNames = { apiConversationHistory: "api_conversation_history.json", uiMessages: "ui_messages.json", mcpSettings: "mcp_settings.json", - customModes: "custom_modes.json", + customModes: "custom_modes.yaml", taskMetadata: "task_metadata.json", } diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index a4d414b52ff..2144e0de584 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -3,6 +3,7 @@ import * as path from "path" import * as fs from "fs/promises" import { fileExistsAtPath } from "./fs" import { GlobalFileNames } from "../shared/globalFileNames" +import * as yaml from "yaml" /** * Migrates old settings files to new file names @@ -15,8 +16,9 @@ export async function migrateSettings( ): Promise { // Legacy file names that need to be migrated to the new names in GlobalFileNames const fileMigrations = [ - { oldName: "cline_custom_modes.json", newName: GlobalFileNames.customModes }, + { oldName: "cline_custom_modes.json", newName: GlobalFileNames.mcpSettings }, { oldName: "cline_mcp_settings.json", newName: GlobalFileNames.mcpSettings }, + // custom_modes.json to custom_modes.yaml is handled separately below ] try { @@ -29,25 +31,87 @@ export async function migrateSettings( } // Process each file migration - for (const migration of fileMigrations) { - const oldPath = path.join(settingsDir, migration.oldName) - const newPath = path.join(settingsDir, migration.newName) - - // Only migrate if old file exists and new file doesn't exist yet - // This ensures we don't overwrite any existing new files - const oldFileExists = await fileExistsAtPath(oldPath) - const newFileExists = await fileExistsAtPath(newPath) - - if (oldFileExists && !newFileExists) { - await fs.rename(oldPath, newPath) - outputChannel.appendLine(`Renamed ${migration.oldName} to ${migration.newName}`) - } else { - outputChannel.appendLine( - `Skipping migration of ${migration.oldName} to ${migration.newName}: ${oldFileExists ? "new file already exists" : "old file not found"}`, - ) + try { + for (const migration of fileMigrations) { + const oldPath = path.join(settingsDir, migration.oldName) + const newPath = path.join(settingsDir, migration.newName) + + // Only migrate if old file exists and new file doesn't exist yet + // This ensures we don't overwrite any existing new files + const oldFileExists = await fileExistsAtPath(oldPath) + const newFileExists = await fileExistsAtPath(newPath) + + if (oldFileExists && !newFileExists) { + await fs.rename(oldPath, newPath) + outputChannel.appendLine(`Renamed ${migration.oldName} to ${migration.newName}`) + } else { + outputChannel.appendLine( + `Skipping migration of ${migration.oldName} to ${migration.newName}: ${oldFileExists ? "new file already exists" : "old file not found"}`, + ) + } } + + // Special migration for custom_modes.json to custom_modes.yaml with content transformation + await migrateCustomModesToYaml(settingsDir, outputChannel) + } catch (error) { + outputChannel.appendLine(`Error in file migrations: ${error}`) } } catch (error) { outputChannel.appendLine(`Error migrating settings files: ${error}`) } } + +/** + * Special migration function to convert custom_modes.json to YAML format + */ +async function migrateCustomModesToYaml(settingsDir: string, outputChannel: vscode.OutputChannel): Promise { + const oldJsonPath = path.join(settingsDir, "custom_modes.json") + const newYamlPath = path.join(settingsDir, GlobalFileNames.customModes) + + // Only proceed if JSON exists and YAML doesn't + const jsonExists = await fileExistsAtPath(oldJsonPath) + const yamlExists = await fileExistsAtPath(newYamlPath) + + if (!jsonExists) { + outputChannel.appendLine("No custom_modes.json found, skipping YAML migration") + return + } + + if (yamlExists) { + outputChannel.appendLine("custom_modes.yaml already exists, skipping migration") + return + } + + try { + // Read JSON content + const jsonContent = await fs.readFile(oldJsonPath, "utf-8") + + try { + // Parse JSON to object + const customModesData = JSON.parse(jsonContent) + + // Convert to YAML + const yamlContent = yaml.stringify(customModesData) + + // Write YAML file + await fs.writeFile(newYamlPath, yamlContent, "utf-8") + + try { + // Delete old JSON file - wrapped in try/catch to handle unlink errors + await fs.unlink(oldJsonPath) + } catch (unlinkError) { + // Just log the error but continue - file migration succeeded + outputChannel.appendLine(`Warning: Could not delete old JSON file: ${unlinkError}`) + } + + outputChannel.appendLine("Successfully migrated custom_modes.json to YAML format") + } catch (parseError) { + // Handle corrupt JSON file + outputChannel.appendLine( + `Error parsing custom_modes.json: ${parseError}. File might be corrupted. Skipping migration.`, + ) + } + } catch (fileError) { + outputChannel.appendLine(`Error reading custom_modes.json: ${fileError}. Skipping migration.`) + } +} From cf8e847a809ee053201c59cea414ef1c71f18f7c Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 10:35:26 -0400 Subject: [PATCH 3/7] Update instructions to use YAML --- src/core/prompts/instructions/create-mode.ts | 47 +++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/src/core/prompts/instructions/create-mode.ts b/src/core/prompts/instructions/create-mode.ts index db63eb96d9c..47e998ff4c0 100644 --- a/src/core/prompts/instructions/create-mode.ts +++ b/src/core/prompts/instructions/create-mode.ts @@ -31,25 +31,30 @@ If asked to create a project mode, create it in .roomodes in the workspace root. - For multi-line text, include newline characters in the string like "This is the first line.\\nThis is the next line.\\n\\nThis is a double line break." -Both files should follow this structure: -{ - "customModes": [ - { - "slug": "designer", // Required: unique slug with lowercase letters, numbers, and hyphens - "name": "Designer", // Required: mode display name - "roleDefinition": "You are Roo, a UI/UX expert specializing in design systems and frontend development. Your expertise includes:\\n- Creating and maintaining design systems\\n- Implementing responsive and accessible web interfaces\\n- Working with CSS, HTML, and modern frontend frameworks\\n- Ensuring consistent user experiences across platforms", // Required: non-empty - "whenToUse": "Use this mode when creating or modifying UI components, implementing design systems, or ensuring responsive web interfaces. This mode is especially effective with CSS, HTML, and modern frontend frameworks.", // Optional but recommended - "groups": [ // Required: array of tool groups (can be empty) - "read", // Read files group (read_file, fetch_instructions, search_files, list_files, list_code_definition_names) - "edit", // Edit files group (apply_diff, write_to_file) - allows editing any file - // Or with file restrictions: - // ["edit", { fileRegex: "\\.md$", description: "Markdown files only" }], // Edit group that only allows editing markdown files - "browser", // Browser group (browser_action) - "command", // Command group (execute_command) - "mcp" // MCP group (use_mcp_tool, access_mcp_resource) - ], - "customInstructions": "Additional instructions for the Designer mode" // Optional - } - ] -}` +Both files should follow this structure (in YAML format): + +customModes: + - slug: designer # Required: unique slug with lowercase letters, numbers, and hyphens + name: Designer # Required: mode display name + roleDefinition: >- + You are Roo, a UI/UX expert specializing in design systems and frontend development. Your expertise includes: + - Creating and maintaining design systems + - Implementing responsive and accessible web interfaces + - Working with CSS, HTML, and modern frontend frameworks + - Ensuring consistent user experiences across platforms # Required: non-empty + whenToUse: >- + Use this mode when creating or modifying UI components, implementing design systems, + or ensuring responsive web interfaces. This mode is especially effective with CSS, + HTML, and modern frontend frameworks. # Optional but recommended + groups: # Required: array of tool groups (can be empty) + - read # Read files group (read_file, fetch_instructions, search_files, list_files, list_code_definition_names) + - edit # Edit files group (apply_diff, write_to_file) - allows editing any file + # Or with file restrictions: + # - - edit + # - fileRegex: \\.md$ + # description: Markdown files only # Edit group that only allows editing markdown files + - browser # Browser group (browser_action) + - command # Command group (execute_command) + - mcp # MCP group (use_mcp_tool, access_mcp_resource) + customInstructions: Additional instructions for the Designer mode # Optional` } From 5d693ab22640beb92fca7fe0452d47768e1167e2 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 10:39:41 -0400 Subject: [PATCH 4/7] Add changeset --- .changeset/gold-meals-tell.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/gold-meals-tell.md diff --git a/.changeset/gold-meals-tell.md b/.changeset/gold-meals-tell.md new file mode 100644 index 00000000000..3cc54103d5b --- /dev/null +++ b/.changeset/gold-meals-tell.md @@ -0,0 +1,5 @@ +--- +"roo-cline": patch +--- + +Use YAML as default custom modes format From bbe440545a18d5b0006af8bc254951c3aa73b9ec Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 11:34:17 -0400 Subject: [PATCH 5/7] PR feedback --- src/utils/migrateSettings.ts | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index 2144e0de584..1cc1317788e 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -5,6 +5,8 @@ import { fileExistsAtPath } from "./fs" import { GlobalFileNames } from "../shared/globalFileNames" import * as yaml from "yaml" +const deprecatedCustomModesJSONFilename = "custom_modes.json" + /** * Migrates old settings files to new file names * @@ -16,9 +18,9 @@ export async function migrateSettings( ): Promise { // Legacy file names that need to be migrated to the new names in GlobalFileNames const fileMigrations = [ - { oldName: "cline_custom_modes.json", newName: GlobalFileNames.mcpSettings }, - { oldName: "cline_mcp_settings.json", newName: GlobalFileNames.mcpSettings }, // custom_modes.json to custom_modes.yaml is handled separately below + { oldName: "cline_custom_modes.json", newName: deprecatedCustomModesJSONFilename }, + { oldName: "cline_mcp_settings.json", newName: GlobalFileNames.mcpSettings }, ] try { @@ -65,7 +67,7 @@ export async function migrateSettings( * Special migration function to convert custom_modes.json to YAML format */ async function migrateCustomModesToYaml(settingsDir: string, outputChannel: vscode.OutputChannel): Promise { - const oldJsonPath = path.join(settingsDir, "custom_modes.json") + const oldJsonPath = path.join(settingsDir, deprecatedCustomModesJSONFilename) const newYamlPath = path.join(settingsDir, GlobalFileNames.customModes) // Only proceed if JSON exists and YAML doesn't @@ -88,7 +90,7 @@ async function migrateCustomModesToYaml(settingsDir: string, outputChannel: vsco try { // Parse JSON to object - const customModesData = JSON.parse(jsonContent) + const customModesData = yaml.parse(jsonContent) // Convert to YAML const yamlContent = yaml.stringify(customModesData) From 5643fd1116da3f9fb78268b2398661a0acbc17fd Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 11:40:10 -0400 Subject: [PATCH 6/7] Fix tests --- src/__tests__/migrateSettings.test.ts | 48 +++++++++------------------ src/utils/migrateSettings.ts | 2 +- 2 files changed, 17 insertions(+), 33 deletions(-) diff --git a/src/__tests__/migrateSettings.test.ts b/src/__tests__/migrateSettings.test.ts index e3b55eee7ac..1400646b334 100644 --- a/src/__tests__/migrateSettings.test.ts +++ b/src/__tests__/migrateSettings.test.ts @@ -16,7 +16,6 @@ jest.mock("fs/promises", () => ({ })) jest.mock("fs") jest.mock("../utils/fs") -jest.mock("yaml") describe("Settings Migration", () => { let mockContext: vscode.ExtensionContext @@ -72,8 +71,8 @@ describe("Settings Migration", () => { // Run the migration await migrateSettings(mockContext, mockOutputChannel) - // Verify expected rename call - expect(mockRename).toHaveBeenCalledWith(legacyClineCustomModesPath, newMcpSettingsPath) + // Verify expected rename call - cline_custom_modes.json should be renamed to custom_modes.json + expect(mockRename).toHaveBeenCalledWith(legacyClineCustomModesPath, legacyCustomModesJson) }) it("should migrate MCP settings file if old file exists and new file doesn't", async () => { @@ -110,14 +109,15 @@ describe("Settings Migration", () => { ;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => { if (path === mockSettingsDir) return true if (path === legacyClineCustomModesPath) return true - if (path === newMcpSettingsPath) return true // Destination already exists + if (path === legacyCustomModesJson) return true // Destination already exists if (path === legacyMcpSettingsPath) return true + if (path === newMcpSettingsPath) return true return false }) await migrateSettings(mockContext, mockOutputChannel) - // Verify rename was not called since destination file exists + // Verify rename was not called since destination files exist expect(mockRename).not.toHaveBeenCalled() }) @@ -141,11 +141,6 @@ describe("Settings Migration", () => { jest.clearAllMocks() const testJsonContent = JSON.stringify({ customModes: [{ slug: "test-mode", name: "Test Mode" }] }) - const testYamlContent = "customModes:\n - slug: test-mode\n name: Test Mode" - - // Mock yaml library - const yamlMock = require("yaml") - yamlMock.stringify.mockReturnValue(testYamlContent) // Setup mock functions const mockWrite = (fs.writeFile as jest.Mock).mockResolvedValue(undefined) @@ -171,7 +166,7 @@ describe("Settings Migration", () => { await migrateSettings(mockContext, mockOutputChannel) // Verify file operations - expect(mockWrite).toHaveBeenCalledWith(newCustomModesYaml, testYamlContent, "utf-8") + expect(mockWrite).toHaveBeenCalledWith(newCustomModesYaml, expect.any(String), "utf-8") expect(mockUnlink).toHaveBeenCalledWith(legacyCustomModesJson) }) @@ -186,7 +181,7 @@ describe("Settings Migration", () => { // Mock file read to return corrupt JSON ;(fs.readFile as jest.Mock).mockImplementation(async (path: any) => { if (path === legacyCustomModesJson) { - return "{ invalid json content" + return "{ invalid json content" // This will cause an error when parsed } throw new Error("File not found: " + path) }) @@ -200,27 +195,16 @@ describe("Settings Migration", () => { return false }) - // Create a corrupted JSON situation by making parse throw - const originalJSONParse = JSON.parse - JSON.parse = jest.fn().mockImplementation(() => { - throw new Error("Invalid JSON") - }) + await migrateSettings(mockContext, mockOutputChannel) + + // Verify error was logged + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("Error parsing custom_modes.json"), + ) - try { - await migrateSettings(mockContext, mockOutputChannel) - - // Verify error was logged - expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( - expect.stringContaining("Error parsing custom_modes.json"), - ) - - // Verify no write/unlink operations were performed - expect(mockWrite).not.toHaveBeenCalled() - expect(mockUnlink).not.toHaveBeenCalled() - } finally { - // Restore original JSON.parse - JSON.parse = originalJSONParse - } + // Verify no write/unlink operations were performed + expect(mockWrite).not.toHaveBeenCalled() + expect(mockUnlink).not.toHaveBeenCalled() }) it("should skip migration when YAML file already exists", async () => { diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index 1cc1317788e..2cf1ee0fafb 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -89,7 +89,7 @@ async function migrateCustomModesToYaml(settingsDir: string, outputChannel: vsco const jsonContent = await fs.readFile(oldJsonPath, "utf-8") try { - // Parse JSON to object + // Parse JSON to object (using the yaml library just to be safe/consistent) const customModesData = yaml.parse(jsonContent) // Convert to YAML From 8383f400a8dd681d27d6c5faff78f5c0d3442321 Mon Sep 17 00:00:00 2001 From: Matt Rubens Date: Tue, 20 May 2025 14:35:05 -0400 Subject: [PATCH 7/7] Preserve custom_modes.json for rollback if necessary --- src/__tests__/migrateSettings.test.ts | 8 +++++++- src/utils/migrateSettings.ts | 14 +++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/src/__tests__/migrateSettings.test.ts b/src/__tests__/migrateSettings.test.ts index 1400646b334..538ff7d5908 100644 --- a/src/__tests__/migrateSettings.test.ts +++ b/src/__tests__/migrateSettings.test.ts @@ -167,7 +167,13 @@ describe("Settings Migration", () => { // Verify file operations expect(mockWrite).toHaveBeenCalledWith(newCustomModesYaml, expect.any(String), "utf-8") - expect(mockUnlink).toHaveBeenCalledWith(legacyCustomModesJson) + // We don't delete the original JSON file to allow for rollback + expect(mockUnlink).not.toHaveBeenCalled() + + // Verify log message mentions preservation of original file + expect(mockOutputChannel.appendLine).toHaveBeenCalledWith( + expect.stringContaining("original JSON file preserved for rollback purposes"), + ) }) it("should handle corrupt JSON gracefully", async () => { diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index 2cf1ee0fafb..406e5bd051f 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -98,15 +98,11 @@ async function migrateCustomModesToYaml(settingsDir: string, outputChannel: vsco // Write YAML file await fs.writeFile(newYamlPath, yamlContent, "utf-8") - try { - // Delete old JSON file - wrapped in try/catch to handle unlink errors - await fs.unlink(oldJsonPath) - } catch (unlinkError) { - // Just log the error but continue - file migration succeeded - outputChannel.appendLine(`Warning: Could not delete old JSON file: ${unlinkError}`) - } - - outputChannel.appendLine("Successfully migrated custom_modes.json to YAML format") + // Keeping the old JSON file for backward compatibility + // This allows users to roll back if needed + outputChannel.appendLine( + "Successfully migrated custom_modes.json to YAML format (original JSON file preserved for rollback purposes)", + ) } catch (parseError) { // Handle corrupt JSON file outputChannel.appendLine(