From fd4c06487e350b1b7961ae559820547ed0834b43 Mon Sep 17 00:00:00 2001 From: Eric Wheeler Date: Tue, 22 Jul 2025 02:23:28 -0700 Subject: [PATCH 1/3] fix: allow auto-approve checkbox to be toggled at any time During model responses, users need the ability to disable auto-approval for safety when they realize the model is about to perform unintended actions. The checkbox was previously disabled when no options were selected or when the menu was open, preventing users from turning off auto-approval when needed most. - Remove disabled state from main auto-approve checkbox - Simplify toggle logic to always allow state changes - Add memoization to reduce unnecessary re-renders that cause flickering - Update tests to reflect new always-enabled behavior This ensures users can always disable auto-approval for safety, especially during time-critical situations when the model is actively responding. Fixes: #6060 Signed-off-by: Eric Wheeler --- .../src/components/chat/AutoApproveMenu.tsx | 19 +++++-------------- .../chat/__tests__/AutoApproveMenu.spec.tsx | 9 ++++++--- 2 files changed, 11 insertions(+), 17 deletions(-) diff --git a/webview-ui/src/components/chat/AutoApproveMenu.tsx b/webview-ui/src/components/chat/AutoApproveMenu.tsx index 2e987b7c49..bd8769f0ca 100644 --- a/webview-ui/src/components/chat/AutoApproveMenu.tsx +++ b/webview-ui/src/components/chat/AutoApproveMenu.tsx @@ -1,4 +1,4 @@ -import { useCallback, useMemo, useState } from "react" +import { memo, useCallback, useMemo, useState } from "react" import { Trans } from "react-i18next" import { VSCodeCheckbox, VSCodeLink, VSCodeTextField } from "@vscode/webview-ui-toolkit/react" @@ -129,11 +129,6 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => { setIsExpanded((prev) => !prev) }, []) - // Disable main checkbox while menu is open or no options selected - const isCheckboxDisabled = useMemo(() => { - return !hasEnabledOptions || isExpanded - }, [hasEnabledOptions, isExpanded]) - const enabledActionsList = Object.entries(toggles) .filter(([_key, value]) => !!value) .map(([key]) => t(autoApproveSettingsConfig[key as AutoApproveSetting].labelKey)) @@ -178,19 +173,15 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => { content={!hasEnabledOptions ? t("chat:autoApprove.selectOptionsFirst") : undefined}> { - if (hasEnabledOptions) { - const newValue = !(autoApprovalEnabled ?? false) - setAutoApprovalEnabled(newValue) - vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue }) - } - // If no options enabled, do nothing + const newValue = !(autoApprovalEnabled ?? false) + setAutoApprovalEnabled(newValue) + vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue }) }} /> @@ -290,4 +281,4 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => { ) } -export default AutoApproveMenu +export default memo(AutoApproveMenu) diff --git a/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx b/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx index 185e5eeec6..de7545d250 100644 --- a/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx @@ -115,7 +115,7 @@ describe("AutoApproveMenu", () => { expect(screen.getByText("Read-only operations")).toBeInTheDocument() }) - it("should not allow toggling master checkbox when no options are selected", () => { + it("should allow toggling master checkbox even when no options are selected", () => { ;(useExtensionState as ReturnType).mockReturnValue({ ...defaultExtensionState, autoApprovalEnabled: false, @@ -128,8 +128,11 @@ describe("AutoApproveMenu", () => { const masterCheckbox = screen.getByRole("checkbox") fireEvent.click(masterCheckbox) - // Should not send any message since no options are selected - expect(mockPostMessage).not.toHaveBeenCalled() + // Should send message to toggle auto-approval even when no options are selected + expect(mockPostMessage).toHaveBeenCalledWith({ + type: "autoApprovalEnabled", + bool: true, + }) }) it("should toggle master checkbox when options are selected", () => { From 484d3a80be957cf2bfdbb685d727e084eb0d8812 Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Thu, 24 Jul 2025 17:06:32 -0500 Subject: [PATCH 2/3] fix: revert change allowing checkbox toggle without options selected - Keep the React.memo optimization to prevent flickering - Keep the ability to toggle during model responses - Restore the check for hasEnabledOptions before allowing toggle - Update test to reflect the original behavior --- webview-ui/src/components/chat/AutoApproveMenu.tsx | 9 ++++++--- .../components/chat/__tests__/AutoApproveMenu.spec.tsx | 9 +++------ 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/webview-ui/src/components/chat/AutoApproveMenu.tsx b/webview-ui/src/components/chat/AutoApproveMenu.tsx index bd8769f0ca..de12352bc8 100644 --- a/webview-ui/src/components/chat/AutoApproveMenu.tsx +++ b/webview-ui/src/components/chat/AutoApproveMenu.tsx @@ -179,9 +179,12 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => { : t("chat:autoApprove.disabledAriaLabel") } onChange={() => { - const newValue = !(autoApprovalEnabled ?? false) - setAutoApprovalEnabled(newValue) - vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue }) + if (hasEnabledOptions) { + const newValue = !(autoApprovalEnabled ?? false) + setAutoApprovalEnabled(newValue) + vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue }) + } + // If no options enabled, do nothing }} /> diff --git a/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx b/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx index de7545d250..185e5eeec6 100644 --- a/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx +++ b/webview-ui/src/components/chat/__tests__/AutoApproveMenu.spec.tsx @@ -115,7 +115,7 @@ describe("AutoApproveMenu", () => { expect(screen.getByText("Read-only operations")).toBeInTheDocument() }) - it("should allow toggling master checkbox even when no options are selected", () => { + it("should not allow toggling master checkbox when no options are selected", () => { ;(useExtensionState as ReturnType).mockReturnValue({ ...defaultExtensionState, autoApprovalEnabled: false, @@ -128,11 +128,8 @@ describe("AutoApproveMenu", () => { const masterCheckbox = screen.getByRole("checkbox") fireEvent.click(masterCheckbox) - // Should send message to toggle auto-approval even when no options are selected - expect(mockPostMessage).toHaveBeenCalledWith({ - type: "autoApprovalEnabled", - bool: true, - }) + // Should not send any message since no options are selected + expect(mockPostMessage).not.toHaveBeenCalled() }) it("should toggle master checkbox when options are selected", () => { From b9990759525fdca726703d7e66bfde65ddc1ca06 Mon Sep 17 00:00:00 2001 From: Daniel Riccio Date: Thu, 24 Jul 2025 17:16:20 -0500 Subject: [PATCH 3/3] fix: add disabled attribute to prevent checkbox interaction when no options selected --- webview-ui/src/components/chat/AutoApproveMenu.tsx | 1 + 1 file changed, 1 insertion(+) diff --git a/webview-ui/src/components/chat/AutoApproveMenu.tsx b/webview-ui/src/components/chat/AutoApproveMenu.tsx index de12352bc8..0feafae15d 100644 --- a/webview-ui/src/components/chat/AutoApproveMenu.tsx +++ b/webview-ui/src/components/chat/AutoApproveMenu.tsx @@ -173,6 +173,7 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => { content={!hasEnabledOptions ? t("chat:autoApprove.selectOptionsFirst") : undefined}>