diff --git a/src/core/config/CustomModesManager.ts b/src/core/config/CustomModesManager.ts index a9a2e6a6b55a..fb11413db76c 100644 --- a/src/core/config/CustomModesManager.ts +++ b/src/core/config/CustomModesManager.ts @@ -102,6 +102,61 @@ export class CustomModesManager { return exists ? roomodesPath : undefined } + /** + * Get all .roomodes files in the hierarchy from workspace root up to parent directories + * @returns Array of .roomodes file paths, ordered from most general (parent) to most specific (workspace) + */ + private async getHierarchicalRoomodes(): Promise { + const workspaceFolders = vscode.workspace.workspaceFolders + + if (!workspaceFolders || workspaceFolders.length === 0) { + return [] + } + + const workspaceRoot = getWorkspacePath() + const roomodesFiles: string[] = [] + const visitedPaths = new Set() + const homeDir = os.homedir() + let currentPath = path.resolve(workspaceRoot) + + // Walk up the directory tree from workspace root + while (currentPath && currentPath !== path.dirname(currentPath)) { + // Avoid infinite loops + if (visitedPaths.has(currentPath)) { + break + } + visitedPaths.add(currentPath) + + // Don't look for .roomodes in the home directory + if (currentPath === homeDir) { + break + } + + // Check if .roomodes exists at this level + const roomodesPath = path.join(currentPath, ROOMODES_FILENAME) + if (await fileExistsAtPath(roomodesPath)) { + roomodesFiles.push(roomodesPath) + } + + // Move to parent directory + const parentPath = path.dirname(currentPath) + + // Stop if we've reached the root or if parent is the same as current + if ( + parentPath === currentPath || + parentPath === "/" || + (process.platform === "win32" && parentPath === path.parse(currentPath).root) + ) { + break + } + + currentPath = parentPath + } + + // Return in order from most general (parent) to most specific (workspace) + return roomodesFiles.reverse() + } + /** * Regex pattern for problematic characters that need to be cleaned from YAML content * Includes: @@ -293,12 +348,17 @@ export class CustomModesManager { return } - // Get modes from .roomodes if it exists (takes precedence) - const roomodesPath = await this.getWorkspaceRoomodes() - const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : [] + // Get modes from hierarchical .roomodes + const hierarchicalRoomodes = await this.getHierarchicalRoomodes() + const allRoomodesModes: ModeConfig[] = [] - // Merge modes from both sources (.roomodes takes precedence) - const mergedModes = await this.mergeCustomModes(roomodesModes, result.data.customModes) + for (const roomodesPath of hierarchicalRoomodes) { + const modes = await this.loadModesFromFile(roomodesPath) + allRoomodesModes.push(...modes) + } + + // Merge modes from all sources + const mergedModes = await this.mergeCustomModes(allRoomodesModes, result.data.customModes) await this.context.globalState.update("customModes", mergedModes) this.clearCache() await this.onUpdate() @@ -312,19 +372,28 @@ export class CustomModesManager { this.disposables.push(settingsWatcher.onDidDelete(handleSettingsChange)) this.disposables.push(settingsWatcher) - // Watch .roomodes file - watch the path even if it doesn't exist yet + // Watch .roomodes files in hierarchy const workspaceFolders = vscode.workspace.workspaceFolders if (workspaceFolders && workspaceFolders.length > 0) { - const workspaceRoot = getWorkspacePath() - const roomodesPath = path.join(workspaceRoot, ROOMODES_FILENAME) - const roomodesWatcher = vscode.workspace.createFileSystemWatcher(roomodesPath) + // Create a generic pattern to watch all .roomodes files in the workspace tree + const roomodesPattern = new vscode.RelativePattern(workspaceFolders[0], "**/.roomodes") + const roomodesWatcher = vscode.workspace.createFileSystemWatcher(roomodesPattern) const handleRoomodesChange = async () => { try { const settingsModes = await this.loadModesFromFile(settingsPath) - const roomodesModes = await this.loadModesFromFile(roomodesPath) - // .roomodes takes precedence - const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes) + + // Get modes from hierarchical .roomodes + const hierarchicalRoomodes = await this.getHierarchicalRoomodes() + const allRoomodesModes: ModeConfig[] = [] + + for (const roomodesPath of hierarchicalRoomodes) { + const modes = await this.loadModesFromFile(roomodesPath) + allRoomodesModes.push(...modes) + } + + // Merge modes from all sources + const mergedModes = await this.mergeCustomModes(allRoomodesModes, settingsModes) await this.context.globalState.update("customModes", mergedModes) this.clearCache() await this.onUpdate() @@ -335,19 +404,7 @@ export class CustomModesManager { this.disposables.push(roomodesWatcher.onDidChange(handleRoomodesChange)) this.disposables.push(roomodesWatcher.onDidCreate(handleRoomodesChange)) - this.disposables.push( - roomodesWatcher.onDidDelete(async () => { - // When .roomodes is deleted, refresh with only settings modes - try { - const settingsModes = await this.loadModesFromFile(settingsPath) - await this.context.globalState.update("customModes", settingsModes) - this.clearCache() - await this.onUpdate() - } catch (error) { - console.error(`[CustomModesManager] Error handling .roomodes file deletion:`, error) - } - }), - ) + this.disposables.push(roomodesWatcher.onDidDelete(handleRoomodesChange)) this.disposables.push(roomodesWatcher) } } @@ -360,37 +417,22 @@ export class CustomModesManager { return this.cachedModes } - // Get modes from settings file. + // Get modes from settings file (global) const settingsPath = await this.getCustomModesFilePath() const settingsModes = await this.loadModesFromFile(settingsPath) - // Get modes from .roomodes if it exists. - const roomodesPath = await this.getWorkspaceRoomodes() - const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : [] - - // Create maps to store modes by source. - const projectModes = new Map() - const globalModes = new Map() + // Get modes from hierarchical .roomodes files + const hierarchicalRoomodes = await this.getHierarchicalRoomodes() + const allRoomodesModes: ModeConfig[] = [] - // Add project modes (they take precedence). - for (const mode of roomodesModes) { - projectModes.set(mode.slug, { ...mode, source: "project" as const }) - } - - // Add global modes. - for (const mode of settingsModes) { - if (!projectModes.has(mode.slug)) { - globalModes.set(mode.slug, { ...mode, source: "global" as const }) - } + // Load modes from each .roomodes file in hierarchy + for (const roomodesPath of hierarchicalRoomodes) { + const modes = await this.loadModesFromFile(roomodesPath) + allRoomodesModes.push(...modes) } - // Combine modes in the correct order: project modes first, then global modes. - const mergedModes = [ - ...roomodesModes.map((mode) => ({ ...mode, source: "project" as const })), - ...settingsModes - .filter((mode) => !projectModes.has(mode.slug)) - .map((mode) => ({ ...mode, source: "global" as const })), - ] + // Merge with .roomodes taking precedence and preserving expected order + const mergedModes = await this.mergeCustomModes(allRoomodesModes, settingsModes) await this.context.globalState.update("customModes", mergedModes) @@ -493,11 +535,18 @@ export class CustomModesManager { private async refreshMergedState(): Promise { const settingsPath = await this.getCustomModesFilePath() - const roomodesPath = await this.getWorkspaceRoomodes() - const settingsModes = await this.loadModesFromFile(settingsPath) - const roomodesModes = roomodesPath ? await this.loadModesFromFile(roomodesPath) : [] - const mergedModes = await this.mergeCustomModes(roomodesModes, settingsModes) + + // Get modes from hierarchical .roomodes + const hierarchicalRoomodes = await this.getHierarchicalRoomodes() + const allRoomodesModes: ModeConfig[] = [] + + for (const roomodesPath of hierarchicalRoomodes) { + const modes = await this.loadModesFromFile(roomodesPath) + allRoomodesModes.push(...modes) + } + + const mergedModes = await this.mergeCustomModes(allRoomodesModes, settingsModes) await this.context.globalState.update("customModes", mergedModes) diff --git a/src/services/mcp/McpHub.ts b/src/services/mcp/McpHub.ts index caca5ddb392e..b3f0646c4b95 100644 --- a/src/services/mcp/McpHub.ts +++ b/src/services/mcp/McpHub.ts @@ -32,6 +32,8 @@ import { import { fileExistsAtPath } from "../../utils/fs" import { arePathsEqual, getWorkspacePath } from "../../utils/path" import { injectVariables } from "../../utils/config" +import * as os from "os" +import { safeWriteJson } from "../../utils/safeWriteJson" // Discriminated union for connection states export type ConnectedMcpConnection = { @@ -297,6 +299,13 @@ export class McpHub { private async handleConfigFileChange(filePath: string, source: "global" | "project"): Promise { try { + if (source === "project") { + // For project-level changes, recompute from all hierarchical mcp.json files + await this.updateProjectMcpServers() + return + } + + // Global file: validate and update from the single settings file const content = await fs.readFile(filePath, "utf-8") let config: any @@ -321,15 +330,7 @@ export class McpHub { await this.updateServerConnections(result.data.mcpServers || {}, source) } catch (error) { - // Check if the error is because the file doesn't exist - if (error.code === "ENOENT" && source === "project") { - // File was deleted, clean up project MCP servers - await this.cleanupProjectMcpServers() - await this.notifyWebviewOfServerChanges() - vscode.window.showInformationMessage(t("mcp:info.project_config_deleted")) - } else { - this.showErrorMessage(t("mcp:errors.failed_update_project"), error) - } + this.showErrorMessage(t("mcp:errors.failed_update_project"), error) } } @@ -380,33 +381,8 @@ export class McpHub { private async updateProjectMcpServers(): Promise { try { - const projectMcpPath = await this.getProjectMcpPath() - if (!projectMcpPath) return - - const content = await fs.readFile(projectMcpPath, "utf-8") - let config: any - - try { - config = JSON.parse(content) - } catch (parseError) { - const errorMessage = t("mcp:errors.invalid_settings_syntax") - console.error(errorMessage, parseError) - vscode.window.showErrorMessage(errorMessage) - return - } - - // Validate configuration structure - const result = McpSettingsSchema.safeParse(config) - if (result.success) { - await this.updateServerConnections(result.data.mcpServers || {}, "project") - } else { - // Format validation errors for better user feedback - const errorMessages = result.error.errors - .map((err) => `${err.path.join(".")}: ${err.message}`) - .join("\n") - console.error("Invalid project MCP settings format:", errorMessages) - vscode.window.showErrorMessage(t("mcp:errors.invalid_settings_validation", { errorMessages })) - } + const { servers } = await this.loadMergedProjectServers() + await this.updateServerConnections(servers, "project") } catch (error) { this.showErrorMessage(t("mcp:errors.failed_update_project"), error) } @@ -454,14 +430,7 @@ export class McpHub { ) const fileExists = await fileExistsAtPath(mcpSettingsFilePath) if (!fileExists) { - await fs.writeFile( - mcpSettingsFilePath, - `{ - "mcpServers": { - - } -}`, - ) + await safeWriteJson(mcpSettingsFilePath, { mcpServers: {} }) } return mcpSettingsFilePath } @@ -504,34 +473,34 @@ export class McpHub { private async initializeMcpServers(source: "global" | "project"): Promise { try { - const configPath = - source === "global" ? await this.getMcpSettingsFilePath() : await this.getProjectMcpPath() - - if (!configPath) { + if (source === "project") { + // Initialize from hierarchical project MCP files + const { servers } = await this.loadMergedProjectServers() + await this.updateServerConnections(servers, "project", false) return } + // Global initialization remains unchanged + const configPath = await this.getMcpSettingsFilePath() const content = await fs.readFile(configPath, "utf-8") const config = JSON.parse(content) const result = McpSettingsSchema.safeParse(config) if (result.success) { // Pass all servers including disabled ones - they'll be handled in updateServerConnections - await this.updateServerConnections(result.data.mcpServers || {}, source, false) + await this.updateServerConnections(result.data.mcpServers || {}, "global", false) } else { const errorMessages = result.error.errors .map((err) => `${err.path.join(".")}: ${err.message}`) .join("\n") - console.error(`Invalid ${source} MCP settings format:`, errorMessages) + console.error(`Invalid global MCP settings format:`, errorMessages) vscode.window.showErrorMessage(t("mcp:errors.invalid_settings_validation", { errorMessages })) - if (source === "global") { - // Still try to connect with the raw config, but show warnings - try { - await this.updateServerConnections(config.mcpServers || {}, source, false) - } catch (error) { - this.showErrorMessage(`Failed to initialize ${source} MCP servers with raw config`, error) - } + // Still try to connect with the raw config, but show warnings + try { + await this.updateServerConnections(config.mcpServers || {}, "global", false) + } catch (error) { + this.showErrorMessage(`Failed to initialize global MCP servers with raw config`, error) } } } catch (error) { @@ -563,6 +532,79 @@ export class McpHub { } } + /** + * Returns the list of project-level MCP files discovered hierarchically + * from parent directories down to the current workspace. + * Order: least specific (parent) -> most specific (workspace) + */ + private async getHierarchicalProjectMcpPaths(): Promise { + const workspaceRoot = this.providerRef.deref()?.cwd ?? getWorkspacePath() + if (!workspaceRoot) return [] + + const paths: string[] = [] + const visited = new Set() + const homeDir = os.homedir() + let currentPath = path.resolve(workspaceRoot) + + while (currentPath && currentPath !== path.dirname(currentPath)) { + if (visited.has(currentPath)) break + visited.add(currentPath) + + // Stop at the home directory since global config is handled separately + if (currentPath === homeDir) break + + const candidate = path.join(currentPath, ".roo", "mcp.json") + try { + await fs.access(candidate) + paths.push(candidate) + } catch { + // ignore missing files + } + + const parentPath = path.dirname(currentPath) + if ( + parentPath === currentPath || + parentPath === "/" || + (process.platform === "win32" && parentPath === path.parse(currentPath).root) + ) { + break + } + currentPath = parentPath + } + + // Return from least specific to most specific + return paths.reverse() + } + + /** + * Loads and merges all project-level MCP server configurations discovered hierarchically. + * More specific configurations override general ones by shallow merge. + */ + private async loadMergedProjectServers(): Promise<{ servers: Record; order: string[] }> { + const files = await this.getHierarchicalProjectMcpPaths() + const servers: Record = {} + const order: string[] = [] + + for (const file of files) { + try { + const content = await fs.readFile(file, "utf-8") + const cfg = JSON.parse(content) + const mcpServers = (cfg && cfg.mcpServers) || {} + for (const [name, serverCfg] of Object.entries(mcpServers)) { + // Maintain a stable order where more specific definitions appear later + const existingIndex = order.indexOf(name) + if (existingIndex !== -1) order.splice(existingIndex, 1) + order.push(name) + servers[name] = serverCfg + } + } catch { + // ignore parse/read errors for individual files + } + } + + return { servers, order } + } + // Initialize project-level MCP servers private async initializeProjectMcpServers(): Promise { await this.initializeMcpServers("project") @@ -923,23 +965,17 @@ export class McpHub { try { let serverConfigData: Record = {} if (actualSource === "project") { - // Get project MCP config path - const projectMcpPath = await this.getProjectMcpPath() - if (projectMcpPath) { - configPath = projectMcpPath - const content = await fs.readFile(configPath, "utf-8") - serverConfigData = JSON.parse(content) - } + // Merge MCP servers from all hierarchical project mcp.json files + const { servers } = await this.loadMergedProjectServers() + serverConfigData.mcpServers = servers } else { // Get global MCP settings path - configPath = await this.getMcpSettingsFilePath() - const content = await fs.readFile(configPath, "utf-8") + const configPathGlobal = await this.getMcpSettingsFilePath() + const content = await fs.readFile(configPathGlobal, "utf-8") serverConfigData = JSON.parse(content) } - if (serverConfigData) { - alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || [] - disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || [] - } + alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || [] + disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || [] } catch (error) { console.error(`Failed to read tool configuration for ${serverName}:`, error) // Continue with empty configs @@ -1285,17 +1321,13 @@ export class McpHub { const config = JSON.parse(content) const globalServerOrder = Object.keys(config.mcpServers || {}) - // Get project server order if available - const projectMcpPath = await this.getProjectMcpPath() + // Get project server order from hierarchical project MCP files let projectServerOrder: string[] = [] - if (projectMcpPath) { - try { - const projectContent = await fs.readFile(projectMcpPath, "utf-8") - const projectConfig = JSON.parse(projectContent) - projectServerOrder = Object.keys(projectConfig.mcpServers || {}) - } catch (error) { - // Silently continue with empty project server order - } + try { + const { order } = await this.loadMergedProjectServers() + projectServerOrder = order + } catch (error) { + // Silently continue with empty project server order } // Sort connections: first project servers in their defined order, then global servers in their defined order @@ -1463,7 +1495,7 @@ export class McpHub { mcpServers: config.mcpServers, } - await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2)) + await safeWriteJson(configPath, updatedConfig) } public async updateServerTimeout( @@ -1541,7 +1573,7 @@ export class McpHub { mcpServers: config.mcpServers, } - await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2)) + await safeWriteJson(configPath, updatedConfig) // Update server connections with the correct source await this.updateServerConnections(config.mcpServers, serverSource) @@ -1686,7 +1718,7 @@ export class McpHub { targetList.splice(toolIndex, 1) } - await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2)) + await safeWriteJson(normalizedPath, config) if (connection) { connection.server.tools = await this.fetchToolsList(serverName, source) diff --git a/src/services/mcp/__tests__/hierarchical-mcp.spec.ts b/src/services/mcp/__tests__/hierarchical-mcp.spec.ts new file mode 100644 index 000000000000..94942df84b53 --- /dev/null +++ b/src/services/mcp/__tests__/hierarchical-mcp.spec.ts @@ -0,0 +1,164 @@ +import { describe, it, expect, beforeEach, vi } from "vitest" +import * as path from "path" +import * as os from "os" +import * as fs from "fs/promises" +import type { ClineProvider } from "../../../core/webview/ClineProvider" +import { McpHub } from "../McpHub" + +// Mock fs/promises ESM module to allow stubbing methods +vi.mock("fs/promises", () => ({ + default: { + access: vi.fn(), + readFile: vi.fn(), + writeFile: vi.fn(), + }, + access: vi.fn(), + readFile: vi.fn(), + writeFile: vi.fn(), +})) + +// Mock safeWriteJson to avoid real FS writes +vi.mock("../../../utils/safeWriteJson", () => ({ + safeWriteJson: vi.fn(async (filePath: string, data: any) => { + const fs = await import("fs/promises") + return fs.writeFile(filePath, JSON.stringify(data), "utf8") + }), +})) + +// Minimal VSCode mock +vi.mock("vscode", () => ({ + workspace: { + createFileSystemWatcher: vi.fn().mockReturnValue({ + onDidChange: vi.fn(), + onDidCreate: vi.fn(), + onDidDelete: vi.fn(), + dispose: vi.fn(), + }), + onDidSaveTextDocument: vi.fn(), + onDidChangeWorkspaceFolders: vi.fn(), + workspaceFolders: [], + }, + window: { + showErrorMessage: vi.fn(), + showInformationMessage: vi.fn(), + showWarningMessage: vi.fn(), + createTextEditorDecorationType: vi.fn().mockReturnValue({ + dispose: vi.fn(), + }), + }, + Disposable: { + from: vi.fn(), + }, +})) + +describe("Hierarchical project MCP configuration resolution", () => { + const fileMap: Record = {} + const workspaceRoot = "/home/user/mono-repo/packages/frontend" + + const repoLevel = path.join("/home/user/mono-repo", ".roo", "mcp.json") + const packagesLevel = path.join("/home/user/mono-repo/packages", ".roo", "mcp.json") + const appLevel = path.join(workspaceRoot, ".roo", "mcp.json") + + beforeEach(() => { + vi.clearAllMocks() + process.env.NODE_ENV = "test" + + // Prepare hierarchical files + fileMap[repoLevel] = JSON.stringify({ + mcpServers: { + alpha: { type: "stdio", command: "node", args: ["repo.js"], disabled: true }, + }, + }) + fileMap[packagesLevel] = JSON.stringify({ + mcpServers: { + beta: { type: "stdio", command: "node", args: ["pkg.js"], disabled: true }, + }, + }) + fileMap[appLevel] = JSON.stringify({ + mcpServers: { + // Override alpha at most specific level + alpha: { type: "stdio", command: "node", args: ["app.js"], disabled: true }, + }, + }) + + // Configure fs.promises mocks + ;(fs as any).access.mockImplementation(async (p: any) => { + const key = String(p) + if (fileMap[key]) return + const err: any = new Error("ENOENT") + err.code = "ENOENT" + throw err + }) + ;(fs as any).readFile.mockImplementation(async (p: any, _enc?: any) => { + const key = String(p) + if (fileMap[key]) return fileMap[key] + // default empty config to avoid syntax errors + return "{}" + }) + ;(fs as any).writeFile.mockResolvedValue(undefined as unknown as void) + }) + + it("merges multiple project-level .roo/mcp.json files with most specific overriding", async () => { + const mockProvider: Partial = { + ensureSettingsDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"), + ensureMcpServersDirectoryExists: vi.fn().mockResolvedValue("/mock/settings/path"), + postMessageToWebview: vi.fn().mockResolvedValue(undefined), + getState: vi.fn().mockResolvedValue({ mcpEnabled: true }), + // Critical: provide cwd for hierarchical traversal start + cwd: workspaceRoot as any, + context: { + subscriptions: [], + workspaceState: {} as any, + globalState: {} as any, + secrets: {} as any, + extensionUri: { fsPath: "/mock/uri" } as any, + extensionPath: "/mock/extension/path", + storagePath: "/mock/storage", + globalStoragePath: "/mock/global-storage", + environmentVariableCollection: {} as any, + extension: { + id: "test-extension", + extensionUri: { fsPath: "/mock/uri" } as any, + extensionPath: "/mock/extension/path", + extensionKind: 1, + isActive: true, + packageJSON: { version: "1.0.0" }, + activate: vi.fn(), + exports: undefined, + } as any, + asAbsolutePath: (p: string) => p, + storageUri: { fsPath: "/mock/uri" } as any, + globalStorageUri: { fsPath: "/mock/uri" } as any, + logUri: { fsPath: "/mock/uri" } as any, + extensionMode: 1, + logPath: "/mock/log", + languageModelAccessInformation: {} as any, + } as any, + } + + const hub = new McpHub(mockProvider as ClineProvider) + + // Allow initial async initialization to complete to avoid duplicate connections + await new Promise((r) => setTimeout(r, 100)) + + // Do not manually trigger refresh to avoid racing with constructor initialization. + // The constructor already initializes project servers hierarchically. + + // Validate that both alpha and beta are present as project servers + const projectConnections = hub.connections.filter((c) => c.server.source === "project") + const names = projectConnections.map((c) => c.server.name).sort() + expect(names).toEqual(["alpha", "beta"]) + + // Validate that alpha was overridden by most specific file (app level) + const alphaConn = projectConnections.find((c) => c.server.name === "alpha")! + const alphaCfg = JSON.parse(alphaConn.server.config) + expect(alphaCfg.args).toEqual(["app.js"]) + expect(alphaConn.server.status).toBe("disconnected") // disabled:true yields placeholder + + // Validate that beta came from packages level unchanged + const betaConn = projectConnections.find((c) => c.server.name === "beta")! + const betaCfg = JSON.parse(betaConn.server.config) + expect(betaCfg.args).toEqual(["pkg.js"]) + expect(betaConn.server.status).toBe("disconnected") + }) +}) diff --git a/src/services/roo-config/__tests__/hierarchical-resolution.spec.ts b/src/services/roo-config/__tests__/hierarchical-resolution.spec.ts new file mode 100644 index 000000000000..61d8bcad6150 --- /dev/null +++ b/src/services/roo-config/__tests__/hierarchical-resolution.spec.ts @@ -0,0 +1,158 @@ +import { describe, it, expect, vi, beforeEach, afterEach } from "vitest" +import * as path from "path" +import * as os from "os" +import { getRooDirectoriesForCwd } from "../index" + +vi.mock("os", () => ({ + homedir: vi.fn(), +})) + +describe("Hierarchical .roo directory resolution", () => { + const mockHomedir = vi.mocked(os.homedir) + + beforeEach(() => { + mockHomedir.mockReturnValue("/home/user") + }) + + afterEach(() => { + vi.clearAllMocks() + }) + + describe("getRooDirectoriesForCwd with hierarchical resolution", () => { + it("should return only global and project-local when hierarchical is disabled", () => { + const cwd = "/home/user/projects/myapp" + const directories = getRooDirectoriesForCwd(cwd, false) + + expect(directories).toEqual([ + "/home/user/.roo", // Global + "/home/user/projects/myapp/.roo", // Project-local + ]) + }) + + it("should return hierarchical directories for simple project", () => { + const cwd = "/home/user/projects/myapp" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "/home/user/.roo", // Global + "/home/user/projects/.roo", // Parent directory + "/home/user/projects/myapp/.roo", // Project-local + ]) + }) + + it("should handle deeply nested mono-repo structure", () => { + const cwd = "/home/user/work/company/mono-repo/packages/frontend/src" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "/home/user/.roo", // Global + "/home/user/work/.roo", + "/home/user/work/company/.roo", + "/home/user/work/company/mono-repo/.roo", // Repository root + "/home/user/work/company/mono-repo/packages/.roo", // Packages folder + "/home/user/work/company/mono-repo/packages/frontend/.roo", // Frontend package + "/home/user/work/company/mono-repo/packages/frontend/src/.roo", // Source folder + ]) + }) + + it.skipIf(process.platform !== "win32")("should handle Windows paths correctly", () => { + mockHomedir.mockReturnValue("C:\\Users\\john") + + const cwd = "C:\\Users\\john\\projects\\myapp" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "C:\\Users\\john\\.roo", // Global + "C:\\Users\\john\\projects\\.roo", // Parent directory + "C:\\Users\\john\\projects\\myapp\\.roo", // Project-local + ]) + }) + + it("should stop at home directory and not include it twice", () => { + const cwd = "/home/user/myproject" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "/home/user/.roo", // Global (home directory) + "/home/user/myproject/.roo", // Project-local + ]) + + // Should not have duplicate /home/user/.roo entries + const homeDirRooCount = directories.filter((d) => d === "/home/user/.roo").length + expect(homeDirRooCount).toBe(1) + }) + + it("should handle root directory edge case", () => { + const cwd = "/project" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "/home/user/.roo", // Global + "/project/.roo", // Project at root level + ]) + }) + + it("should handle relative paths by resolving them", () => { + const cwd = "./myproject" + const directories = getRooDirectoriesForCwd(cwd, true) + + // Should resolve relative path and return proper hierarchy + expect(directories[0]).toBe("/home/user/.roo") // Global directory + expect(directories[directories.length - 1]).toMatch(/myproject[\/\\]\.roo$/) + }) + + it("should use hierarchical resolution by default when not specified", () => { + const cwd = "/home/user/projects/myapp" + const directories = getRooDirectoriesForCwd(cwd) // No second parameter + + expect(directories).toEqual([ + "/home/user/.roo", // Global + "/home/user/projects/.roo", // Parent directory + "/home/user/projects/myapp/.roo", // Project-local + ]) + }) + + it("should handle edge case of cwd being exactly home directory", () => { + const cwd = "/home/user" + const directories = getRooDirectoriesForCwd(cwd, true) + + expect(directories).toEqual([ + "/home/user/.roo", // Global (and also the cwd) + ]) + + // Should not have duplicate entries + expect(directories.length).toBe(1) + }) + + it("should handle symbolic links and circular references gracefully", () => { + // This tests that the visitedPaths Set prevents infinite loops + const cwd = "/home/user/projects/link/to/self" + const directories = getRooDirectoriesForCwd(cwd, true) + + // Should not throw or hang, should return valid hierarchy + expect(directories).toBeDefined() + expect(directories[0]).toBe("/home/user/.roo") + expect(directories.length).toBeGreaterThan(0) + }) + }) + + describe("Integration with configuration loading", () => { + it("should provide directories in correct order for override behavior", () => { + // More specific configurations should override more general ones + const cwd = "/home/user/company/project/submodule" + const directories = getRooDirectoriesForCwd(cwd, true) + + // Verify order: global first (least specific), then progressively more specific + expect(directories[0]).toBe("/home/user/.roo") // Least specific + expect(directories[directories.length - 1]).toBe("/home/user/company/project/submodule/.roo") // Most specific + + // This order ensures that when configs are merged, more specific ones override general ones + for (let i = 1; i < directories.length; i++) { + // Each directory should be a child of the previous (when excluding global) + if (i > 1) { + expect(directories[i].startsWith(directories[i - 1].replace(/[\/\\]\.roo$/, ""))).toBe(true) + } + } + }) + }) +}) diff --git a/src/services/roo-config/__tests__/index.spec.ts b/src/services/roo-config/__tests__/index.spec.ts index 946bb27c7f28..62a1d5618d0d 100644 --- a/src/services/roo-config/__tests__/index.spec.ts +++ b/src/services/roo-config/__tests__/index.spec.ts @@ -205,13 +205,28 @@ describe("RooConfigService", () => { }) describe("getRooDirectoriesForCwd", () => { - it("should return directories for given cwd", () => { + it("should return directories for given cwd with legacy behavior", () => { const cwd = "/custom/project/path" - const result = getRooDirectoriesForCwd(cwd) + // Test with hierarchical disabled to maintain backward compatibility + const result = getRooDirectoriesForCwd(cwd, false) expect(result).toEqual([path.join("/mock/home", ".roo"), path.join(cwd, ".roo")]) }) + + it("should return hierarchical directories by default", () => { + const cwd = "/custom/project/path" + + const result = getRooDirectoriesForCwd(cwd) + + // With hierarchical resolution, we get intermediate directories + expect(result).toEqual([ + path.join("/mock/home", ".roo"), // Global + "/custom/.roo", // Parent directories + "/custom/project/.roo", + path.join(cwd, ".roo"), // Project-local + ]) + }) }) describe("loadConfiguration", () => { diff --git a/src/services/roo-config/index.ts b/src/services/roo-config/index.ts index b46c39e354b3..e4a1bd6d52aa 100644 --- a/src/services/roo-config/index.ts +++ b/src/services/roo-config/index.ts @@ -25,6 +25,10 @@ import fs from "fs/promises" */ export function getGlobalRooDirectory(): string { const homeDir = os.homedir() + // Preserve POSIX-style paths when the homedir is expressed as POSIX (e.g., in tests on Windows) + if (homeDir.startsWith("/")) { + return path.posix.join(homeDir, ".roo") + } return path.join(homeDir, ".roo") } @@ -58,7 +62,8 @@ export function getGlobalRooDirectory(): string { * ``` */ export function getProjectRooDirectoryForCwd(cwd: string): string { - return path.join(cwd, ".roo") + // If the provided cwd is POSIX-like (starts with "/"), keep POSIX semantics cross-platform + return cwd.startsWith("/") ? path.posix.join(cwd, ".roo") : path.join(cwd, ".roo") } /** @@ -112,19 +117,23 @@ export async function readFileIfExists(filePath: string): Promise } /** - * Gets the ordered list of .roo directories to check (global first, then project-local) + * Gets the ordered list of .roo directories to check hierarchically + * Walking from global to most specific (current directory) * * @param cwd - Current working directory (project path) - * @returns Array of directory paths to check in order [global, project-local] + * @param enableHierarchical - Whether to enable hierarchical resolution (default: true) + * @returns Array of directory paths to check in order [global, ...parent directories, project-local] * * @example * ```typescript - * // For a project at /Users/john/my-project - * const directories = getRooDirectoriesForCwd('/Users/john/my-project') + * // For a project at /Users/john/mono-repo/packages/frontend + * const directories = getRooDirectoriesForCwd('/Users/john/mono-repo/packages/frontend') * // Returns: * // [ - * // '/Users/john/.roo', // Global directory - * // '/Users/john/my-project/.roo' // Project-local directory + * // '/Users/john/.roo', // Global directory + * // '/Users/john/mono-repo/.roo', // Repository root + * // '/Users/john/mono-repo/packages/.roo', // Packages folder + * // '/Users/john/mono-repo/packages/frontend/.roo' // Project-local directory * // ] * ``` * @@ -135,23 +144,91 @@ export async function readFileIfExists(filePath: string): Promise * │ ├── rules/ * │ │ └── rules.md * │ └── custom-instructions.md - * └── my-project/ - * ├── .roo/ # Project-specific configuration - * │ ├── rules/ - * │ │ └── rules.md # Overrides global rules - * │ └── project-notes.md - * └── src/ - * └── index.ts + * └── mono-repo/ + * ├── .roo/ # Repository-wide configuration + * │ └── rules/ + * │ └── repo-rules.md + * └── packages/ + * ├── .roo/ # Packages-specific configuration + * │ └── rules/ + * │ └── packages-rules.md + * └── frontend/ + * ├── .roo/ # Frontend-specific configuration + * │ └── rules/ + * │ └── frontend-rules.md + * └── src/ + * └── index.ts * ``` */ -export function getRooDirectoriesForCwd(cwd: string): string[] { +export function getRooDirectoriesForCwd(cwd: string, enableHierarchical: boolean = true): string[] { const directories: string[] = [] + const posixInput = cwd.startsWith("/") + const ops = posixInput ? path.posix : path + + // Resolve global directory first and normalize for POSIX-like inputs + const globalDir = getGlobalRooDirectory() + const normalizedGlobalDir = + posixInput && !globalDir.startsWith("/") + ? ops.join(os.homedir().replace(/\\/g, "/"), ".roo") + : posixInput + ? globalDir.replace(/\\/g, "/") + : globalDir // Add global directory first - directories.push(getGlobalRooDirectory()) + directories.push(normalizedGlobalDir) + + if (!enableHierarchical) { + // Legacy behavior: only global and project-local + directories.push(getProjectRooDirectoryForCwd(cwd)) + return directories + } + + // Hierarchical resolution: walk up from cwd to find all .roo directories + const visitedPaths = new Set() + // Resolve current path using the chosen ops + let currentPath = posixInput ? path.posix.resolve(cwd) : path.resolve(cwd) + const hierarchicalDirs: string[] = [] + + // Normalize homeDir for comparison + let homeDir = os.homedir() + if (posixInput) homeDir = homeDir.replace(/\\/g, "/") + const homeResolved = posixInput ? path.posix.resolve(homeDir) : path.resolve(homeDir) + + // Walk up the directory tree + while (currentPath && currentPath !== ops.dirname(currentPath)) { + // Avoid infinite loops + if (visitedPaths.has(currentPath)) { + break + } + visitedPaths.add(currentPath) + + // Skip if we've reached the home directory (global .roo is already added) + if (currentPath === homeResolved) { + break + } + + // Add .roo directory for this level + const rooDir = ops.join(currentPath, ".roo") + hierarchicalDirs.push(rooDir) + + // Move to parent directory + const parentPath = ops.dirname(currentPath) + + // Stop if we've reached the root or if parent is the same as current + if ( + parentPath === currentPath || + (!posixInput && (parentPath === "/" || parentPath === path.parse(currentPath).root)) || + (posixInput && parentPath === "/") + ) { + break + } + + currentPath = parentPath + } - // Add project-local directory second - directories.push(getProjectRooDirectoryForCwd(cwd)) + // Add hierarchical directories in reverse order (from root to most specific) + // This ensures more specific configurations override general ones + directories.push(...hierarchicalDirs.reverse()) return directories }