From 47eaf274d6b688eae7302c3e4b8289b084765755 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Tue, 8 Apr 2025 15:37:10 -0600 Subject: [PATCH] agent: Only require confirmation for batch tool when subset of tool calls require confirmation (#28363) Release Notes: - agent: Only require confirmation for batch tool when subset of tool calls require confirmation --- crates/agent/src/thread.rs | 2 +- crates/agent/src/tool_use.rs | 2 +- crates/assistant_tool/src/assistant_tool.rs | 2 +- crates/assistant_tools/src/bash_tool.rs | 2 +- crates/assistant_tools/src/batch_tool.rs | 13 +++++++++++-- crates/assistant_tools/src/code_symbols_tool.rs | 2 +- crates/assistant_tools/src/copy_path_tool.rs | 2 +- crates/assistant_tools/src/create_directory_tool.rs | 2 +- crates/assistant_tools/src/create_file_tool.rs | 2 +- crates/assistant_tools/src/delete_path_tool.rs | 2 +- crates/assistant_tools/src/diagnostics_tool.rs | 2 +- crates/assistant_tools/src/fetch_tool.rs | 2 +- .../assistant_tools/src/find_replace_file_tool.rs | 2 +- crates/assistant_tools/src/list_directory_tool.rs | 2 +- crates/assistant_tools/src/move_path_tool.rs | 2 +- crates/assistant_tools/src/now_tool.rs | 2 +- crates/assistant_tools/src/open_tool.rs | 2 +- crates/assistant_tools/src/path_search_tool.rs | 2 +- crates/assistant_tools/src/read_file_tool.rs | 2 +- crates/assistant_tools/src/regex_search_tool.rs | 2 +- crates/assistant_tools/src/symbol_info_tool.rs | 2 +- crates/assistant_tools/src/thinking_tool.rs | 2 +- crates/context_server/src/context_server_tool.rs | 2 +- 23 files changed, 33 insertions(+), 24 deletions(-) diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index fd3192bdf0..3b5cd98d09 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -1414,7 +1414,7 @@ impl Thread { for tool_use in pending_tool_uses.iter() { if let Some(tool) = self.tools.tool(&tool_use.name, cx) { - if tool.needs_confirmation() + if tool.needs_confirmation(&tool_use.input, cx) && !AssistantSettings::get_global(cx).always_allow_tool_actions { self.tool_use.confirm_tool_use( diff --git a/crates/agent/src/tool_use.rs b/crates/agent/src/tool_use.rs index b71c0348c3..8fce4ac67a 100644 --- a/crates/agent/src/tool_use.rs +++ b/crates/agent/src/tool_use.rs @@ -201,7 +201,7 @@ impl ToolUseState { let (icon, needs_confirmation) = if let Some(tool) = self.tools.tool(&tool_use.name, cx) { - (tool.icon(), tool.needs_confirmation()) + (tool.icon(), tool.needs_confirmation(&tool_use.input, cx)) } else { (IconName::Cog, false) }; diff --git a/crates/assistant_tool/src/assistant_tool.rs b/crates/assistant_tool/src/assistant_tool.rs index 65c844e554..59879b80d3 100644 --- a/crates/assistant_tool/src/assistant_tool.rs +++ b/crates/assistant_tool/src/assistant_tool.rs @@ -48,7 +48,7 @@ pub trait Tool: 'static + Send + Sync { /// Returns true iff the tool needs the users's confirmation /// before having permission to run. - fn needs_confirmation(&self) -> bool; + fn needs_confirmation(&self, input: &serde_json::Value, cx: &App) -> bool; /// Returns the JSON schema that describes the tool's input. fn input_schema(&self, _: LanguageModelToolSchemaFormat) -> serde_json::Value { diff --git a/crates/assistant_tools/src/bash_tool.rs b/crates/assistant_tools/src/bash_tool.rs index e010e81113..d0b6b885d6 100644 --- a/crates/assistant_tools/src/bash_tool.rs +++ b/crates/assistant_tools/src/bash_tool.rs @@ -29,7 +29,7 @@ impl Tool for BashTool { "bash".to_string() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/batch_tool.rs b/crates/assistant_tools/src/batch_tool.rs index 30d436f9e8..751d2f8272 100644 --- a/crates/assistant_tools/src/batch_tool.rs +++ b/crates/assistant_tools/src/batch_tool.rs @@ -151,8 +151,17 @@ impl Tool for BatchTool { "batch_tool".into() } - fn needs_confirmation(&self) -> bool { - true + fn needs_confirmation(&self, input: &serde_json::Value, cx: &App) -> bool { + serde_json::from_value::(input.clone()) + .map(|input| { + let working_set = ToolWorkingSet::default(); + input.invocations.iter().any(|invocation| { + working_set + .tool(&invocation.name, cx) + .map_or(false, |tool| tool.needs_confirmation(&invocation.input, cx)) + }) + }) + .unwrap_or(false) } fn description(&self) -> String { diff --git a/crates/assistant_tools/src/code_symbols_tool.rs b/crates/assistant_tools/src/code_symbols_tool.rs index f6f6dc2855..dccff43fbf 100644 --- a/crates/assistant_tools/src/code_symbols_tool.rs +++ b/crates/assistant_tools/src/code_symbols_tool.rs @@ -79,7 +79,7 @@ impl Tool for CodeSymbolsTool { "code_symbols".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/copy_path_tool.rs b/crates/assistant_tools/src/copy_path_tool.rs index 7a77b3257b..c5995d6ff9 100644 --- a/crates/assistant_tools/src/copy_path_tool.rs +++ b/crates/assistant_tools/src/copy_path_tool.rs @@ -43,7 +43,7 @@ impl Tool for CopyPathTool { "copy_path".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/create_directory_tool.rs b/crates/assistant_tools/src/create_directory_tool.rs index d7d80e1f2d..2ca7b27303 100644 --- a/crates/assistant_tools/src/create_directory_tool.rs +++ b/crates/assistant_tools/src/create_directory_tool.rs @@ -33,7 +33,7 @@ impl Tool for CreateDirectoryTool { "create_directory".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/create_file_tool.rs b/crates/assistant_tools/src/create_file_tool.rs index b4f72a5f5a..26dd12b6c8 100644 --- a/crates/assistant_tools/src/create_file_tool.rs +++ b/crates/assistant_tools/src/create_file_tool.rs @@ -40,7 +40,7 @@ impl Tool for CreateFileTool { "create_file".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/delete_path_tool.rs b/crates/assistant_tools/src/delete_path_tool.rs index 22b1b45e50..b831f14b42 100644 --- a/crates/assistant_tools/src/delete_path_tool.rs +++ b/crates/assistant_tools/src/delete_path_tool.rs @@ -33,7 +33,7 @@ impl Tool for DeletePathTool { "delete_path".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/diagnostics_tool.rs b/crates/assistant_tools/src/diagnostics_tool.rs index 2882cbefc6..704e1e6d57 100644 --- a/crates/assistant_tools/src/diagnostics_tool.rs +++ b/crates/assistant_tools/src/diagnostics_tool.rs @@ -46,7 +46,7 @@ impl Tool for DiagnosticsTool { "diagnostics".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/fetch_tool.rs b/crates/assistant_tools/src/fetch_tool.rs index 6c24bf3ba1..90a4c0ca05 100644 --- a/crates/assistant_tools/src/fetch_tool.rs +++ b/crates/assistant_tools/src/fetch_tool.rs @@ -116,7 +116,7 @@ impl Tool for FetchTool { "fetch".to_string() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/find_replace_file_tool.rs b/crates/assistant_tools/src/find_replace_file_tool.rs index e18763be20..dfcceb90fe 100644 --- a/crates/assistant_tools/src/find_replace_file_tool.rs +++ b/crates/assistant_tools/src/find_replace_file_tool.rs @@ -129,7 +129,7 @@ impl Tool for FindReplaceFileTool { "find_replace_file".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/list_directory_tool.rs b/crates/assistant_tools/src/list_directory_tool.rs index 4d76653dfd..2a0615fa54 100644 --- a/crates/assistant_tools/src/list_directory_tool.rs +++ b/crates/assistant_tools/src/list_directory_tool.rs @@ -44,7 +44,7 @@ impl Tool for ListDirectoryTool { "list_directory".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/move_path_tool.rs b/crates/assistant_tools/src/move_path_tool.rs index 5dbbc2373e..b44023f122 100644 --- a/crates/assistant_tools/src/move_path_tool.rs +++ b/crates/assistant_tools/src/move_path_tool.rs @@ -42,7 +42,7 @@ impl Tool for MovePathTool { "move_path".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/now_tool.rs b/crates/assistant_tools/src/now_tool.rs index e793899855..45279daa3a 100644 --- a/crates/assistant_tools/src/now_tool.rs +++ b/crates/assistant_tools/src/now_tool.rs @@ -33,7 +33,7 @@ impl Tool for NowTool { "now".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/open_tool.rs b/crates/assistant_tools/src/open_tool.rs index 8aa336e264..a64a50edbf 100644 --- a/crates/assistant_tools/src/open_tool.rs +++ b/crates/assistant_tools/src/open_tool.rs @@ -23,7 +23,7 @@ impl Tool for OpenTool { "open".to_string() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true } diff --git a/crates/assistant_tools/src/path_search_tool.rs b/crates/assistant_tools/src/path_search_tool.rs index 78662a604f..604084395c 100644 --- a/crates/assistant_tools/src/path_search_tool.rs +++ b/crates/assistant_tools/src/path_search_tool.rs @@ -41,7 +41,7 @@ impl Tool for PathSearchTool { "path_search".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/read_file_tool.rs b/crates/assistant_tools/src/read_file_tool.rs index b0823ee70f..eb4f5c7a77 100644 --- a/crates/assistant_tools/src/read_file_tool.rs +++ b/crates/assistant_tools/src/read_file_tool.rs @@ -51,7 +51,7 @@ impl Tool for ReadFileTool { "read_file".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/regex_search_tool.rs b/crates/assistant_tools/src/regex_search_tool.rs index 948afcf1fd..809b407547 100644 --- a/crates/assistant_tools/src/regex_search_tool.rs +++ b/crates/assistant_tools/src/regex_search_tool.rs @@ -44,7 +44,7 @@ impl Tool for RegexSearchTool { "regex_search".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/symbol_info_tool.rs b/crates/assistant_tools/src/symbol_info_tool.rs index d04cce22ed..6d86679b38 100644 --- a/crates/assistant_tools/src/symbol_info_tool.rs +++ b/crates/assistant_tools/src/symbol_info_tool.rs @@ -72,7 +72,7 @@ impl Tool for SymbolInfoTool { "symbol_info".into() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/assistant_tools/src/thinking_tool.rs b/crates/assistant_tools/src/thinking_tool.rs index b189edc0b6..4a5800a6ad 100644 --- a/crates/assistant_tools/src/thinking_tool.rs +++ b/crates/assistant_tools/src/thinking_tool.rs @@ -24,7 +24,7 @@ impl Tool for ThinkingTool { "thinking".to_string() } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { false } diff --git a/crates/context_server/src/context_server_tool.rs b/crates/context_server/src/context_server_tool.rs index 31801ddfc7..ac952e4bd2 100644 --- a/crates/context_server/src/context_server_tool.rs +++ b/crates/context_server/src/context_server_tool.rs @@ -49,7 +49,7 @@ impl Tool for ContextServerTool { } } - fn needs_confirmation(&self) -> bool { + fn needs_confirmation(&self, _: &serde_json::Value, _: &App) -> bool { true }