Merge pull request #384 from RooVetGit/auto_approve_menu_fixes

Fixes to the auto approve menu

.changeset/curvy-ants-help.md

@@ -0,0 +1,5 @@
+---
+"roo-cline": patch
+---
+
+Bug fixes to the auto approve menu

src/core/webview/ClineProvider.ts

@@ -98,6 +98,7 @@ type GlobalStateKey =
 	| "modeApiConfigs"
 	| "customPrompts"
 	| "enhancementApiConfigId"
+	| "autoApprovalEnabled"
 
 export const GlobalFileNames = {
 	apiConversationHistory: "api_conversation_history.json",
@@ -875,6 +876,10 @@ export class ClineProvider implements vscode.WebviewViewProvider {
 						await this.updateGlobalState("enhancementApiConfigId", message.text)
 						await this.postStateToWebview()
 						break
+					case "autoApprovalEnabled":
+						await this.updateGlobalState("autoApprovalEnabled", message.bool)
+						await this.postStateToWebview()
+						break
 					case "enhancePrompt":
 						if (message.text) {
 							try {

src/shared/ExtensionMessage.ts

@@ -30,6 +30,7 @@ export interface ExtensionMessage {
 		| "requestVsCodeLmModels"
 		| "updatePrompt"
 		| "systemPrompt"
+		| "autoApprovalEnabled"
 	text?: string
 	action?:
 		| "chatButtonClicked"

src/shared/WebviewMessage.ts

@@ -72,6 +72,7 @@ export interface WebviewMessage {
 		| "getSystemPrompt"
 		| "systemPrompt"
 		| "enhancementApiConfigId"
+		| "autoApprovalEnabled"
 	text?: string
 	disabled?: boolean
 	askResponse?: ClineAskResponse

webview-ui/src/components/chat/AutoApproveMenu.tsx

@@ -1,6 +1,7 @@
 import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
 import { useCallback, useState } from "react"
 import { useExtensionState } from "../../context/ExtensionStateContext"
+import { vscode } from "../../utils/vscode"
 
 interface AutoApproveAction {
 	id: string
@@ -50,7 +51,7 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
 		},
 		{
 			id: "executeCommands",
-			label: "Execute safe commands",
+			label: "Execute approved commands",
 			shortName: "Commands",
 			enabled: alwaysAllowExecute ?? false,
 			description:
@@ -89,12 +90,41 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
 		.join(", ")
 
 	// Individual checkbox handlers - each one only updates its own state
-	const handleReadOnlyChange = useCallback(() => setAlwaysAllowReadOnly(!(alwaysAllowReadOnly ?? false)), [alwaysAllowReadOnly, setAlwaysAllowReadOnly])
-	const handleWriteChange = useCallback(() => setAlwaysAllowWrite(!(alwaysAllowWrite ?? false)), [alwaysAllowWrite, setAlwaysAllowWrite])
-	const handleExecuteChange = useCallback(() => setAlwaysAllowExecute(!(alwaysAllowExecute ?? false)), [alwaysAllowExecute, setAlwaysAllowExecute])
-	const handleBrowserChange = useCallback(() => setAlwaysAllowBrowser(!(alwaysAllowBrowser ?? false)), [alwaysAllowBrowser, setAlwaysAllowBrowser])
-	const handleMcpChange = useCallback(() => setAlwaysAllowMcp(!(alwaysAllowMcp ?? false)), [alwaysAllowMcp, setAlwaysAllowMcp])
-	const handleRetryChange = useCallback(() => setAlwaysApproveResubmit(!(alwaysApproveResubmit ?? false)), [alwaysApproveResubmit, setAlwaysApproveResubmit])
+	const handleReadOnlyChange = useCallback(() => {
+		const newValue = !(alwaysAllowReadOnly ?? false)
+		setAlwaysAllowReadOnly(newValue)
+		vscode.postMessage({ type: "alwaysAllowReadOnly", bool: newValue })
+	}, [alwaysAllowReadOnly, setAlwaysAllowReadOnly])
+
+	const handleWriteChange = useCallback(() => {
+		const newValue = !(alwaysAllowWrite ?? false)
+		setAlwaysAllowWrite(newValue)
+		vscode.postMessage({ type: "alwaysAllowWrite", bool: newValue })
+	}, [alwaysAllowWrite, setAlwaysAllowWrite])
+
+	const handleExecuteChange = useCallback(() => {
+		const newValue = !(alwaysAllowExecute ?? false)
+		setAlwaysAllowExecute(newValue)
+		vscode.postMessage({ type: "alwaysAllowExecute", bool: newValue })
+	}, [alwaysAllowExecute, setAlwaysAllowExecute])
+
+	const handleBrowserChange = useCallback(() => {
+		const newValue = !(alwaysAllowBrowser ?? false)
+		setAlwaysAllowBrowser(newValue)
+		vscode.postMessage({ type: "alwaysAllowBrowser", bool: newValue })
+	}, [alwaysAllowBrowser, setAlwaysAllowBrowser])
+
+	const handleMcpChange = useCallback(() => {
+		const newValue = !(alwaysAllowMcp ?? false)
+		setAlwaysAllowMcp(newValue)
+		vscode.postMessage({ type: "alwaysAllowMcp", bool: newValue })
+	}, [alwaysAllowMcp, setAlwaysAllowMcp])
+
+	const handleRetryChange = useCallback(() => {
+		const newValue = !(alwaysApproveResubmit ?? false)
+		setAlwaysApproveResubmit(newValue)
+		vscode.postMessage({ type: "alwaysApproveResubmit", bool: newValue })
+	}, [alwaysApproveResubmit, setAlwaysApproveResubmit])
 
 	// Map action IDs to their specific handlers
 	const actionHandlers: Record<AutoApproveAction['id'], () => void> = {
@@ -129,7 +159,11 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
 				<div onClick={(e) => e.stopPropagation()}>
 					<VSCodeCheckbox
 						checked={autoApprovalEnabled ?? false}
-						onChange={() => setAutoApprovalEnabled(!(autoApprovalEnabled ?? false))}
+						onChange={() => {
+							const newValue = !(autoApprovalEnabled ?? false)
+							setAutoApprovalEnabled(newValue)
+							vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
+						}}
 					/>
 				</div>
 				<div style={{

webview-ui/src/components/chat/__tests__/AutoApproveMenu.test.tsx

@@ -109,7 +109,7 @@ describe("AutoApproveMenu", () => {
         // Verify menu items are visible
         expect(screen.getByText("Read files and directories")).toBeInTheDocument()
         expect(screen.getByText("Edit files")).toBeInTheDocument()
-        expect(screen.getByText("Execute safe commands")).toBeInTheDocument()
+        expect(screen.getByText("Execute approved commands")).toBeInTheDocument()
         expect(screen.getByText("Use the browser")).toBeInTheDocument()
         expect(screen.getByText("Use MCP servers")).toBeInTheDocument()
         expect(screen.getByText("Retry failed requests")).toBeInTheDocument()
@@ -139,7 +139,7 @@ describe("AutoApproveMenu", () => {
         expect(defaultMockState.setAlwaysAllowWrite).toHaveBeenCalledWith(true)
         
         // Click execute commands checkbox
-        fireEvent.click(screen.getByText("Execute safe commands"))
+        fireEvent.click(screen.getByText("Execute approved commands"))
         expect(defaultMockState.setAlwaysAllowExecute).toHaveBeenCalledWith(true)
     })
 

webview-ui/src/components/chat/__tests__/ChatView.test.tsx

@@ -144,104 +144,6 @@ describe('ChatView - Auto Approval Tests', () => {
     jest.clearAllMocks()
   })
 
-  it('defaults autoApprovalEnabled to true if any individual auto-approval flags are true', async () => {
-    render(
-      <ExtensionStateContextProvider>
-        <ChatView
-          isHidden={false}
-          showAnnouncement={false}
-          hideAnnouncement={() => {}}
-          showHistoryView={() => {}}
-        />
-      </ExtensionStateContextProvider>
-    )
-
-    // Test cases with different individual flags
-    const testCases = [
-      { alwaysAllowBrowser: true },
-      { alwaysAllowReadOnly: true },
-      { alwaysAllowWrite: true },
-      { alwaysAllowExecute: true },
-      { alwaysAllowMcp: true }
-    ]
-
-    for (const flags of testCases) {
-      // Reset state
-      mockPostMessage({
-        ...flags,
-        clineMessages: []
-      })
-
-      // Send an action that should be auto-approved
-      mockPostMessage({
-        ...flags,
-        clineMessages: [
-          {
-            type: 'say',
-            say: 'task',
-            ts: Date.now() - 2000,
-            text: 'Initial task'
-          },
-          {
-            type: 'ask',
-            ask: flags.alwaysAllowBrowser ? 'browser_action_launch' :
-                 flags.alwaysAllowReadOnly ? 'tool' :
-                 flags.alwaysAllowWrite ? 'tool' :
-                 flags.alwaysAllowExecute ? 'command' :
-                 'use_mcp_server',
-            ts: Date.now(),
-            text: flags.alwaysAllowBrowser ? JSON.stringify({ action: 'launch', url: 'http://example.com' }) :
-                  flags.alwaysAllowReadOnly ? JSON.stringify({ tool: 'readFile', path: 'test.txt' }) :
-                  flags.alwaysAllowWrite ? JSON.stringify({ tool: 'editedExistingFile', path: 'test.txt' }) :
-                  flags.alwaysAllowExecute ? 'npm test' :
-                  JSON.stringify({ type: 'use_mcp_tool', serverName: 'test', toolName: 'test' }),
-            partial: false
-          }
-        ]
-      })
-
-      // Wait for auto-approval
-      await waitFor(() => {
-        expect(vscode.postMessage).toHaveBeenCalledWith({
-          type: 'askResponse',
-          askResponse: 'yesButtonClicked'
-        })
-      })
-    }
-
-    // Verify no auto-approval when no flags are true
-    jest.clearAllMocks()
-    mockPostMessage({
-      alwaysAllowBrowser: false,
-      alwaysAllowReadOnly: false,
-      alwaysAllowWrite: false,
-      alwaysAllowExecute: false,
-      alwaysAllowMcp: false,
-      clineMessages: [
-        {
-          type: 'say',
-          say: 'task',
-          ts: Date.now() - 2000,
-          text: 'Initial task'
-        },
-        {
-          type: 'ask',
-          ask: 'browser_action_launch',
-          ts: Date.now(),
-          text: JSON.stringify({ action: 'launch', url: 'http://example.com' }),
-          partial: false
-        }
-      ]
-    })
-
-    // Wait a bit to ensure no auto-approval happens
-    await new Promise(resolve => setTimeout(resolve, 100))
-    expect(vscode.postMessage).not.toHaveBeenCalledWith({
-      type: 'askResponse',
-      askResponse: 'yesButtonClicked'
-    })
-  })
-
   it('does not auto-approve any actions when autoApprovalEnabled is false', () => {
     render(
       <ExtensionStateContextProvider>

webview-ui/src/context/ExtensionStateContext.tsx

@@ -124,16 +124,6 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
 		switch (message.type) {
 			case "state": {
 				const newState = message.state!
-				// Set autoApprovalEnabled to true if undefined and any individual flag is true
-				if (newState.autoApprovalEnabled === undefined) {
-					newState.autoApprovalEnabled = !!(
-						newState.alwaysAllowBrowser ||
-						newState.alwaysAllowReadOnly ||
-						newState.alwaysAllowWrite ||
-						newState.alwaysAllowExecute ||
-						newState.alwaysAllowMcp ||
-						newState.alwaysApproveResubmit)
-				}
 				setState(prevState => ({
 					...prevState,
 					...newState