Fixes to the auto approve menu

This commit is contained in:
Matt Rubens
2025-01-16 03:37:23 -05:00
parent 6e3919f5e2
commit ee344facda
8 changed files with 56 additions and 118 deletions

View File

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

View File

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

View File

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

View File

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

View File

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

View File

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

View File

@@ -144,104 +144,6 @@ describe('ChatView - Auto Approval Tests', () => {
jest.clearAllMocks() 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', () => { it('does not auto-approve any actions when autoApprovalEnabled is false', () => {
render( render(
<ExtensionStateContextProvider> <ExtensionStateContextProvider>

View File

@@ -124,16 +124,6 @@ export const ExtensionStateContextProvider: React.FC<{ children: React.ReactNode
switch (message.type) { switch (message.type) {
case "state": { case "state": {
const newState = message.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 => ({ setState(prevState => ({
...prevState, ...prevState,
...newState ...newState