fix(settings): buffer Save-managed settings in cachedState until Save

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

@@ -5,19 +5,35 @@ import { useExtensionState } from "@src/context/ExtensionStateContext"
 import { useAppTranslation } from "@src/i18n/TranslationContext"
 import { vscode } from "@src/utils/vscode"
 
-const McpEnabledToggle = () => {
-	const { mcpEnabled, setMcpEnabled } = useExtensionState()
+interface McpEnabledToggleProps {
+	mcpEnabled?: boolean
+	setMcpEnabled?: (value: boolean) => void
+}
+
+const McpEnabledToggle = ({
+	mcpEnabled: propsMcpEnabled,
+	setMcpEnabled: propsSetMcpEnabled,
+}: McpEnabledToggleProps = {}) => {
+	const { mcpEnabled: contextMcpEnabled, setMcpEnabled: contextSetMcpEnabled } = useExtensionState()
 	const { t } = useAppTranslation()
 
+	// When rendered inside SettingsView the value is buffered in `cachedState` and
+	// only persisted on Save. Fall back to live extension state when used uncontrolled.
+	const mcpEnabled = propsMcpEnabled ?? contextMcpEnabled
+
 	const handleChange = (e: Event | FormEvent<HTMLElement>) => {
 		const target = ("target" in e ? e.target : null) as HTMLInputElement | null
 
 		if (!target) {
 			return
 		}
 
-		setMcpEnabled(target.checked)
-		vscode.postMessage({ type: "updateSettings", updatedSettings: { mcpEnabled: target.checked } })
+		if (propsSetMcpEnabled) {
+			propsSetMcpEnabled(target.checked)
+		} else {
+			contextSetMcpEnabled(target.checked)
+			vscode.postMessage({ type: "updateSettings", updatedSettings: { mcpEnabled: target.checked } })
+		}
 	}
 
 	return (

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

@@ -28,8 +28,17 @@ import McpResourceRow from "./McpResourceRow"
 import McpEnabledToggle from "./McpEnabledToggle"
 import { McpErrorRow } from "./McpErrorRow"
 
-const McpView = () => {
-	const { mcpServers: servers, alwaysAllowMcp, mcpEnabled } = useExtensionState()
+interface McpViewProps {
+	mcpEnabled?: boolean
+	setMcpEnabled?: (value: boolean) => void
+}
+
+const McpView = ({ mcpEnabled: propsMcpEnabled, setMcpEnabled }: McpViewProps = {}) => {
+	const { mcpServers: servers, alwaysAllowMcp, mcpEnabled: contextMcpEnabled } = useExtensionState()
+
+	// When rendered inside SettingsView the value is buffered in `cachedState` and
+	// only persisted on Save. Fall back to live extension state when used uncontrolled.
+	const mcpEnabled = propsMcpEnabled ?? contextMcpEnabled
 
 	const { t } = useAppTranslation()
 	const { isOverThreshold, title, message } = useTooManyTools()
@@ -55,7 +64,7 @@ const McpView = () => {
 					</Trans>
 				</div>
 
-				<McpEnabledToggle />
+				<McpEnabledToggle mcpEnabled={mcpEnabled} setMcpEnabled={setMcpEnabled} />
 
 				{mcpEnabled && (
 					<>

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

@@ -0,0 +1,58 @@
+// npx vitest src/components/mcp/__tests__/McpEnabledToggle.spec.tsx
+
+import { render, screen, fireEvent } from "@/utils/test-utils"
+
+import McpEnabledToggle from "../McpEnabledToggle"
+import { vscode } from "@src/utils/vscode"
+
+vi.mock("@src/utils/vscode", () => ({
+	vscode: {
+		postMessage: vi.fn(),
+	},
+}))
+
+vi.mock("@src/i18n/TranslationContext", () => ({
+	useAppTranslation: () => ({ t: (key: string) => key }),
+}))
+
+const contextSetMcpEnabled = vi.fn()
+vi.mock("@src/context/ExtensionStateContext", () => ({
+	useExtensionState: () => ({
+		mcpEnabled: true,
+		setMcpEnabled: contextSetMcpEnabled,
+	}),
+}))
+
+const getCheckbox = () => screen.getByRole("checkbox")
+
+describe("McpEnabledToggle - Save/Discard contract", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
+	// Case 6: controlled (inside SettingsView) must buffer, not persist before Save.
+	it("buffers via the setter prop without persisting before Save when controlled", () => {
+		const setMcpEnabled = vi.fn()
+		render(<McpEnabledToggle mcpEnabled={true} setMcpEnabled={setMcpEnabled} />)
+
+		fireEvent.click(getCheckbox())
+
+		expect(setMcpEnabled).toHaveBeenCalledWith(false)
+		expect(vscode.postMessage).not.toHaveBeenCalled()
+		// Must not touch live extension state either.
+		expect(contextSetMcpEnabled).not.toHaveBeenCalled()
+	})
+
+	// Regression guard: uncontrolled usage keeps the original immediate behavior.
+	it("persists immediately via live state when used uncontrolled", () => {
+		render(<McpEnabledToggle />)
+
+		fireEvent.click(getCheckbox())
+
+		expect(contextSetMcpEnabled).toHaveBeenCalledWith(false)
+		expect(vscode.postMessage).toHaveBeenCalledWith({
+			type: "updateSettings",
+			updatedSettings: { mcpEnabled: false },
+		})
+	})
+})

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

@@ -0,0 +1,77 @@
+import { fireEvent, render, screen } from "@/utils/test-utils"
+
+import McpView from "../McpView"
+
+const setMcpEnabled = vi.fn()
+
+vi.mock("@src/context/ExtensionStateContext", () => ({
+	useExtensionState: () => ({
+		mcpServers: [],
+		alwaysAllowMcp: false,
+		mcpEnabled: false,
+	}),
+}))
+
+vi.mock("@src/i18n/TranslationContext", () => ({
+	useAppTranslation: () => ({ t: (key: string) => key }),
+}))
+
+vi.mock("@src/hooks/useTooManyTools", () => ({
+	useTooManyTools: () => ({ isOverThreshold: false, title: "", message: "" }),
+}))
+
+vi.mock("@src/components/settings/Section", () => ({
+	Section: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+}))
+
+vi.mock("@src/components/settings/SectionHeader", () => ({
+	SectionHeader: ({ children }: { children: React.ReactNode }) => <h2>{children}</h2>,
+}))
+
+vi.mock("@src/components/ui", () => ({
+	Button: ({ children, onClick }: { children: React.ReactNode; onClick: () => void }) => (
+		<button onClick={onClick}>{children}</button>
+	),
+	Dialog: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	DialogContent: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	DialogHeader: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	DialogTitle: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	DialogDescription: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	DialogFooter: ({ children }: { children: React.ReactNode }) => <div>{children}</div>,
+	ToggleSwitch: () => null,
+	StandardTooltip: ({ children }: { children: React.ReactNode }) => <>{children}</>,
+}))
+
+vi.mock("../McpEnabledToggle", () => ({
+	default: ({
+		mcpEnabled,
+		setMcpEnabled: setEnabled,
+	}: {
+		mcpEnabled: boolean
+		setMcpEnabled?: (value: boolean) => void
+	}) => (
+		<button data-testid="mcp-enabled-toggle" onClick={() => setEnabled?.(!mcpEnabled)}>
+			{String(mcpEnabled)}
+		</button>
+	),
+}))
+
+describe("McpView", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
+	it("uses extension state when no buffered value is provided", () => {
+		render(<McpView />)
+
+		expect(screen.getByTestId("mcp-enabled-toggle")).toHaveTextContent("false")
+	})
+
+	it("uses and updates the buffered value when controlled", () => {
+		render(<McpView mcpEnabled={true} setMcpEnabled={setMcpEnabled} />)
+
+		fireEvent.click(screen.getByTestId("mcp-enabled-toggle"))
+
+		expect(setMcpEnabled).toHaveBeenCalledWith(false)
+	})
+})

webview-ui/src/components/settings/AutoApproveSettings.tsx

@@ -88,7 +88,6 @@ export const AutoApproveSettings = ({
 			const newCommands = [...currentCommands, commandInput]
 			setCachedStateField("allowedCommands", newCommands)
 			setCommandInput("")
-			vscode.postMessage({ type: "updateSettings", updatedSettings: { allowedCommands: newCommands } })
 		}
 	}
 
@@ -99,7 +98,6 @@ export const AutoApproveSettings = ({
 			const newCommands = [...currentCommands, deniedCommandInput]
 			setCachedStateField("deniedCommands", newCommands)
 			setDeniedCommandInput("")
-			vscode.postMessage({ type: "updateSettings", updatedSettings: { deniedCommands: newCommands } })
 		}
 	}
 
@@ -317,11 +315,6 @@ export const AutoApproveSettings = ({
 									onClick={() => {
 										const newCommands = (allowedCommands ?? []).filter((_, i) => i !== index)
 										setCachedStateField("allowedCommands", newCommands)
-
-										vscode.postMessage({
-											type: "updateSettings",
-											updatedSettings: { allowedCommands: newCommands },
-										})
 									}}>
 									<div className="flex flex-row items-center gap-1">
 										<div>{cmd}</div>
@@ -376,11 +369,6 @@ export const AutoApproveSettings = ({
 									onClick={() => {
 										const newCommands = (deniedCommands ?? []).filter((_, i) => i !== index)
 										setCachedStateField("deniedCommands", newCommands)
-
-										vscode.postMessage({
-											type: "updateSettings",
-											updatedSettings: { deniedCommands: newCommands },
-										})
 									}}>
 									<div className="flex flex-row items-center gap-1">
 										<div>{cmd}</div>

webview-ui/src/components/settings/ContextManagementSettings.tsx

@@ -24,7 +24,6 @@ import { SetCachedStateField } from "./types"
 import { SectionHeader } from "./SectionHeader"
 import { Section } from "./Section"
 import { SearchableSetting } from "./SearchableSetting"
-import { vscode } from "@/utils/vscode"
 
 type ContextManagementSettingsProps = HTMLAttributes<HTMLDivElement> & {
 	autoCondenseContext: boolean
@@ -139,7 +138,6 @@ export const ContextManagementSettings = ({
 			}
 
 			setCachedStateField("profileThresholds", newThresholds)
-			vscode.postMessage({ type: "updateSettings", updatedSettings: { profileThresholds: newThresholds } })
 		}
 	}
 	return (

webview-ui/src/components/settings/PromptsSettings.tsx

@@ -208,11 +208,6 @@ const PromptsSettings = ({
 										}
 
 										setIncludeTaskHistoryInEnhance(target.checked)
-
-										vscode.postMessage({
-											type: "updateSettings",
-											updatedSettings: { includeTaskHistoryInEnhance: target.checked },
-										})
 									}}>
 									<span className="font-medium">
 										{t("prompts:supportPrompts.enhance.includeTaskHistory")}

webview-ui/src/components/settings/SettingsView.tsx

@@ -900,7 +900,12 @@ const SettingsView = forwardRef<SettingsViewRef, SettingsViewProps>(({ onDone, t
 						{renderTab === "modes" && <ModesView />}
 
 						{/* MCP Section */}
-						{renderTab === "mcp" && <McpView />}
+						{renderTab === "mcp" && (
+							<McpView
+								mcpEnabled={mcpEnabled}
+								setMcpEnabled={(value) => setCachedStateField("mcpEnabled", value)}
+							/>
+						)}
 
 						{/* Worktrees Section */}
 						{renderTab === "worktrees" && <WorktreesView />}

webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx

@@ -0,0 +1,100 @@
+// npx vitest src/components/settings/__tests__/AutoApproveSettings.spec.tsx
+
+import { render, screen, fireEvent } from "@/utils/test-utils"
+
+import { AutoApproveSettings } from "../AutoApproveSettings"
+import { vscode } from "@/utils/vscode"
+
+vi.mock("@/utils/vscode", () => ({
+	vscode: {
+		postMessage: vi.fn(),
+	},
+}))
+
+vi.mock("@/i18n/TranslationContext", () => ({
+	useAppTranslation: () => ({ t: (key: string) => key }),
+}))
+
+// AutoApproveSettings reads a couple of live-state values that are genuinely
+// immediate actions (autoApprovalEnabled). Those are out of scope for the
+// Save/Discard buffering contract, so we just provide inert stand-ins.
+vi.mock("@/context/ExtensionStateContext", () => ({
+	useExtensionState: () => ({
+		autoApprovalEnabled: false,
+		setAutoApprovalEnabled: vi.fn(),
+	}),
+}))
+
+vi.mock("@/hooks/useAutoApprovalToggles", () => ({
+	useAutoApprovalToggles: () => ({}),
+}))
+
+vi.mock("@/hooks/useAutoApprovalState", () => ({
+	useAutoApprovalState: () => ({ effectiveAutoApprovalEnabled: false, hasEnabledOptions: false }),
+}))
+
+const renderSettings = (overrides = {}) => {
+	const setCachedStateField = vi.fn()
+	const props = {
+		alwaysAllowExecute: true, // reveal the command list section
+		allowedCommands: [] as string[],
+		deniedCommands: [] as string[],
+		setCachedStateField,
+		...overrides,
+	}
+	render(<AutoApproveSettings {...(props as any)} />)
+	return { setCachedStateField }
+}
+
+// A change is "Save-managed" if it must NOT reach the extension host before Save.
+const expectNoImmediateUpdateSettings = () => {
+	expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "updateSettings" }))
+}
+
+describe("AutoApproveSettings - Save/Discard contract", () => {
+	beforeEach(() => {
+		vi.clearAllMocks()
+	})
+
+	// Case 1: allowedCommands add
+	it("buffers an added allowed command without persisting before Save", () => {
+		const { setCachedStateField } = renderSettings()
+
+		fireEvent.change(screen.getByTestId("command-input"), { target: { value: "npm test" } })
+		fireEvent.click(screen.getByTestId("add-command-button"))
+
+		expect(setCachedStateField).toHaveBeenCalledWith("allowedCommands", ["npm test"])
+		expectNoImmediateUpdateSettings()
+	})
+
+	// Case 2: allowedCommands remove
+	it("buffers a removed allowed command without persisting before Save", () => {
+		const { setCachedStateField } = renderSettings({ allowedCommands: ["npm test"] })
+
+		fireEvent.click(screen.getByTestId("remove-command-0"))
+
+		expect(setCachedStateField).toHaveBeenCalledWith("allowedCommands", [])
+		expectNoImmediateUpdateSettings()
+	})
+
+	// Case 3a: deniedCommands add
+	it("buffers an added denied command without persisting before Save", () => {
+		const { setCachedStateField } = renderSettings()
+
+		fireEvent.change(screen.getByTestId("denied-command-input"), { target: { value: "rm -rf" } })
+		fireEvent.click(screen.getByTestId("add-denied-command-button"))
+
+		expect(setCachedStateField).toHaveBeenCalledWith("deniedCommands", ["rm -rf"])
+		expectNoImmediateUpdateSettings()
+	})
+
+	// Case 3b: deniedCommands remove
+	it("buffers a removed denied command without persisting before Save", () => {
+		const { setCachedStateField } = renderSettings({ deniedCommands: ["rm -rf"] })
+
+		fireEvent.click(screen.getByTestId("remove-denied-command-0"))
+
+		expect(setCachedStateField).toHaveBeenCalledWith("deniedCommands", [])
+		expectNoImmediateUpdateSettings()
+	})
+})