fix: prevent unnecessary MCP server refresh on settings save (#6772) (#6779)

2026-08-28 05:27:24 +00:00 · 2025-08-06 18:37:51 -06:00 · 2025-08-06 18:37:51 -06:00 · 8d05bc179b
commit 8d05bc179b
parent c99ccf0bb0
2 changed files with 159 additions and 4 deletions
--- a/src/core/webview/tests/webviewMessageHandler.spec.ts
+++ b/src/core/webview/tests/webviewMessageHandler.spec.ts
@ -35,6 +35,7 @@ const mockClineProvider = {
 	getCurrentCline: vi.fn(),
 	getTaskWithId: vi.fn(),
 	initClineWithHistoryItem: vi.fn(),
+	getMcpHub: vi.fn(),
 } as unknown as ClineProvider

 import { t } from "../../../i18n"
@ -576,3 +577,150 @@ describe("webviewMessageHandler - message dialog preferences", () => {
 		})
 	})
 })
+
+describe("webviewMessageHandler - mcpEnabled", () => {
+	let mockMcpHub: any
+
+	beforeEach(() => {
+		vi.clearAllMocks()
+
+		// Create a mock McpHub instance
+		mockMcpHub = {
+			handleMcpEnabledChange: vi.fn().mockResolvedValue(undefined),
+		}
+
+		// Mock the getMcpHub method to return our mock McpHub
+		mockClineProvider.getMcpHub = vi.fn().mockReturnValue(mockMcpHub)
+
+		// Reset the contextProxy getValue mock
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined)
+	})
+
+	it("should not refresh MCP servers when value does not change (true to true)", async () => {
+		// Setup: mcpEnabled is already true
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(true)
+
+		// Act: Send mcpEnabled message with same value
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: true,
+		})
+
+		// Assert: handleMcpEnabledChange should not be called
+		expect(mockMcpHub.handleMcpEnabledChange).not.toHaveBeenCalled()
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", true)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should not refresh MCP servers when value does not change (false to false)", async () => {
+		// Setup: mcpEnabled is already false
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(false)
+
+		// Act: Send mcpEnabled message with same value
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: false,
+		})
+
+		// Assert: handleMcpEnabledChange should not be called
+		expect(mockMcpHub.handleMcpEnabledChange).not.toHaveBeenCalled()
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", false)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should refresh MCP servers when value changes from true to false", async () => {
+		// Setup: mcpEnabled is true
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(true)
+
+		// Act: Send mcpEnabled message with false
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: false,
+		})
+
+		// Assert: handleMcpEnabledChange should be called
+		expect(mockMcpHub.handleMcpEnabledChange).toHaveBeenCalledWith(false)
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", false)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should refresh MCP servers when value changes from false to true", async () => {
+		// Setup: mcpEnabled is false
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(false)
+
+		// Act: Send mcpEnabled message with true
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: true,
+		})
+
+		// Assert: handleMcpEnabledChange should be called
+		expect(mockMcpHub.handleMcpEnabledChange).toHaveBeenCalledWith(true)
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", true)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should handle undefined values with defaults correctly", async () => {
+		// Setup: mcpEnabled is undefined (defaults to true)
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined)
+
+		// Act: Send mcpEnabled message with undefined (defaults to true)
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: undefined,
+		})
+
+		// Assert: Should use default value (true) and not trigger refresh since both are true
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", true)
+		expect(mockMcpHub.handleMcpEnabledChange).not.toHaveBeenCalled()
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should handle when mcpEnabled changes from undefined to false", async () => {
+		// Setup: mcpEnabled is undefined (defaults to true)
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(undefined)
+
+		// Act: Send mcpEnabled message with false
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: false,
+		})
+
+		// Assert: Should trigger refresh since undefined defaults to true and we're changing to false
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", false)
+		expect(mockMcpHub.handleMcpEnabledChange).toHaveBeenCalledWith(false)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+
+	it("should not call handleMcpEnabledChange when McpHub is not available", async () => {
+		// Setup: No McpHub instance available
+		mockClineProvider.getMcpHub = vi.fn().mockReturnValue(null)
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(true)
+
+		// Act: Send mcpEnabled message with false
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: false,
+		})
+
+		// Assert: State should be updated but handleMcpEnabledChange should not be called
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", false)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+		// No error should be thrown
+	})
+
+	it("should always update state even when value doesn't change", async () => {
+		// Setup: mcpEnabled is true
+		vi.mocked(mockClineProvider.contextProxy.getValue).mockReturnValue(true)
+
+		// Act: Send mcpEnabled message with same value
+		await webviewMessageHandler(mockClineProvider, {
+			type: "mcpEnabled",
+			bool: true,
+		})
+
+		// Assert: State should still be updated to ensure consistency
+		expect(mockClineProvider.contextProxy.setValue).toHaveBeenCalledWith("mcpEnabled", true)
+		expect(mockClineProvider.postStateToWebview).toHaveBeenCalled()
+	})
+})
--- a/src/core/webview/webviewMessageHandler.ts
+++ b/src/core/webview/webviewMessageHandler.ts
@ -900,12 +900,19 @@ export const webviewMessageHandler = async (
 		}
 		case "mcpEnabled":
 			const mcpEnabled = message.bool ?? true
+			const currentMcpEnabled = getGlobalState("mcpEnabled") ?? true
+
+			// Always update the state to ensure consistency
 			await updateGlobalState("mcpEnabled", mcpEnabled)

-			// Delegate MCP enable/disable logic to McpHub
-			const mcpHubInstance = provider.getMcpHub()
-			if (mcpHubInstance) {
-				await mcpHubInstance.handleMcpEnabledChange(mcpEnabled)
+			// Only refresh MCP connections if the value actually changed
+			// This prevents expensive MCP server refresh operations when saving unrelated settings
+			if (currentMcpEnabled !== mcpEnabled) {
+				// Delegate MCP enable/disable logic to McpHub
+				const mcpHubInstance = provider.getMcpHub()
+				if (mcpHubInstance) {
+					await mcpHubInstance.handleMcpEnabledChange(mcpEnabled)
+				}
 			}

 			await provider.postStateToWebview()