-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix: auto-approval settings now properly apply during active tasks #8642
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -279,6 +279,89 @@ export async function presentAssistantMessage(cline: Task) { | |
| progressStatus?: ToolProgressStatus, | ||
| isProtected?: boolean, | ||
| ) => { | ||
| // Check auto-approval settings before asking user | ||
| const state = await cline.providerRef.deref()?.getState() | ||
|
|
||
| // Check if we should auto-approve based on tool type and settings | ||
| if (state) { | ||
| let shouldAutoApprove = false | ||
|
|
||
| // Handle tool approval | ||
| if (type === "tool" && partialMessage) { | ||
| try { | ||
| const toolInfo = JSON.parse(partialMessage) | ||
| const toolName = toolInfo.tool || block.name | ||
|
|
||
| // Check for read-only file operations | ||
| if ( | ||
| toolName === "readFile" || | ||
| toolName === "list_files" || | ||
| toolName === "search_files" || | ||
| toolName === "list_code_definition_names" | ||
| ) { | ||
| if (toolInfo.isOutsideWorkspace) { | ||
| shouldAutoApprove = state.alwaysAllowReadOnlyOutsideWorkspace ?? false | ||
|
Comment on lines
+302
to
+303
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Logic Error: The code checks Looking at how
Neither includes an
The workspace context needs to be determined differently, likely by examining the tool's parameters or the file path being operated on. |
||
| } else { | ||
| shouldAutoApprove = state.alwaysAllowReadOnly ?? false | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type Safety Issue: State properties used here are not defined in the state type The code references several auto-approval properties on
These properties should either:
Without these properties being properly typed and available in the state object, TypeScript may not catch errors and the feature won't work at runtime. |
||
| } | ||
| } | ||
| // Check for write file operations | ||
| else if ( | ||
| toolName === "writeFile" || | ||
| toolName === "write_to_file" || | ||
| toolName === "apply_diff" || | ||
| toolName === "insert_content" || | ||
| toolName === "search_and_replace" | ||
| ) { | ||
| if (isProtected) { | ||
| shouldAutoApprove = state.alwaysAllowWriteProtected ?? false | ||
| } else if (toolInfo.isOutsideWorkspace) { | ||
| shouldAutoApprove = state.alwaysAllowWriteOutsideWorkspace ?? false | ||
| } else { | ||
| shouldAutoApprove = state.alwaysAllowWrite ?? false | ||
| } | ||
| } | ||
| // Check for command execution | ||
| else if (toolName === "execute_command") { | ||
| shouldAutoApprove = state.alwaysAllowExecute ?? false | ||
| } | ||
| // Check for browser actions | ||
| else if (toolName === "browser_action") { | ||
| shouldAutoApprove = state.alwaysAllowBrowser ?? false | ||
| } | ||
| // Check for follow-up questions | ||
| else if (toolName === "ask_followup_question") { | ||
| shouldAutoApprove = state.alwaysAllowFollowupQuestions ?? false | ||
| } | ||
| // Check for todo list updates | ||
| else if (toolName === "update_todo_list") { | ||
| shouldAutoApprove = state.alwaysAllowUpdateTodoList ?? false | ||
| } | ||
| } catch (error) { | ||
| // If parsing fails or any error occurs, fall through to normal approval flow | ||
| console.debug("Error checking auto-approval settings for tool:", error) | ||
| } | ||
| } | ||
| // Handle command approval | ||
| else if (type === "command") { | ||
| shouldAutoApprove = state.alwaysAllowExecute ?? false | ||
| } | ||
| // Handle browser action launch | ||
| else if (type === "browser_action_launch") { | ||
| shouldAutoApprove = state.alwaysAllowBrowser ?? false | ||
| } | ||
| // Handle MCP server usage | ||
| else if (type === "use_mcp_server") { | ||
| shouldAutoApprove = state.alwaysAllowMcp ?? false | ||
| } | ||
|
|
||
| if (shouldAutoApprove) { | ||
| // Auto-approve without asking user | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| // Normal approval flow - ask the user | ||
| const { response, text, images } = await cline.ask( | ||
| type, | ||
| partialMessage, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Critical Bug: Tool name mismatch will prevent auto-approval from ever working
The tool names checked here use camelCase (e.g.,
readFile,writeFile) but the actual tool names in this codebase use snake_case (e.g.,read_file,write_to_file).Evidence from the same file:
case "execute_command":case "read_file":case "write_to_file":case "apply_diff":case "insert_content":case "search_and_replace":This means the conditions will never match, and auto-approval will never trigger. All tool names in the checks should use snake_case to match the actual tool names used throughout the codebase.