Skip to content

Commit 349b09d

Browse files
committed
feat(mcp): hierarchical project-level mcp.json resolution and config merge; use safeWriteJson for atomic writes; add tests
1 parent 1b499c4 commit 349b09d

File tree

2 files changed

+279
-83
lines changed

2 files changed

+279
-83
lines changed

src/services/mcp/McpHub.ts

Lines changed: 115 additions & 83 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import {
3232
import { fileExistsAtPath } from "../../utils/fs"
3333
import { arePathsEqual, getWorkspacePath } from "../../utils/path"
3434
import { injectVariables } from "../../utils/config"
35+
import * as os from "os"
36+
import { safeWriteJson } from "../../utils/safeWriteJson"
3537

3638
// Discriminated union for connection states
3739
export type ConnectedMcpConnection = {
@@ -297,6 +299,13 @@ export class McpHub {
297299

298300
private async handleConfigFileChange(filePath: string, source: "global" | "project"): Promise<void> {
299301
try {
302+
if (source === "project") {
303+
// For project-level changes, recompute from all hierarchical mcp.json files
304+
await this.updateProjectMcpServers()
305+
return
306+
}
307+
308+
// Global file: validate and update from the single settings file
300309
const content = await fs.readFile(filePath, "utf-8")
301310
let config: any
302311

@@ -321,15 +330,7 @@ export class McpHub {
321330

322331
await this.updateServerConnections(result.data.mcpServers || {}, source)
323332
} catch (error) {
324-
// Check if the error is because the file doesn't exist
325-
if (error.code === "ENOENT" && source === "project") {
326-
// File was deleted, clean up project MCP servers
327-
await this.cleanupProjectMcpServers()
328-
await this.notifyWebviewOfServerChanges()
329-
vscode.window.showInformationMessage(t("mcp:info.project_config_deleted"))
330-
} else {
331-
this.showErrorMessage(t("mcp:errors.failed_update_project"), error)
332-
}
333+
this.showErrorMessage(t("mcp:errors.failed_update_project"), error)
333334
}
334335
}
335336

@@ -380,33 +381,8 @@ export class McpHub {
380381

381382
private async updateProjectMcpServers(): Promise<void> {
382383
try {
383-
const projectMcpPath = await this.getProjectMcpPath()
384-
if (!projectMcpPath) return
385-
386-
const content = await fs.readFile(projectMcpPath, "utf-8")
387-
let config: any
388-
389-
try {
390-
config = JSON.parse(content)
391-
} catch (parseError) {
392-
const errorMessage = t("mcp:errors.invalid_settings_syntax")
393-
console.error(errorMessage, parseError)
394-
vscode.window.showErrorMessage(errorMessage)
395-
return
396-
}
397-
398-
// Validate configuration structure
399-
const result = McpSettingsSchema.safeParse(config)
400-
if (result.success) {
401-
await this.updateServerConnections(result.data.mcpServers || {}, "project")
402-
} else {
403-
// Format validation errors for better user feedback
404-
const errorMessages = result.error.errors
405-
.map((err) => `${err.path.join(".")}: ${err.message}`)
406-
.join("\n")
407-
console.error("Invalid project MCP settings format:", errorMessages)
408-
vscode.window.showErrorMessage(t("mcp:errors.invalid_settings_validation", { errorMessages }))
409-
}
384+
const { servers } = await this.loadMergedProjectServers()
385+
await this.updateServerConnections(servers, "project")
410386
} catch (error) {
411387
this.showErrorMessage(t("mcp:errors.failed_update_project"), error)
412388
}
@@ -454,14 +430,7 @@ export class McpHub {
454430
)
455431
const fileExists = await fileExistsAtPath(mcpSettingsFilePath)
456432
if (!fileExists) {
457-
await fs.writeFile(
458-
mcpSettingsFilePath,
459-
`{
460-
"mcpServers": {
461-
462-
}
463-
}`,
464-
)
433+
await safeWriteJson(mcpSettingsFilePath, { mcpServers: {} })
465434
}
466435
return mcpSettingsFilePath
467436
}
@@ -504,34 +473,34 @@ export class McpHub {
504473

505474
private async initializeMcpServers(source: "global" | "project"): Promise<void> {
506475
try {
507-
const configPath =
508-
source === "global" ? await this.getMcpSettingsFilePath() : await this.getProjectMcpPath()
509-
510-
if (!configPath) {
476+
if (source === "project") {
477+
// Initialize from hierarchical project MCP files
478+
const { servers } = await this.loadMergedProjectServers()
479+
await this.updateServerConnections(servers, "project", false)
511480
return
512481
}
513482

483+
// Global initialization remains unchanged
484+
const configPath = await this.getMcpSettingsFilePath()
514485
const content = await fs.readFile(configPath, "utf-8")
515486
const config = JSON.parse(content)
516487
const result = McpSettingsSchema.safeParse(config)
517488

518489
if (result.success) {
519490
// Pass all servers including disabled ones - they'll be handled in updateServerConnections
520-
await this.updateServerConnections(result.data.mcpServers || {}, source, false)
491+
await this.updateServerConnections(result.data.mcpServers || {}, "global", false)
521492
} else {
522493
const errorMessages = result.error.errors
523494
.map((err) => `${err.path.join(".")}: ${err.message}`)
524495
.join("\n")
525-
console.error(`Invalid ${source} MCP settings format:`, errorMessages)
496+
console.error(`Invalid global MCP settings format:`, errorMessages)
526497
vscode.window.showErrorMessage(t("mcp:errors.invalid_settings_validation", { errorMessages }))
527498

528-
if (source === "global") {
529-
// Still try to connect with the raw config, but show warnings
530-
try {
531-
await this.updateServerConnections(config.mcpServers || {}, source, false)
532-
} catch (error) {
533-
this.showErrorMessage(`Failed to initialize ${source} MCP servers with raw config`, error)
534-
}
499+
// Still try to connect with the raw config, but show warnings
500+
try {
501+
await this.updateServerConnections(config.mcpServers || {}, "global", false)
502+
} catch (error) {
503+
this.showErrorMessage(`Failed to initialize global MCP servers with raw config`, error)
535504
}
536505
}
537506
} catch (error) {
@@ -563,6 +532,79 @@ export class McpHub {
563532
}
564533
}
565534

535+
/**
536+
* Returns the list of project-level MCP files discovered hierarchically
537+
* from parent directories down to the current workspace.
538+
* Order: least specific (parent) -> most specific (workspace)
539+
*/
540+
private async getHierarchicalProjectMcpPaths(): Promise<string[]> {
541+
const workspaceRoot = this.providerRef.deref()?.cwd ?? getWorkspacePath()
542+
if (!workspaceRoot) return []
543+
544+
const paths: string[] = []
545+
const visited = new Set<string>()
546+
const homeDir = os.homedir()
547+
let currentPath = path.resolve(workspaceRoot)
548+
549+
while (currentPath && currentPath !== path.dirname(currentPath)) {
550+
if (visited.has(currentPath)) break
551+
visited.add(currentPath)
552+
553+
// Stop at the home directory since global config is handled separately
554+
if (currentPath === homeDir) break
555+
556+
const candidate = path.join(currentPath, ".roo", "mcp.json")
557+
try {
558+
await fs.access(candidate)
559+
paths.push(candidate)
560+
} catch {
561+
// ignore missing files
562+
}
563+
564+
const parentPath = path.dirname(currentPath)
565+
if (
566+
parentPath === currentPath ||
567+
parentPath === "/" ||
568+
(process.platform === "win32" && parentPath === path.parse(currentPath).root)
569+
) {
570+
break
571+
}
572+
currentPath = parentPath
573+
}
574+
575+
// Return from least specific to most specific
576+
return paths.reverse()
577+
}
578+
579+
/**
580+
* Loads and merges all project-level MCP server configurations discovered hierarchically.
581+
* More specific configurations override general ones by shallow merge.
582+
*/
583+
private async loadMergedProjectServers(): Promise<{ servers: Record<string, any>; order: string[] }> {
584+
const files = await this.getHierarchicalProjectMcpPaths()
585+
const servers: Record<string, any> = {}
586+
const order: string[] = []
587+
588+
for (const file of files) {
589+
try {
590+
const content = await fs.readFile(file, "utf-8")
591+
const cfg = JSON.parse(content)
592+
const mcpServers = (cfg && cfg.mcpServers) || {}
593+
for (const [name, serverCfg] of Object.entries(mcpServers)) {
594+
// Maintain a stable order where more specific definitions appear later
595+
const existingIndex = order.indexOf(name)
596+
if (existingIndex !== -1) order.splice(existingIndex, 1)
597+
order.push(name)
598+
servers[name] = serverCfg
599+
}
600+
} catch {
601+
// ignore parse/read errors for individual files
602+
}
603+
}
604+
605+
return { servers, order }
606+
}
607+
566608
// Initialize project-level MCP servers
567609
private async initializeProjectMcpServers(): Promise<void> {
568610
await this.initializeMcpServers("project")
@@ -923,23 +965,17 @@ export class McpHub {
923965
try {
924966
let serverConfigData: Record<string, any> = {}
925967
if (actualSource === "project") {
926-
// Get project MCP config path
927-
const projectMcpPath = await this.getProjectMcpPath()
928-
if (projectMcpPath) {
929-
configPath = projectMcpPath
930-
const content = await fs.readFile(configPath, "utf-8")
931-
serverConfigData = JSON.parse(content)
932-
}
968+
// Merge MCP servers from all hierarchical project mcp.json files
969+
const { servers } = await this.loadMergedProjectServers()
970+
serverConfigData.mcpServers = servers
933971
} else {
934972
// Get global MCP settings path
935-
configPath = await this.getMcpSettingsFilePath()
936-
const content = await fs.readFile(configPath, "utf-8")
973+
const configPathGlobal = await this.getMcpSettingsFilePath()
974+
const content = await fs.readFile(configPathGlobal, "utf-8")
937975
serverConfigData = JSON.parse(content)
938976
}
939-
if (serverConfigData) {
940-
alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
941-
disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || []
942-
}
977+
alwaysAllowConfig = serverConfigData.mcpServers?.[serverName]?.alwaysAllow || []
978+
disabledToolsList = serverConfigData.mcpServers?.[serverName]?.disabledTools || []
943979
} catch (error) {
944980
console.error(`Failed to read tool configuration for ${serverName}:`, error)
945981
// Continue with empty configs
@@ -1285,17 +1321,13 @@ export class McpHub {
12851321
const config = JSON.parse(content)
12861322
const globalServerOrder = Object.keys(config.mcpServers || {})
12871323

1288-
// Get project server order if available
1289-
const projectMcpPath = await this.getProjectMcpPath()
1324+
// Get project server order from hierarchical project MCP files
12901325
let projectServerOrder: string[] = []
1291-
if (projectMcpPath) {
1292-
try {
1293-
const projectContent = await fs.readFile(projectMcpPath, "utf-8")
1294-
const projectConfig = JSON.parse(projectContent)
1295-
projectServerOrder = Object.keys(projectConfig.mcpServers || {})
1296-
} catch (error) {
1297-
// Silently continue with empty project server order
1298-
}
1326+
try {
1327+
const { order } = await this.loadMergedProjectServers()
1328+
projectServerOrder = order
1329+
} catch (error) {
1330+
// Silently continue with empty project server order
12991331
}
13001332

13011333
// Sort connections: first project servers in their defined order, then global servers in their defined order
@@ -1463,7 +1495,7 @@ export class McpHub {
14631495
mcpServers: config.mcpServers,
14641496
}
14651497

1466-
await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2))
1498+
await safeWriteJson(configPath, updatedConfig)
14671499
}
14681500

14691501
public async updateServerTimeout(
@@ -1541,7 +1573,7 @@ export class McpHub {
15411573
mcpServers: config.mcpServers,
15421574
}
15431575

1544-
await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2))
1576+
await safeWriteJson(configPath, updatedConfig)
15451577

15461578
// Update server connections with the correct source
15471579
await this.updateServerConnections(config.mcpServers, serverSource)
@@ -1686,7 +1718,7 @@ export class McpHub {
16861718
targetList.splice(toolIndex, 1)
16871719
}
16881720

1689-
await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
1721+
await safeWriteJson(normalizedPath, config)
16901722

16911723
if (connection) {
16921724
connection.server.tools = await this.fetchToolsList(serverName, source)

0 commit comments

Comments
 (0)