From 76a3fdc256adb0ff3a56d0f650c7d0b0c97cad5a Mon Sep 17 00:00:00 2001
From: Matt Rubens <mrubens@users.noreply.github.com>
Date: Thu, 10 Jul 2025 14:36:09 -0400
Subject: [PATCH] Clean up MCP tool disabling (#5576)

---
 webview-ui/src/components/mcp/McpToolRow.tsx  |  58 ++++----
 webview-ui/src/components/mcp/McpView.tsx     |  64 +++------
 .../mcp/__tests__/McpToolRow.spec.tsx         | 130 +++++++++++++++---
 .../ui/__tests__/toggle-switch.spec.tsx       | 101 ++++++++++++++
 webview-ui/src/components/ui/index.ts         |   1 +
 .../src/components/ui/toggle-switch.tsx       |  68 +++++++++
 6 files changed, 334 insertions(+), 88 deletions(-)
 create mode 100644 webview-ui/src/components/ui/__tests__/toggle-switch.spec.tsx
 create mode 100644 webview-ui/src/components/ui/toggle-switch.tsx

diff --git a/webview-ui/src/components/mcp/McpToolRow.tsx b/webview-ui/src/components/mcp/McpToolRow.tsx
index 58b938f9f1..aa57b18fd9 100644
--- a/webview-ui/src/components/mcp/McpToolRow.tsx
+++ b/webview-ui/src/components/mcp/McpToolRow.tsx
@@ -4,7 +4,7 @@ import { McpTool } from "@roo/mcp"
 
 import { useAppTranslation } from "@src/i18n/TranslationContext"
 import { vscode } from "@src/utils/vscode"
-import { StandardTooltip } from "@/components/ui"
+import { StandardTooltip, ToggleSwitch } from "@/components/ui"
 
 type McpToolRowProps = {
 	tool: McpTool
@@ -16,6 +16,8 @@ type McpToolRowProps = {
 
 const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatContext = false }: McpToolRowProps) => {
 	const { t } = useAppTranslation()
+	const isToolEnabled = tool.enabledForPrompt ?? true
+
 	const handleAlwaysAllowChange = () => {
 		if (!serverName) return
 		vscode.postMessage({
@@ -46,17 +48,29 @@ const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatCo
 				onClick={(e) => e.stopPropagation()}>
 				{/* Tool name section */}
 				<div className="flex items-center min-w-0 flex-1">
-					<span className="codicon codicon-symbol-method mr-2 flex-shrink-0 text-vscode-symbolIcon-methodForeground"></span>
+					<span
+						className={`codicon codicon-symbol-method mr-2 flex-shrink-0 ${
+							isToolEnabled
+								? "text-vscode-symbolIcon-methodForeground"
+								: "text-vscode-descriptionForeground opacity-60"
+						}`}></span>
 					<StandardTooltip content={tool.name}>
-						<span className="font-medium truncate text-vscode-foreground">{tool.name}</span>
+						<span
+							className={`font-medium truncate ${
+								isToolEnabled
+									? "text-vscode-foreground"
+									: "text-vscode-descriptionForeground opacity-60"
+							}`}>
+							{tool.name}
+						</span>
 					</StandardTooltip>
 				</div>
 
 				{/* Controls section */}
 				{serverName && (
 					<div className="flex items-center gap-4 flex-shrink-0">
-						{/* Always Allow checkbox */}
-						{alwaysAllowMcp && (
+						{/* Always Allow checkbox - only show when tool is enabled */}
+						{alwaysAllowMcp && isToolEnabled && (
 							<VSCodeCheckbox
 								checked={tool.alwaysAllow}
 								onChange={handleAlwaysAllowChange}
@@ -68,35 +82,31 @@ const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatCo
 							</VSCodeCheckbox>
 						)}
 
-						{/* Enabled eye button - only show in settings context */}
+						{/* Enabled toggle switch - only show in settings context */}
 						{!isInChatContext && (
 							<StandardTooltip content={t("mcp:tool.togglePromptInclusion")}>
-								<button
-									role="button"
-									aria-pressed={tool.enabledForPrompt}
+								<ToggleSwitch
+									checked={isToolEnabled}
+									onChange={handleEnabledForPromptChange}
+									size="medium"
 									aria-label={t("mcp:tool.togglePromptInclusion")}
-									className={`p-1 rounded hover:bg-vscode-toolbar-hoverBackground transition-colors ${
-										tool.enabledForPrompt
-											? "text-vscode-foreground"
-											: "text-vscode-descriptionForeground opacity-60"
-									}`}
-									onClick={handleEnabledForPromptChange}
-									data-tool-prompt-toggle={tool.name}>
-									<span
-										className={`codicon ${
-											tool.enabledForPrompt ? "codicon-eye-closed" : "codicon-eye"
-										} text-base`}
-									/>
-								</button>
+									data-testid={`tool-prompt-toggle-${tool.name}`}
+								/>
 							</StandardTooltip>
 						)}
 					</div>
 				)}
 			</div>
 			{tool.description && (
-				<div className="mt-1 text-xs text-vscode-descriptionForeground opacity-80">{tool.description}</div>
+				<div
+					className={`mt-1 text-xs text-vscode-descriptionForeground ${
+						isToolEnabled ? "opacity-80" : "opacity-40"
+					}`}>
+					{tool.description}
+				</div>
 			)}
-			{tool.inputSchema &&
+			{isToolEnabled &&
+				tool.inputSchema &&
 				"properties" in tool.inputSchema &&
 				Object.keys(tool.inputSchema.properties as Record<string, any>).length > 0 && (
 					<div className="mt-2 text-xs border border-vscode-panel-border rounded p-2">
diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx
index 2c83b432cc..b95ed2608c 100644
--- a/webview-ui/src/components/mcp/McpView.tsx
+++ b/webview-ui/src/components/mcp/McpView.tsx
@@ -22,6 +22,7 @@ import {
 	DialogTitle,
 	DialogDescription,
 	DialogFooter,
+	ToggleSwitch,
 } from "@src/components/ui"
 import { buildDocLink } from "@src/utils/docLinks"
 
@@ -295,54 +296,6 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
 						style={{ marginRight: "8px" }}>
 						<span className="codicon codicon-refresh" style={{ fontSize: "14px" }}></span>
 					</Button>
-					<div
-						role="switch"
-						aria-checked={!server.disabled}
-						tabIndex={0}
-						style={{
-							width: "20px",
-							height: "10px",
-							backgroundColor: server.disabled
-								? "var(--vscode-titleBar-inactiveForeground)"
-								: "var(--vscode-button-background)",
-							borderRadius: "5px",
-							position: "relative",
-							cursor: "pointer",
-							transition: "background-color 0.2s",
-							opacity: server.disabled ? 0.4 : 0.8,
-						}}
-						onClick={() => {
-							vscode.postMessage({
-								type: "toggleMcpServer",
-								serverName: server.name,
-								source: server.source || "global",
-								disabled: !server.disabled,
-							})
-						}}
-						onKeyDown={(e) => {
-							if (e.key === "Enter" || e.key === " ") {
-								e.preventDefault()
-								vscode.postMessage({
-									type: "toggleMcpServer",
-									serverName: server.name,
-									source: server.source || "global",
-									disabled: !server.disabled,
-								})
-							}
-						}}>
-						<div
-							style={{
-								width: "6px",
-								height: "6px",
-								backgroundColor: "var(--vscode-titleBar-activeForeground)",
-								borderRadius: "50%",
-								position: "absolute",
-								top: "2px",
-								left: server.disabled ? "2px" : "12px",
-								transition: "left 0.2s",
-							}}
-						/>
-					</div>
 				</div>
 				<div
 					style={{
@@ -353,6 +306,21 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
 						marginLeft: "8px",
 					}}
 				/>
+				<div style={{ marginLeft: "8px" }}>
+					<ToggleSwitch
+						checked={!server.disabled}
+						onChange={() => {
+							vscode.postMessage({
+								type: "toggleMcpServer",
+								serverName: server.name,
+								source: server.source || "global",
+								disabled: !server.disabled,
+							})
+						}}
+						size="medium"
+						aria-label={`Toggle ${server.name} server`}
+					/>
+				</div>
 			</div>
 
 			{server.status === "connected" ? (
diff --git a/webview-ui/src/components/mcp/__tests__/McpToolRow.spec.tsx b/webview-ui/src/components/mcp/__tests__/McpToolRow.spec.tsx
index 2bfe3b1338..f686a23f00 100644
--- a/webview-ui/src/components/mcp/__tests__/McpToolRow.spec.tsx
+++ b/webview-ui/src/components/mcp/__tests__/McpToolRow.spec.tsx
@@ -144,40 +144,40 @@ describe("McpToolRow", () => {
 		expect(screen.getByText("Second parameter")).toBeInTheDocument()
 	})
 
-	it("shows eye button when serverName is provided and not in chat context", () => {
+	it("shows toggle switch when serverName is provided and not in chat context", () => {
 		render(<McpToolRow tool={mockTool} serverName="test-server" />)
 
-		const eyeButton = screen.getByRole("button", { name: "Toggle prompt inclusion" })
-		expect(eyeButton).toBeInTheDocument()
+		const toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
+		expect(toggleSwitch).toBeInTheDocument()
 	})
 
-	it("hides eye button when isInChatContext is true", () => {
+	it("hides toggle switch when isInChatContext is true", () => {
 		render(<McpToolRow tool={mockTool} serverName="test-server" isInChatContext={true} />)
 
-		const eyeButton = screen.queryByRole("button", { name: "Toggle prompt inclusion" })
-		expect(eyeButton).not.toBeInTheDocument()
+		const toggleSwitch = screen.queryByRole("switch", { name: "Toggle prompt inclusion" })
+		expect(toggleSwitch).not.toBeInTheDocument()
 	})
 
-	it("shows correct eye icon based on enabledForPrompt state", () => {
-		// Test when enabled (should show eye-closed icon)
+	it("shows correct toggle switch state based on enabledForPrompt", () => {
+		// Test when enabled (should be checked)
 		const { rerender } = render(<McpToolRow tool={mockTool} serverName="test-server" />)
 
-		let eyeIcon = screen.getByRole("button", { name: "Toggle prompt inclusion" }).querySelector("span")
-		expect(eyeIcon).toHaveClass("codicon-eye-closed")
+		let toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
+		expect(toggleSwitch).toHaveAttribute("aria-checked", "true")
 
-		// Test when disabled (should show eye icon)
+		// Test when disabled (should not be checked)
 		const disabledTool = { ...mockTool, enabledForPrompt: false }
 		rerender(<McpToolRow tool={disabledTool} serverName="test-server" />)
 
-		eyeIcon = screen.getByRole("button", { name: "Toggle prompt inclusion" }).querySelector("span")
-		expect(eyeIcon).toHaveClass("codicon-eye")
+		toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
+		expect(toggleSwitch).toHaveAttribute("aria-checked", "false")
 	})
 
-	it("sends message to toggle enabledForPrompt when eye button is clicked", () => {
+	it("sends message to toggle enabledForPrompt when toggle switch is clicked", () => {
 		render(<McpToolRow tool={mockTool} serverName="test-server" />)
 
-		const eyeButton = screen.getByRole("button", { name: "Toggle prompt inclusion" })
-		fireEvent.click(eyeButton)
+		const toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
+		fireEvent.click(toggleSwitch)
 
 		expect(vscode.postMessage).toHaveBeenCalledWith({
 			type: "toggleToolEnabledForPrompt",
@@ -187,4 +187,102 @@ describe("McpToolRow", () => {
 			isEnabled: false,
 		})
 	})
+
+	it("hides always allow checkbox when tool is disabled", () => {
+		const disabledTool = { ...mockTool, enabledForPrompt: false }
+		render(<McpToolRow tool={disabledTool} serverName="test-server" alwaysAllowMcp={true} />)
+
+		expect(screen.queryByText("Always allow")).not.toBeInTheDocument()
+	})
+
+	it("shows always allow checkbox when tool is enabled", () => {
+		const enabledTool = { ...mockTool, enabledForPrompt: true }
+		render(<McpToolRow tool={enabledTool} serverName="test-server" alwaysAllowMcp={true} />)
+
+		expect(screen.getByText("Always allow")).toBeInTheDocument()
+	})
+
+	it("hides parameters section when tool is disabled", () => {
+		const disabledToolWithSchema = {
+			...mockTool,
+			enabledForPrompt: false,
+			inputSchema: {
+				type: "object",
+				properties: {
+					param1: {
+						type: "string",
+						description: "First parameter",
+					},
+				},
+				required: ["param1"],
+			},
+		}
+
+		render(<McpToolRow tool={disabledToolWithSchema} serverName="test-server" />)
+
+		expect(screen.queryByText("Parameters")).not.toBeInTheDocument()
+		expect(screen.queryByText("param1")).not.toBeInTheDocument()
+		expect(screen.queryByText("First parameter")).not.toBeInTheDocument()
+	})
+
+	it("shows parameters section when tool is enabled", () => {
+		const enabledToolWithSchema = {
+			...mockTool,
+			enabledForPrompt: true,
+			inputSchema: {
+				type: "object",
+				properties: {
+					param1: {
+						type: "string",
+						description: "First parameter",
+					},
+				},
+				required: ["param1"],
+			},
+		}
+
+		render(<McpToolRow tool={enabledToolWithSchema} serverName="test-server" />)
+
+		expect(screen.getByText("Parameters")).toBeInTheDocument()
+		expect(screen.getByText("param1")).toBeInTheDocument()
+		expect(screen.getByText("First parameter")).toBeInTheDocument()
+	})
+
+	it("grays out tool name and description when tool is disabled", () => {
+		const disabledTool = {
+			...mockTool,
+			enabledForPrompt: false,
+			description: "A disabled tool",
+		}
+		render(<McpToolRow tool={disabledTool} serverName="test-server" />)
+
+		const toolName = screen.getByText("test-tool")
+		const toolDescription = screen.getByText("A disabled tool")
+
+		// Check that the tool name has the grayed out classes
+		expect(toolName).toHaveClass("text-vscode-descriptionForeground", "opacity-60")
+
+		// Check that the description has reduced opacity
+		expect(toolDescription).toHaveClass("opacity-40")
+	})
+
+	it("shows normal styling for tool name and description when tool is enabled", () => {
+		const enabledTool = {
+			...mockTool,
+			enabledForPrompt: true,
+			description: "An enabled tool",
+		}
+		render(<McpToolRow tool={enabledTool} serverName="test-server" />)
+
+		const toolName = screen.getByText("test-tool")
+		const toolDescription = screen.getByText("An enabled tool")
+
+		// Check that the tool name has normal styling
+		expect(toolName).toHaveClass("text-vscode-foreground")
+		expect(toolName).not.toHaveClass("text-vscode-descriptionForeground", "opacity-60")
+
+		// Check that the description has normal opacity
+		expect(toolDescription).toHaveClass("opacity-80")
+		expect(toolDescription).not.toHaveClass("opacity-40")
+	})
 })
diff --git a/webview-ui/src/components/ui/__tests__/toggle-switch.spec.tsx b/webview-ui/src/components/ui/__tests__/toggle-switch.spec.tsx
new file mode 100644
index 0000000000..e394e76139
--- /dev/null
+++ b/webview-ui/src/components/ui/__tests__/toggle-switch.spec.tsx
@@ -0,0 +1,101 @@
+import React from "react"
+import { render, fireEvent, screen } from "@/utils/test-utils"
+
+import { ToggleSwitch } from "../toggle-switch"
+
+describe("ToggleSwitch", () => {
+	it("renders with correct initial state", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={true} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		expect(toggle).toBeInTheDocument()
+		expect(toggle).toHaveAttribute("aria-checked", "true")
+		expect(toggle).toHaveAttribute("aria-label", "Test toggle")
+	})
+
+	it("renders unchecked state correctly", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		expect(toggle).toHaveAttribute("aria-checked", "false")
+	})
+
+	it("calls onChange when clicked", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		fireEvent.click(toggle)
+
+		expect(onChange).toHaveBeenCalledTimes(1)
+	})
+
+	it("calls onChange when Enter key is pressed", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		fireEvent.keyDown(toggle, { key: "Enter" })
+
+		expect(onChange).toHaveBeenCalledTimes(1)
+	})
+
+	it("calls onChange when Space key is pressed", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		fireEvent.keyDown(toggle, { key: " " })
+
+		expect(onChange).toHaveBeenCalledTimes(1)
+	})
+
+	it("does not call onChange when disabled", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} disabled={true} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		fireEvent.click(toggle)
+		fireEvent.keyDown(toggle, { key: "Enter" })
+
+		expect(onChange).not.toHaveBeenCalled()
+	})
+
+	it("has correct tabIndex when disabled", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} disabled={true} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		expect(toggle).toHaveAttribute("tabindex", "-1")
+	})
+
+	it("renders with custom data-testid", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} data-testid="custom-toggle" />)
+
+		const toggle = screen.getByTestId("custom-toggle")
+		expect(toggle).toBeInTheDocument()
+	})
+
+	it("supports medium size", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} size="medium" aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		expect(toggle).toBeInTheDocument()
+		// Medium size should be 20px x 10px
+		expect(toggle).toHaveStyle({ width: "20px", height: "10px" })
+	})
+
+	it("defaults to small size", () => {
+		const onChange = vi.fn()
+		render(<ToggleSwitch checked={false} onChange={onChange} aria-label="Test toggle" />)
+
+		const toggle = screen.getByRole("switch")
+		expect(toggle).toBeInTheDocument()
+		// Small size should be 16px x 8px
+		expect(toggle).toHaveStyle({ width: "16px", height: "8px" })
+	})
+})
diff --git a/webview-ui/src/components/ui/index.ts b/webview-ui/src/components/ui/index.ts
index c36d3b4769..ee28b964c5 100644
--- a/webview-ui/src/components/ui/index.ts
+++ b/webview-ui/src/components/ui/index.ts
@@ -18,3 +18,4 @@ export * from "./select"
 export * from "./textarea"
 export * from "./tooltip"
 export * from "./standard-tooltip"
+export * from "./toggle-switch"
diff --git a/webview-ui/src/components/ui/toggle-switch.tsx b/webview-ui/src/components/ui/toggle-switch.tsx
new file mode 100644
index 0000000000..c2488851b5
--- /dev/null
+++ b/webview-ui/src/components/ui/toggle-switch.tsx
@@ -0,0 +1,68 @@
+import React from "react"
+
+export interface ToggleSwitchProps {
+	checked: boolean
+	onChange: () => void
+	disabled?: boolean
+	size?: "small" | "medium"
+	"aria-label"?: string
+	"data-testid"?: string
+}
+
+export const ToggleSwitch: React.FC<ToggleSwitchProps> = ({
+	checked,
+	onChange,
+	disabled = false,
+	size = "small",
+	"aria-label": ariaLabel,
+	"data-testid": dataTestId,
+}) => {
+	const dimensions = size === "small" ? { width: 16, height: 8, dotSize: 4 } : { width: 20, height: 10, dotSize: 6 }
+
+	const handleKeyDown = (e: React.KeyboardEvent) => {
+		if (e.key === "Enter" || e.key === " ") {
+			e.preventDefault()
+			if (!disabled) {
+				onChange()
+			}
+		}
+	}
+
+	return (
+		<div
+			role="switch"
+			aria-checked={checked}
+			aria-label={ariaLabel}
+			tabIndex={disabled ? -1 : 0}
+			data-testid={dataTestId}
+			style={{
+				width: `${dimensions.width}px`,
+				height: `${dimensions.height}px`,
+				backgroundColor: checked
+					? "var(--vscode-button-background)"
+					: "var(--vscode-titleBar-inactiveForeground)",
+				borderRadius: `${dimensions.height / 2}px`,
+				position: "relative",
+				cursor: disabled ? "not-allowed" : "pointer",
+				transition: "background-color 0.2s",
+				opacity: disabled ? 0.4 : checked ? 0.8 : 0.6,
+			}}
+			onClick={disabled ? undefined : onChange}
+			onKeyDown={handleKeyDown}>
+			<div
+				style={{
+					width: `${dimensions.dotSize}px`,
+					height: `${dimensions.dotSize}px`,
+					backgroundColor: "var(--vscode-titleBar-activeForeground)",
+					borderRadius: "50%",
+					position: "absolute",
+					top: `${(dimensions.height - dimensions.dotSize) / 2}px`,
+					left: checked
+						? `${dimensions.width - dimensions.dotSize - (dimensions.height - dimensions.dotSize) / 2}px`
+						: `${(dimensions.height - dimensions.dotSize) / 2}px`,
+					transition: "left 0.2s",
+				}}
+			/>
+		</div>
+	)
+}