Skip to content

Commit 6587316

Browse files
committed
fix(mcp): prevent server reconnection when toggling disabled state
Fixed bug where MCP servers would reconnect instead of staying disabled when toggled off. The issue was that toggleServerDisabled() used stale in-memory config instead of reading the fresh config from disk after writing the disabled flag. Changes: Added readServerConfigFromFile() helper to read and validate server config from disk Updated disable path to read fresh config before calling connectToServer() Updated enable path to read fresh config before calling connectToServer() This ensures the disabled: true flag is properly read, causing connectToServer() to create a disabled placeholder connection instead of actually connecting the server. + refactor(mcp): use safeWriteJson for atomic config writes Replace JSON.stringify + fs.writeFile with safeWriteJson in McpHub.ts to prevent data corruption through atomic writes with file locking.
1 parent 9db94d3 commit 6587316

File tree

1 file changed

+69
-16
lines changed

1 file changed

+69
-16
lines changed

src/services/mcp/McpHub.ts

Lines changed: 69 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
import { fileExistsAtPath } from "../../utils/fs"
3333
import { arePathsEqual, getWorkspacePath } from "../../utils/path"
3434
import { injectVariables } from "../../utils/config"
35+
import { safeWriteJson } from "../../utils/safeWriteJson"
3536

3637
// Discriminated union for connection states
3738
export type ConnectedMcpConnection = {
@@ -1375,13 +1376,16 @@ export class McpHub {
13751376
this.removeFileWatchersForServer(serverName)
13761377
await this.deleteConnection(serverName, serverSource)
13771378
// Re-add as a disabled connection
1378-
await this.connectToServer(serverName, JSON.parse(connection.server.config), serverSource)
1379+
// Re-read config from file to get updated disabled state
1380+
const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
1381+
await this.connectToServer(serverName, updatedConfig, serverSource)
13791382
} else if (!disabled && connection.server.status === "disconnected") {
13801383
// If enabling a disabled server, connect it
1381-
const config = JSON.parse(connection.server.config)
1384+
// Re-read config from file to get updated disabled state
1385+
const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
13821386
await this.deleteConnection(serverName, serverSource)
13831387
// When re-enabling, file watchers will be set up in connectToServer
1384-
await this.connectToServer(serverName, config, serverSource)
1388+
await this.connectToServer(serverName, updatedConfig, serverSource)
13851389
} else if (connection.server.status === "connected") {
13861390
// Only refresh capabilities if connected
13871391
connection.server.tools = await this.fetchToolsList(serverName, serverSource)
@@ -1403,6 +1407,57 @@ export class McpHub {
14031407
}
14041408
}
14051409

1410+
/**
1411+
* Helper method to read a server's configuration from the appropriate settings file
1412+
* @param serverName The name of the server to read
1413+
* @param source Whether to read from the global or project config
1414+
* @returns The validated server configuration
1415+
*/
1416+
private async readServerConfigFromFile(
1417+
serverName: string,
1418+
source: "global" | "project" = "global",
1419+
): Promise<z.infer<typeof ServerConfigSchema>> {
1420+
// Determine which config file to read
1421+
let configPath: string
1422+
if (source === "project") {
1423+
const projectMcpPath = await this.getProjectMcpPath()
1424+
if (!projectMcpPath) {
1425+
throw new Error("Project MCP configuration file not found")
1426+
}
1427+
configPath = projectMcpPath
1428+
} else {
1429+
configPath = await this.getMcpSettingsFilePath()
1430+
}
1431+
1432+
// Ensure the settings file exists and is accessible
1433+
try {
1434+
await fs.access(configPath)
1435+
} catch (error) {
1436+
console.error("Settings file not accessible:", error)
1437+
throw new Error("Settings file not accessible")
1438+
}
1439+
1440+
// Read and parse the config file
1441+
const content = await fs.readFile(configPath, "utf-8")
1442+
const config = JSON.parse(content)
1443+
1444+
// Validate the config structure
1445+
if (!config || typeof config !== "object") {
1446+
throw new Error("Invalid config structure")
1447+
}
1448+
1449+
if (!config.mcpServers || typeof config.mcpServers !== "object") {
1450+
throw new Error("No mcpServers section in config")
1451+
}
1452+
1453+
if (!config.mcpServers[serverName]) {
1454+
throw new Error(`Server ${serverName} not found in config`)
1455+
}
1456+
1457+
// Validate and return the server config
1458+
return this.validateServerConfig(config.mcpServers[serverName], serverName)
1459+
}
1460+
14061461
/**
14071462
* Helper method to update a server's configuration in the appropriate settings file
14081463
* @param serverName The name of the server to update
@@ -1471,13 +1526,12 @@ export class McpHub {
14711526

14721527
// Set flag to prevent file watcher from triggering server restart
14731528
this.isProgrammaticUpdate = true
1474-
try {
1475-
await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2))
1476-
// Delay longer than debounce timer (500ms) to ensure watcher event is processed while flag is set
1477-
await delay(600)
1478-
} finally {
1529+
await safeWriteJson(configPath, updatedConfig)
1530+
1531+
// Reset flag after watcher debounce period (non-blocking)
1532+
delay(600).then(() => {
14791533
this.isProgrammaticUpdate = false
1480-
}
1534+
})
14811535
}
14821536

14831537
public async updateServerTimeout(
@@ -1555,7 +1609,7 @@ export class McpHub {
15551609
mcpServers: config.mcpServers,
15561610
}
15571611

1558-
await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2))
1612+
await safeWriteJson(configPath, updatedConfig)
15591613

15601614
// Update server connections with the correct source
15611615
await this.updateServerConnections(config.mcpServers, serverSource)
@@ -1702,13 +1756,12 @@ export class McpHub {
17021756

17031757
// Set flag to prevent file watcher from triggering server restart
17041758
this.isProgrammaticUpdate = true
1705-
try {
1706-
await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
1707-
// Delay longer than debounce timer (500ms) to ensure watcher event is processed while flag is set
1708-
await delay(600)
1709-
} finally {
1759+
await safeWriteJson(normalizedPath, config)
1760+
1761+
// Reset flag after watcher debounce period (non-blocking)
1762+
delay(600).then(() => {
17101763
this.isProgrammaticUpdate = false
1711-
}
1764+
})
17121765

17131766
if (connection) {
17141767
connection.server.tools = await this.fetchToolsList(serverName, source)

0 commit comments

Comments
 (0)