Skip to content

Commit 76a3fdc

Browse files
authored
Clean up MCP tool disabling (#5576)
1 parent 6fa918c commit 76a3fdc

File tree

6 files changed

+334
-88
lines changed

6 files changed

+334
-88
lines changed

webview-ui/src/components/mcp/McpToolRow.tsx

Lines changed: 34 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { McpTool } from "@roo/mcp"
44

55
import { useAppTranslation } from "@src/i18n/TranslationContext"
66
import { vscode } from "@src/utils/vscode"
7-
import { StandardTooltip } from "@/components/ui"
7+
import { StandardTooltip, ToggleSwitch } from "@/components/ui"
88

99
type McpToolRowProps = {
1010
tool: McpTool
@@ -16,6 +16,8 @@ type McpToolRowProps = {
1616

1717
const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatContext = false }: McpToolRowProps) => {
1818
const { t } = useAppTranslation()
19+
const isToolEnabled = tool.enabledForPrompt ?? true
20+
1921
const handleAlwaysAllowChange = () => {
2022
if (!serverName) return
2123
vscode.postMessage({
@@ -46,17 +48,29 @@ const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatCo
4648
onClick={(e) => e.stopPropagation()}>
4749
{/* Tool name section */}
4850
<div className="flex items-center min-w-0 flex-1">
49-
<span className="codicon codicon-symbol-method mr-2 flex-shrink-0 text-vscode-symbolIcon-methodForeground"></span>
51+
<span
52+
className={`codicon codicon-symbol-method mr-2 flex-shrink-0 ${
53+
isToolEnabled
54+
? "text-vscode-symbolIcon-methodForeground"
55+
: "text-vscode-descriptionForeground opacity-60"
56+
}`}></span>
5057
<StandardTooltip content={tool.name}>
51-
<span className="font-medium truncate text-vscode-foreground">{tool.name}</span>
58+
<span
59+
className={`font-medium truncate ${
60+
isToolEnabled
61+
? "text-vscode-foreground"
62+
: "text-vscode-descriptionForeground opacity-60"
63+
}`}>
64+
{tool.name}
65+
</span>
5266
</StandardTooltip>
5367
</div>
5468

5569
{/* Controls section */}
5670
{serverName && (
5771
<div className="flex items-center gap-4 flex-shrink-0">
58-
{/* Always Allow checkbox */}
59-
{alwaysAllowMcp && (
72+
{/* Always Allow checkbox - only show when tool is enabled */}
73+
{alwaysAllowMcp && isToolEnabled && (
6074
<VSCodeCheckbox
6175
checked={tool.alwaysAllow}
6276
onChange={handleAlwaysAllowChange}
@@ -68,35 +82,31 @@ const McpToolRow = ({ tool, serverName, serverSource, alwaysAllowMcp, isInChatCo
6882
</VSCodeCheckbox>
6983
)}
7084

71-
{/* Enabled eye button - only show in settings context */}
85+
{/* Enabled toggle switch - only show in settings context */}
7286
{!isInChatContext && (
7387
<StandardTooltip content={t("mcp:tool.togglePromptInclusion")}>
74-
<button
75-
role="button"
76-
aria-pressed={tool.enabledForPrompt}
88+
<ToggleSwitch
89+
checked={isToolEnabled}
90+
onChange={handleEnabledForPromptChange}
91+
size="medium"
7792
aria-label={t("mcp:tool.togglePromptInclusion")}
78-
className={`p-1 rounded hover:bg-vscode-toolbar-hoverBackground transition-colors ${
79-
tool.enabledForPrompt
80-
? "text-vscode-foreground"
81-
: "text-vscode-descriptionForeground opacity-60"
82-
}`}
83-
onClick={handleEnabledForPromptChange}
84-
data-tool-prompt-toggle={tool.name}>
85-
<span
86-
className={`codicon ${
87-
tool.enabledForPrompt ? "codicon-eye-closed" : "codicon-eye"
88-
} text-base`}
89-
/>
90-
</button>
93+
data-testid={`tool-prompt-toggle-${tool.name}`}
94+
/>
9195
</StandardTooltip>
9296
)}
9397
</div>
9498
)}
9599
</div>
96100
{tool.description && (
97-
<div className="mt-1 text-xs text-vscode-descriptionForeground opacity-80">{tool.description}</div>
101+
<div
102+
className={`mt-1 text-xs text-vscode-descriptionForeground ${
103+
isToolEnabled ? "opacity-80" : "opacity-40"
104+
}`}>
105+
{tool.description}
106+
</div>
98107
)}
99-
{tool.inputSchema &&
108+
{isToolEnabled &&
109+
tool.inputSchema &&
100110
"properties" in tool.inputSchema &&
101111
Object.keys(tool.inputSchema.properties as Record<string, any>).length > 0 && (
102112
<div className="mt-2 text-xs border border-vscode-panel-border rounded p-2">

webview-ui/src/components/mcp/McpView.tsx

Lines changed: 16 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import {
2222
DialogTitle,
2323
DialogDescription,
2424
DialogFooter,
25+
ToggleSwitch,
2526
} from "@src/components/ui"
2627
import { buildDocLink } from "@src/utils/docLinks"
2728

@@ -295,54 +296,6 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
295296
style={{ marginRight: "8px" }}>
296297
<span className="codicon codicon-refresh" style={{ fontSize: "14px" }}></span>
297298
</Button>
298-
<div
299-
role="switch"
300-
aria-checked={!server.disabled}
301-
tabIndex={0}
302-
style={{
303-
width: "20px",
304-
height: "10px",
305-
backgroundColor: server.disabled
306-
? "var(--vscode-titleBar-inactiveForeground)"
307-
: "var(--vscode-button-background)",
308-
borderRadius: "5px",
309-
position: "relative",
310-
cursor: "pointer",
311-
transition: "background-color 0.2s",
312-
opacity: server.disabled ? 0.4 : 0.8,
313-
}}
314-
onClick={() => {
315-
vscode.postMessage({
316-
type: "toggleMcpServer",
317-
serverName: server.name,
318-
source: server.source || "global",
319-
disabled: !server.disabled,
320-
})
321-
}}
322-
onKeyDown={(e) => {
323-
if (e.key === "Enter" || e.key === " ") {
324-
e.preventDefault()
325-
vscode.postMessage({
326-
type: "toggleMcpServer",
327-
serverName: server.name,
328-
source: server.source || "global",
329-
disabled: !server.disabled,
330-
})
331-
}
332-
}}>
333-
<div
334-
style={{
335-
width: "6px",
336-
height: "6px",
337-
backgroundColor: "var(--vscode-titleBar-activeForeground)",
338-
borderRadius: "50%",
339-
position: "absolute",
340-
top: "2px",
341-
left: server.disabled ? "2px" : "12px",
342-
transition: "left 0.2s",
343-
}}
344-
/>
345-
</div>
346299
</div>
347300
<div
348301
style={{
@@ -353,6 +306,21 @@ const ServerRow = ({ server, alwaysAllowMcp }: { server: McpServer; alwaysAllowM
353306
marginLeft: "8px",
354307
}}
355308
/>
309+
<div style={{ marginLeft: "8px" }}>
310+
<ToggleSwitch
311+
checked={!server.disabled}
312+
onChange={() => {
313+
vscode.postMessage({
314+
type: "toggleMcpServer",
315+
serverName: server.name,
316+
source: server.source || "global",
317+
disabled: !server.disabled,
318+
})
319+
}}
320+
size="medium"
321+
aria-label={`Toggle ${server.name} server`}
322+
/>
323+
</div>
356324
</div>
357325

358326
{server.status === "connected" ? (

webview-ui/src/components/mcp/__tests__/McpToolRow.spec.tsx

Lines changed: 114 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -144,40 +144,40 @@ describe("McpToolRow", () => {
144144
expect(screen.getByText("Second parameter")).toBeInTheDocument()
145145
})
146146

147-
it("shows eye button when serverName is provided and not in chat context", () => {
147+
it("shows toggle switch when serverName is provided and not in chat context", () => {
148148
render(<McpToolRow tool={mockTool} serverName="test-server" />)
149149

150-
const eyeButton = screen.getByRole("button", { name: "Toggle prompt inclusion" })
151-
expect(eyeButton).toBeInTheDocument()
150+
const toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
151+
expect(toggleSwitch).toBeInTheDocument()
152152
})
153153

154-
it("hides eye button when isInChatContext is true", () => {
154+
it("hides toggle switch when isInChatContext is true", () => {
155155
render(<McpToolRow tool={mockTool} serverName="test-server" isInChatContext={true} />)
156156

157-
const eyeButton = screen.queryByRole("button", { name: "Toggle prompt inclusion" })
158-
expect(eyeButton).not.toBeInTheDocument()
157+
const toggleSwitch = screen.queryByRole("switch", { name: "Toggle prompt inclusion" })
158+
expect(toggleSwitch).not.toBeInTheDocument()
159159
})
160160

161-
it("shows correct eye icon based on enabledForPrompt state", () => {
162-
// Test when enabled (should show eye-closed icon)
161+
it("shows correct toggle switch state based on enabledForPrompt", () => {
162+
// Test when enabled (should be checked)
163163
const { rerender } = render(<McpToolRow tool={mockTool} serverName="test-server" />)
164164

165-
let eyeIcon = screen.getByRole("button", { name: "Toggle prompt inclusion" }).querySelector("span")
166-
expect(eyeIcon).toHaveClass("codicon-eye-closed")
165+
let toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
166+
expect(toggleSwitch).toHaveAttribute("aria-checked", "true")
167167

168-
// Test when disabled (should show eye icon)
168+
// Test when disabled (should not be checked)
169169
const disabledTool = { ...mockTool, enabledForPrompt: false }
170170
rerender(<McpToolRow tool={disabledTool} serverName="test-server" />)
171171

172-
eyeIcon = screen.getByRole("button", { name: "Toggle prompt inclusion" }).querySelector("span")
173-
expect(eyeIcon).toHaveClass("codicon-eye")
172+
toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
173+
expect(toggleSwitch).toHaveAttribute("aria-checked", "false")
174174
})
175175

176-
it("sends message to toggle enabledForPrompt when eye button is clicked", () => {
176+
it("sends message to toggle enabledForPrompt when toggle switch is clicked", () => {
177177
render(<McpToolRow tool={mockTool} serverName="test-server" />)
178178

179-
const eyeButton = screen.getByRole("button", { name: "Toggle prompt inclusion" })
180-
fireEvent.click(eyeButton)
179+
const toggleSwitch = screen.getByRole("switch", { name: "Toggle prompt inclusion" })
180+
fireEvent.click(toggleSwitch)
181181

182182
expect(vscode.postMessage).toHaveBeenCalledWith({
183183
type: "toggleToolEnabledForPrompt",
@@ -187,4 +187,102 @@ describe("McpToolRow", () => {
187187
isEnabled: false,
188188
})
189189
})
190+
191+
it("hides always allow checkbox when tool is disabled", () => {
192+
const disabledTool = { ...mockTool, enabledForPrompt: false }
193+
render(<McpToolRow tool={disabledTool} serverName="test-server" alwaysAllowMcp={true} />)
194+
195+
expect(screen.queryByText("Always allow")).not.toBeInTheDocument()
196+
})
197+
198+
it("shows always allow checkbox when tool is enabled", () => {
199+
const enabledTool = { ...mockTool, enabledForPrompt: true }
200+
render(<McpToolRow tool={enabledTool} serverName="test-server" alwaysAllowMcp={true} />)
201+
202+
expect(screen.getByText("Always allow")).toBeInTheDocument()
203+
})
204+
205+
it("hides parameters section when tool is disabled", () => {
206+
const disabledToolWithSchema = {
207+
...mockTool,
208+
enabledForPrompt: false,
209+
inputSchema: {
210+
type: "object",
211+
properties: {
212+
param1: {
213+
type: "string",
214+
description: "First parameter",
215+
},
216+
},
217+
required: ["param1"],
218+
},
219+
}
220+
221+
render(<McpToolRow tool={disabledToolWithSchema} serverName="test-server" />)
222+
223+
expect(screen.queryByText("Parameters")).not.toBeInTheDocument()
224+
expect(screen.queryByText("param1")).not.toBeInTheDocument()
225+
expect(screen.queryByText("First parameter")).not.toBeInTheDocument()
226+
})
227+
228+
it("shows parameters section when tool is enabled", () => {
229+
const enabledToolWithSchema = {
230+
...mockTool,
231+
enabledForPrompt: true,
232+
inputSchema: {
233+
type: "object",
234+
properties: {
235+
param1: {
236+
type: "string",
237+
description: "First parameter",
238+
},
239+
},
240+
required: ["param1"],
241+
},
242+
}
243+
244+
render(<McpToolRow tool={enabledToolWithSchema} serverName="test-server" />)
245+
246+
expect(screen.getByText("Parameters")).toBeInTheDocument()
247+
expect(screen.getByText("param1")).toBeInTheDocument()
248+
expect(screen.getByText("First parameter")).toBeInTheDocument()
249+
})
250+
251+
it("grays out tool name and description when tool is disabled", () => {
252+
const disabledTool = {
253+
...mockTool,
254+
enabledForPrompt: false,
255+
description: "A disabled tool",
256+
}
257+
render(<McpToolRow tool={disabledTool} serverName="test-server" />)
258+
259+
const toolName = screen.getByText("test-tool")
260+
const toolDescription = screen.getByText("A disabled tool")
261+
262+
// Check that the tool name has the grayed out classes
263+
expect(toolName).toHaveClass("text-vscode-descriptionForeground", "opacity-60")
264+
265+
// Check that the description has reduced opacity
266+
expect(toolDescription).toHaveClass("opacity-40")
267+
})
268+
269+
it("shows normal styling for tool name and description when tool is enabled", () => {
270+
const enabledTool = {
271+
...mockTool,
272+
enabledForPrompt: true,
273+
description: "An enabled tool",
274+
}
275+
render(<McpToolRow tool={enabledTool} serverName="test-server" />)
276+
277+
const toolName = screen.getByText("test-tool")
278+
const toolDescription = screen.getByText("An enabled tool")
279+
280+
// Check that the tool name has normal styling
281+
expect(toolName).toHaveClass("text-vscode-foreground")
282+
expect(toolName).not.toHaveClass("text-vscode-descriptionForeground", "opacity-60")
283+
284+
// Check that the description has normal opacity
285+
expect(toolDescription).toHaveClass("opacity-80")
286+
expect(toolDescription).not.toHaveClass("opacity-40")
287+
})
190288
})

0 commit comments

Comments
 (0)