From 4223f1c3fe94323ba7cae3c67c9db5bc072ff70b Mon Sep 17 00:00:00 2001 From: Junyong Park Date: Wed, 8 Jul 2026 20:58:18 +0900 Subject: [PATCH 1/2] fix(settings): buffer Save-managed settings in cachedState until Save MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Several controls inside SettingsView wrote their changes to the extension host immediately via `updateSettings`, instead of buffering them in local `cachedState` and waiting for the user to click Save. Because Discard reverts edits by resetting `cachedState` to the persisted state, any control that already posted `updateSettings` poisoned that source of truth — so Discard could no longer undo the change, silently breaking the Save/Discard contract. Route all five Save-managed values exclusively through `cachedState` / `setCachedStateField` while SettingsView is open: - allowedCommands / deniedCommands (AutoApproveSettings): drop the immediate updateSettings on add/remove. - profileThresholds (ContextManagementSettings): drop the immediate updateSettings on the non-default profile branch; remove the now-unused vscode import. - includeTaskHistoryInEnhance (PromptsSettings): drop the immediate updateSettings on toggle. - mcpEnabled (McpEnabledToggle / McpView): adopt the existing "props first, fall back to context" pattern so SettingsView can pass the buffered value and a cachedState-backed setter; uncontrolled usage keeps the original immediate behavior. Genuinely-immediate actions (autoApprovalEnabled, per-server MCP toggle/delete/restart/timeout) are not in the Save payload and are left unchanged. The extension host side is correct and untouched. Add/update tests at the webview-ui component layer: buffering assertions for each control, a McpEnabledToggle controlled/uncontrolled regression, and a SettingsView Discard regression. Signed-off-by: JunyongParkDev --- .../src/components/mcp/McpEnabledToggle.tsx | 24 ++++- webview-ui/src/components/mcp/McpView.tsx | 15 ++- .../mcp/__tests__/McpEnabledToggle.spec.tsx | 58 ++++++++++ .../settings/AutoApproveSettings.tsx | 12 --- .../settings/ContextManagementSettings.tsx | 2 - .../components/settings/PromptsSettings.tsx | 5 - .../src/components/settings/SettingsView.tsx | 7 +- .../__tests__/AutoApproveSettings.spec.tsx | 100 ++++++++++++++++++ .../ContextManagementSettings.spec.tsx | 33 +++++- .../__tests__/PromptsSettings.spec.tsx | 62 +++++++++++ .../settings/__tests__/SettingsView.spec.tsx | 63 ++++++++--- 11 files changed, 338 insertions(+), 43 deletions(-) create mode 100644 webview-ui/src/components/mcp/__tests__/McpEnabledToggle.spec.tsx create mode 100644 webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx create mode 100644 webview-ui/src/components/settings/__tests__/PromptsSettings.spec.tsx diff --git a/webview-ui/src/components/mcp/McpEnabledToggle.tsx b/webview-ui/src/components/mcp/McpEnabledToggle.tsx index 1fd30034b5..3e6e5cf9ff 100644 --- a/webview-ui/src/components/mcp/McpEnabledToggle.tsx +++ b/webview-ui/src/components/mcp/McpEnabledToggle.tsx @@ -5,10 +5,22 @@ 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) => { const target = ("target" in e ? e.target : null) as HTMLInputElement | null @@ -16,8 +28,12 @@ const McpEnabledToggle = () => { 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 ( diff --git a/webview-ui/src/components/mcp/McpView.tsx b/webview-ui/src/components/mcp/McpView.tsx index 75a9a1a380..72724d72e8 100644 --- a/webview-ui/src/components/mcp/McpView.tsx +++ b/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 = () => { - + {mcpEnabled && ( <> diff --git a/webview-ui/src/components/mcp/__tests__/McpEnabledToggle.spec.tsx b/webview-ui/src/components/mcp/__tests__/McpEnabledToggle.spec.tsx new file mode 100644 index 0000000000..f59efce971 --- /dev/null +++ b/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() + + 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() + + fireEvent.click(getCheckbox()) + + expect(contextSetMcpEnabled).toHaveBeenCalledWith(false) + expect(vscode.postMessage).toHaveBeenCalledWith({ + type: "updateSettings", + updatedSettings: { mcpEnabled: false }, + }) + }) +}) diff --git a/webview-ui/src/components/settings/AutoApproveSettings.tsx b/webview-ui/src/components/settings/AutoApproveSettings.tsx index 40e1658f5f..90b82dc4be 100644 --- a/webview-ui/src/components/settings/AutoApproveSettings.tsx +++ b/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 }, - }) }}>
{cmd}
@@ -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 }, - }) }}>
{cmd}
diff --git a/webview-ui/src/components/settings/ContextManagementSettings.tsx b/webview-ui/src/components/settings/ContextManagementSettings.tsx index eaa566f36a..c7081391f9 100644 --- a/webview-ui/src/components/settings/ContextManagementSettings.tsx +++ b/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 & { autoCondenseContext: boolean @@ -139,7 +138,6 @@ export const ContextManagementSettings = ({ } setCachedStateField("profileThresholds", newThresholds) - vscode.postMessage({ type: "updateSettings", updatedSettings: { profileThresholds: newThresholds } }) } } return ( diff --git a/webview-ui/src/components/settings/PromptsSettings.tsx b/webview-ui/src/components/settings/PromptsSettings.tsx index 54babbcfcb..959b8bdd3e 100644 --- a/webview-ui/src/components/settings/PromptsSettings.tsx +++ b/webview-ui/src/components/settings/PromptsSettings.tsx @@ -208,11 +208,6 @@ const PromptsSettings = ({ } setIncludeTaskHistoryInEnhance(target.checked) - - vscode.postMessage({ - type: "updateSettings", - updatedSettings: { includeTaskHistoryInEnhance: target.checked }, - }) }}> {t("prompts:supportPrompts.enhance.includeTaskHistory")} diff --git a/webview-ui/src/components/settings/SettingsView.tsx b/webview-ui/src/components/settings/SettingsView.tsx index 14d6fa4413..c6c8176a4c 100644 --- a/webview-ui/src/components/settings/SettingsView.tsx +++ b/webview-ui/src/components/settings/SettingsView.tsx @@ -900,7 +900,12 @@ const SettingsView = forwardRef(({ onDone, t {renderTab === "modes" && } {/* MCP Section */} - {renderTab === "mcp" && } + {renderTab === "mcp" && ( + setCachedStateField("mcpEnabled", value)} + /> + )} {/* Worktrees Section */} {renderTab === "worktrees" && } diff --git a/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/AutoApproveSettings.spec.tsx new file mode 100644 index 0000000000..8b736280ee --- /dev/null +++ b/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() + 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() + }) +}) diff --git a/webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx index 57b3066b4d..10d5d0fe4f 100644 --- a/webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/ContextManagementSettings.spec.tsx @@ -2,6 +2,7 @@ import { render, screen, fireEvent, waitFor } from "@/utils/test-utils" import { ContextManagementSettings } from "../ContextManagementSettings" +import { vscode } from "@/utils/vscode" // Mock the translation hook vi.mock("@/hooks/useAppTranslation", () => ({ @@ -47,8 +48,16 @@ vi.mock("@/components/ui", () => ({ {children} ), - Select: ({ children, ...props }: any) => ( -
+ Select: ({ children, value, onValueChange, ...props }: any) => ( +
+ {/* Hidden trigger lets tests drive profile selection deterministically: + set data-next-value on the button, then click it. */} +
), @@ -391,6 +400,26 @@ describe("ContextManagementSettings", () => { render() expect(screen.getByText("75%")).toBeInTheDocument() }) + + // Case 4: profileThresholds must buffer through cachedState, not persist before Save. + it("buffers profile threshold changes without persisting before Save", () => { + const mockSetCachedStateField = vitest.fn() + const props = { ...autoCondenseProps, setCachedStateField: mockSetCachedStateField } + render() + + // Select a non-default profile so the slider edits profileThresholds. + const profileTrigger = screen.getByTestId("threshold-profile-change") + profileTrigger.setAttribute("data-next-value", "config-1") + fireEvent.click(profileTrigger) + + // Move the threshold slider for that profile. + const slider = screen.getByTestId("condense-threshold-slider") + slider.focus() + fireEvent.keyDown(slider, { key: "ArrowRight" }) + + expect(mockSetCachedStateField).toHaveBeenCalledWith("profileThresholds", { "config-1": 76 }) + expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "updateSettings" })) + }) }) it("handles boundary values for sliders", () => { diff --git a/webview-ui/src/components/settings/__tests__/PromptsSettings.spec.tsx b/webview-ui/src/components/settings/__tests__/PromptsSettings.spec.tsx new file mode 100644 index 0000000000..7676f201be --- /dev/null +++ b/webview-ui/src/components/settings/__tests__/PromptsSettings.spec.tsx @@ -0,0 +1,62 @@ +// npx vitest src/components/settings/__tests__/PromptsSettings.spec.tsx + +import { render, screen, fireEvent } from "@/utils/test-utils" + +import PromptsSettings from "../PromptsSettings" +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 }), +})) + +// PromptsSettings falls back to context when props are absent; we always pass +// props here (the SettingsView wiring), so the context values are inert. +vi.mock("@src/context/ExtensionStateContext", () => ({ + useExtensionState: () => ({ + listApiConfigMeta: [], + enhancementApiConfigId: "", + setEnhancementApiConfigId: vi.fn(), + includeTaskHistoryInEnhance: true, + setIncludeTaskHistoryInEnhance: vi.fn(), + }), +})) + +const renderSettings = (overrides = {}) => { + const setIncludeTaskHistoryInEnhance = vi.fn() + const props = { + customSupportPrompts: {}, + setCustomSupportPrompts: vi.fn(), + includeTaskHistoryInEnhance: true, + setIncludeTaskHistoryInEnhance, + ...overrides, + } + render() + return { setIncludeTaskHistoryInEnhance } +} + +describe("PromptsSettings - Save/Discard contract", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + // Case 5: includeTaskHistoryInEnhance must buffer, not persist before Save. + it("buffers includeTaskHistoryInEnhance toggle without persisting before Save", () => { + const { setIncludeTaskHistoryInEnhance } = renderSettings({ includeTaskHistoryInEnhance: true }) + + // The ENHANCE support option is active by default, so the checkbox is present. + const checkbox = screen + .getByText("prompts:supportPrompts.enhance.includeTaskHistory") + .closest("label")! + .querySelector('input[type="checkbox"]')! + fireEvent.click(checkbox) + + expect(setIncludeTaskHistoryInEnhance).toHaveBeenCalledWith(false) + expect(vscode.postMessage).not.toHaveBeenCalledWith(expect.objectContaining({ type: "updateSettings" })) + }) +}) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx index 6434eadb23..b55a65bfe4 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.spec.tsx @@ -629,13 +629,13 @@ describe("SettingsView - Allowed Commands", () => { // Verify command was added expect(within(content).getByText("npm test")).toBeInTheDocument() - // Verify VSCode message was sent - expect(vscode.postMessage).toHaveBeenCalledWith({ - type: "updateSettings", - updatedSettings: { - allowedCommands: ["npm test"], - }, - }) + // Adding a command must NOT persist before Save; it only buffers in cachedState. + expect(vscode.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ allowedCommands: ["npm test"] }), + }), + ) }) it("removes command from the list", () => { @@ -663,13 +663,13 @@ describe("SettingsView - Allowed Commands", () => { // Verify command was removed expect(within(content).queryByText("npm test")).not.toBeInTheDocument() - // Verify VSCode message was sent - expect(vscode.postMessage).toHaveBeenLastCalledWith({ - type: "updateSettings", - updatedSettings: { - allowedCommands: [], - }, - }) + // Removing a command must NOT persist before Save; it only buffers in cachedState. + expect(vscode.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ allowedCommands: expect.anything() }), + }), + ) }) describe("SettingsView - Tab Navigation", () => { @@ -776,4 +776,39 @@ describe("SettingsView - Duplicate Commands", () => { }), ) }) + + it("does not persist allowed commands when discarding unsaved changes", () => { + // Render once and get the activateTab helper + const { activateTab, getSettingsContent } = renderSettingsView() + + // Activate the autoApprove tab + activateTab("autoApprove") + + const content = getSettingsContent() + // Enable always allow execute + const executeCheckbox = within(content).getByTestId("always-allow-execute-toggle") + fireEvent.click(executeCheckbox) + + // Add a command (buffers into cachedState, must not persist yet) + const input = within(content).getByTestId("command-input") + fireEvent.change(input, { target: { value: "npm test" } }) + const addButton = within(content).getByTestId("add-command-button") + fireEvent.click(addButton) + + // Click Done, which opens the unsaved-changes dialog since a change is detected + const doneButton = screen.getByText("settings:common.done") + fireEvent.click(doneButton) + + // Confirm discard + const discardButton = screen.getByTestId("alert-dialog-action") + fireEvent.click(discardButton) + + // Discarding must never persist the buffered command edit + expect(vscode.postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ allowedCommands: ["npm test"] }), + }), + ) + }) }) From c6dacd468ee112c2b5be128887656bca5a01098d Mon Sep 17 00:00:00 2001 From: Junyong Park Date: Sat, 11 Jul 2026 00:41:18 +0900 Subject: [PATCH 2/2] test(settings): cover buffered MCP enablement --- .../components/mcp/__tests__/McpView.spec.tsx | 77 +++++++++++++++++++ .../SettingsView.unsaved-changes.spec.tsx | 46 +++++++++-- 2 files changed, 116 insertions(+), 7 deletions(-) create mode 100644 webview-ui/src/components/mcp/__tests__/McpView.spec.tsx diff --git a/webview-ui/src/components/mcp/__tests__/McpView.spec.tsx b/webview-ui/src/components/mcp/__tests__/McpView.spec.tsx new file mode 100644 index 0000000000..957797d1e9 --- /dev/null +++ b/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 }) =>
{children}
, +})) + +vi.mock("@src/components/settings/SectionHeader", () => ({ + SectionHeader: ({ children }: { children: React.ReactNode }) =>

{children}

, +})) + +vi.mock("@src/components/ui", () => ({ + Button: ({ children, onClick }: { children: React.ReactNode; onClick: () => void }) => ( + + ), + Dialog: ({ children }: { children: React.ReactNode }) =>
{children}
, + DialogContent: ({ children }: { children: React.ReactNode }) =>
{children}
, + DialogHeader: ({ children }: { children: React.ReactNode }) =>
{children}
, + DialogTitle: ({ children }: { children: React.ReactNode }) =>
{children}
, + DialogDescription: ({ children }: { children: React.ReactNode }) =>
{children}
, + DialogFooter: ({ children }: { children: React.ReactNode }) =>
{children}
, + ToggleSwitch: () => null, + StandardTooltip: ({ children }: { children: React.ReactNode }) => <>{children}, +})) + +vi.mock("../McpEnabledToggle", () => ({ + default: ({ + mcpEnabled, + setMcpEnabled: setEnabled, + }: { + mcpEnabled: boolean + setMcpEnabled?: (value: boolean) => void + }) => ( + + ), +})) + +describe("McpView", () => { + beforeEach(() => { + vi.clearAllMocks() + }) + + it("uses extension state when no buffered value is provided", () => { + render() + + expect(screen.getByTestId("mcp-enabled-toggle")).toHaveTextContent("false") + }) + + it("uses and updates the buffered value when controlled", () => { + render() + + fireEvent.click(screen.getByTestId("mcp-enabled-toggle")) + + expect(setMcpEnabled).toHaveBeenCalledWith(false) + }) +}) diff --git a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx index 567838cc30..88428a077d 100644 --- a/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx +++ b/webview-ui/src/components/settings/__tests__/SettingsView.unsaved-changes.spec.tsx @@ -4,13 +4,9 @@ import { QueryClient, QueryClientProvider } from "@tanstack/react-query" import React from "react" import SettingsView from "../SettingsView" +import { vscode } from "@src/utils/vscode" -// Mock vscode API -const mockPostMessage = vi.fn() -const mockVscode = { - postMessage: mockPostMessage, -} -;(global as any).acquireVsCodeApi = () => mockVscode +const postMessage = vi.spyOn(vscode, "postMessage").mockImplementation(() => {}) // Mock the extension state context vi.mock("@src/context/ExtensionStateContext", () => ({ @@ -170,7 +166,14 @@ vi.mock("@src/components/modes/ModesView", () => ({ })) vi.mock("@src/components/mcp/McpView", () => ({ - default: () => null, + default: ({ mcpEnabled, setMcpEnabled }: any) => ( + + ), })) // Mock Tab components @@ -596,4 +599,33 @@ describe("SettingsView - Unsaved Changes Detection", () => { // No dialog should appear expect(screen.queryByText("settings:unsavedChangesDialog.title")).not.toBeInTheDocument() }) + + it("buffers MCP enablement until Save", async () => { + render( + + + , + ) + + const toggle = await screen.findByTestId("mcp-enabled-toggle") + fireEvent.click(toggle) + + await waitFor(() => expect(toggle).toHaveAttribute("data-mcp-enabled", "true")) + expect(postMessage).not.toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ mcpEnabled: true }), + }), + ) + + await waitFor(() => expect(screen.getByTestId("save-button")).toBeEnabled()) + fireEvent.click(screen.getByTestId("save-button")) + + expect(postMessage).toHaveBeenCalledWith( + expect.objectContaining({ + type: "updateSettings", + updatedSettings: expect.objectContaining({ mcpEnabled: true }), + }), + ) + }) })