Skip to content

Commit 8ad4fff

Browse files
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]>
1 parent 187979a commit 8ad4fff

File tree

10 files changed

+215
-14
lines changed

10 files changed

+215
-14
lines changed

.changeset/heavy-eyes-reply.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": patch
3+
---
4+
5+
Add settings migration to support renaming legacy settings files to new format

src/__mocks__/fs/promises.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -152,6 +152,22 @@ const mockFs = {
152152
throw error
153153
}),
154154

155+
rename: jest.fn().mockImplementation(async (oldPath: string, newPath: string) => {
156+
// Check if the old file exists
157+
if (mockFiles.has(oldPath)) {
158+
// Copy content to new path
159+
const content = mockFiles.get(oldPath)
160+
mockFiles.set(newPath, content)
161+
// Delete old file
162+
mockFiles.delete(oldPath)
163+
return Promise.resolve()
164+
}
165+
// If old file doesn't exist, throw an error
166+
const error = new Error(`ENOENT: no such file or directory, rename '${oldPath}'`)
167+
;(error as any).code = "ENOENT"
168+
throw error
169+
}),
170+
155171
constants: jest.requireActual("fs").constants,
156172

157173
// Expose mock data for test assertions
@@ -162,7 +178,7 @@ const mockFs = {
162178
_setInitialMockData: () => {
163179
// Set up default MCP settings
164180
mockFiles.set(
165-
"/mock/settings/path/cline_mcp_settings.json",
181+
"/mock/settings/path/mcp_settings.json",
166182
JSON.stringify({
167183
mcpServers: {
168184
"test-server": {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
import * as vscode from "vscode"
2+
import * as path from "path"
3+
import * as fs from "fs/promises"
4+
import { fileExistsAtPath } from "../utils/fs"
5+
import { GlobalFileNames } from "../shared/globalFileNames"
6+
import { migrateSettings } from "../utils/migrateSettings"
7+
8+
// Mock dependencies
9+
jest.mock("vscode")
10+
jest.mock("fs/promises")
11+
jest.mock("fs")
12+
jest.mock("../utils/fs")
13+
// We're testing the real migrateSettings function
14+
15+
describe("Settings Migration", () => {
16+
let mockContext: vscode.ExtensionContext
17+
let mockOutputChannel: vscode.OutputChannel
18+
const mockStoragePath = "/mock/storage"
19+
const mockSettingsDir = path.join(mockStoragePath, "settings")
20+
21+
// Legacy file names
22+
const legacyCustomModesPath = path.join(mockSettingsDir, "cline_custom_modes.json")
23+
const legacyMcpSettingsPath = path.join(mockSettingsDir, "cline_mcp_settings.json")
24+
25+
// New file names
26+
const newCustomModesPath = path.join(mockSettingsDir, GlobalFileNames.customModes)
27+
const newMcpSettingsPath = path.join(mockSettingsDir, GlobalFileNames.mcpSettings)
28+
29+
beforeEach(() => {
30+
jest.clearAllMocks()
31+
32+
// Mock output channel
33+
mockOutputChannel = {
34+
appendLine: jest.fn(),
35+
append: jest.fn(),
36+
clear: jest.fn(),
37+
show: jest.fn(),
38+
hide: jest.fn(),
39+
dispose: jest.fn(),
40+
} as unknown as vscode.OutputChannel
41+
42+
// Mock extension context
43+
mockContext = {
44+
globalStorageUri: { fsPath: mockStoragePath },
45+
} as unknown as vscode.ExtensionContext
46+
47+
// The fs/promises mock is already set up in src/__mocks__/fs/promises.ts
48+
// We don't need to manually mock these methods
49+
50+
// Set global outputChannel for all tests
51+
;(global as any).outputChannel = mockOutputChannel
52+
})
53+
54+
it("should migrate custom modes file if old file exists and new file doesn't", async () => {
55+
const mockCustomModesContent = '{"customModes":[{"slug":"test-mode"}]}' as string
56+
57+
// Mock file existence checks
58+
;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => {
59+
if (path === mockSettingsDir) return true
60+
if (path === legacyCustomModesPath) return true
61+
if (path === newCustomModesPath) return false
62+
return false
63+
})
64+
65+
await migrateSettings(mockContext, mockOutputChannel)
66+
67+
// Verify file was renamed
68+
expect(fs.rename).toHaveBeenCalledWith(legacyCustomModesPath, newCustomModesPath)
69+
})
70+
71+
it("should migrate MCP settings file if old file exists and new file doesn't", async () => {
72+
const mockMcpSettingsContent = '{"mcpServers":{"test-server":{}}}' as string
73+
74+
// Mock file existence checks
75+
;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => {
76+
if (path === mockSettingsDir) return true
77+
if (path === legacyMcpSettingsPath) return true
78+
if (path === newMcpSettingsPath) return false
79+
return false
80+
})
81+
82+
await migrateSettings(mockContext, mockOutputChannel)
83+
84+
// Verify file was renamed
85+
expect(fs.rename).toHaveBeenCalledWith(legacyMcpSettingsPath, newMcpSettingsPath)
86+
})
87+
88+
it("should not migrate if new file already exists", async () => {
89+
// Mock file existence checks
90+
;(fileExistsAtPath as jest.Mock).mockImplementation(async (path: string) => {
91+
if (path === mockSettingsDir) return true
92+
if (path === legacyCustomModesPath) return true
93+
if (path === newCustomModesPath) return true
94+
if (path === legacyMcpSettingsPath) return true
95+
if (path === newMcpSettingsPath) return true
96+
return false
97+
})
98+
99+
await migrateSettings(mockContext, mockOutputChannel)
100+
101+
// Verify no files were renamed
102+
expect(fs.rename).not.toHaveBeenCalled()
103+
})
104+
105+
it("should handle errors gracefully", async () => {
106+
// Mock file existence checks to throw an error
107+
;(fileExistsAtPath as jest.Mock).mockRejectedValue(new Error("Test error"))
108+
109+
// Set the global outputChannel for the test
110+
;(global as any).outputChannel = mockOutputChannel
111+
112+
await migrateSettings(mockContext, mockOutputChannel)
113+
114+
// Verify error was logged
115+
expect(mockOutputChannel.appendLine).toHaveBeenCalledWith(
116+
expect.stringContaining("Error migrating settings files"),
117+
)
118+
})
119+
})

src/core/config/CustomModesManager.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { ModeConfig } from "../../shared/modes"
66
import { fileExistsAtPath } from "../../utils/fs"
77
import { arePathsEqual, getWorkspacePath } from "../../utils/path"
88
import { logger } from "../../utils/logging"
9+
import { GlobalFileNames } from "../../shared/globalFileNames"
910

1011
const ROOMODES_FILENAME = ".roomodes"
1112

@@ -113,7 +114,7 @@ export class CustomModesManager {
113114

114115
async getCustomModesFilePath(): Promise<string> {
115116
const settingsDir = await this.ensureSettingsDirectoryExists()
116-
const filePath = path.join(settingsDir, "cline_custom_modes.json")
117+
const filePath = path.join(settingsDir, GlobalFileNames.customModes)
117118
const fileExists = await fileExistsAtPath(filePath)
118119
if (!fileExists) {
119120
await this.queueWrite(async () => {

src/core/config/__tests__/CustomModesManager.test.ts

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { CustomModesManager } from "../CustomModesManager"
77
import { ModeConfig } from "../../../shared/modes"
88
import { fileExistsAtPath } from "../../../utils/fs"
99
import { getWorkspacePath, arePathsEqual } from "../../../utils/path"
10+
import { GlobalFileNames } from "../../../shared/globalFileNames"
1011

1112
jest.mock("vscode")
1213
jest.mock("fs/promises")
@@ -21,7 +22,7 @@ describe("CustomModesManager", () => {
2122

2223
// Use path.sep to ensure correct path separators for the current platform
2324
const mockStoragePath = `${path.sep}mock${path.sep}settings`
24-
const mockSettingsPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
25+
const mockSettingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
2526
const mockRoomodes = `${path.sep}mock${path.sep}workspace${path.sep}.roomodes`
2627

2728
beforeEach(() => {
@@ -333,17 +334,16 @@ describe("CustomModesManager", () => {
333334
expect(mockOnUpdate).toHaveBeenCalled()
334335
})
335336
})
336-
337337
describe("File Operations", () => {
338338
it("creates settings directory if it doesn't exist", async () => {
339-
const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
339+
const settingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
340340
await manager.getCustomModesFilePath()
341341

342-
expect(fs.mkdir).toHaveBeenCalledWith(path.dirname(configPath), { recursive: true })
342+
expect(fs.mkdir).toHaveBeenCalledWith(path.dirname(settingsPath), { recursive: true })
343343
})
344344

345345
it("creates default config if file doesn't exist", async () => {
346-
const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
346+
const settingsPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
347347

348348
// Mock fileExists to return false first time, then true
349349
let firstCall = true
@@ -358,13 +358,13 @@ describe("CustomModesManager", () => {
358358
await manager.getCustomModesFilePath()
359359

360360
expect(fs.writeFile).toHaveBeenCalledWith(
361-
configPath,
361+
settingsPath,
362362
expect.stringMatching(/^\{\s+"customModes":\s+\[\s*\]\s*\}$/),
363363
)
364364
})
365365

366366
it("watches file for changes", async () => {
367-
const configPath = path.join(mockStoragePath, "settings", "cline_custom_modes.json")
367+
const configPath = path.join(mockStoragePath, "settings", GlobalFileNames.customModes)
368368

369369
;(fs.readFile as jest.Mock).mockResolvedValue(JSON.stringify({ customModes: [] }))
370370
;(arePathsEqual as jest.Mock).mockImplementation((path1: string, path2: string) => {

src/core/prompts/sections/modes.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,11 +2,12 @@ import * as path from "path"
22
import * as vscode from "vscode"
33
import { promises as fs } from "fs"
44
import { ModeConfig, getAllModesWithPrompts } from "../../../shared/modes"
5+
import { GlobalFileNames } from "../../../shared/globalFileNames"
56

67
export async function getModesSection(context: vscode.ExtensionContext): Promise<string> {
78
const settingsDir = path.join(context.globalStorageUri.fsPath, "settings")
89
await fs.mkdir(settingsDir, { recursive: true })
9-
const customModesPath = path.join(settingsDir, "cline_custom_modes.json")
10+
const customModesPath = path.join(settingsDir, GlobalFileNames.customModes)
1011

1112
// Get all modes with their overrides from extension state
1213
const allModes = await getAllModesWithPrompts(context)

src/extension.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
import * as vscode from "vscode"
22
import * as dotenvx from "@dotenvx/dotenvx"
3+
import * as path from "path"
34

45
// Load environment variables from .env file
56
try {
67
// Specify path to .env file in the project root directory
7-
const envPath = __dirname + "/../.env"
8+
const envPath = path.join(__dirname, "..", ".env")
89
dotenvx.config({ path: envPath })
910
} catch (e) {
1011
// Silently handle environment loading errors
@@ -21,6 +22,7 @@ import { McpServerManager } from "./services/mcp/McpServerManager"
2122
import { telemetryService } from "./services/telemetry/TelemetryService"
2223
import { TerminalRegistry } from "./integrations/terminal/TerminalRegistry"
2324
import { API } from "./exports/api"
25+
import { migrateSettings } from "./utils/migrateSettings"
2426

2527
import { handleUri, registerCommands, registerCodeActions, registerTerminalActions } from "./activate"
2628
import { formatLanguage } from "./shared/language"
@@ -38,12 +40,15 @@ let extensionContext: vscode.ExtensionContext
3840

3941
// This method is called when your extension is activated.
4042
// Your extension is activated the very first time the command is executed.
41-
export function activate(context: vscode.ExtensionContext) {
43+
export async function activate(context: vscode.ExtensionContext) {
4244
extensionContext = context
4345
outputChannel = vscode.window.createOutputChannel("Roo-Code")
4446
context.subscriptions.push(outputChannel)
4547
outputChannel.appendLine("Roo-Code extension activated")
4648

49+
// Migrate old settings to new
50+
await migrateSettings(context, outputChannel)
51+
4752
// Initialize telemetry service after environment variables are loaded.
4853
telemetryService.initialize()
4954

src/services/mcp/__tests__/McpHub.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ jest.mock("../../../core/webview/ClineProvider")
1414
describe("McpHub", () => {
1515
let mcpHub: McpHubType
1616
let mockProvider: Partial<ClineProvider>
17-
const mockSettingsPath = "/mock/settings/path/cline_mcp_settings.json"
17+
const mockSettingsPath = "/mock/settings/path/mcp_settings.json"
1818

1919
beforeEach(() => {
2020
jest.clearAllMocks()

src/shared/globalFileNames.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ export const GlobalFileNames = {
44
glamaModels: "glama_models.json",
55
openRouterModels: "openrouter_models.json",
66
requestyModels: "requesty_models.json",
7-
mcpSettings: "cline_mcp_settings.json",
7+
mcpSettings: "mcp_settings.json",
88
unboundModels: "unbound_models.json",
9+
customModes: "custom_modes.json",
910
}

src/utils/migrateSettings.ts

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import * as vscode from "vscode"
2+
import * as path from "path"
3+
import * as fs from "fs/promises"
4+
import { fileExistsAtPath } from "./fs"
5+
import { GlobalFileNames } from "../shared/globalFileNames"
6+
7+
/**
8+
* Migrates old settings files to new file names
9+
*
10+
* TODO: Remove this migration code in September 2025 (6 months after implementation)
11+
*/
12+
export async function migrateSettings(
13+
context: vscode.ExtensionContext,
14+
outputChannel: vscode.OutputChannel,
15+
): Promise<void> {
16+
// Legacy file names that need to be migrated to the new names in GlobalFileNames
17+
const fileMigrations = [
18+
{ oldName: "cline_custom_modes.json", newName: GlobalFileNames.customModes },
19+
{ oldName: "cline_mcp_settings.json", newName: GlobalFileNames.mcpSettings },
20+
]
21+
22+
try {
23+
const settingsDir = path.join(context.globalStorageUri.fsPath, "settings")
24+
25+
// Check if settings directory exists first
26+
if (!(await fileExistsAtPath(settingsDir))) {
27+
outputChannel.appendLine("No settings directory found, no migrations necessary")
28+
return
29+
}
30+
31+
// Process each file migration
32+
for (const migration of fileMigrations) {
33+
const oldPath = path.join(settingsDir, migration.oldName)
34+
const newPath = path.join(settingsDir, migration.newName)
35+
36+
// Only migrate if old file exists and new file doesn't exist yet
37+
// This ensures we don't overwrite any existing new files
38+
const oldFileExists = await fileExistsAtPath(oldPath)
39+
const newFileExists = await fileExistsAtPath(newPath)
40+
41+
if (oldFileExists && !newFileExists) {
42+
await fs.rename(oldPath, newPath)
43+
outputChannel.appendLine(`Renamed ${migration.oldName} to ${migration.newName}`)
44+
} else {
45+
outputChannel.appendLine(
46+
`Skipping migration of ${migration.oldName} to ${migration.newName}: ${oldFileExists ? "new file already exists" : "old file not found"}`,
47+
)
48+
}
49+
}
50+
} catch (error) {
51+
outputChannel.appendLine(`Error migrating settings files: ${error}`)
52+
}
53+
}

0 commit comments

Comments
 (0)