Browse Source

Removed hardcoded filenames to GlobalFileNames constants. (#1904)

* Replace hardcoded custom modes filename with GlobalFileNames constant

* Remove 'cline' prefix from mcpSettings filename

* Use GlobalFileNames.customModes in modes.ts

* replace two more `cline_mcp_settings` with `mcp_settings` in tests.

* feat: add settings file migration for new file naming convention

- Implement migrateSettings function to rename legacy settings files to new format
- Migrate cline_custom_modes.json to new custom modes filename
- Migrate cline_mcp_settings.json to new MCP settings filename
- Add TODO to remove migration code in September 2025 (6 months after implementation)
- Make activate function async to support migration on startup

* Add associated changeset

* removed unused import

* refactor: move migrateSettings to dedicated utility file

- Extract migrateSettings function from extension.ts to src/utils/migrateSettings.ts
- Update extension.ts to import and use the extracted function
- Update tests to use the real implementation
- Improve dependency injection by passing outputChannel as parameter
- Enhance maintainability by isolating temporary migration code (to be removed Sept 2025)

* Update src/extension.ts

---------

Co-authored-by: Matt Rubens <[email protected]>
Steven T. Cramer 9 months ago
parent
commit
8ad4fff0d2

+ 5 - 0
.changeset/heavy-eyes-reply.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Add settings migration to support renaming legacy settings files to new format

+ 17 - 1
src/__mocks__/fs/promises.ts

@@ -152,6 +152,22 @@ const mockFs = {
 		throw error
 	}),
 
+	rename: jest.fn().mockImplementation(async (oldPath: string, newPath: string) => {
+		// Check if the old file exists
+		if (mockFiles.has(oldPath)) {
+			// Copy content to new path
+			const content = mockFiles.get(oldPath)
+			mockFiles.set(newPath, content)
+			// Delete old file
+			mockFiles.delete(oldPath)
+			return Promise.resolve()
+		}
+		// If old file doesn't exist, throw an error
+		const error = new Error(`ENOENT: no such file or directory, rename '${oldPath}'`)
+		;(error as any).code = "ENOENT"
+		throw error
+	}),
+
 	constants: jest.requireActual("fs").constants,
 
 	// Expose mock data for test assertions
