Skip to content

Commit 4da326a

Browse files
mrubensmrubens
authored
Merge pull request #384 from RooVetGit/auto_approve_menu_fixes
Fixes to the auto approve menu
2 parents 6e3919f + ee344fa commit 4da326a

File tree

8 files changed

+56
-118
lines changed

8 files changed

+56
-118
lines changed

‎.changeset/curvy-ants-help.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"roo-cline": patch
3+
---
4+
5+
Bug fixes to the auto approve menu

‎src/core/webview/ClineProvider.ts‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ type GlobalStateKey =
9898
| "modeApiConfigs"
9999
| "customPrompts"
100100
| "enhancementApiConfigId"
101+
| "autoApprovalEnabled"
101102

102103
export const GlobalFileNames = {
103104
apiConversationHistory: "api_conversation_history.json",
@@ -875,6 +876,10 @@ export class ClineProvider implements vscode.WebviewViewProvider {
875876
await this.updateGlobalState("enhancementApiConfigId", message.text)
876877
await this.postStateToWebview()
877878
break
879+
case "autoApprovalEnabled":
880+
await this.updateGlobalState("autoApprovalEnabled", message.bool)
881+
await this.postStateToWebview()
882+
break
878883
case "enhancePrompt":
879884
if (message.text) {
880885
try {

‎src/shared/ExtensionMessage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ export interface ExtensionMessage {
3030
| "requestVsCodeLmModels"
3131
| "updatePrompt"
3232
| "systemPrompt"
33+
| "autoApprovalEnabled"
3334
text?: string
3435
action?:
3536
| "chatButtonClicked"

‎src/shared/WebviewMessage.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ export interface WebviewMessage {
7272
| "getSystemPrompt"
7373
| "systemPrompt"
7474
| "enhancementApiConfigId"
75+
| "autoApprovalEnabled"
7576
text?: string
7677
disabled?: boolean
7778
askResponse?: ClineAskResponse

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

Lines changed: 42 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { VSCodeCheckbox } from "@vscode/webview-ui-toolkit/react"
22
import { useCallback, useState } from "react"
33
import { useExtensionState } from "../../context/ExtensionStateContext"
4+
import { vscode } from "../../utils/vscode"
45

56
interface AutoApproveAction {
67
id: string
@@ -50,7 +51,7 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
5051
},
5152
{
5253
id: "executeCommands",
53-
label: "Execute safe commands",
54+
label: "Execute approved commands",
5455
shortName: "Commands",
5556
enabled: alwaysAllowExecute ?? false,
5657
description:
@@ -89,12 +90,41 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
8990
.join(", ")
9091

9192
// Individual checkbox handlers - each one only updates its own state
92-
const handleReadOnlyChange = useCallback(() => setAlwaysAllowReadOnly(!(alwaysAllowReadOnly ?? false)), [alwaysAllowReadOnly, setAlwaysAllowReadOnly])
93-
const handleWriteChange = useCallback(() => setAlwaysAllowWrite(!(alwaysAllowWrite ?? false)), [alwaysAllowWrite, setAlwaysAllowWrite])
94-
const handleExecuteChange = useCallback(() => setAlwaysAllowExecute(!(alwaysAllowExecute ?? false)), [alwaysAllowExecute, setAlwaysAllowExecute])
95-
const handleBrowserChange = useCallback(() => setAlwaysAllowBrowser(!(alwaysAllowBrowser ?? false)), [alwaysAllowBrowser, setAlwaysAllowBrowser])
96-
const handleMcpChange = useCallback(() => setAlwaysAllowMcp(!(alwaysAllowMcp ?? false)), [alwaysAllowMcp, setAlwaysAllowMcp])
97-
const handleRetryChange = useCallback(() => setAlwaysApproveResubmit(!(alwaysApproveResubmit ?? false)), [alwaysApproveResubmit, setAlwaysApproveResubmit])
93+
const handleReadOnlyChange = useCallback(() => {
94+
const newValue = !(alwaysAllowReadOnly ?? false)
95+
setAlwaysAllowReadOnly(newValue)
96+
vscode.postMessage({ type: "alwaysAllowReadOnly", bool: newValue })
97+
}, [alwaysAllowReadOnly, setAlwaysAllowReadOnly])
98+
99+
const handleWriteChange = useCallback(() => {
100+
const newValue = !(alwaysAllowWrite ?? false)
101+
setAlwaysAllowWrite(newValue)
102+
vscode.postMessage({ type: "alwaysAllowWrite", bool: newValue })
103+
}, [alwaysAllowWrite, setAlwaysAllowWrite])
104+
105+
const handleExecuteChange = useCallback(() => {
106+
const newValue = !(alwaysAllowExecute ?? false)
107+
setAlwaysAllowExecute(newValue)
108+
vscode.postMessage({ type: "alwaysAllowExecute", bool: newValue })
109+
}, [alwaysAllowExecute, setAlwaysAllowExecute])
110+
111+
const handleBrowserChange = useCallback(() => {
112+
const newValue = !(alwaysAllowBrowser ?? false)
113+
setAlwaysAllowBrowser(newValue)
114+
vscode.postMessage({ type: "alwaysAllowBrowser", bool: newValue })
115+
}, [alwaysAllowBrowser, setAlwaysAllowBrowser])
116+
117+
const handleMcpChange = useCallback(() => {
118+
const newValue = !(alwaysAllowMcp ?? false)
119+
setAlwaysAllowMcp(newValue)
120+
vscode.postMessage({ type: "alwaysAllowMcp", bool: newValue })
121+
}, [alwaysAllowMcp, setAlwaysAllowMcp])
122+
123+
const handleRetryChange = useCallback(() => {
124+
const newValue = !(alwaysApproveResubmit ?? false)
125+
setAlwaysApproveResubmit(newValue)
126+
vscode.postMessage({ type: "alwaysApproveResubmit", bool: newValue })
127+
}, [alwaysApproveResubmit, setAlwaysApproveResubmit])
98128

99129
// Map action IDs to their specific handlers
100130
const actionHandlers: Record<AutoApproveAction['id'], () => void> = {
@@ -129,7 +159,11 @@ const AutoApproveMenu = ({ style }: AutoApproveMenuProps) => {
129159
<div onClick={(e) => e.stopPropagation()}>
130160
<VSCodeCheckbox
131161
checked={autoApprovalEnabled ?? false}
132-
onChange={() => setAutoApprovalEnabled(!(autoApprovalEnabled ?? false))}
162+
onChange={() => {
163+
const newValue = !(autoApprovalEnabled ?? false)
164+
setAutoApprovalEnabled(newValue)
165+
vscode.postMessage({ type: "autoApprovalEnabled", bool: newValue })
166+
}}
133167
/>
134168
</div>
135169
<div style={{

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,7 +109,7 @@ describe("AutoApproveMenu", () => {
109109
// Verify menu items are visible
110110
expect(screen.getByText("Read files and directories")).toBeInTheDocument()
111111
expect(screen.getByText("Edit files")).toBeInTheDocument()
112-
expect(screen.getByText("Execute safe commands")).toBeInTheDocument()
112+
expect(screen.getByText("Execute approved commands")).toBeInTheDocument()
113113
expect(screen.getByText("Use the browser")).toBeInTheDocument()
114114
expect(screen.getByText("Use MCP servers")).toBeInTheDocument()
115115
expect(screen.getByText("Retry failed requests")).toBeInTheDocument()
@@ -139,7 +139,7 @@ describe("AutoApproveMenu", () => {
139139
expect(defaultMockState.setAlwaysAllowWrite).toHaveBeenCalledWith(true)
140140

141141
// Click execute commands checkbox
142-
fireEvent.click(screen.getByText("Execute safe commands"))
142+
fireEvent.click(screen.getByText("Execute approved commands"))
143143
expect(defaultMockState.setAlwaysAllowExecute).toHaveBeenCalledWith(true)
144144
})
145145

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

Lines changed: 0 additions & 98 deletions
Original file line numberDiff line numberDiff line change
@@ -144,104 +144,6 @@ describe('ChatView - Auto Approval Tests', () => {
144144
jest.clearAllMocks()
145145
})
146146

147-
it('defaults autoApprovalEnabled to true if any individual auto-approval flags are true', async () => {
148-
render(
149-
<ExtensionStateContextProvider>
150-
<ChatView
151-
isHidden={false}
152-
showAnnouncement={false}
153-
hideAnnouncement={() => {}}
154-
showHistoryView={() => {}}
155-
/>
156-
</ExtensionStateContextProvider>
157-
)
158-
159-
// Test cases with different individual flags
160-
const testCases = [
161-
{ alwaysAllowBrowser: true },
162-
{ alwaysAllowReadOnly: true },
163-
{ alwaysAllowWrite: true },
164-
{ alwaysAllowExecute: true },
165-
{ alwaysAllowMcp: true }
166-
]
167-
168-
for (const flags of testCases) {
169-
// Reset state
170-
mockPostMessage({
171-
...flags,
172-
clineMessages: []
173-
})
174-
175-
// Send an action that should be auto-approved
176-
mockPostMessage({
177-
...flags,
178-
clineMessages: [
179-
{
180-
type: 'say',
181-
say: 'task',
182-
ts: Date.now() - 2000,
183-
text: 'Initial task'
184-
},
185-
{
186-
type: 'ask',
187-
ask: flags.alwaysAllowBrowser ? 'browser_action_launch' :
188-
flags.alwaysAllowReadOnly ? 'tool' :
189-
flags.alwaysAllowWrite ? 'tool' :
190-
flags.alwaysAllowExecute ? 'command' :
191-
'use_mcp_server',
192-
ts: Date.now(),
193-
text: flags.alwaysAllowBrowser ? JSON.stringify({ action: 'launch', url: 'http://example.com' }) :
194-
flags.alwaysAllowReadOnly ? JSON.stringify({ tool: 'readFile', path: 'test.txt' }) :
195-
flags.alwaysAllowWrite ? JSON.stringify({ tool: 'editedExistingFile', path: 'test.txt' }) :
196-
flags.alwaysAllowExecute ? 'npm test' :
197-
JSON.stringify({ type: 'use_mcp_tool', serverName: 'test', toolName: 'test' }),
198-
partial: false
199-
}
200-
]
201-
})
202-
203-
// Wait for auto-approval
204-
await waitFor(() => {
205-
expect(vscode.postMessage).toHaveBeenCalledWith({
206-
type: 'askResponse',
207-
askResponse: 'yesButtonClicked'
208-
})
209-
})
210-
}
211-
212-
// Verify no auto-approval when no flags are true
213-
jest.clearAllMocks()
214-
mockPostMessage({
215-
alwaysAllowBrowser: false,
216-
alwaysAllowReadOnly: false,
217-
alwaysAllowWrite: false,
218-
alwaysAllowExecute: false,
219-
alwaysAllowMcp: false,
220-
clineMessages: [
221-
{
222-
type: 'say',
223-
say: 'task',
224-
ts: Date.now() - 2000,
225-
text: 'Initial task'
226-
},
227-
{
228-
type: 'ask',
229-
ask: 'browser_action_launch',
230-
ts: Date.now(),
231-
text: JSON.stringify({ action: 'launch', url: 'http://example.com' }),
232-
partial: false
233-
}
234-
]
235-
})
236-
237-
// Wait a bit to ensure no auto-approval happens
238-
await new Promise(resolve => setTimeout(resolve, 100))
239-
expect(vscode.postMessage).not.toHaveBeenCalledWith({
240-
type: 'askResponse',
241-
askResponse: 'yesButtonClicked'
242-
})
243-
})
244-
245147
it('does not auto-approve any actions when autoApprovalEnabled is false', () => {
246148
render(
247149
<ExtensionStateContextProvider>

‎webview-ui/src/context/ExtensionStateContext.tsx‎

Lines changed: 0 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -124,16 +124,6 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
124124
switch (message.type) {
125125
case "state": {
126126
const newState = message.state!
127-
// Set autoApprovalEnabled to true if undefined and any individual flag is true
128-
if (newState.autoApprovalEnabled === undefined) {
129-
newState.autoApprovalEnabled = !!(
130-
newState.alwaysAllowBrowser ||
131-
newState.alwaysAllowReadOnly ||
132-
newState.alwaysAllowWrite ||
133-
newState.alwaysAllowExecute ||
134-
newState.alwaysAllowMcp ||
135-
newState.alwaysApproveResubmit)
136-
}
137127
setState(prevState => ({
138128
...prevState,
139129
...newState

0 commit comments

Comments
 (0)