Skip to content

Commit 04d29fa

Browse files
committed
fix: prevent MCP server restart when toggling tool auto-approve
- Added configsEqualExcludingToolSettings helper to compare configs without alwaysAllow and disabledTools - Modified updateServerConnections to use smart comparison that only restarts servers when essential config changes - When only tool settings change, update config and refresh tools list without restarting the server - Fixes #7189 where MCP connections were closing when clicking auto-approve checkbox
1 parent 241df17 commit 04d29fa

File tree

1 file changed

+47
-10
lines changed

1 file changed

+47
-10
lines changed

src/services/mcp/McpHub.ts

Lines changed: 47 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1023,6 +1023,28 @@ export class McpHub {
10231023
})
10241024
}
10251025

1026+
/**
1027+
* Compares two server configurations to determine if a restart is needed.
1028+
* Excludes alwaysAllow and disabledTools from comparison as these don't require restart.
1029+
* @param config1 First configuration to compare
1030+
* @param config2 Second configuration to compare
1031+
* @returns true if configurations are equal (no restart needed), false otherwise
1032+
*/
1033+
private configsEqualExcludingToolSettings(config1: any, config2: any): boolean {
1034+
// Create copies of the configs without alwaysAllow and disabledTools
1035+
const config1Copy = { ...config1 }
1036+
const config2Copy = { ...config2 }
1037+
1038+
// Remove fields that don't require server restart
1039+
delete config1Copy.alwaysAllow
1040+
delete config1Copy.disabledTools
1041+
delete config2Copy.alwaysAllow
1042+
delete config2Copy.disabledTools
1043+
1044+
// Compare the remaining fields
1045+
return deepEqual(config1Copy, config2Copy)
1046+
}
1047+
10261048
async updateServerConnections(
10271049
newServers: Record<string, any>,
10281050
source: "global" | "project" = "global",
@@ -1071,17 +1093,32 @@ export class McpHub {
10711093
} catch (error) {
10721094
this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
10731095
}
1074-
} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
1075-
// Existing server with changed config
1076-
try {
1077-
// Only setup file watcher for enabled servers
1078-
if (!validatedConfig.disabled) {
1079-
this.setupFileWatcher(name, validatedConfig, source)
1096+
} else {
1097+
// Use smart comparison that excludes alwaysAllow and disabledTools
1098+
const currentConfig = JSON.parse(currentConnection.server.config)
1099+
const needsRestart = !this.configsEqualExcludingToolSettings(currentConfig, config)
1100+
1101+
if (needsRestart) {
1102+
// Existing server with changed config that requires restart
1103+
try {
1104+
// Only setup file watcher for enabled servers
1105+
if (!validatedConfig.disabled) {
1106+
this.setupFileWatcher(name, validatedConfig, source)
1107+
}
1108+
await this.deleteConnection(name, source)
1109+
await this.connectToServer(name, validatedConfig, source)
1110+
} catch (error) {
1111+
this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error)
1112+
}
1113+
} else {
1114+
// Only tool settings changed, update without restart
1115+
// Update the stored config to reflect the new alwaysAllow/disabledTools
1116+
currentConnection.server.config = JSON.stringify(config)
1117+
1118+
// Refresh the tools list to reflect the new settings
1119+
if (currentConnection.server.status === "connected") {
1120+
currentConnection.server.tools = await this.fetchToolsList(name, source)
10801121
}
1081-
await this.deleteConnection(name, source)
1082-
await this.connectToServer(name, validatedConfig, source)
1083-
} catch (error) {
1084-
this.showErrorMessage(`Failed to reconnect MCP server ${name}`, error)
10851122
}
10861123
}
10871124
// If server exists with same config, do nothing

0 commit comments

Comments
 (0)