fix: honor per-mode MCP allowlist for access_mcp_resource and prevent debounced flush from clobbering concurrent mode edits

src/core/prompts/tools/__tests__/filter-tools-for-mode.spec.ts

@@ -89,3 +89,61 @@ describe("filterNativeToolsForMode - disabledTools", () => {
 		expect(resultNames).not.toContain("edit")
 	})
 })
+
+describe("filterNativeToolsForMode - access_mcp_resource allowlist", () => {
+	const nativeTools: OpenAI.Chat.ChatCompletionTool[] = [makeTool("read_file"), makeTool("access_mcp_resource")]
+
+	// Minimal McpHub stub exposing only getServers(), which is all the resource
+	// availability check uses.
+	function makeMcpHub(servers: Array<{ name: string; resources?: unknown[] }>): any {
+		return {
+			getServers: () => servers,
+		}
+	}
+
+	it("keeps access_mcp_resource when an allowed server has resources", () => {
+		const mcpHub = makeMcpHub([{ name: "allowed-server", resources: [{ uri: "res://x" }] }])
+
+		const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}, mcpHub, [
+			"allowed-server",
+		])
+
+		const resultNames = result.map((t) => (t as any).function.name)
+		expect(resultNames).toContain("access_mcp_resource")
+	})
+
+	it("removes access_mcp_resource when only a disallowed server has resources", () => {
+		// The server with resources is NOT in the allowlist, so the restricted
+		// mode must not retain access_mcp_resource.
+		const mcpHub = makeMcpHub([
+			{ name: "allowed-server", resources: [] },
+			{ name: "blocked-server", resources: [{ uri: "res://secret" }] },
+		])
+
+		const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}, mcpHub, [
+			"allowed-server",
+		])
+
+		const resultNames = result.map((t) => (t as any).function.name)
+		expect(resultNames).not.toContain("access_mcp_resource")
+		expect(resultNames).toContain("read_file")
+	})
+
+	it("considers all servers when no allowlist is provided (unrestricted mode)", () => {
+		const mcpHub = makeMcpHub([{ name: "any-server", resources: [{ uri: "res://y" }] }])
+
+		const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}, mcpHub)
+
+		const resultNames = result.map((t) => (t as any).function.name)
+		expect(resultNames).toContain("access_mcp_resource")
+	})
+
+	it("removes access_mcp_resource when the allowlist is empty", () => {
+		const mcpHub = makeMcpHub([{ name: "some-server", resources: [{ uri: "res://z" }] }])
+
+		const result = filterNativeToolsForMode(nativeTools, "code", undefined, undefined, undefined, {}, mcpHub, [])
+
+		const resultNames = result.map((t) => (t as any).function.name)
+		expect(resultNames).not.toContain("access_mcp_resource")
+	})
+})

src/core/prompts/tools/filter-tools-for-mode.ts

