fix: prevent MCP servers from restarting repeatedly on settings save

.changeset/fix-mcp-restart-loop.md

@@ -0,0 +1,5 @@
+---
+"kilo-code": patch
+---
+
+Fix MCP servers restarting repeatedly when saving settings

src/services/mcp/McpHub.ts

@@ -47,13 +47,15 @@ import { McpOAuthService, OAuthTokens } from "./oauth"
 // Discriminated union for connection states
 export type ConnectedMcpConnection = {
 	type: "connected"
+	rawConfig: string // kilocode_change: pre-injection validated config for accurate deep-equal comparison
 	server: McpServer
 	client: Client
 	transport: StdioClientTransport | SSEClientTransport | StreamableHTTPClientTransport
 }
 
 export type DisconnectedMcpConnection = {
 	type: "disconnected"
+	rawConfig: string // kilocode_change: pre-injection validated config for accurate deep-equal comparison
 	server: McpServer
 	client: null
 	transport: null
@@ -188,6 +190,9 @@ export class McpHub {
 	// kilocode_change start - MCP OAuth Authorization
 	private oauthService?: McpOAuthService
 	// kilocode_change end
+	// kilocode_change start - Intentional disconnect guard for auto-reconnect
+	private intentionalDisconnects: Set<string> = new Set()
+	// kilocode_change end
 	// kilocode_change start - Auto-reconnect on disconnect
 	private reconnectAttempts: Map<string, number> = new Map()
 	private reconnectTimers: Map<string, NodeJS.Timeout> = new Map()
@@ -587,6 +592,19 @@ export class McpHub {
 		this.reconnectTimers.set(key, timer)
 	}
 
+	/**
+	 * Attempts to schedule a reconnect only if the disconnect was not intentional.
+	 * This prevents auto-reconnect from firing when we programmatically close connections.
+	 * @param serverName The name of the server
+	 * @param source The server source (global or project)
+	 */
+	private scheduleReconnectIfNotIntentional(serverName: string, source: "global" | "project"): void {
+		const intentionalKey = `${source}-${serverName}`
+		if (!this.intentionalDisconnects.has(intentionalKey)) {
+			this.scheduleReconnect(serverName, source)
+		}
+	}
+
 	/**
 	 * Cancels any scheduled reconnect for a server.
 	 * @param serverName The name of the server
@@ -747,6 +765,11 @@ export class McpHub {
 		// Set new timer
 		const timer = setTimeout(async () => {
 			this.configChangeDebounceTimers.delete(key)
+			// kilocode_change: Re-check flag inside callback - it may have been set
+			// after the initial check but before the debounce timer fires
+			if (this.isProgrammaticUpdate) {
+				return
+			}
 			await this.handleConfigFileChange(filePath, source)
 		}, 500) // 500ms debounce
 
@@ -1088,6 +1111,7 @@ export class McpHub {
 	): DisconnectedMcpConnection {
 		return {
 			type: "disconnected",
+			rawConfig: JSON.stringify(config), // kilocode_change: store pre-injection config
 			server: {
 				name,
 				config: JSON.stringify(config),
@@ -1203,8 +1227,8 @@ export class McpHub {
 						this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on error
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on error (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 
 				transport.onclose = async () => {
@@ -1213,8 +1237,8 @@ export class McpHub {
 						connection.server.status = "disconnected"
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on close
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on close (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 
 				// transport.stderr is only available after the process has been started. However we can't start it separately from the .connect() call because it also starts the transport. And we can't place this after the connect call since we need to capture the stderr stream before the connection is established, in order to capture errors during the connection process.
@@ -1274,8 +1298,8 @@ export class McpHub {
 						this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on error
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on error (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 
 				transport.onclose = async () => {
@@ -1284,8 +1308,8 @@ export class McpHub {
 						connection.server.status = "disconnected"
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on close
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on close (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 			} else if (configInjected.type === "sse") {
 				// SSE connection
@@ -1336,8 +1360,8 @@ export class McpHub {
 						this.appendErrorMessage(connection, error instanceof Error ? error.message : `${error}`)
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on error
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on error (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 
 				transport.onclose = async () => {
@@ -1346,8 +1370,8 @@ export class McpHub {
 						connection.server.status = "disconnected"
 					}
 					await this.notifyWebviewOfServerChanges()
-					// kilocode_change - Schedule auto-reconnect on close
-					this.scheduleReconnect(name, source)
+					// kilocode_change - Schedule auto-reconnect on close (skip if intentional disconnect)
+					this.scheduleReconnectIfNotIntentional(name, source)
 				}
 			} else {
 				// Should not happen if validateServerConfig is correct
@@ -1396,6 +1420,7 @@ export class McpHub {
 			// Create a connected connection
 			const connection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: JSON.stringify(config), // kilocode_change: store pre-injection config for accurate comparison
 				server: {
 					name,
 					config: JSON.stringify(configInjected),
@@ -1420,6 +1445,9 @@ export class McpHub {
 			connection.server.instructions = client.getInstructions()
 			// kilocode_change - Reset reconnect attempts on successful connection
 			this.resetReconnectAttempts(name, source)
+			// kilocode_change - Clear intentional disconnect flag on successful connection
+			const intentionalKey = `${source}-${name}`
+			this.intentionalDisconnects.delete(intentionalKey)
 
 			this.kiloNotificationService.connect(name, connection.client)
 
@@ -1692,6 +1720,10 @@ export class McpHub {
 		for (const connection of connections) {
 			try {
 				if (connection.type === "connected") {
+					// kilocode_change: Mark as intentional disconnect to prevent onclose from scheduling auto-reconnect
+					const connSource = connection.server.source || "global"
+					const intentionalKey = `${connSource}-${name}`
+					this.intentionalDisconnects.add(intentionalKey)
 					// kilocode_change start
 					// Fire-and-forget: don't await close() calls as they can block
 					// waiting for the subprocess to exit. The MCP SDK's transport.close()
@@ -1774,7 +1806,8 @@ export class McpHub {
 				} catch (error) {
 					this.showErrorMessage(`Failed to connect to new MCP server ${name}`, error)
 				}
-			} else if (!deepEqual(JSON.parse(currentConnection.server.config), config)) {
+			} else if (!deepEqual(JSON.parse(currentConnection.rawConfig), validatedConfig)) {
+				// kilocode_change: compare raw (pre-injection) configs
 				// Existing server with changed config
 				try {
 					// Only setup file watcher for enabled servers
@@ -2589,6 +2622,7 @@ export class McpHub {
 		}
 		this.reconnectTimers.clear()
 		this.reconnectAttempts.clear()
+		this.intentionalDisconnects.clear()
 		// kilocode_change end
 
 		this.removeAllFileWatchers()

src/services/mcp/__tests__/McpHub.spec.ts

@@ -307,6 +307,7 @@ describe("McpHub", () => {
 			// Directly set up a connected connection
 			const connectedConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					config: JSON.stringify({ command: "node", args: ["test.js"] }),
@@ -331,6 +332,7 @@ describe("McpHub", () => {
 			// Now test with a disconnected connection
 			const disconnectedConnection: DisconnectedMcpConnection = {
 				type: "disconnected",
+				rawConfig: "{}",
 				server: {
 					name: "disabled-server",
 					config: JSON.stringify({ command: "node", args: ["test.js"], disabled: true }),
@@ -797,6 +799,7 @@ describe("McpHub", () => {
 			// Set up mock connection without alwaysAllow
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					type: "stdio",
@@ -846,6 +849,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					type: "stdio",
@@ -895,6 +899,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					type: "stdio",
@@ -941,6 +946,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					config: "test-server-config",
@@ -989,6 +995,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					config: "test-server-config",
@@ -1036,6 +1043,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					config: "test-server-config",
@@ -1087,6 +1095,7 @@ describe("McpHub", () => {
 			// Set up mock connection
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					type: "stdio",
@@ -1119,6 +1128,7 @@ describe("McpHub", () => {
 			const mockConnections: McpConnection[] = [
 				{
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "enabled-server",
 						config: "{}",
@@ -1130,6 +1140,7 @@ describe("McpHub", () => {
 				} as ConnectedMcpConnection,
 				{
 					type: "disconnected",
+					rawConfig: "{}",
 					server: {
 						name: "disabled-server",
 						config: "{}",
@@ -1152,6 +1163,7 @@ describe("McpHub", () => {
 			const mockConnections: McpConnection[] = [
 				{
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "shared-server",
 						config: '{"source":"global"}',
@@ -1164,6 +1176,7 @@ describe("McpHub", () => {
 				} as ConnectedMcpConnection,
 				{
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "shared-server",
 						config: '{"source":"project"}',
@@ -1176,6 +1189,7 @@ describe("McpHub", () => {
 				} as ConnectedMcpConnection,
 				{
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "unique-global-server",
 						config: "{}",
@@ -1209,6 +1223,7 @@ describe("McpHub", () => {
 			const mockConnections: McpConnection[] = [
 				{
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "global-only-server",
 						config: "{}",
@@ -1297,6 +1312,7 @@ describe("McpHub", () => {
 			// Mock the connection with a minimal client implementation
 			const mockConnection: ConnectedMcpConnection = {
 				type: "connected",
+				rawConfig: "{}",
 				server: {
 					name: "test-server",
 					config: JSON.stringify({}),
@@ -1361,6 +1377,7 @@ describe("McpHub", () => {
 			it("should use default timeout of 60 seconds if not specified", async () => {
 				const mockConnection: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						config: JSON.stringify({ type: "stdio", command: "test" }), // No timeout specified
@@ -1385,6 +1402,7 @@ describe("McpHub", () => {
 			it("should apply configured timeout to tool calls", async () => {
 				const mockConnection: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						config: JSON.stringify({ type: "stdio", command: "test", timeout: 120 }), // 2 minutes
@@ -1426,6 +1444,7 @@ describe("McpHub", () => {
 				// Set up mock connection
 				const mockConnection: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						type: "stdio",
@@ -1472,6 +1491,7 @@ describe("McpHub", () => {
 				// Set up mock connection before updating
 				const mockConnectionInitial: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						type: "stdio",
@@ -1496,6 +1516,7 @@ describe("McpHub", () => {
 				// Setup connection with invalid timeout
 				const mockConnectionInvalid: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						config: JSON.stringify({
@@ -1542,6 +1563,7 @@ describe("McpHub", () => {
 				// Set up mock connection
 				const mockConnection: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						type: "stdio",
@@ -1582,6 +1604,7 @@ describe("McpHub", () => {
 				// Set up mock connection
 				const mockConnection: ConnectedMcpConnection = {
 					type: "connected",
+					rawConfig: "{}",
 					server: {
 						name: "test-server",
 						type: "stdio",