@@ -162,7 +178,7 @@ const mockFs = {
 	_setInitialMockData: () => {
 		// Set up default MCP settings
 		mockFiles.set(
-			"/mock/settings/path/cline_mcp_settings.json",
+			"/mock/settings/path/mcp_settings.json",
 			JSON.stringify({
 				mcpServers: {
 					"test-server": {

+ 119 - 0
src/__tests__/migrateSettings.test.ts

@@ -0,0 +1,119 @@
+import * as vscode from "vscode"
+import * as path from "path"
+import * as fs from "fs/promises"
+import { fileExistsAtPath } from "../utils/fs"
+import { GlobalFileNames } from "../shared/globalFileNames"
+import { migrateSettings } from "../utils/migrateSettings"
+
+// Mock dependencies
+jest.mock("vscode")
+jest.mock("fs/promises")
+jest.mock("fs")
+jest.mock("../utils/fs")
+// We're testing the real migrateSettings function
+
+describe("Settings Migration", () => {
+	let mockContext: vscode.ExtensionContext
+	let mockOutputChannel: vscode.OutputChannel
+	const mockStoragePath = "/mock/storage"
+	const mockSettingsDir = path.join(mockStoragePath, "settings")
+
+	// Legacy file names
+	const legacyCustomModesPath = 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 newMcpSettingsPath = path.join(mockSettingsDir, GlobalFileNames.mcpSettings)
+
+	beforeEach(() => {
+		jest.clearAllMocks()
+
+		// Mock output channel
+		mockOutputChannel = {
+			appendLine: jest.fn(),
+			append: jest.fn(),
+			clear: jest.fn(),
+			show: jest.fn(),
+			hide: jest.fn(),
+			dispose: jest.fn(),
+		} as unknown as vscode.OutputChannel
+
+		// Mock extension context
+		mockContext = {
+			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 () => {
+		const mockCustomModesContent = '{"customModes":[{"slug":"test-mode"}]}' as string
+
+		// Mock file existence checks
+		;(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
+		})
+
+		await migrateSettings(mockContext, mockOutputChannel)
+
+		// Verify file was renamed
+		expect(fs.rename).toHaveBeenCalledWith(legacyCustomModesPath, newCustomModesPath)
+	})
+
+	it("should migrate MCP settings file if old file exists and new file doesn't", async () => {
+		const mockMcpSettingsContent = '{"mcpServers":{"test-server":{}}}' as string
+
+		// Mock file existence checks
+		;(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
+		})
+
+		await migrateSettings(mockContext, mockOutputChannel)
+
+		// Verify file was renamed
+		expect(fs.rename).toHaveBeenCalledWith(legacyMcpSettingsPath, newMcpSettingsPath)
+	})
+
+	it("should not migrate if new file already exists", async () => {
+		// Mock file existence checks
+		;(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 === legacyMcpSettingsPath) return true
+			if (path === newMcpSettingsPath) return true
+			return false
+		})
+
+		await migrateSettings(mockContext, mockOutputChannel)
+
+		// Verify no files were renamed
+		expect(fs.rename).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"))
+
+		// Set the global outputChannel for the test
+		;(global as any).outputChannel = mockOutputChannel
+
+		await migrateSettings(mockContext, mockOutputChannel)
+
+		// Verify error was logged
+		expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(
+			expect.stringContaining("Error migrating settings files"),
+		)
+	})
+})

+ 2 - 1
src/core/config/CustomModesManager.ts

@@ -6,6 +6,7 @@ import { ModeConfig } from "../../shared/modes"
 import { fileExistsAtPath } from "../../utils/fs"
 import { arePathsEqual, getWorkspacePath } from "../../utils/path"
 import { logger } from "../../utils/logging"
+import { GlobalFileNames } from "../../shared/globalFileNames"
 
 const ROOMODES_FILENAME = ".roomodes"
 
@@ -113,7 +114,7 @@ export class CustomModesManager {
 
 	async getCustomModesFilePath(): Promise<string> {
 		const settingsDir = await this.ensureSettingsDirectoryExists()
-		const filePath = path.join(settingsDir, "cline_custom_modes.json")
+		const filePath = path.join(settingsDir, GlobalFileNames.customModes)
 		const fileExists = await fileExistsAtPath(filePath)
 		if (!fileExists) {
 			await this.queueWrite(async () => {

+ 7 - 7
src/core/config/__tests__/CustomModesManager.test.ts

@@ -7,6 +7,7 @@ import { CustomModesManager } from "../CustomModesManager"
 import { ModeConfig } from "../../../shared/modes"
 import { fileExistsAtPath } from "../../../utils/fs"
 import { getWorkspacePath, arePathsEqual } from "../../../utils/path"
+import { GlobalFileNames } from "../../../shared/globalFileNames"
 
 jest.mock("vscode")
 jest.mock("fs/promises")
@@ -21,7 +22,7 @@ describe("CustomModesManager", () => {
 
 	// Use path.sep to ensure correct path separators for the current platform
 	const mockStoragePath = `${path.sep}mock${path.sep}settings`
-	const mockSettingsPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
+	const mockSettingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
 	const mockRoomodes = `${path.sep}mock${path.sep}workspace${path.sep}.roomodes`
 
 	beforeEach(() => {
@@ -333,17 +334,16 @@ describe("CustomModesManager", () => {
 			expect(mockOnUpdate).toHaveBeenCalled()
 		})
 	})
-
 	describe("File Operations", () => {
 		it("creates settings directory if it doesn't exist", async () => {
-			const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
+			const settingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
 			await manager.getCustomModesFilePath()
 
-			expect(fs.mkdir).toHaveBeenCalledWith(path.dirname(configPath), { recursive: true })
+			expect(fs.mkdir).toHaveBeenCalledWith(path.dirname(settingsPath), { recursive: true })
 		})
 
 		it("creates default config if file doesn't exist", async () => {
-			const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
+			const settingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
 
 			// Mock fileExists to return false first time, then true
 			let firstCall = true
@@ -358,13 +358,13 @@ describe("CustomModesManager", () => {
 			await manager.getCustomModesFilePath()
 
 			expect(fs.writeFile).toHaveBeenCalledWith(
-				configPath,
+				settingsPath,
 				expect.stringMatching(/^\{\s+"customModes":\s+\[\s*\]\s*\}$/),
 			)
 		})
 
 		it("watches file for changes", async () => {
-			const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
+			const configPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
 
 			;(fs.readFile as jest.Mock).mockResolvedValue(JSON.stringify({ customModes: [] }))
 			;(arePathsEqual as jest.Mock).mockImplementation((path1: string, path2: string) => {

+ 2 - 1
src/core/prompts/sections/modes.ts

@@ -2,11 +2,12 @@ import * as path from "path"
 import * as vscode from "vscode"
 import { promises as fs } from "fs"
 import { ModeConfig, getAllModesWithPrompts } from "../../../shared/modes"
+import { GlobalFileNames } from "../../../shared/globalFileNames"
 
 export async function getModesSection(context: vscode.ExtensionContext): Promise<string> {
 	const settingsDir = path.join(context.globalStorageUri.fsPath, "settings")
 	await fs.mkdir(settingsDir, { recursive: true })
-	const customModesPath = path.join(settingsDir, "cline_custom_modes.json")
+	const customModesPath = path.join(settingsDir, GlobalFileNames.customModes)
 
 	// Get all modes with their overrides from extension state
 	const allModes = await getAllModesWithPrompts(context)

+ 7 - 2
src/extension.ts

@@ -1,10 +1,11 @@
 import * as vscode from "vscode"
 import * as dotenvx from "@dotenvx/dotenvx"
+import * as path from "path"
 
 // Load environment variables from .env file
 try {
 	// Specify path to .env file in the project root directory
-	const envPath = __dirname + "/../.env"
+	const envPath = path.join(__dirname, "..", ".env")
 	dotenvx.config({ path: envPath })
 } catch (e) {
 	// Silently handle environment loading errors
@@ -21,6 +22,7 @@ import { McpServerManager } from "./services/mcp/McpServerManager"
 import { telemetryService } from "./services/telemetry/TelemetryService"
 import { TerminalRegistry } from "./integrations/terminal/TerminalRegistry"
 import { API } from "./exports/api"
+import { migrateSettings } from "./utils/migrateSettings"
 
 import { handleUri, registerCommands, registerCodeActions, registerTerminalActions } from "./activate"
 import { formatLanguage } from "./shared/language"
@@ -38,12 +40,15 @@ let extensionContext: vscode.ExtensionContext
 
 // This method is called when your extension is activated.
 // Your extension is activated the very first time the command is executed.
-export function activate(context: vscode.ExtensionContext) {
+export async function activate(context: vscode.ExtensionContext) {
 	extensionContext = context
 	outputChannel = vscode.window.createOutputChannel("Roo-Code")
 	context.subscriptions.push(outputChannel)
 	outputChannel.appendLine("Roo-Code extension activated")
 
+	// Migrate old settings to new
+	await migrateSettings(context, outputChannel)
+
 	// Initialize telemetry service after environment variables are loaded.
 	telemetryService.initialize()
 

+ 1 - 1
src/services/mcp/__tests__/McpHub.test.ts

@@ -14,7 +14,7 @@ jest.mock("../../../core/webview/ClineProvider")
 describe("McpHub", () => {
 	let mcpHub: McpHubType
 	let mockProvider: Partial<ClineProvider>
-	const mockSettingsPath = "/mock/settings/path/cline_mcp_settings.json"
+	const mockSettingsPath = "/mock/settings/path/mcp_settings.json"
 
 	beforeEach(() => {
 		jest.clearAllMocks()

+ 2 - 1
src/shared/globalFileNames.ts

@@ -4,6 +4,7 @@ export const GlobalFileNames = {
 	glamaModels: "glama_models.json",
 	openRouterModels: "openrouter_models.json",
 	requestyModels: "requesty_models.json",
-	mcpSettings: "cline_mcp_settings.json",
+	mcpSettings: "mcp_settings.json",
 	unboundModels: "unbound_models.json",
+	customModes: "custom_modes.json",
 }

+ 53 - 0
src/utils/migrateSettings.ts

@@ -0,0 +1,53 @@
+import * as vscode from "vscode"
+import * as path from "path"
+import * as fs from "fs/promises"
+import { fileExistsAtPath } from "./fs"
+import { GlobalFileNames } from "../shared/globalFileNames"
+
+/**
+ * Migrates old settings files to new file names
+ *
+ * TODO: Remove this migration code in September 2025 (6 months after implementation)
+ */
+export async function migrateSettings(
+	context: vscode.ExtensionContext,
+	outputChannel: vscode.OutputChannel,
+): Promise<void> {
+	// 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_mcp_settings.json", newName: GlobalFileNames.mcpSettings },
+	]
+
+	try {
+		const settingsDir = path.join(context.globalStorageUri.fsPath, "settings")
+
+		// Check if settings directory exists first
+		if (!(await fileExistsAtPath(settingsDir))) {
+			outputChannel.appendLine("No settings directory found, no migrations necessary")
+			return
+		}
+
+		// 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"}`,
+				)
+			}
+		}
+	} catch (error) {
+		outputChannel.appendLine(`Error migrating settings files: ${error}`)
+	}
+}