From 65da2c2c6a8093c5c0058865d9f6540edcf8283c Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Thu, 5 Feb 2026 14:28:28 -0500 Subject: [PATCH] Use AgentTool::NAME const instead of name() method and hardcoded strings (#48506) This PR removes the `fn name()` default method from the `AgentTool` trait (which just returned `Self::NAME`) and replaces all call sites with direct use of the `NAME` associated constant. This lets us use it as a single source of truth in macros that want to access it at compile time. This PR also replaces hardcoded tool name string literals throughout the codebase with the corresponding `NAME` constants, so tool name references stay in sync if a tool is renamed. Intentionally not changed: - **Display strings** like `"Thinking"` and `"Subagent"` (user-facing UI labels) - **Serde attributes** like `#[serde(rename = "thinking")]` (wire format) - **The test assertion** `assert_eq!(SubagentTool::NAME, "subagent")` (replacing the literal would make it tautological) - **MCP server names** like `"mcp:srv:terminal"` (not tool identifiers) Release Notes: - N/A --- crates/agent/src/edit_agent/evals.rs | 117 ++++++++------- .../agent/src/tests/edit_file_thread_test.rs | 23 +-- crates/agent/src/tests/mod.rs | 133 +++++++++--------- crates/agent/src/tests/test_tools.rs | 26 +--- crates/agent/src/thread.rs | 39 ++--- crates/agent/src/tool_permissions.rs | 79 ++++++++--- crates/agent/src/tools.rs | 63 ++++++--- crates/agent/src/tools/copy_path_tool.rs | 10 +- .../agent/src/tools/create_directory_tool.rs | 8 +- crates/agent/src/tools/delete_path_tool.rs | 8 +- crates/agent/src/tools/diagnostics_tool.rs | 4 +- crates/agent/src/tools/edit_file_tool.rs | 18 +-- crates/agent/src/tools/fetch_tool.rs | 8 +- crates/agent/src/tools/find_path_tool.rs | 4 +- crates/agent/src/tools/grep_tool.rs | 4 +- crates/agent/src/tools/list_directory_tool.rs | 4 +- crates/agent/src/tools/move_path_tool.rs | 10 +- crates/agent/src/tools/now_tool.rs | 4 +- crates/agent/src/tools/open_tool.rs | 6 +- crates/agent/src/tools/read_file_tool.rs | 4 +- .../src/tools/restore_file_from_disk_tool.rs | 4 +- crates/agent/src/tools/save_file_tool.rs | 8 +- .../src/tools/streaming_edit_file_tool.rs | 19 +-- crates/agent/src/tools/subagent_tool.rs | 8 +- crates/agent/src/tools/terminal_tool.rs | 8 +- crates/agent/src/tools/thinking_tool.rs | 4 +- crates/agent/src/tools/web_search_tool.rs | 8 +- crates/agent_ui/src/acp/thread_view.rs | 45 +++--- .../manage_profiles_modal.rs | 26 ++-- crates/eval/src/examples/planets.rs | 4 +- 30 files changed, 384 insertions(+), 322 deletions(-) diff --git a/crates/agent/src/edit_agent/evals.rs b/crates/agent/src/edit_agent/evals.rs index 8af2e04cd58b2e31860269b82ea6c78180e1c930..0cf5c2e934f0c0cf33982fecf7a409d32245e381 100644 --- a/crates/agent/src/edit_agent/evals.rs +++ b/crates/agent/src/edit_agent/evals.rs @@ -1,6 +1,7 @@ use super::*; use crate::{ - EditFileMode, EditFileToolInput, GrepToolInput, ListDirectoryToolInput, ReadFileToolInput, + AgentTool, EditFileMode, EditFileTool, EditFileToolInput, GrepTool, GrepToolInput, + ListDirectoryTool, ListDirectoryToolInput, ReadFileTool, ReadFileToolInput, }; use Role::*; use client::{Client, UserStore}; @@ -119,7 +120,7 @@ fn eval_extract_handle_command_output() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: None, @@ -129,13 +130,17 @@ fn eval_extract_handle_command_output() { ), message( User, - [tool_result("tool_1", "read_file", input_file_content)], + [tool_result( + "tool_1", + ReadFileTool::NAME, + input_file_content, + )], ), message( Assistant, [tool_use( "tool_2", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -180,7 +185,7 @@ fn eval_delete_run_git_blame() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: None, @@ -190,13 +195,17 @@ fn eval_delete_run_git_blame() { ), message( User, - [tool_result("tool_1", "read_file", input_file_content)], + [tool_result( + "tool_1", + ReadFileTool::NAME, + input_file_content, + )], ), message( Assistant, [tool_use( "tool_2", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -241,7 +250,7 @@ fn eval_translate_doc_comments() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: None, @@ -251,13 +260,17 @@ fn eval_translate_doc_comments() { ), message( User, - [tool_result("tool_1", "read_file", input_file_content)], + [tool_result( + "tool_1", + ReadFileTool::NAME, + input_file_content, + )], ), message( Assistant, [tool_use( "tool_2", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -318,7 +331,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(971), @@ -330,7 +343,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { User, [tool_result( "tool_1", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 971..1050), )], ), @@ -338,7 +351,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { Assistant, [tool_use( "tool_2", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(1050), @@ -350,7 +363,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { User, [tool_result( "tool_2", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 1050..1100), )], ), @@ -358,7 +371,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { Assistant, [tool_use( "tool_3", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(1100), @@ -370,7 +383,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { User, [tool_result( "tool_3", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 1100..1150), )], ), @@ -378,7 +391,7 @@ fn eval_use_wasi_sdk_in_compile_parser_to_wasm() { Assistant, [tool_use( "tool_4", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -425,7 +438,7 @@ fn eval_disable_cursor_blinking() { Assistant, [tool_use( "tool_1", - "grep", + GrepTool::NAME, GrepToolInput { regex: "blink".into(), include_pattern: None, @@ -438,7 +451,7 @@ fn eval_disable_cursor_blinking() { User, [tool_result( "tool_1", - "grep", + GrepTool::NAME, [ lines(input_file_content, 100..400), lines(input_file_content, 800..1300), @@ -464,7 +477,7 @@ fn eval_disable_cursor_blinking() { Assistant, [tool_use( "tool_4", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -516,7 +529,7 @@ fn eval_from_pixels_constructor() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: None, @@ -526,13 +539,17 @@ fn eval_from_pixels_constructor() { ), message( User, - [tool_result("tool_1", "read_file", input_file_content)], + [tool_result( + "tool_1", + ReadFileTool::NAME, + input_file_content, + )], ), message( Assistant, [tool_use( "tool_2", - "grep", + GrepTool::NAME, GrepToolInput { regex: "mod\\s+tests".into(), include_pattern: Some("font-kit/src/canvas.rs".into()), @@ -541,12 +558,15 @@ fn eval_from_pixels_constructor() { }, )], ), - message(User, [tool_result("tool_2", "grep", "No matches found")]), + message( + User, + [tool_result("tool_2", GrepTool::NAME, "No matches found")], + ), message( Assistant, [tool_use( "tool_3", - "grep", + GrepTool::NAME, GrepToolInput { regex: "mod\\s+tests".into(), include_pattern: Some("font-kit/src/**/*.rs".into()), @@ -555,12 +575,15 @@ fn eval_from_pixels_constructor() { }, )], ), - message(User, [tool_result("tool_3", "grep", "No matches found")]), + message( + User, + [tool_result("tool_3", GrepTool::NAME, "No matches found")], + ), message( Assistant, [tool_use( "tool_4", - "grep", + GrepTool::NAME, GrepToolInput { regex: "#\\[test\\]".into(), include_pattern: Some("font-kit/src/**/*.rs".into()), @@ -573,7 +596,7 @@ fn eval_from_pixels_constructor() { User, [tool_result( "tool_4", - "grep", + GrepTool::NAME, indoc! {" Found 6 matches: @@ -667,7 +690,7 @@ fn eval_from_pixels_constructor() { Assistant, [tool_use( "tool_5", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -710,7 +733,7 @@ fn eval_zode() { [ tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: "root/eval/react.py".into(), start_line: None, @@ -719,7 +742,7 @@ fn eval_zode() { ), tool_use( "tool_2", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: "root/eval/react_test.py".into(), start_line: None, @@ -733,12 +756,12 @@ fn eval_zode() { [ tool_result( "tool_1", - "read_file", + ReadFileTool::NAME, include_str!("evals/fixtures/zode/react.py"), ), tool_result( "tool_2", - "read_file", + ReadFileTool::NAME, include_str!("evals/fixtures/zode/react_test.py"), ), ], @@ -751,7 +774,7 @@ fn eval_zode() { ), tool_use( "tool_3", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -821,7 +844,7 @@ fn eval_add_overwrite_test() { Assistant, [tool_use( "tool_1", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: None, @@ -833,7 +856,7 @@ fn eval_add_overwrite_test() { User, [tool_result( "tool_1", - "read_file", + ReadFileTool::NAME, indoc! {" pub struct ActionLog [L13-20] tracked_buffers [L15] @@ -920,7 +943,7 @@ fn eval_add_overwrite_test() { ), tool_use( "tool_2", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(953), @@ -933,7 +956,7 @@ fn eval_add_overwrite_test() { User, [tool_result( "tool_2", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 953..1010), )], ), @@ -945,7 +968,7 @@ fn eval_add_overwrite_test() { ), tool_use( "tool_3", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(1012), @@ -958,7 +981,7 @@ fn eval_add_overwrite_test() { User, [tool_result( "tool_3", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 1012..1120), )], ), @@ -968,7 +991,7 @@ fn eval_add_overwrite_test() { text("Now let's look at how `buffer_created` is implemented:"), tool_use( "tool_4", - "read_file", + ReadFileTool::NAME, ReadFileToolInput { path: input_file_path.into(), start_line: Some(271), @@ -981,7 +1004,7 @@ fn eval_add_overwrite_test() { User, [tool_result( "tool_4", - "read_file", + ReadFileTool::NAME, lines(input_file_content, 271..276), )], ), @@ -1002,7 +1025,7 @@ fn eval_add_overwrite_test() { "}), tool_use( "tool_5", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: edit_description.into(), path: input_file_path.into(), @@ -1056,7 +1079,7 @@ fn eval_create_empty_file() { "}), tool_use( "toolu_01GAF8TtsgpjKxCr8fgQLDgR", - "list_directory", + ListDirectoryTool::NAME, ListDirectoryToolInput { path: "root".to_string(), }, @@ -1067,7 +1090,7 @@ fn eval_create_empty_file() { User, [tool_result( "toolu_01GAF8TtsgpjKxCr8fgQLDgR", - "list_directory", + ListDirectoryTool::NAME, "root/TODO\nroot/TODO2\nroot/new.txt\n", )], ), @@ -1079,7 +1102,7 @@ fn eval_create_empty_file() { "}), tool_use( "toolu_01Tb3iQ9griqSYMmVuykQPWU", - "edit_file", + EditFileTool::NAME, EditFileToolInput { display_description: "Create empty TODO3 file".to_string(), mode: EditFileMode::Create, @@ -1173,7 +1196,7 @@ impl EvalInput { .content .iter() .flat_map(|content| match content { - MessageContent::ToolUse(tool_use) if tool_use.name == "edit_file".into() => { + MessageContent::ToolUse(tool_use) if tool_use.name == EditFileTool::NAME.into() => { Some(tool_use) } _ => None, diff --git a/crates/agent/src/tests/edit_file_thread_test.rs b/crates/agent/src/tests/edit_file_thread_test.rs index 67a0aa07255f15445ae236e2042864acba9833c4..e016889a97a69a265c10a022c70a66ec52aae450 100644 --- a/crates/agent/src/tests/edit_file_thread_test.rs +++ b/crates/agent/src/tests/edit_file_thread_test.rs @@ -1,4 +1,5 @@ use super::*; +use crate::{AgentTool, EditFileTool, ReadFileTool}; use acp_thread::UserMessageId; use action_log::ActionLog; use fs::FakeFs; @@ -74,7 +75,7 @@ async fn test_edit_file_tool_in_thread_context(cx: &mut TestAppContext) { // Model calls read_file tool let read_tool_use = LanguageModelToolUse { id: "read_tool_1".into(), - name: "read_file".into(), + name: ReadFileTool::NAME.into(), raw_input: json!({"path": "project/src/main.rs"}).to_string(), input: json!({"path": "project/src/main.rs"}), is_input_complete: true, @@ -96,7 +97,7 @@ async fn test_edit_file_tool_in_thread_context(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_text_chunk("I'll edit the file now."); let edit_tool_use = LanguageModelToolUse { id: "edit_tool_1".into(), - name: "edit_file".into(), + name: EditFileTool::NAME.into(), raw_input: json!({ "display_description": "Change greeting message", "path": "project/src/main.rs", @@ -261,7 +262,7 @@ async fn test_subagent_uses_read_file_tool(cx: &mut TestAppContext) { let mut tools: BTreeMap> = BTreeMap::new(); tools.insert( - "read_file".into(), + ReadFileTool::NAME.into(), crate::ReadFileTool::new(fake_parent_thread.downgrade(), project.clone(), action_log) .erase(), ); @@ -288,7 +289,7 @@ async fn test_subagent_uses_read_file_tool(cx: &mut TestAppContext) { // Verify the subagent has the read_file tool subagent.read_with(cx, |thread, _| { assert!( - thread.has_registered_tool("read_file"), + thread.has_registered_tool(ReadFileTool::NAME), "subagent should have read_file tool" ); }); @@ -304,7 +305,7 @@ async fn test_subagent_uses_read_file_tool(cx: &mut TestAppContext) { // Simulate the model calling the read_file tool let read_tool_use = LanguageModelToolUse { id: "read_tool_1".into(), - name: "read_file".into(), + name: ReadFileTool::NAME.into(), raw_input: json!({"path": "project/src/lib.rs"}).to_string(), input: json!({"path": "project/src/lib.rs"}), is_input_complete: true, @@ -414,12 +415,12 @@ async fn test_subagent_uses_edit_file_tool(cx: &mut TestAppContext) { let mut tools: BTreeMap> = BTreeMap::new(); tools.insert( - "read_file".into(), + ReadFileTool::NAME.into(), crate::ReadFileTool::new(parent_thread.downgrade(), project.clone(), action_log) .erase(), ); tools.insert( - "edit_file".into(), + EditFileTool::NAME.into(), crate::EditFileTool::new( project.clone(), parent_thread.downgrade(), @@ -464,11 +465,11 @@ async fn test_subagent_uses_edit_file_tool(cx: &mut TestAppContext) { // Verify the subagent has the tools subagent.read_with(cx, |thread, _| { assert!( - thread.has_registered_tool("read_file"), + thread.has_registered_tool(ReadFileTool::NAME), "subagent should have read_file tool" ); assert!( - thread.has_registered_tool("edit_file"), + thread.has_registered_tool(EditFileTool::NAME), "subagent should have edit_file tool" ); }); @@ -484,7 +485,7 @@ async fn test_subagent_uses_edit_file_tool(cx: &mut TestAppContext) { // First, model calls read_file to see the current content let read_tool_use = LanguageModelToolUse { id: "read_tool_1".into(), - name: "read_file".into(), + name: ReadFileTool::NAME.into(), raw_input: json!({"path": "project/src/config.rs"}).to_string(), input: json!({"path": "project/src/config.rs"}), is_input_complete: true, @@ -511,7 +512,7 @@ async fn test_subagent_uses_edit_file_tool(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_text_chunk("I'll update the version now."); let edit_tool_use = LanguageModelToolUse { id: "edit_tool_1".into(), - name: "edit_file".into(), + name: EditFileTool::NAME.into(), raw_input: json!({ "display_description": "Update version to 2.0.0", "path": "project/src/config.rs", diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index 1ae188a6440bec0375fd76dab23facb4a5c60590..3b6a047256efff90796c184b63cc2c0b89d94714 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -559,7 +559,7 @@ async fn test_prompt_caching(cx: &mut TestAppContext) { let tool_use = LanguageModelToolUse { id: "tool_1".into(), - name: EchoTool::name().into(), + name: EchoTool::NAME.into(), raw_input: json!({"text": "test"}).to_string(), input: json!({"text": "test"}), is_input_complete: true, @@ -573,7 +573,7 @@ async fn test_prompt_caching(cx: &mut TestAppContext) { let completion = fake_model.pending_completions().pop().unwrap(); let tool_result = LanguageModelToolResult { tool_use_id: "tool_1".into(), - tool_name: EchoTool::name().into(), + tool_name: EchoTool::NAME.into(), is_error: false, content: "test".into(), output: Some("test".into()), @@ -650,7 +650,7 @@ async fn test_basic_tool_calls(cx: &mut TestAppContext) { // Test a tool calls that's likely to complete *after* streaming stops. let events = thread .update(cx, |thread, cx| { - thread.remove_tool(&EchoTool::name()); + thread.remove_tool(&EchoTool::NAME); thread.add_tool(DelayTool); thread.send( UserMessageId::new(), @@ -754,7 +754,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "tool_id_1".into(), - name: ToolRequiringPermission::name().into(), + name: ToolRequiringPermission::NAME.into(), raw_input: "{}".into(), input: json!({}), is_input_complete: true, @@ -764,7 +764,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "tool_id_2".into(), - name: ToolRequiringPermission::name().into(), + name: ToolRequiringPermission::NAME.into(), raw_input: "{}".into(), input: json!({}), is_input_complete: true, @@ -796,14 +796,14 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { vec![ language_model::MessageContent::ToolResult(LanguageModelToolResult { tool_use_id: tool_call_auth_1.tool_call.tool_call_id.0.to_string().into(), - tool_name: ToolRequiringPermission::name().into(), + tool_name: ToolRequiringPermission::NAME.into(), is_error: false, content: "Allowed".into(), output: Some("Allowed".into()) }), language_model::MessageContent::ToolResult(LanguageModelToolResult { tool_use_id: tool_call_auth_2.tool_call.tool_call_id.0.to_string().into(), - tool_name: ToolRequiringPermission::name().into(), + tool_name: ToolRequiringPermission::NAME.into(), is_error: true, content: "Permission to run tool denied by user".into(), output: Some("Permission to run tool denied by user".into()) @@ -815,7 +815,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "tool_id_3".into(), - name: ToolRequiringPermission::name().into(), + name: ToolRequiringPermission::NAME.into(), raw_input: "{}".into(), input: json!({}), is_input_complete: true, @@ -841,7 +841,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { vec![language_model::MessageContent::ToolResult( LanguageModelToolResult { tool_use_id: tool_call_auth_3.tool_call.tool_call_id.0.to_string().into(), - tool_name: ToolRequiringPermission::name().into(), + tool_name: ToolRequiringPermission::NAME.into(), is_error: false, content: "Allowed".into(), output: Some("Allowed".into()) @@ -853,7 +853,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "tool_id_4".into(), - name: ToolRequiringPermission::name().into(), + name: ToolRequiringPermission::NAME.into(), raw_input: "{}".into(), input: json!({}), is_input_complete: true, @@ -869,7 +869,7 @@ async fn test_tool_authorization(cx: &mut TestAppContext) { vec![language_model::MessageContent::ToolResult( LanguageModelToolResult { tool_use_id: "tool_id_4".into(), - tool_name: ToolRequiringPermission::name().into(), + tool_name: ToolRequiringPermission::NAME.into(), is_error: false, content: "Allowed".into(), output: Some("Allowed".into()) @@ -970,7 +970,8 @@ async fn next_tool_call_authorization( #[test] fn test_permission_options_terminal_with_pattern() { let permission_options = - ToolPermissionContext::new("terminal", "cargo build --release").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -989,7 +990,7 @@ fn test_permission_options_terminal_with_pattern() { #[test] fn test_permission_options_edit_file_with_path_pattern() { let permission_options = - ToolPermissionContext::new("edit_file", "src/main.rs").build_permission_options(); + ToolPermissionContext::new(EditFileTool::NAME, "src/main.rs").build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1005,8 +1006,8 @@ fn test_permission_options_edit_file_with_path_pattern() { #[test] fn test_permission_options_fetch_with_domain_pattern() { - let permission_options = - ToolPermissionContext::new("fetch", "https://docs.rs/gpui").build_permission_options(); + let permission_options = ToolPermissionContext::new(FetchTool::NAME, "https://docs.rs/gpui") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1022,8 +1023,9 @@ fn test_permission_options_fetch_with_domain_pattern() { #[test] fn test_permission_options_without_pattern() { - let permission_options = ToolPermissionContext::new("terminal", "./deploy.sh --production") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, "./deploy.sh --production") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1042,7 +1044,8 @@ fn test_permission_options_without_pattern() { #[test] fn test_permission_option_ids_for_terminal() { let permission_options = - ToolPermissionContext::new("terminal", "cargo build --release").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -1143,14 +1146,14 @@ async fn test_profiles(cx: &mut TestAppContext) { "test-1": { "name": "Test Profile 1", "tools": { - EchoTool::name(): true, - DelayTool::name(): true, + EchoTool::NAME: true, + DelayTool::NAME: true, } }, "test-2": { "name": "Test Profile 2", "tools": { - InfiniteTool::name(): true, + InfiniteTool::NAME: true, } } } @@ -1179,7 +1182,7 @@ async fn test_profiles(cx: &mut TestAppContext) { .iter() .map(|tool| tool.name.clone()) .collect(); - assert_eq!(tool_names, vec![DelayTool::name(), EchoTool::name()]); + assert_eq!(tool_names, vec![DelayTool::NAME, EchoTool::NAME]); fake_model.end_last_completion_stream(); // Switch to test-2 profile, and verify that it has only the infinite tool. @@ -1198,7 +1201,7 @@ async fn test_profiles(cx: &mut TestAppContext) { .iter() .map(|tool| tool.name.clone()) .collect(); - assert_eq!(tool_names, vec![InfiniteTool::name()]); + assert_eq!(tool_names, vec![InfiniteTool::NAME]); } #[gpui::test] @@ -1223,7 +1226,7 @@ async fn test_mcp_tools(cx: &mut TestAppContext) { "name": "Test Profile", "enable_all_context_servers": true, "tools": { - EchoTool::name(): true, + EchoTool::NAME: true, } }, } @@ -1388,11 +1391,11 @@ async fn test_mcp_tool_truncation(cx: &mut TestAppContext) { "name": "Test Profile", "enable_all_context_servers": true, "tools": { - EchoTool::name(): true, - DelayTool::name(): true, - WordListTool::name(): true, - ToolRequiringPermission::name(): true, - InfiniteTool::name(): true, + EchoTool::NAME: true, + DelayTool::NAME: true, + WordListTool::NAME: true, + ToolRequiringPermission::NAME: true, + InfiniteTool::NAME: true, } }, } @@ -1646,7 +1649,7 @@ async fn test_terminal_tool_cancellation_captures_output(cx: &mut TestAppContext fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_1".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 1000", "cd": "."}"#.into(), input: json!({"command": "sleep 1000", "cd": "."}), is_input_complete: true, @@ -1929,7 +1932,7 @@ async fn test_truncate_while_terminal_tool_running(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_1".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 1000", "cd": "."}"#.into(), input: json!({"command": "sleep 1000", "cd": "."}), is_input_complete: true, @@ -1993,7 +1996,7 @@ async fn test_cancel_multiple_concurrent_terminal_tools(cx: &mut TestAppContext) fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_1".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 1000", "cd": "."}"#.into(), input: json!({"command": "sleep 1000", "cd": "."}), is_input_complete: true, @@ -2003,7 +2006,7 @@ async fn test_cancel_multiple_concurrent_terminal_tools(cx: &mut TestAppContext) fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_2".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 2000", "cd": "."}"#.into(), input: json!({"command": "sleep 2000", "cd": "."}), is_input_complete: true, @@ -2106,7 +2109,7 @@ async fn test_terminal_tool_stopped_via_terminal_card_button(cx: &mut TestAppCon fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_1".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 1000", "cd": "."}"#.into(), input: json!({"command": "sleep 1000", "cd": "."}), is_input_complete: true, @@ -2200,7 +2203,7 @@ async fn test_terminal_tool_timeout_expires(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "terminal_tool_1".into(), - name: "terminal".into(), + name: TerminalTool::NAME.into(), raw_input: r#"{"command": "sleep 1000", "cd": ".", "timeout_ms": 100}"#.into(), input: json!({"command": "sleep 1000", "cd": ".", "timeout_ms": 100}), is_input_complete: true, @@ -2679,7 +2682,7 @@ async fn test_building_request_with_pending_tools(cx: &mut TestAppContext) { let permission_tool_use = LanguageModelToolUse { id: "tool_id_1".into(), - name: ToolRequiringPermission::name().into(), + name: ToolRequiringPermission::NAME.into(), raw_input: "{}".into(), input: json!({}), is_input_complete: true, @@ -2687,7 +2690,7 @@ async fn test_building_request_with_pending_tools(cx: &mut TestAppContext) { }; let echo_tool_use = LanguageModelToolUse { id: "tool_id_2".into(), - name: EchoTool::name().into(), + name: EchoTool::NAME.into(), raw_input: json!({"text": "test"}).to_string(), input: json!({"text": "test"}), is_input_complete: true, @@ -2890,7 +2893,7 @@ async fn test_tool_updates_to_completion(cx: &mut TestAppContext) { fake_model.send_last_completion_stream_event(LanguageModelCompletionEvent::ToolUse( LanguageModelToolUse { id: "1".into(), - name: ThinkingTool::name().into(), + name: ThinkingTool::NAME.into(), raw_input: input.to_string(), input, is_input_complete: false, @@ -3085,7 +3088,7 @@ async fn test_send_retry_finishes_tool_calls_on_error(cx: &mut TestAppContext) { let tool_use_1 = LanguageModelToolUse { id: "tool_1".into(), - name: EchoTool::name().into(), + name: EchoTool::NAME.into(), raw_input: json!({"text": "test"}).to_string(), input: json!({"text": "test"}), is_input_complete: true, @@ -3253,14 +3256,14 @@ async fn setup(cx: &mut TestAppContext, model: TestModel) -> ThreadTest { "test-profile": { "name": "Test Profile", "tools": { - EchoTool::name(): true, - DelayTool::name(): true, - WordListTool::name(): true, - ToolRequiringPermission::name(): true, - InfiniteTool::name(): true, - CancellationAwareTool::name(): true, - ThinkingTool::name(): true, - "terminal": true, + EchoTool::NAME: true, + DelayTool::NAME: true, + WordListTool::NAME: true, + ToolRequiringPermission::NAME: true, + InfiniteTool::NAME: true, + CancellationAwareTool::NAME: true, + ThinkingTool::NAME: true, + (TerminalTool::NAME): true, } } } @@ -3657,7 +3660,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "terminal".into(), + TerminalTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Confirm, always_allow: vec![], @@ -3710,7 +3713,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( - "terminal".into(), + TerminalTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Deny, always_allow: vec![ @@ -3768,7 +3771,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = true; settings.tool_permissions.tools.insert( - "terminal".into(), + TerminalTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -3814,7 +3817,7 @@ async fn test_terminal_tool_permission_rules(cx: &mut TestAppContext) { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = true; settings.tool_permissions.tools.insert( - "terminal".into(), + TerminalTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Deny, always_allow: vec![], @@ -3883,7 +3886,7 @@ async fn test_subagent_tool_is_present_when_feature_flag_enabled(cx: &mut TestAp thread.read_with(cx, |thread, _| { assert!( - thread.has_registered_tool("subagent"), + thread.has_registered_tool(SubagentTool::NAME), "subagent tool should be present when feature flag is enabled" ); }); @@ -3980,7 +3983,7 @@ async fn test_max_subagent_depth_prevents_tool_registration(cx: &mut TestAppCont deep_subagent.read_with(cx, |thread, _| { assert_eq!(thread.depth(), MAX_SUBAGENT_DEPTH); assert!( - !thread.has_registered_tool("subagent"), + !thread.has_registered_tool(SubagentTool::NAME), "subagent tool should not be present at max depth" ); }); @@ -4794,7 +4797,7 @@ async fn test_subagent_uses_tool_and_returns_result(cx: &mut TestAppContext) { let tool_use = LanguageModelToolUse { id: "tool_call_1".into(), - name: EchoTool::name().into(), + name: EchoTool::NAME.into(), raw_input: json!({"text": "hello world"}).to_string(), input: json!({"text": "hello world"}), is_input_complete: true, @@ -5041,7 +5044,7 @@ async fn test_edit_file_tool_deny_rule_blocks_edit(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "edit_file".into(), + EditFileTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5109,7 +5112,7 @@ async fn test_delete_path_tool_deny_rule_blocks_deletion(cx: &mut TestAppContext cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "delete_path".into(), + DeletePathTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5163,7 +5166,7 @@ async fn test_move_path_tool_denies_if_destination_denied(cx: &mut TestAppContex cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "move_path".into(), + MovePathTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5219,7 +5222,7 @@ async fn test_move_path_tool_denies_if_source_denied(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "move_path".into(), + MovePathTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5275,7 +5278,7 @@ async fn test_copy_path_tool_deny_rule_blocks_copy(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "copy_path".into(), + CopyPathTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5332,7 +5335,7 @@ async fn test_save_file_tool_denies_if_any_path_denied(cx: &mut TestAppContext) cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "save_file".into(), + SaveFileTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5385,7 +5388,7 @@ async fn test_save_file_tool_respects_deny_rules(cx: &mut TestAppContext) { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( - "save_file".into(), + SaveFileTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5426,7 +5429,7 @@ async fn test_web_search_tool_deny_rule_blocks_search(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "web_search".into(), + WebSearchTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5470,7 +5473,7 @@ async fn test_edit_file_tool_allow_rule_skips_confirmation(cx: &mut TestAppConte let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( - "edit_file".into(), + EditFileTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Confirm, always_allow: vec![agent_settings::CompiledRegex::new(r"\.md$", false).unwrap()], @@ -5534,7 +5537,7 @@ async fn test_fetch_tool_deny_rule_blocks_url(cx: &mut TestAppContext) { cx.update(|cx| { let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.tool_permissions.tools.insert( - "fetch".into(), + FetchTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Allow, always_allow: vec![], @@ -5575,7 +5578,7 @@ async fn test_fetch_tool_allow_rule_skips_confirmation(cx: &mut TestAppContext) let mut settings = agent_settings::AgentSettings::get_global(cx).clone(); settings.always_allow_tool_actions = false; settings.tool_permissions.tools.insert( - "fetch".into(), + FetchTool::NAME.into(), agent_settings::ToolRules { default_mode: settings::ToolPermissionMode::Confirm, always_allow: vec![agent_settings::CompiledRegex::new(r"docs\.rs", false).unwrap()], diff --git a/crates/agent/src/tests/test_tools.rs b/crates/agent/src/tests/test_tools.rs index 90974f4d5d66745c9d691959e9d2ba2a5addd9ca..eb9018960180a167951cbba5458aa110377f2b16 100644 --- a/crates/agent/src/tests/test_tools.rs +++ b/crates/agent/src/tests/test_tools.rs @@ -18,9 +18,7 @@ impl AgentTool for EchoTool { type Input = EchoToolInput; type Output = String; - fn name() -> &'static str { - "echo" - } + const NAME: &'static str = "echo"; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -57,9 +55,7 @@ impl AgentTool for DelayTool { type Input = DelayToolInput; type Output = String; - fn name() -> &'static str { - "delay" - } + const NAME: &'static str = "delay"; fn initial_title( &self, @@ -103,9 +99,7 @@ impl AgentTool for ToolRequiringPermission { type Input = ToolRequiringPermissionInput; type Output = String; - fn name() -> &'static str { - "tool_requiring_permission" - } + const NAME: &'static str = "tool_requiring_permission"; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -126,7 +120,7 @@ impl AgentTool for ToolRequiringPermission { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), "", settings); + let decision = decide_permission_from_settings(Self::NAME, "", settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -160,9 +154,7 @@ impl AgentTool for InfiniteTool { type Input = InfiniteToolInput; type Output = String; - fn name() -> &'static str { - "infinite" - } + const NAME: &'static str = "infinite"; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -214,9 +206,7 @@ impl AgentTool for CancellationAwareTool { type Input = CancellationAwareToolInput; type Output = String; - fn name() -> &'static str { - "cancellation_aware" - } + const NAME: &'static str = "cancellation_aware"; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -271,9 +261,7 @@ impl AgentTool for WordListTool { type Input = WordListInput; type Output = String; - fn name() -> &'static str { - "word_list" - } + const NAME: &'static str = "word_list"; fn kind() -> acp::ToolKind { acp::ToolKind::Other diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index fd301dda5773578a9bb7d52ea5e54bffb237a41f..3afc1d22a5b13b6bfb56bdaff778ffd4e4cb5e1e 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -664,28 +664,28 @@ impl ToolPermissionContext { // Check if the user's shell supports POSIX-like command chaining. // See the doc comment above for the full explanation of why this is needed. - let shell_supports_always_allow = if tool_name == TerminalTool::name() { + let shell_supports_always_allow = if tool_name == TerminalTool::NAME { ShellKind::system().supports_posix_chaining() } else { true }; - let (pattern, pattern_display) = if tool_name == TerminalTool::name() { + let (pattern, pattern_display) = if tool_name == TerminalTool::NAME { ( extract_terminal_pattern(input_value), extract_terminal_pattern_display(input_value), ) - } else if tool_name == EditFileTool::name() - || tool_name == DeletePathTool::name() - || tool_name == MovePathTool::name() - || tool_name == CreateDirectoryTool::name() - || tool_name == SaveFileTool::name() + } else if tool_name == EditFileTool::NAME + || tool_name == DeletePathTool::NAME + || tool_name == MovePathTool::NAME + || tool_name == CreateDirectoryTool::NAME + || tool_name == SaveFileTool::NAME { ( extract_path_pattern(input_value), extract_path_pattern_display(input_value), ) - } else if tool_name == FetchTool::name() { + } else if tool_name == FetchTool::NAME { ( extract_url_pattern(input_value), extract_url_pattern_display(input_value), @@ -721,7 +721,7 @@ impl ToolPermissionContext { ); if let (Some(pattern), Some(display)) = (pattern, pattern_display) { - let button_text = if tool_name == TerminalTool::name() { + let button_text = if tool_name == TerminalTool::NAME { format!("Always for `{}` commands", display) } else { format!("Always for `{}`", display) @@ -1285,7 +1285,12 @@ impl Thread { } pub fn add_tool(&mut self, tool: T) { - self.tools.insert(T::name().into(), tool.erase()); + debug_assert!( + !self.tools.contains_key(T::NAME), + "Duplicate tool name: {}", + T::NAME, + ); + self.tools.insert(T::NAME.into(), tool.erase()); } pub fn remove_tool(&mut self, name: &str) -> bool { @@ -2338,8 +2343,8 @@ impl Thread { .iter() .filter_map(|(tool_name, tool)| { // For streaming_edit_file, check profile against "edit_file" since that's what users configure - let profile_tool_name = if tool_name == "streaming_edit_file" { - "edit_file" + let profile_tool_name = if tool_name == StreamingEditFileTool::NAME { + EditFileTool::NAME } else { tool_name.as_ref() }; @@ -2348,10 +2353,10 @@ impl Thread { && profile.is_tool_enabled(profile_tool_name) { match (tool_name.as_ref(), use_streaming_edit_tool) { - ("streaming_edit_file", false) | ("edit_file", true) => None, - ("streaming_edit_file", true) => { + (StreamingEditFileTool::NAME, false) | (EditFileTool::NAME, true) => None, + (StreamingEditFileTool::NAME, true) => { // Expose streaming tool as "edit_file" - Some((SharedString::from("edit_file"), tool.clone())) + Some((SharedString::from(EditFileTool::NAME), tool.clone())) } _ => Some((truncate(tool_name), tool.clone())), } @@ -2687,7 +2692,7 @@ where type Input: for<'de> Deserialize<'de> + Serialize + JsonSchema; type Output: for<'de> Deserialize<'de> + Serialize + Into; - fn name() -> &'static str; + const NAME: &'static str; fn description() -> SharedString { let schema = schemars::schema_for!(Self::Input); @@ -2796,7 +2801,7 @@ where T: AgentTool, { fn name(&self) -> SharedString { - T::name().into() + T::NAME.into() } fn description(&self) -> SharedString { diff --git a/crates/agent/src/tool_permissions.rs b/crates/agent/src/tool_permissions.rs index 62742622bf909455dfd8998a87393fca0e08c5ec..2437b64df803d307946efc098031fb8e486f31df 100644 --- a/crates/agent/src/tool_permissions.rs +++ b/crates/agent/src/tool_permissions.rs @@ -46,7 +46,7 @@ fn check_hardcoded_security_rules( shell_kind: ShellKind, ) -> Option { // Currently only terminal tool has hardcoded rules - if tool_name != TerminalTool::name() { + if tool_name != TerminalTool::NAME { return None; } @@ -165,7 +165,7 @@ impl ToolPermissionDecision { // // If parsing fails or the shell syntax is unsupported, always_allow is // disabled for this command (we set allow_enabled to false to signal this). - if tool_name == TerminalTool::name() { + if tool_name == TerminalTool::NAME { // Our shell parser (brush-parser) only supports POSIX-like shell syntax. // See the doc comment above for the list of compatible/incompatible shells. if !shell_kind.supports_posix_chaining() { @@ -306,7 +306,9 @@ pub fn decide_permission_from_settings( #[cfg(test)] mod tests { use super::*; + use crate::AgentTool; use crate::pattern_extraction::extract_terminal_pattern; + use crate::tools::{EditFileTool, TerminalTool}; use agent_settings::{CompiledRegex, InvalidRegexPattern, ToolRules}; use std::sync::Arc; @@ -332,7 +334,7 @@ mod tests { impl PermTest { fn new(input: &'static str) -> Self { Self { - tool: "terminal", + tool: TerminalTool::NAME, input, mode: ToolPermissionMode::Confirm, allow: vec![], @@ -444,7 +446,7 @@ mod tests { fn no_rules(input: &str, global: bool) -> ToolPermissionDecision { ToolPermissionDecision::from_input( - "terminal", + TerminalTool::NAME, input, &ToolPermissions { tools: collections::HashMap::default(), @@ -666,7 +668,7 @@ mod tests { fn other_tool_not_affected() { let mut tools = collections::HashMap::default(); tools.insert( - Arc::from("terminal"), + Arc::from(TerminalTool::NAME), ToolRules { default_mode: ToolPermissionMode::Deny, always_allow: vec![], @@ -676,7 +678,7 @@ mod tests { }, ); tools.insert( - Arc::from("edit_file"), + Arc::from(EditFileTool::NAME), ToolRules { default_mode: ToolPermissionMode::Allow, always_allow: vec![], @@ -688,16 +690,28 @@ mod tests { let p = ToolPermissions { tools }; // With always_allow_tool_actions=true, even default_mode: Deny is overridden assert_eq!( - ToolPermissionDecision::from_input("terminal", "x", &p, true, ShellKind::Posix), + ToolPermissionDecision::from_input(TerminalTool::NAME, "x", &p, true, ShellKind::Posix), ToolPermissionDecision::Allow ); // With always_allow_tool_actions=false, default_mode: Deny is respected assert!(matches!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + TerminalTool::NAME, + "x", + &p, + false, + ShellKind::Posix + ), ToolPermissionDecision::Deny(_) )); assert_eq!( - ToolPermissionDecision::from_input("edit_file", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + EditFileTool::NAME, + "x", + &p, + false, + ShellKind::Posix + ), ToolPermissionDecision::Allow ); } @@ -718,7 +732,13 @@ mod tests { let p = ToolPermissions { tools }; // "terminal" should not match "term" rules, so falls back to Confirm (no rules) assert_eq!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + TerminalTool::NAME, + "x", + &p, + false, + ShellKind::Posix + ), ToolPermissionDecision::Confirm ); } @@ -728,7 +748,7 @@ mod tests { fn invalid_pattern_blocks() { let mut tools = collections::HashMap::default(); tools.insert( - Arc::from("terminal"), + Arc::from(TerminalTool::NAME), ToolRules { default_mode: ToolPermissionMode::Allow, always_allow: vec![CompiledRegex::new("echo", false).unwrap()], @@ -746,12 +766,24 @@ mod tests { }; // With global=true, all checks are bypassed including invalid pattern check assert!(matches!( - ToolPermissionDecision::from_input("terminal", "echo hi", &p, true, ShellKind::Posix), + ToolPermissionDecision::from_input( + TerminalTool::NAME, + "echo hi", + &p, + true, + ShellKind::Posix + ), ToolPermissionDecision::Allow )); // With global=false, invalid patterns block the tool assert!(matches!( - ToolPermissionDecision::from_input("terminal", "echo hi", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + TerminalTool::NAME, + "echo hi", + &p, + false, + ShellKind::Posix + ), ToolPermissionDecision::Deny(_) )); } @@ -928,7 +960,7 @@ mod tests { fn mcp_doesnt_collide_with_builtin() { let mut tools = collections::HashMap::default(); tools.insert( - Arc::from("terminal"), + Arc::from(TerminalTool::NAME), ToolRules { default_mode: ToolPermissionMode::Deny, always_allow: vec![], @@ -949,7 +981,13 @@ mod tests { ); let p = ToolPermissions { tools }; assert!(matches!( - ToolPermissionDecision::from_input("terminal", "x", &p, false, ShellKind::Posix), + ToolPermissionDecision::from_input( + TerminalTool::NAME, + "x", + &p, + false, + ShellKind::Posix + ), ToolPermissionDecision::Deny(_) )); assert_eq!( @@ -1035,7 +1073,7 @@ mod tests { fn multiple_invalid_patterns_pluralizes_message() { let mut tools = collections::HashMap::default(); tools.insert( - Arc::from("terminal"), + Arc::from(TerminalTool::NAME), ToolRules { default_mode: ToolPermissionMode::Allow, always_allow: vec![], @@ -1057,8 +1095,13 @@ mod tests { ); let p = ToolPermissions { tools }; - let result = - ToolPermissionDecision::from_input("terminal", "echo hi", &p, false, ShellKind::Posix); + let result = ToolPermissionDecision::from_input( + TerminalTool::NAME, + "echo hi", + &p, + false, + ShellKind::Posix, + ); match result { ToolPermissionDecision::Deny(msg) => { assert!( diff --git a/crates/agent/src/tools.rs b/crates/agent/src/tools.rs index 29c4bfe6673b34f655282d8fc86c49d27fabca06..debfa39962a018de3baaf484db314a1c715897d7 100644 --- a/crates/agent/src/tools.rs +++ b/crates/agent/src/tools.rs @@ -21,8 +21,6 @@ mod thinking_tool; mod web_search_tool; use crate::AgentTool; -use feature_flags::{FeatureFlagAppExt, SubagentsFeatureFlag}; -use gpui::App; use language_model::{LanguageModelRequestTool, LanguageModelToolSchemaFormat}; pub use context_server_registry::*; @@ -49,34 +47,57 @@ pub use web_search_tool::*; macro_rules! tools { ($($tool:ty),* $(,)?) => { - /// A list of all built-in tool names - pub fn supported_built_in_tool_names(provider: Option, cx: &App) -> Vec { - let mut tools: Vec = [ - $( - (if let Some(provider) = provider.as_ref() { - <$tool>::supports_provider(provider) - } else { - true - }) - .then(|| <$tool>::name().to_string()), - )* - ] - .into_iter() - .flatten() - .collect(); + /// Every built-in tool name, determined at compile time. + pub const ALL_TOOL_NAMES: &[&str] = &[ + $(<$tool>::NAME,)* + ]; + + const _: () = { + const fn str_eq(a: &str, b: &str) -> bool { + let a = a.as_bytes(); + let b = b.as_bytes(); + if a.len() != b.len() { + return false; + } + let mut i = 0; + while i < a.len() { + if a[i] != b[i] { + return false; + } + i += 1; + } + true + } - if !cx.has_flag::() { - tools.retain(|name| name != SubagentTool::name()); + const NAMES: &[&str] = ALL_TOOL_NAMES; + let mut i = 0; + while i < NAMES.len() { + let mut j = i + 1; + while j < NAMES.len() { + if str_eq(NAMES[i], NAMES[j]) { + panic!("Duplicate tool name in tools! macro"); + } + j += 1; + } + i += 1; } + }; - tools + /// Returns whether the tool with the given name supports the given provider. + pub fn tool_supports_provider(name: &str, provider: &language_model::LanguageModelProviderId) -> bool { + $( + if name == <$tool>::NAME { + return <$tool>::supports_provider(provider); + } + )* + false } /// A list of all built-in tools pub fn built_in_tools() -> impl Iterator { fn language_model_tool() -> LanguageModelRequestTool { LanguageModelRequestTool { - name: T::name().to_string(), + name: T::NAME.to_string(), description: T::description().to_string(), input_schema: T::input_schema(LanguageModelToolSchemaFormat::JsonSchema).to_value(), } diff --git a/crates/agent/src/tools/copy_path_tool.rs b/crates/agent/src/tools/copy_path_tool.rs index 33fdcf3631eed6815ae2d1b1cecd4932f0b728a7..fdeb426f64bf0a5a31fc917f0fa83bd5cf5c9eec 100644 --- a/crates/agent/src/tools/copy_path_tool.rs +++ b/crates/agent/src/tools/copy_path_tool.rs @@ -55,9 +55,7 @@ impl AgentTool for CopyPathTool { type Input = CopyPathToolInput; type Output = String; - fn name() -> &'static str { - "copy_path" - } + const NAME: &'static str = "copy_path"; fn kind() -> ToolKind { ToolKind::Move @@ -86,13 +84,13 @@ impl AgentTool for CopyPathTool { let settings = AgentSettings::get_global(cx); let source_decision = - decide_permission_from_settings(Self::name(), &input.source_path, settings); + decide_permission_from_settings(Self::NAME, &input.source_path, settings); if let ToolPermissionDecision::Deny(reason) = source_decision { return Task::ready(Err(anyhow!("{}", reason))); } let dest_decision = - decide_permission_from_settings(Self::name(), &input.destination_path, settings); + decide_permission_from_settings(Self::NAME, &input.destination_path, settings); if let ToolPermissionDecision::Deny(reason) = dest_decision { return Task::ready(Err(anyhow!("{}", reason))); } @@ -104,7 +102,7 @@ impl AgentTool for CopyPathTool { let src = MarkdownInlineCode(&input.source_path); let dest = MarkdownInlineCode(&input.destination_path); let context = crate::ToolPermissionContext { - tool_name: "copy_path".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.source_path.clone(), }; Some(event_stream.authorize(format!("Copy {src} to {dest}"), context, cx)) diff --git a/crates/agent/src/tools/create_directory_tool.rs b/crates/agent/src/tools/create_directory_tool.rs index ba2adf2debe23f168d2c54b87d193f1d25c1dc47..daaa5c946ea685cc3e2a113d47364a83a6de5c73 100644 --- a/crates/agent/src/tools/create_directory_tool.rs +++ b/crates/agent/src/tools/create_directory_tool.rs @@ -46,9 +46,7 @@ impl AgentTool for CreateDirectoryTool { type Input = CreateDirectoryToolInput; type Output = String; - fn name() -> &'static str { - "create_directory" - } + const NAME: &'static str = "create_directory"; fn kind() -> ToolKind { ToolKind::Read @@ -73,7 +71,7 @@ impl AgentTool for CreateDirectoryTool { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &input.path, settings); + let decision = decide_permission_from_settings(Self::NAME, &input.path, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -82,7 +80,7 @@ impl AgentTool for CreateDirectoryTool { } ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { - tool_name: "create_directory".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.path.clone(), }; Some(event_stream.authorize( diff --git a/crates/agent/src/tools/delete_path_tool.rs b/crates/agent/src/tools/delete_path_tool.rs index f92f5171eaecb229a962b296e0932d627e93ea6b..5787b867bcf22595321266eb05e734903c7dbd31 100644 --- a/crates/agent/src/tools/delete_path_tool.rs +++ b/crates/agent/src/tools/delete_path_tool.rs @@ -49,9 +49,7 @@ impl AgentTool for DeletePathTool { type Input = DeletePathToolInput; type Output = String; - fn name() -> &'static str { - "delete_path" - } + const NAME: &'static str = "delete_path"; fn kind() -> ToolKind { ToolKind::Delete @@ -78,7 +76,7 @@ impl AgentTool for DeletePathTool { let path = input.path; let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &path, settings); + let decision = decide_permission_from_settings(Self::NAME, &path, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -87,7 +85,7 @@ impl AgentTool for DeletePathTool { } ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { - tool_name: "delete_path".to_string(), + tool_name: Self::NAME.to_string(), input_value: path.clone(), }; Some(event_stream.authorize( diff --git a/crates/agent/src/tools/diagnostics_tool.rs b/crates/agent/src/tools/diagnostics_tool.rs index 28d2d2c7e7e127f5afc106999e9d1d459f10dc2c..8479bbbf3dbff7eef51be9a0d85bd2ff79b91935 100644 --- a/crates/agent/src/tools/diagnostics_tool.rs +++ b/crates/agent/src/tools/diagnostics_tool.rs @@ -64,9 +64,7 @@ impl AgentTool for DiagnosticsTool { type Input = DiagnosticsToolInput; type Output = String; - fn name() -> &'static str { - "diagnostics" - } + const NAME: &'static str = "diagnostics"; fn kind() -> acp::ToolKind { acp::ToolKind::Read diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index bc7e5b5289937d6212c662f97238e43ea185684d..0b63587a5aae031c17c0890269de86c63a09f306 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -1,3 +1,5 @@ +use super::restore_file_from_disk_tool::RestoreFileFromDiskTool; +use super::save_file_tool::SaveFileTool; use crate::{ AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, @@ -161,7 +163,7 @@ impl EditFileTool { ) -> Task> { let path_str = input.path.to_string_lossy(); let settings = agent_settings::AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &path_str, settings); + let decision = decide_permission_from_settings(Self::NAME, &path_str, settings); match decision { ToolPermissionDecision::Allow => return Task::ready(Ok(())), @@ -179,7 +181,7 @@ impl EditFileTool { component.as_os_str() == <_ as AsRef>::as_ref(&local_settings_folder) }) { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: Self::NAME.to_string(), input_value: path_str.to_string(), }; return event_stream.authorize( @@ -196,7 +198,7 @@ impl EditFileTool { && canonical_path.starts_with(paths::config_dir()) { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: Self::NAME.to_string(), input_value: path_str.to_string(), }; return event_stream.authorize( @@ -220,7 +222,7 @@ impl EditFileTool { Task::ready(Ok(())) } else { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: Self::NAME.to_string(), input_value: path_str.to_string(), }; event_stream.authorize(&input.display_description, context, cx) @@ -232,9 +234,7 @@ impl AgentTool for EditFileTool { type Input = EditFileToolInput; type Output = EditFileToolOutput; - fn name() -> &'static str { - "edit_file" - } + const NAME: &'static str = "edit_file"; fn kind() -> acp::ToolKind { acp::ToolKind::Edit @@ -342,8 +342,8 @@ impl AgentTool for EditFileTool { let last_read = thread.file_read_times.get(abs_path).copied(); let current = buffer.read(cx).file().and_then(|file| file.disk_state().mtime()); let dirty = buffer.read(cx).is_dirty(); - let has_save = thread.has_tool("save_file"); - let has_restore = thread.has_tool("restore_file_from_disk"); + let has_save = thread.has_tool(SaveFileTool::NAME); + let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME); (last_read, current, dirty, has_save, has_restore) })?; diff --git a/crates/agent/src/tools/fetch_tool.rs b/crates/agent/src/tools/fetch_tool.rs index 59d6b56063943a9c9db46844a5d8901de918c95a..65fd3a9e5311392b92dd2dc594eccae0768c6bd2 100644 --- a/crates/agent/src/tools/fetch_tool.rs +++ b/crates/agent/src/tools/fetch_tool.rs @@ -122,9 +122,7 @@ impl AgentTool for FetchTool { type Input = FetchToolInput; type Output = String; - fn name() -> &'static str { - "fetch" - } + const NAME: &'static str = "fetch"; fn kind() -> acp::ToolKind { acp::ToolKind::Fetch @@ -148,7 +146,7 @@ impl AgentTool for FetchTool { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &input.url, settings); + let decision = decide_permission_from_settings(Self::NAME, &input.url, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -157,7 +155,7 @@ impl AgentTool for FetchTool { } ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { - tool_name: "fetch".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.url.clone(), }; Some(event_stream.authorize( diff --git a/crates/agent/src/tools/find_path_tool.rs b/crates/agent/src/tools/find_path_tool.rs index 5f64a9cbbf569e4729777b3a4e266e788961dadd..202d33b22d7d49e5e582ff58b6f866cc68f7ced3 100644 --- a/crates/agent/src/tools/find_path_tool.rs +++ b/crates/agent/src/tools/find_path_tool.rs @@ -86,9 +86,7 @@ impl AgentTool for FindPathTool { type Input = FindPathToolInput; type Output = FindPathToolOutput; - fn name() -> &'static str { - "find_path" - } + const NAME: &'static str = "find_path"; fn kind() -> acp::ToolKind { acp::ToolKind::Search diff --git a/crates/agent/src/tools/grep_tool.rs b/crates/agent/src/tools/grep_tool.rs index 886b52226f0b351d81771be475db6645873640ee..09e9c9e4fef73575cb8974a6cbea2f327a50034f 100644 --- a/crates/agent/src/tools/grep_tool.rs +++ b/crates/agent/src/tools/grep_tool.rs @@ -80,9 +80,7 @@ impl AgentTool for GrepTool { type Input = GrepToolInput; type Output = String; - fn name() -> &'static str { - "grep" - } + const NAME: &'static str = "grep"; fn kind() -> acp::ToolKind { acp::ToolKind::Search diff --git a/crates/agent/src/tools/list_directory_tool.rs b/crates/agent/src/tools/list_directory_tool.rs index b7ceba5abf39ecd9a50a07b627a35f7231f4a339..4e642dba4f826d50ebcd0d04e7f99209d0be22c7 100644 --- a/crates/agent/src/tools/list_directory_tool.rs +++ b/crates/agent/src/tools/list_directory_tool.rs @@ -51,9 +51,7 @@ impl AgentTool for ListDirectoryTool { type Input = ListDirectoryToolInput; type Output = String; - fn name() -> &'static str { - "list_directory" - } + const NAME: &'static str = "list_directory"; fn kind() -> ToolKind { ToolKind::Read diff --git a/crates/agent/src/tools/move_path_tool.rs b/crates/agent/src/tools/move_path_tool.rs index 7fc56af2ddde848bbc4bfb9a21ab1a4156dd033c..77091e22d81985608f32565c0133469516bdadee 100644 --- a/crates/agent/src/tools/move_path_tool.rs +++ b/crates/agent/src/tools/move_path_tool.rs @@ -57,9 +57,7 @@ impl AgentTool for MovePathTool { type Input = MovePathToolInput; type Output = String; - fn name() -> &'static str { - "move_path" - } + const NAME: &'static str = "move_path"; fn kind() -> ToolKind { ToolKind::Move @@ -100,13 +98,13 @@ impl AgentTool for MovePathTool { let settings = AgentSettings::get_global(cx); let source_decision = - decide_permission_from_settings(Self::name(), &input.source_path, settings); + decide_permission_from_settings(Self::NAME, &input.source_path, settings); if let ToolPermissionDecision::Deny(reason) = source_decision { return Task::ready(Err(anyhow!("{}", reason))); } let dest_decision = - decide_permission_from_settings(Self::name(), &input.destination_path, settings); + decide_permission_from_settings(Self::NAME, &input.destination_path, settings); if let ToolPermissionDecision::Deny(reason) = dest_decision { return Task::ready(Err(anyhow!("{}", reason))); } @@ -118,7 +116,7 @@ impl AgentTool for MovePathTool { let src = MarkdownInlineCode(&input.source_path); let dest = MarkdownInlineCode(&input.destination_path); let context = crate::ToolPermissionContext { - tool_name: "move_path".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.source_path.clone(), }; Some(event_stream.authorize(format!("Move {src} to {dest}"), context, cx)) diff --git a/crates/agent/src/tools/now_tool.rs b/crates/agent/src/tools/now_tool.rs index 3387c0a617017991f8b2590868864287f399ec28..cb1a05f412340b5a31096851bd973892a51a9775 100644 --- a/crates/agent/src/tools/now_tool.rs +++ b/crates/agent/src/tools/now_tool.rs @@ -33,9 +33,7 @@ impl AgentTool for NowTool { type Input = NowToolInput; type Output = String; - fn name() -> &'static str { - "now" - } + const NAME: &'static str = "now"; fn kind() -> acp::ToolKind { acp::ToolKind::Other diff --git a/crates/agent/src/tools/open_tool.rs b/crates/agent/src/tools/open_tool.rs index ccbe29153e632e9a2a735f54868991d450d1cdfc..dac797769fa692a32c659f67cefdd92655b1a567 100644 --- a/crates/agent/src/tools/open_tool.rs +++ b/crates/agent/src/tools/open_tool.rs @@ -38,9 +38,7 @@ impl AgentTool for OpenTool { type Input = OpenToolInput; type Output = String; - fn name() -> &'static str { - "open" - } + const NAME: &'static str = "open"; fn kind() -> ToolKind { ToolKind::Execute @@ -67,7 +65,7 @@ impl AgentTool for OpenTool { // If path_or_url turns out to be a path in the project, make it absolute. let abs_path = to_absolute_path(&input.path_or_url, self.project.clone(), cx); let context = crate::ToolPermissionContext { - tool_name: "open".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.path_or_url.clone(), }; let authorize = diff --git a/crates/agent/src/tools/read_file_tool.rs b/crates/agent/src/tools/read_file_tool.rs index 8b13452e9357921a1f7a43a51a3364594b481c42..f6395742331ab17947c6885e186aab31bb8c826c 100644 --- a/crates/agent/src/tools/read_file_tool.rs +++ b/crates/agent/src/tools/read_file_tool.rs @@ -79,9 +79,7 @@ impl AgentTool for ReadFileTool { type Input = ReadFileToolInput; type Output = LanguageModelToolResultContent; - fn name() -> &'static str { - "read_file" - } + const NAME: &'static str = "read_file"; fn kind() -> acp::ToolKind { acp::ToolKind::Read diff --git a/crates/agent/src/tools/restore_file_from_disk_tool.rs b/crates/agent/src/tools/restore_file_from_disk_tool.rs index 0429706c5d9b449b7b54d19655fadeadd5adfbbc..bf329a02bd0c0e8a0dcadf61e9b3b269d7906c76 100644 --- a/crates/agent/src/tools/restore_file_from_disk_tool.rs +++ b/crates/agent/src/tools/restore_file_from_disk_tool.rs @@ -39,9 +39,7 @@ impl AgentTool for RestoreFileFromDiskTool { type Input = RestoreFileFromDiskToolInput; type Output = String; - fn name() -> &'static str { - "restore_file_from_disk" - } + const NAME: &'static str = "restore_file_from_disk"; fn kind() -> acp::ToolKind { acp::ToolKind::Other diff --git a/crates/agent/src/tools/save_file_tool.rs b/crates/agent/src/tools/save_file_tool.rs index 44f701941fc7b30825e4b131d6fb5206a7a4a4d7..c199df414c54e22556a3d6dc562ce93c28fc5762 100644 --- a/crates/agent/src/tools/save_file_tool.rs +++ b/crates/agent/src/tools/save_file_tool.rs @@ -41,9 +41,7 @@ impl AgentTool for SaveFileTool { type Input = SaveFileToolInput; type Output = String; - fn name() -> &'static str { - "save_file" - } + const NAME: &'static str = "save_file"; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -72,7 +70,7 @@ impl AgentTool for SaveFileTool { for path in &input.paths { let path_str = path.to_string_lossy(); - let decision = decide_permission_from_settings(Self::name(), &path_str, settings); + let decision = decide_permission_from_settings(Self::NAME, &path_str, settings); match decision { ToolPermissionDecision::Allow => {} ToolPermissionDecision::Deny(reason) => { @@ -113,7 +111,7 @@ impl AgentTool for SaveFileTool { .map(|p| p.to_string_lossy().to_string()) .unwrap_or_default(); let context = crate::ToolPermissionContext { - tool_name: "save_file".to_string(), + tool_name: Self::NAME.to_string(), input_value: first_path, }; Some(event_stream.authorize(title, context, cx)) diff --git a/crates/agent/src/tools/streaming_edit_file_tool.rs b/crates/agent/src/tools/streaming_edit_file_tool.rs index 3591d8fe3044ab436ba015a374b918276471f3da..915c25726222ac5f0c2c1582631242bb3a0b2e4a 100644 --- a/crates/agent/src/tools/streaming_edit_file_tool.rs +++ b/crates/agent/src/tools/streaming_edit_file_tool.rs @@ -1,3 +1,6 @@ +use super::edit_file_tool::EditFileTool; +use super::restore_file_from_disk_tool::RestoreFileFromDiskTool; +use super::save_file_tool::SaveFileTool; use crate::{ AgentTool, Templates, Thread, ToolCallEventStream, ToolPermissionDecision, decide_permission_from_settings, edit_agent::streaming_fuzzy_matcher::StreamingFuzzyMatcher, @@ -168,7 +171,7 @@ impl StreamingEditFileTool { ) -> Task> { let path_str = input.path.to_string_lossy(); let settings = agent_settings::AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &path_str, settings); + let decision = decide_permission_from_settings(Self::NAME, &path_str, settings); match decision { ToolPermissionDecision::Allow => return Task::ready(Ok(())), @@ -184,7 +187,7 @@ impl StreamingEditFileTool { component.as_os_str() == <_ as AsRef>::as_ref(&local_settings_folder) }) { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: EditFileTool::NAME.to_string(), input_value: path_str.to_string(), }; return event_stream.authorize( @@ -198,7 +201,7 @@ impl StreamingEditFileTool { && canonical_path.starts_with(paths::config_dir()) { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: EditFileTool::NAME.to_string(), input_value: path_str.to_string(), }; return event_stream.authorize( @@ -218,7 +221,7 @@ impl StreamingEditFileTool { Task::ready(Ok(())) } else { let context = crate::ToolPermissionContext { - tool_name: "edit_file".to_string(), + tool_name: EditFileTool::NAME.to_string(), input_value: path_str.to_string(), }; event_stream.authorize(&input.display_description, context, cx) @@ -230,9 +233,7 @@ impl AgentTool for StreamingEditFileTool { type Input = StreamingEditFileToolInput; type Output = StreamingEditFileToolOutput; - fn name() -> &'static str { - "streaming_edit_file" - } + const NAME: &'static str = "streaming_edit_file"; fn kind() -> acp::ToolKind { acp::ToolKind::Edit @@ -330,8 +331,8 @@ impl AgentTool for StreamingEditFileTool { .file() .and_then(|file| file.disk_state().mtime()); let dirty = buffer.read(cx).is_dirty(); - let has_save = thread.has_tool("save_file"); - let has_restore = thread.has_tool("restore_file_from_disk"); + let has_save = thread.has_tool(SaveFileTool::NAME); + let has_restore = thread.has_tool(RestoreFileFromDiskTool::NAME); (last_read, current, dirty, has_save, has_restore) })?; diff --git a/crates/agent/src/tools/subagent_tool.rs b/crates/agent/src/tools/subagent_tool.rs index 17de5284caa14ab37c0ac12e90717662b7e0e243..ec7fa937b7e9ec3168f107e3a7bb50e8cf948da4 100644 --- a/crates/agent/src/tools/subagent_tool.rs +++ b/crates/agent/src/tools/subagent_tool.rs @@ -129,9 +129,7 @@ impl AgentTool for SubagentTool { type Input = SubagentToolInput; type Output = String; - fn name() -> &'static str { - acp_thread::SUBAGENT_TOOL_NAME - } + const NAME: &'static str = acp_thread::SUBAGENT_TOOL_NAME; fn kind() -> acp::ToolKind { acp::ToolKind::Other @@ -529,7 +527,7 @@ mod tests { #[test] fn test_subagent_tool_name() { - assert_eq!(SubagentTool::name(), "subagent"); + assert_eq!(SubagentTool::NAME, "subagent"); } #[test] @@ -542,7 +540,7 @@ struct SubagentDisplayConnection; impl AgentConnection for SubagentDisplayConnection { fn telemetry_id(&self) -> SharedString { - "subagent".into() + acp_thread::SUBAGENT_TOOL_NAME.into() } fn auth_methods(&self) -> &[acp::AuthMethod] { diff --git a/crates/agent/src/tools/terminal_tool.rs b/crates/agent/src/tools/terminal_tool.rs index 5e2b7ead1e237841e135757ecfb0f59309ffcb64..37537642bc047e1a1dbdb4ee8600ef576feea5d9 100644 --- a/crates/agent/src/tools/terminal_tool.rs +++ b/crates/agent/src/tools/terminal_tool.rs @@ -65,9 +65,7 @@ impl AgentTool for TerminalTool { type Input = TerminalToolInput; type Output = String; - fn name() -> &'static str { - "terminal" - } + const NAME: &'static str = "terminal"; fn kind() -> acp::ToolKind { acp::ToolKind::Execute @@ -97,7 +95,7 @@ impl AgentTool for TerminalTool { }; let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &input.command, settings); + let decision = decide_permission_from_settings(Self::NAME, &input.command, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -106,7 +104,7 @@ impl AgentTool for TerminalTool { } ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { - tool_name: "terminal".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.command.clone(), }; Some(event_stream.authorize(self.initial_title(Ok(input.clone()), cx), context, cx)) diff --git a/crates/agent/src/tools/thinking_tool.rs b/crates/agent/src/tools/thinking_tool.rs index 96024326f6f1610f500972b1a98be45258e3966b..10e7b01bfbd88f3973e8618207170d6991ced579 100644 --- a/crates/agent/src/tools/thinking_tool.rs +++ b/crates/agent/src/tools/thinking_tool.rs @@ -21,9 +21,7 @@ impl AgentTool for ThinkingTool { type Input = ThinkingToolInput; type Output = String; - fn name() -> &'static str { - "thinking" - } + const NAME: &'static str = "thinking"; fn kind() -> acp::ToolKind { acp::ToolKind::Think diff --git a/crates/agent/src/tools/web_search_tool.rs b/crates/agent/src/tools/web_search_tool.rs index 7bc9602a074d7ffe799a7022d98e99ab4ff337c1..65a51b6cb3befb0c47f32e769cd3fd0332f50589 100644 --- a/crates/agent/src/tools/web_search_tool.rs +++ b/crates/agent/src/tools/web_search_tool.rs @@ -46,9 +46,7 @@ impl AgentTool for WebSearchTool { type Input = WebSearchToolInput; type Output = WebSearchToolOutput; - fn name() -> &'static str { - "web_search" - } + const NAME: &'static str = "web_search"; fn kind() -> acp::ToolKind { acp::ToolKind::Fetch @@ -74,7 +72,7 @@ impl AgentTool for WebSearchTool { cx: &mut App, ) -> Task> { let settings = AgentSettings::get_global(cx); - let decision = decide_permission_from_settings(Self::name(), &input.query, settings); + let decision = decide_permission_from_settings(Self::NAME, &input.query, settings); let authorize = match decision { ToolPermissionDecision::Allow => None, @@ -83,7 +81,7 @@ impl AgentTool for WebSearchTool { } ToolPermissionDecision::Confirm => { let context = crate::ToolPermissionContext { - tool_name: "web_search".to_string(), + tool_name: Self::NAME.to_string(), input_value: input.query.clone(), }; Some(event_stream.authorize( diff --git a/crates/agent_ui/src/acp/thread_view.rs b/crates/agent_ui/src/acp/thread_view.rs index d44a9c7b94aa339eb52cf1010dc479deceb017ff..ce17e9325cc3ed813593d02cbc087bc9515cf9c3 100644 --- a/crates/agent_ui/src/acp/thread_view.rs +++ b/crates/agent_ui/src/acp/thread_view.rs @@ -2293,7 +2293,7 @@ pub(crate) mod tests { AgentSessionList, AgentSessionListRequest, AgentSessionListResponse, StubAgentConnection, }; use action_log::ActionLog; - use agent::ToolPermissionContext; + use agent::{AgentTool, EditFileTool, FetchTool, TerminalTool, ToolPermissionContext}; use agent_client_protocol::SessionId; use editor::MultiBufferOffset; use fs::FakeFs; @@ -4056,8 +4056,9 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `cargo build --release`") .kind(acp::ToolKind::Edit); - let permission_options = ToolPermissionContext::new("terminal", "cargo build --release") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4163,8 +4164,8 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Edit `src/main.rs`") .kind(acp::ToolKind::Edit); - let permission_options = - ToolPermissionContext::new("edit_file", "src/main.rs").build_permission_options(); + let permission_options = ToolPermissionContext::new(EditFileTool::NAME, "src/main.rs") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4251,7 +4252,8 @@ pub(crate) mod tests { .kind(acp::ToolKind::Fetch); let permission_options = - ToolPermissionContext::new("fetch", "https://docs.rs/gpui").build_permission_options(); + ToolPermissionContext::new(FetchTool::NAME, "https://docs.rs/gpui") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4338,8 +4340,9 @@ pub(crate) mod tests { .kind(acp::ToolKind::Edit); // No pattern button since ./deploy.sh doesn't match the alphanumeric pattern - let permission_options = ToolPermissionContext::new("terminal", "./deploy.sh --production") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, "./deploy.sh --production") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4437,7 +4440,7 @@ pub(crate) mod tests { acp::ToolCall::new(tool_call_id.clone(), "Run `cargo test`").kind(acp::ToolKind::Edit); let permission_options = - ToolPermissionContext::new("terminal", "cargo test").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, "cargo test").build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4520,8 +4523,8 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `npm install`").kind(acp::ToolKind::Edit); - let permission_options = - ToolPermissionContext::new("terminal", "npm install").build_permission_options(); + let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "npm install") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4609,8 +4612,8 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `cargo build`").kind(acp::ToolKind::Edit); - let permission_options = - ToolPermissionContext::new("terminal", "cargo build").build_permission_options(); + let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "cargo build") + .build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4688,8 +4691,8 @@ pub(crate) mod tests { let tool_call = acp::ToolCall::new(tool_call_id.clone(), "Run `npm install`").kind(acp::ToolKind::Edit); - let permission_options = - ToolPermissionContext::new("terminal", "npm install").build_permission_options(); + let permission_options = ToolPermissionContext::new(TerminalTool::NAME, "npm install") + .build_permission_options(); // Verify we have the expected options let PermissionOptions::Dropdown(choices) = &permission_options else { @@ -4791,7 +4794,7 @@ pub(crate) mod tests { acp::ToolCall::new(tool_call_id.clone(), "Run `git push`").kind(acp::ToolKind::Edit); let permission_options = - ToolPermissionContext::new("terminal", "git push").build_permission_options(); + ToolPermissionContext::new(TerminalTool::NAME, "git push").build_permission_options(); let connection = StubAgentConnection::new().with_permission_requests(HashMap::from_iter([( @@ -4850,8 +4853,9 @@ pub(crate) mod tests { #[gpui::test] async fn test_option_id_transformation_for_allow() { - let permission_options = ToolPermissionContext::new("terminal", "cargo build --release") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); @@ -4874,8 +4878,9 @@ pub(crate) mod tests { #[gpui::test] async fn test_option_id_transformation_for_deny() { - let permission_options = ToolPermissionContext::new("terminal", "cargo build --release") - .build_permission_options(); + let permission_options = + ToolPermissionContext::new(TerminalTool::NAME, "cargo build --release") + .build_permission_options(); let PermissionOptions::Dropdown(choices) = permission_options else { panic!("Expected dropdown permission options"); diff --git a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs index c79d68f5b1243cd69eb016ab65d9f3a31da9a8c4..c9305ef29012b1150b53444fe76aa73cc0a56cd8 100644 --- a/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs +++ b/crates/agent_ui/src/agent_configuration/manage_profiles_modal.rs @@ -2,9 +2,10 @@ mod profile_modal_header; use std::sync::Arc; -use agent::ContextServerRegistry; +use agent::{AgentTool, ContextServerRegistry, SubagentTool}; use agent_settings::{AgentProfile, AgentProfileId, AgentSettings, builtin_profiles}; use editor::Editor; +use feature_flags::{FeatureFlagAppExt as _, SubagentsFeatureFlag}; use fs::Fs; use gpui::{DismissEvent, Entity, EventEmitter, FocusHandle, Focusable, Subscription, prelude::*}; use language_model::{LanguageModel, LanguageModelRegistry}; @@ -350,14 +351,21 @@ impl ManageProfilesModal { return; }; - //todo: This causes the web search tool to show up even it only works when using zed hosted models - let tool_names: Vec> = agent::supported_built_in_tool_names( - self.active_model.as_ref().map(|model| model.provider_id()), - cx, - ) - .into_iter() - .map(|s| Arc::from(s)) - .collect(); + let provider = self.active_model.as_ref().map(|model| model.provider_id()); + let tool_names: Vec> = agent::ALL_TOOL_NAMES + .iter() + .copied() + .filter(|name| { + let supported_by_provider = provider.as_ref().map_or(true, |provider| { + agent::tool_supports_provider(name, provider) + }); + let enabled_by_feature_flag = + *name != SubagentTool::NAME || cx.has_flag::(); + + supported_by_provider && enabled_by_feature_flag + }) + .map(Arc::from) + .collect(); let tool_picker = cx.new(|cx| { let delegate = ToolPickerDelegate::builtin_tools( diff --git a/crates/eval/src/examples/planets.rs b/crates/eval/src/examples/planets.rs index 6b6ca0e3fe75633c49f11f24a24835dc58886a01..1ef257a55db82abe3dab9ef006176df4b12cec5f 100644 --- a/crates/eval/src/examples/planets.rs +++ b/crates/eval/src/examples/planets.rs @@ -36,9 +36,9 @@ impl Example for Planets { let mut terminal_tool_uses = 0; for tool_use in response.tool_calls() { - if tool_use.name == OpenTool::name() { + if tool_use.name == OpenTool::NAME { open_tool_uses += 1; - } else if tool_use.name == TerminalTool::name() { + } else if tool_use.name == TerminalTool::NAME { terminal_tool_uses += 1; } }