@@ -220,6 +220,9 @@ export function applyModelToolCustomization(
  * @param codeIndexManager - Code index manager for codebase_search feature check
  * @param settings - Additional settings for tool filtering (includes modelInfo for model-specific customization)
  * @param mcpHub - MCP hub for checking available resources
+ * @param allowedMcpServers - Optional allowlist of MCP server names for the current mode. When
+ *   provided, the resource-availability check only considers servers in this list, so a mode that
+ *   restricts MCP servers cannot retain `access_mcp_resource` based on resources from disallowed servers.
  * @returns Filtered array of tools allowed for the mode
  */
 export function filterNativeToolsForMode(
@@ -230,6 +233,7 @@ export function filterNativeToolsForMode(
 	codeIndexManager?: CodeIndexManager,
 	settings?: Record<string, any>,
 	mcpHub?: McpHub,
+	allowedMcpServers?: string[],
 ): OpenAI.Chat.ChatCompletionTool[] {
 	// Get mode configuration and all tools for this mode
 	const modeSlug = mode ?? defaultModeSlug
@@ -301,8 +305,10 @@ export function filterNativeToolsForMode(
 		}
 	}
 
-	// Conditionally exclude access_mcp_resource if MCP is not enabled or there are no resources
-	if (!mcpHub || !hasAnyMcpResources(mcpHub)) {
+	// Conditionally exclude access_mcp_resource if MCP is not enabled or there are no resources.
+	// When the mode restricts MCP servers via allowedMcpServers, only resources from allowed
+	// servers count — otherwise a restricted mode could still read resources from disallowed servers.
+	if (!mcpHub || !hasAnyMcpResources(mcpHub, allowedMcpServers)) {
 		allowedToolNames.delete("access_mcp_resource")
 	}
 
@@ -330,10 +336,18 @@ export function filterNativeToolsForMode(
 }
 
 /**
- * Helper function to check if any MCP server has resources available
+ * Helper function to check if any MCP server has resources available.
+ *
+ * When `allowedServers` is provided, only servers whose name is in the allowlist are considered.
+ * This keeps the `access_mcp_resource` availability check consistent with the mode's MCP server
+ * allowlist so a restricted mode cannot retain the tool based on resources from disallowed servers.
  */
-function hasAnyMcpResources(mcpHub: McpHub): boolean {
-	const servers = mcpHub.getServers()
+function hasAnyMcpResources(mcpHub: McpHub, allowedServers?: string[]): boolean {
+	let servers = mcpHub.getServers()
+	if (allowedServers) {
+		const allowSet = new Set(allowedServers)
+		servers = servers.filter((server) => allowSet.has(server.name))
+	}
 	return servers.some((server) => server.resources && server.resources.length > 0)
 }
 

src/core/task/build-tools.ts

@@ -114,7 +114,13 @@ export async function buildNativeToolsArrayWithRestrictions(options: BuildToolsO
 		supportsImages,
 	})
 
-	// Filter native tools based on mode restrictions.
+	// Resolve mode config to get allowedMcpServers for MCP server filtering.
+	const modeConfig = getModeBySlug(mode ?? defaultModeSlug, customModes)
+	const allowedMcpServers = modeConfig?.allowedMcpServers
+
+	// Filter native tools based on mode restrictions. The allowlist is forwarded so the
+	// access_mcp_resource availability check only considers resources from allowed servers;
+	// otherwise a restricted mode could still read resources from disallowed servers.
 	const filteredNativeTools = filterNativeToolsForMode(
 		nativeTools,
 		mode,
@@ -123,12 +129,9 @@ export async function buildNativeToolsArrayWithRestrictions(options: BuildToolsO
 		codeIndexManager,
 		filterSettings,
 		mcpHub,
+		allowedMcpServers,
 	)
 
-	// Resolve mode config to get allowedMcpServers for MCP server filtering.
-	const modeConfig = getModeBySlug(mode ?? defaultModeSlug, customModes)
-	const allowedMcpServers = modeConfig?.allowedMcpServers
-
 	// Filter MCP tools based on mode restrictions.
 	const mcpTools = getMcpServerTools(mcpHub, allowedMcpServers)
 	const filteredMcpTools = filterMcpToolsForMode(mcpTools, mode, customModes, experiments)

webview-ui/src/__mocks__/@vscode/webview-ui-toolkit/react.tsx

@@ -1,8 +1,14 @@
 import React from "react"
 
 export const VSCodeCheckbox = ({ children, onChange, checked, "data-testid": dataTestId, ...props }: any) => (
-	<label data-testid={dataTestId}>
-		<input type="checkbox" checked={checked} onChange={(e: any) => onChange?.(e)} {...props} />
+	<label>
+		<input
+			type="checkbox"
+			data-testid={dataTestId}
+			checked={checked}
+			onChange={(e: any) => onChange?.(e)}
+			{...props}
+		/>
 		{children}
 	</label>
 )
@@ -52,3 +58,31 @@ export const VSCodeTextField = ({ value, onInput, "data-testid": dataTestId, chi
 		{children}
 	</div>
 )
+
+export const VSCodeButton = ({ children, onClick, "data-testid": dataTestId, ...props }: any) => (
+	<button data-testid={dataTestId} onClick={onClick} {...props}>
+		{children}
+	</button>
+)
+
+export const VSCodeBadge = ({ children, ...props }: any) => <span {...props}>{children}</span>
+
+export const VSCodeProgressRing = (props: any) => <div role="progressbar" {...props} />
+
+export const VSCodeDropdown = ({ children, value, onChange, "data-testid": dataTestId, ...props }: any) => (
+	<select data-testid={dataTestId} value={value} onChange={(e: any) => onChange?.(e)} {...props}>
+		{children}
+	</select>
+)
+
+export const VSCodeOption = ({ children, value, ...props }: any) => (
+	<option value={value} {...props}>
+		{children}
+	</option>
+)
+
+export const VSCodePanels = ({ children, ...props }: any) => <div {...props}>{children}</div>
+
+export const VSCodePanelTab = ({ children, ...props }: any) => <div {...props}>{children}</div>
+
+export const VSCodePanelView = ({ children, ...props }: any) => <div {...props}>{children}</div>

webview-ui/src/components/modes/McpServerRestriction.tsx

@@ -61,6 +61,18 @@ const McpServerRestriction: React.FC<McpServerRestrictionProps> = ({ customMode,
 	// Reseed-on-mode-switch is keyed on slug; track the last slug we saw.
 	const lastSlugRef = useRef(customMode.slug)
 
+	// Always hold the latest `customMode` and `onCommit` so the debounced flush
+	// merges `allowedMcpServers` into the freshest mode snapshot instead of the
+	// stale one captured when the timeout was scheduled. Without this, an edit to
+	// another field of the same mode within the 150 ms debounce window would be
+	// clobbered when this flush spreads an outdated `customMode`.
+	const latestCustomModeRef = useRef(customMode)
+	const latestOnCommitRef = useRef(onCommit)
+	useEffect(() => {
+		latestCustomModeRef.current = customMode
+		latestOnCommitRef.current = onCommit
+	})
+
 	// Reseed when the user switches to a different mode.
 	useEffect(() => {
 		if (lastSlugRef.current !== customMode.slug) {
@@ -98,10 +110,13 @@ const McpServerRestriction: React.FC<McpServerRestrictionProps> = ({ customMode,
 		}
 		const handle = setTimeout(() => {
 			lastFlushedRef.current = cachedAllowedMcpServers
-			onCommit(customMode.slug, {
-				...customMode,
+			// Merge into the freshest mode snapshot (via refs) so a concurrent edit to
+			// another field within the debounce window is not clobbered by a stale spread.
+			const latestCustomMode = latestCustomModeRef.current
+			latestOnCommitRef.current(latestCustomMode.slug, {
+				...latestCustomMode,
 				allowedMcpServers: cachedAllowedMcpServers,
-				source: customMode.source || "global",
+				source: latestCustomMode.source || "global",
 			})
 		}, 150)
 		return () => clearTimeout(handle)

webview-ui/src/components/modes/__tests__/McpServerRestriction.spec.tsx

@@ -138,8 +138,9 @@ describe("MCP Server Restriction UI", () => {
 			expect(screen.getByTestId("restrict-mcp-servers-toggle")).toBeInTheDocument()
 		})
 
-		const toggleLabel = screen.getByTestId("restrict-mcp-servers-toggle")
-		const checkbox = toggleLabel.querySelector("input[type='checkbox']") as HTMLInputElement
+		// The toolkit mock forwards `data-testid` to the inner
+		// <input type="checkbox">, so getByTestId resolves to the checkbox input directly.
+		const checkbox = screen.getByTestId("restrict-mcp-servers-toggle") as HTMLInputElement
 		fireEvent.click(checkbox)
 
 		await waitFor(() => {
@@ -165,8 +166,8 @@ describe("MCP Server Restriction UI", () => {
 			expect(screen.getByTestId("restrict-mcp-servers-toggle")).toBeInTheDocument()
 		})
 
-		const toggleLabel = screen.getByTestId("restrict-mcp-servers-toggle")
-		const checkbox = toggleLabel.querySelector("input[type='checkbox']") as HTMLInputElement
+		// data-testid is forwarded to the inner checkbox input by the toolkit mock.
+		const checkbox = screen.getByTestId("restrict-mcp-servers-toggle") as HTMLInputElement
 		fireEvent.click(checkbox)
 
 		await waitFor(() => {
@@ -222,17 +223,16 @@ describe("McpServerRestriction subcomponent — flicker regressions", () => {
 			const onCommit = vitest.fn()
 			render(<McpServerRestriction customMode={baseCustomMode} mcpServers={mcpServers} onCommit={onCommit} />)
 
-			const toggleLabel = screen.getByTestId("restrict-mcp-servers-toggle")
-			const checkbox = toggleLabel.querySelector("input[type='checkbox']") as HTMLInputElement
+			// The toolkit mock forwards `data-testid` to the inner checkbox input,
+			// so getByTestId resolves to the <input type="checkbox"> directly.
+			const checkbox = screen.getByTestId("restrict-mcp-servers-toggle") as HTMLInputElement
 			expect(checkbox.checked).toBe(false)
 			expect(screen.queryByTestId("mcp-server-list")).not.toBeInTheDocument()
 
 			fireEvent.click(checkbox)
 
 			// Synchronous post-click assertions — no advanceTimers, no host echo.
-			const checkboxAfter = screen
-				.getByTestId("restrict-mcp-servers-toggle")
-				.querySelector("input[type='checkbox']") as HTMLInputElement
+			const checkboxAfter = screen.getByTestId("restrict-mcp-servers-toggle") as HTMLInputElement
 			expect(checkboxAfter.checked).toBe(true)
 			expect(screen.getByTestId("mcp-server-list")).toBeInTheDocument()
 			// Debounced flush has not fired yet.
@@ -314,10 +314,10 @@ describe("McpServerRestriction subcomponent — flicker regressions", () => {
 			const initialCommits = onRender.mock.calls.length
 			expect(initialCommits).toBeGreaterThan(0)
 
-			// (a) Click — exactly 1 additional commit.
-			const serverCheckbox = screen
-				.getByTestId("mcp-server-checkbox-server-a")
-				.querySelector("input[type='checkbox']") as HTMLInputElement
+			// (a) Click — exactly 1 additional commit. The toolkit mock forwards
+			// `data-testid` to the inner checkbox input, so getByTestId resolves
+			// to the <input type="checkbox"> directly.
+			const serverCheckbox = screen.getByTestId("mcp-server-checkbox-server-a") as HTMLInputElement
 			fireEvent.click(serverCheckbox)
 			const afterClickCommits = onRender.mock.calls.length
 			expect(afterClickCommits - initialCommits).toBe(1)
@@ -340,4 +340,57 @@ describe("McpServerRestriction subcomponent — flicker regressions", () => {
 			vitest.useRealTimers()
 		}
 	})
+
+	/**
+	 * Test 4 — Concurrent-edit safety: the debounced flush must not clobber a
+	 * newer edit to another field of the same mode made within the 150 ms
+	 * window.
+	 *
+	 * Repro: the user toggles a per-server checkbox (schedules the debounced
+	 * flush), then within the debounce window another field of the same mode
+	 * (e.g. `name`) changes — the parent re-renders the component with an
+	 * updated `customMode`. Before the fix, the flush spread the STALE
+	 * `customMode` captured when the timeout was scheduled, sending the old
+	 * `name` back to the host and clobbering the newer value.
+	 *
+	 * After the fix, the flush merges `allowedMcpServers` into the freshest
+	 * `customMode` (via a ref), so the committed snapshot carries the updated
+	 * `name`.
+	 */
+	it("Test 4: debounced flush merges into the latest customMode, not the stale snapshot", () => {
+		vitest.useFakeTimers()
+		try {
+			const onCommit = vitest.fn()
+			const customMode = { ...baseCustomMode, name: "Old Name", allowedMcpServers: [] as string[] }
+
+			const { rerender } = render(
+				<McpServerRestriction customMode={customMode} mcpServers={mcpServers} onCommit={onCommit} />,
+			)
+
+			// User edits the allowlist (schedules the debounced flush). The toolkit
+			// mock forwards `data-testid` to the inner checkbox input, so getByTestId
+			// resolves to the <input type="checkbox"> directly.
+			const serverCheckbox = screen.getByTestId("mcp-server-checkbox-server-a") as HTMLInputElement
+			fireEvent.click(serverCheckbox)
+			expect(onCommit).not.toHaveBeenCalled() // debounce hasn't fired yet
+
+			// Within the debounce window, another field of the same mode changes.
+			const updatedCustomMode = { ...customMode, name: "New Name" }
+			rerender(
+				<McpServerRestriction customMode={updatedCustomMode} mcpServers={mcpServers} onCommit={onCommit} />,
+			)
+
+			// Let the 150 ms debounce fire.
+			vitest.advanceTimersByTime(200)
+
+			expect(onCommit).toHaveBeenCalledTimes(1)
+			const [, committedConfig] = onCommit.mock.calls[0]
+			// The newer field value must be preserved (not clobbered by the stale snapshot).
+			expect(committedConfig.name).toBe("New Name")
+			// And the user's allowlist edit must still be present.
+			expect(committedConfig.allowedMcpServers).toEqual(["server-a"])
+		} finally {
+			vitest.useRealTimers()
+		}
+	})
 })