fix: prevent MCP server restart when toggling tool permissions (#8633)

* fix: prevent MCP server restart when toggling tool permissions

Add isProgrammaticUpdate flag to distinguish between programmatic config
updates and user-initiated file changes. Skip file watcher processing
during programmatic updates to prevent unnecessary server restarts.

* 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.

* fix(mcp): prevent race condition in isProgrammaticUpdate flag

Replace multiple independent reset timers with a single timer that gets
cleared and rescheduled on each programmatic config update. This prevents
the flag from being reset prematurely when multiple rapid updates occur,
which could cause unwanted server restarts during the file watcher's
debounce period.

+ fix(mcp): ensure isProgrammaticUpdate flag cleanup with try-finally

Wrap safeWriteJson() calls in try-finally blocks to guarantee the
isProgrammaticUpdate flag is always reset, even if the write operation
fails. This prevents the flag from being stuck at true indefinitely,
which would cause subsequent user-initiated config changes to be
silently ignored.

Author: heyseth

src/services/mcp/McpHub.ts

@@ -32,6 +32,7 @@ import {
 import { fileExistsAtPath } from "../../utils/fs"
 import { arePathsEqual, getWorkspacePath } from "../../utils/path"
 import { injectVariables } from "../../utils/config"
+import { safeWriteJson } from "../../utils/safeWriteJson"
 
 // Discriminated union for connection states
 export type ConnectedMcpConnection = {
@@ -151,6 +152,8 @@ export class McpHub {
 	isConnecting: boolean = false
 	private refCount: number = 0 // Reference counter for active clients
 	private configChangeDebounceTimers: Map<string, NodeJS.Timeout> = new Map()
+	private isProgrammaticUpdate: boolean = false
+	private flagResetTimer?: NodeJS.Timeout
 
 	constructor(provider: ClineProvider) {
 		this.providerRef = new WeakRef(provider)
@@ -278,6 +281,11 @@ export class McpHub {
 	 * Debounced wrapper for handling config file changes
 	 */
 	private debounceConfigChange(filePath: string, source: "global" | "project"): void {
+		// Skip processing if this is a programmatic update to prevent unnecessary server restarts
+		if (this.isProgrammaticUpdate) {
+			return
+		}
+
 		const key = `${source}-${filePath}`
 
 		// Clear existing timer if any
@@ -1369,13 +1377,16 @@ export class McpHub {
 						this.removeFileWatchersForServer(serverName)
 						await this.deleteConnection(serverName, serverSource)
 						// Re-add as a disabled connection
-						await this.connectToServer(serverName, JSON.parse(connection.server.config), serverSource)
+						// Re-read config from file to get updated disabled state
+					const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
+					await this.connectToServer(serverName, updatedConfig, serverSource)
 					} else if (!disabled && connection.server.status === "disconnected") {
 						// If enabling a disabled server, connect it
-						const config = JSON.parse(connection.server.config)
+						// Re-read config from file to get updated disabled state
+						const updatedConfig = await this.readServerConfigFromFile(serverName, serverSource)
 						await this.deleteConnection(serverName, serverSource)
 						// When re-enabling, file watchers will be set up in connectToServer
-						await this.connectToServer(serverName, config, serverSource)
+						await this.connectToServer(serverName, updatedConfig, serverSource)
 					} else if (connection.server.status === "connected") {
 						// Only refresh capabilities if connected
 						connection.server.tools = await this.fetchToolsList(serverName, serverSource)
@@ -1397,6 +1408,57 @@ export class McpHub {
 		}
 	}
 
+	/**
+	 * Helper method to read a server's configuration from the appropriate settings file
+	 * @param serverName The name of the server to read
+	 * @param source Whether to read from the global or project config
+	 * @returns The validated server configuration
+	 */
+	private async readServerConfigFromFile(
+		serverName: string,
+		source: "global" | "project" = "global",
+	): Promise<z.infer<typeof ServerConfigSchema>> {
+		// Determine which config file to read
+		let configPath: string
+		if (source === "project") {
+			const projectMcpPath = await this.getProjectMcpPath()
+			if (!projectMcpPath) {
+				throw new Error("Project MCP configuration file not found")
+			}
+			configPath = projectMcpPath
+		} else {
+			configPath = await this.getMcpSettingsFilePath()
+		}
+
+		// Ensure the settings file exists and is accessible
+		try {
+			await fs.access(configPath)
+		} catch (error) {
+			console.error("Settings file not accessible:", error)
+			throw new Error("Settings file not accessible")
+		}
+
+		// Read and parse the config file
+		const content = await fs.readFile(configPath, "utf-8")
+		const config = JSON.parse(content)
+
+		// Validate the config structure
+		if (!config || typeof config !== "object") {
+			throw new Error("Invalid config structure")
+		}
+
+		if (!config.mcpServers || typeof config.mcpServers !== "object") {
+			throw new Error("No mcpServers section in config")
+		}
+
+		if (!config.mcpServers[serverName]) {
+			throw new Error(`Server ${serverName} not found in config`)
+		}
+
+		// Validate and return the server config
+		return this.validateServerConfig(config.mcpServers[serverName], serverName)
+	}
+
 	/**
 	 * Helper method to update a server's configuration in the appropriate settings file
 	 * @param serverName The name of the server to update
@@ -1463,7 +1525,20 @@ export class McpHub {
 			mcpServers: config.mcpServers,
 		}
 
-		await fs.writeFile(configPath, JSON.stringify(updatedConfig, null, 2))
+		// Set flag to prevent file watcher from triggering server restart
+		if (this.flagResetTimer) {
+			clearTimeout(this.flagResetTimer)
+		}
+		this.isProgrammaticUpdate = true
+		try {
+			await safeWriteJson(configPath, updatedConfig)
+		} finally {
+			// Reset flag after watcher debounce period (non-blocking)
+			this.flagResetTimer = setTimeout(() => {
+				this.isProgrammaticUpdate = false
+				this.flagResetTimer = undefined
+			}, 600)
+		}
 	}
 
 	public async updateServerTimeout(
@@ -1541,7 +1616,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 +1761,20 @@ export class McpHub {
 			targetList.splice(toolIndex, 1)
 		}
 
-		await fs.writeFile(normalizedPath, JSON.stringify(config, null, 2))
+		// Set flag to prevent file watcher from triggering server restart
+		if (this.flagResetTimer) {
+			clearTimeout(this.flagResetTimer)
+		}
+		this.isProgrammaticUpdate = true
+		try {
+			await safeWriteJson(normalizedPath, config)
+		} finally {
+			// Reset flag after watcher debounce period (non-blocking)
+			this.flagResetTimer = setTimeout(() => {
+				this.isProgrammaticUpdate = false
+				this.flagResetTimer = undefined
+			}, 600)
+		}
 
 		if (connection) {
 			connection.server.tools = await this.fetchToolsList(serverName, source)
@@ -1796,6 +1884,13 @@ export class McpHub {
 		}
 		this.configChangeDebounceTimers.clear()
 
+		// Clear flag reset timer and reset programmatic update flag
+		if (this.flagResetTimer) {
+			clearTimeout(this.flagResetTimer)
+			this.flagResetTimer = undefined
+		}
+		this.isProgrammaticUpdate = false
+
 		this.removeAllFileWatchers()
 		for (const connection of this.connections) {
 			try {