fix: restore MCP tool auto-approval behavior for issue #9214

src/core/auto-approval/__tests__/checkAutoApproval.spec.ts

@@ -0,0 +1,288 @@
+import { describe, it, expect } from "vitest"
+import { checkAutoApproval } from "../index"
+import type { ExtensionState } from "../../../shared/ExtensionMessage"
+import type { McpServer } from "../../../shared/mcp"
+
+describe("checkAutoApproval", () => {
+	describe("MCP tool auto-approval", () => {
+		it("should auto-approve MCP tools when alwaysAllowMcp is true and tool has no explicit alwaysAllow", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [
+					{
+						name: "test-server",
+						config: "test",
+						status: "connected",
+						tools: [
+							{
+								name: "test-tool",
+								description: "Test tool",
+								// No alwaysAllow property - should default to auto-approve
+							},
+						],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("approve")
+		})
+
+		it("should auto-approve MCP tools when alwaysAllowMcp is true and tool has alwaysAllow=true", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [
+					{
+						name: "test-server",
+						config: "test",
+						status: "connected",
+						tools: [
+							{
+								name: "test-tool",
+								description: "Test tool",
+								alwaysAllow: true,
+							},
+						],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("approve")
+		})
+
+		it("should NOT auto-approve MCP tools when tool explicitly has alwaysAllow=false", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [
+					{
+						name: "test-server",
+						config: "test",
+						status: "connected",
+						tools: [
+							{
+								name: "test-tool",
+								description: "Test tool",
+								alwaysAllow: false, // Explicitly disabled
+							},
+						],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("ask")
+		})
+
+		it("should NOT auto-approve MCP tools when alwaysAllowMcp is false", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: false, // Global MCP auto-approval disabled
+				mcpServers: [
+					{
+						name: "test-server",
+						config: "test",
+						status: "connected",
+						tools: [
+							{
+								name: "test-tool",
+								description: "Test tool",
+							},
+						],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("ask")
+		})
+
+		it("should NOT auto-approve when autoApprovalEnabled is false", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: false, // Auto-approval completely disabled
+				alwaysAllowMcp: true,
+				mcpServers: [],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("ask")
+		})
+
+		it("should auto-approve MCP resources when alwaysAllowMcp is true", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "access_mcp_resource",
+				serverName: "test-server",
+				resourceUri: "test://resource",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			expect(result.decision).toBe("approve")
+		})
+
+		it("should handle missing or unknown servers gracefully", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [
+					{
+						name: "different-server",
+						config: "test",
+						status: "connected",
+						tools: [],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "unknown-server",
+				toolName: "test-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			// Should still auto-approve since alwaysAllowMcp is true and tool doesn't exist to have alwaysAllow=false
+			expect(result.decision).toBe("approve")
+		})
+
+		it("should handle missing tools gracefully", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [
+					{
+						name: "test-server",
+						config: "test",
+						status: "connected",
+						tools: [
+							{
+								name: "different-tool",
+								description: "Different tool",
+							},
+						],
+					} as McpServer,
+				],
+			}
+
+			const mcpServerUse = JSON.stringify({
+				type: "use_mcp_tool",
+				serverName: "test-server",
+				toolName: "unknown-tool",
+			})
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: mcpServerUse,
+			})
+
+			// Should still auto-approve since alwaysAllowMcp is true and tool doesn't exist to have alwaysAllow=false
+			expect(result.decision).toBe("approve")
+		})
+
+		it("should handle invalid JSON gracefully", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [],
+			}
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: "invalid json",
+			})
+
+			expect(result.decision).toBe("ask")
+		})
+
+		it("should handle missing text gracefully", async () => {
+			const state: Partial<ExtensionState> = {
+				autoApprovalEnabled: true,
+				alwaysAllowMcp: true,
+				mcpServers: [],
+			}
+
+			const result = await checkAutoApproval({
+				state: state as any,
+				ask: "use_mcp_server",
+				text: undefined,
+			})
+
+			expect(result.decision).toBe("ask")
+		})
+	})
+})

src/core/auto-approval/index.ts

@@ -2,6 +2,7 @@ import { type ClineAsk, type McpServerUse, type FollowUpData, isNonBlockingAsk }
 
 import type { ClineSayTool, ExtensionState } from "../../shared/ExtensionMessage"
 import { ClineAskResponse } from "../../shared/WebviewMessage"
+import type { McpServer, McpTool } from "../../shared/mcp"
 
 import { isWriteToolAction, isReadOnlyToolAction } from "./tools"
 import { isMcpToolAlwaysAllowed } from "./mcp"
@@ -99,9 +100,18 @@ export async function checkAutoApproval({
 			const mcpServerUse = JSON.parse(text) as McpServerUse
 
 			if (mcpServerUse.type === "use_mcp_tool") {
-				return state.alwaysAllowMcp === true && isMcpToolAlwaysAllowed(mcpServerUse, state.mcpServers)
-					? { decision: "approve" }
-					: { decision: "ask" }
+				// Check if global MCP auto-approval is enabled
+				if (state.alwaysAllowMcp === true) {
+					// If the tool has explicit alwaysAllow set to false, respect that
+					const server = state.mcpServers?.find((s) => s.name === mcpServerUse.serverName)
+					const tool = server?.tools?.find((t) => t.name === mcpServerUse.toolName)
+					if (tool?.alwaysAllow === false) {
+						return { decision: "ask" }
+					}
+					// Otherwise auto-approve (including when alwaysAllow is true or undefined)
+					return { decision: "approve" }
+				}
+				return { decision: "ask" }
 			} else if (mcpServerUse.type === "access_mcp_resource") {
 				return state.alwaysAllowMcp === true ? { decision: "approve" } : { decision: "ask" }
 			}