From 5eb6a465dcf71ecfa3602b6421e4c7b5824589cd Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Tue, 5 May 2026 14:11:24 +0200 Subject: [PATCH 1/6] agent: Remove `display_description` from edit tool (#55752) We did not really use it in practice (we would only display it in the tool card header until we received a path), so as is it just wastes tokens. Therefore removing it. Release Notes: - agent: Reduce token usage when LLM edits file --- crates/agent/src/tests/mod.rs | 3 - crates/agent/src/tools/edit_file_tool.rs | 298 ++++----------------- crates/agent/src/tools/tool_permissions.rs | 13 +- 3 files changed, 51 insertions(+), 263 deletions(-) diff --git a/crates/agent/src/tests/mod.rs b/crates/agent/src/tests/mod.rs index c2efda7673d..2a4e9c255fb 100644 --- a/crates/agent/src/tests/mod.rs +++ b/crates/agent/src/tests/mod.rs @@ -6061,7 +6061,6 @@ async fn test_edit_file_tool_deny_rule_blocks_edit(cx: &mut TestAppContext) { let task = cx.update(|cx| { tool.run( ToolInput::resolved(crate::EditFileToolInput { - display_description: "Edit sensitive file".to_string(), path: "root/sensitive_config.txt".into(), mode: crate::EditFileMode::Edit, content: None, @@ -6496,7 +6495,6 @@ async fn test_edit_file_tool_allow_rule_skips_confirmation(cx: &mut TestAppConte let _task = cx.update(|cx| { tool.run( ToolInput::resolved(crate::EditFileToolInput { - display_description: "Edit README".to_string(), path: "root/README.md".into(), mode: crate::EditFileMode::Edit, content: None, @@ -6569,7 +6567,6 @@ async fn test_edit_file_tool_allow_still_prompts_for_local_settings(cx: &mut Tes let _task = cx.update(|cx| { tool.run( ToolInput::resolved(crate::EditFileToolInput { - display_description: "Edit local settings".to_string(), path: "root/.zed/settings.json".into(), mode: crate::EditFileMode::Edit, content: None, diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index a51e9883224..0e6493953c9 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -49,18 +49,6 @@ const DEFAULT_UI_TEXT: &str = "Editing file"; /// - Use the `list_directory` tool to verify the parent directory exists and is the correct location #[derive(Clone, Debug, Serialize, Deserialize, JsonSchema)] pub struct EditFileToolInput { - /// A one-line, user-friendly markdown description of the edit. This will be shown in the UI. - /// - /// Be terse, but also descriptive in what you want to achieve with this edit. Avoid generic instructions. - /// - /// NEVER mention the file path in this description. - /// - /// Fix API endpoint URLs - /// Update copyright year in `page_footer` - /// - /// Make sure to include this field before all the others in the input object so that we can display it immediately. - pub display_description: String, - /// The full path of the file to create or modify in the project. /// /// WARNING: When specifying which file path need changing, you MUST start each path with one of the project's root directories. @@ -131,8 +119,6 @@ pub struct Edit { #[derive(Clone, Default, Debug, Deserialize)] struct EditFileToolPartialInput { - #[serde(default)] - display_description: Option, #[serde(default)] path: Option, #[serde(default, deserialize_with = "deserialize_maybe_stringified")] @@ -278,14 +264,12 @@ impl EditFileTool { fn authorize( &self, path: &PathBuf, - description: &str, event_stream: &ToolCallEventStream, cx: &mut App, ) -> Task> { super::tool_permissions::authorize_file_edit( EditFileTool::NAME, path, - description, &self.thread, event_stream, cx, @@ -360,14 +344,12 @@ impl EditFileTool { && path_complete && let EditFileToolPartialInput { path: Some(path), - display_description: Some(display_description), mode: Some(mode), .. } = &parsed { match EditSession::new( PathBuf::from(path), - display_description, *mode, self, event_stream, @@ -400,7 +382,6 @@ impl EditFileTool { } else { match EditSession::new( full_input.path.clone(), - &full_input.display_description, full_input.mode, self, event_stream, @@ -505,12 +486,6 @@ impl AgentTool for EditFileTool { .unwrap_or_else(|| path.to_string()) .into(); } - - let description = input.display_description.unwrap_or_default(); - let description = description.trim(); - if !description.is_empty() { - return description.to_string().into(); - } } DEFAULT_UI_TEXT.into() @@ -881,7 +856,6 @@ impl EditPipeline { impl EditSession { async fn new( path: PathBuf, - display_description: &str, mode: EditFileMode, tool: &EditFileTool, event_stream: &ToolCallEventStream, @@ -901,7 +875,7 @@ impl EditSession { ToolCallUpdateFields::new().locations(vec![ToolCallLocation::new(abs_path.clone())]), ); - cx.update(|cx| tool.authorize(&path, &display_description, event_stream, cx)) + cx.update(|cx| tool.authorize(&path, event_stream, cx)) .await .map_err(|e| e.to_string())?; @@ -1296,7 +1270,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Create new file".into(), path: "root/dir/new_file.txt".into(), mode: EditFileMode::Write, content: Some("Hello, World!".into()), @@ -1323,7 +1296,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Overwrite file".into(), path: "root/file.txt".into(), mode: EditFileMode::Write, content: Some("new content".into()), @@ -1353,7 +1325,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit lines".into(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1385,7 +1356,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit multiple lines".into(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1426,7 +1396,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit adjacent lines".into(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1467,7 +1436,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit multiple lines in ascending order".into(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1504,7 +1472,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Some edit".into(), path: "root/nonexistent_file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1540,7 +1507,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit file".into(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -1573,18 +1539,16 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Send partials simulating LLM streaming: description first, then path, then mode - sender.send_partial(json!({"display_description": "Edit lines"})); + sender.send_partial(json!({})); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt" })); cx.run_until_parked(); // Path is NOT yet complete because mode hasn't appeared — no buffer open yet sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit" })); @@ -1592,7 +1556,6 @@ mod tests { // Now send the final complete input sender.send_full(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 2", "new_text": "modified line 2"}] @@ -1615,14 +1578,12 @@ mod tests { // Send partial with path but NO mode — path should NOT be treated as complete sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file" })); cx.run_until_parked(); // Now the path grows and mode appears sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write" })); @@ -1630,7 +1591,6 @@ mod tests { // Send final sender.send_full(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new content" @@ -1653,7 +1613,7 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Send a partial - sender.send_partial(json!({"display_description": "Edit"})); + sender.send_partial(json!({})); cx.run_until_parked(); // Cancel during streaming @@ -1686,24 +1646,21 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Simulate fine-grained streaming of the JSON - sender.send_partial(json!({"display_description": "Edit multiple"})); + sender.send_partial(json!({})); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 1"}] @@ -1711,7 +1668,6 @@ mod tests { cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1723,7 +1679,6 @@ mod tests { // Send final complete input sender.send_full(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1750,18 +1705,16 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Stream partials for create mode - sender.send_partial(json!({"display_description": "Create new file"})); + sender.send_partial(json!({})); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "Hello, " @@ -1770,7 +1723,6 @@ mod tests { // Final with full content sender.send_full(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "Hello, World!" @@ -1793,7 +1745,6 @@ mod tests { // Send final immediately with no partials (simulates non-streaming path) sender.send_full(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 2", "new_text": "modified line 2"}] @@ -1818,11 +1769,10 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Stream description, path, mode - sender.send_partial(json!({"display_description": "Edit multiple lines"})); + sender.send_partial(json!({})); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit" })); @@ -1830,7 +1780,6 @@ mod tests { // First edit starts streaming (old_text only, still in progress) sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 1"}] @@ -1857,7 +1806,6 @@ mod tests { // Second edit appears — this proves the first edit is complete, so it // should be applied immediately during streaming sender.send_partial(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1884,7 +1832,6 @@ mod tests { // Send final complete input sender.send_full(json!({ - "display_description": "Edit multiple lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1917,7 +1864,6 @@ mod tests { // Setup: description + path + mode sender.send_partial(json!({ - "display_description": "Edit three lines", "path": "root/file.txt", "mode": "edit" })); @@ -1925,7 +1871,6 @@ mod tests { // Edit 1 in progress sender.send_partial(json!({ - "display_description": "Edit three lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "aaa", "new_text": "AAA"}] @@ -1934,7 +1879,6 @@ mod tests { // Edit 2 appears — edit 1 is now complete and should be applied sender.send_partial(json!({ - "display_description": "Edit three lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1957,7 +1901,6 @@ mod tests { // Edit 3 appears — edit 2 is now complete and should be applied sender.send_partial(json!({ - "display_description": "Edit three lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -1980,7 +1923,6 @@ mod tests { // Send final sender.send_full(json!({ - "display_description": "Edit three lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -2007,7 +1949,6 @@ mod tests { // Setup sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit" })); @@ -2015,7 +1956,6 @@ mod tests { // Edit 1 (valid) in progress — not yet complete (no second edit) sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -2027,7 +1967,6 @@ mod tests { // Edit 2 appears (will fail to match) — this makes edit 1 complete. // Edit 1 should be applied. Edit 2 is still in-progress (last edit). sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -2054,7 +1993,6 @@ mod tests { // Edit 3 appears — this makes edit 2 "complete", triggering its // resolution which should fail (old_text doesn't exist in the file). sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -2101,14 +2039,12 @@ mod tests { // Setup + single edit that stays in-progress (no second edit to prove completion) sender.send_partial(json!({ - "display_description": "Single edit", "path": "root/file.txt", "mode": "edit", })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Single edit", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "hello world", "new_text": "goodbye world"}] @@ -2133,7 +2069,6 @@ mod tests { // Send final — the edit is applied during finalization sender.send_full(json!({ - "display_description": "Single edit", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "hello world", "new_text": "goodbye world"}] @@ -2156,20 +2091,16 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Send progressively more complete partial snapshots, as the LLM would - sender.send_partial(json!({ - "display_description": "Edit lines" - })); + sender.send_partial(json!({})); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 2", "new_text": "modified line 2"}] @@ -2178,7 +2109,6 @@ mod tests { // Send the final complete input sender.send_full(json!({ - "display_description": "Edit lines", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "line 2", "new_text": "modified line 2"}] @@ -2201,9 +2131,7 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Send a partial then drop the sender without sending final - sender.send_partial(json!({ - "display_description": "Edit file" - })); + sender.send_partial(json!({})); cx.run_until_parked(); drop(sender); @@ -2227,15 +2155,13 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); // Buffer several partials before sending the final - sender.send_partial(json!({"display_description": "Create"})); - sender.send_partial(json!({"display_description": "Create", "path": "root/dir/new.txt"})); + sender.send_partial(json!({})); + sender.send_partial(json!({"path": "root/dir/new.txt"})); sender.send_partial(json!({ - "display_description": "Create", "path": "root/dir/new.txt", "mode": "write" })); sender.send_full(json!({ - "display_description": "Create", "path": "root/dir/new.txt", "mode": "write", "content": "streamed content" @@ -2419,14 +2345,12 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Create main function", "path": "root/src/main.rs", "mode": "write" })); cx.run_until_parked(); sender.send_full(json!({ - "display_description": "Create main function", "path": "root/src/main.rs", "mode": "write", "content": UNFORMATTED_CONTENT @@ -2477,14 +2401,12 @@ mod tests { let task = cx.update(|cx| tool2.run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Update main function", "path": "root/src/main.rs", "mode": "write" })); cx.run_until_parked(); sender.send_full(json!({ - "display_description": "Update main function", "path": "root/src/main.rs", "mode": "write", "content": UNFORMATTED_CONTENT @@ -2540,7 +2462,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Create main function".into(), path: "root/src/main.rs".into(), mode: EditFileMode::Write, content: Some(CONTENT_WITH_TRAILING_WHITESPACE.into()), @@ -2588,7 +2509,6 @@ mod tests { .update(|cx| { tool2.run( ToolInput::resolved(EditFileToolInput { - display_description: "Update main function".into(), path: "root/src/main.rs".into(), mode: EditFileMode::Write, content: Some(CONTENT_WITH_TRAILING_WHITESPACE.into()), @@ -2617,52 +2537,40 @@ mod tests { // Test 1: Path with .zed component should require confirmation let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = cx.update(|cx| { - tool.authorize( - &PathBuf::from(".zed/settings.json"), - "test 1", - &stream_tx, - cx, - ) - }); + let _auth = + cx.update(|cx| tool.authorize(&PathBuf::from(".zed/settings.json"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; assert_eq!( event.tool_call.fields.title, - Some("test 1 (local settings)".into()) + Some("Edit `.zed/settings.json` (local settings)".into()) ); // Test 2: Path outside project should require confirmation let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = - cx.update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), "test 2", &stream_tx, cx)); + let _auth = cx.update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; - assert_eq!(event.tool_call.fields.title, Some("test 2".into())); + assert_eq!( + event.tool_call.fields.title, + Some("Edit `/etc/hosts`".into()) + ); // Test 3: Relative path without .zed should not require confirmation let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - cx.update(|cx| { - tool.authorize(&PathBuf::from("root/src/main.rs"), "test 3", &stream_tx, cx) - }) - .await - .unwrap(); + cx.update(|cx| tool.authorize(&PathBuf::from("root/src/main.rs"), &stream_tx, cx)) + .await + .unwrap(); assert!(stream_rx.try_recv().is_err()); // Test 4: Path with .zed in the middle should require confirmation let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = cx.update(|cx| { - tool.authorize( - &PathBuf::from("root/.zed/tasks.json"), - "test 4", - &stream_tx, - cx, - ) - }); + let _auth = + cx.update(|cx| tool.authorize(&PathBuf::from("root/.zed/tasks.json"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; assert_eq!( event.tool_call.fields.title, - Some("test 4 (local settings)".into()) + Some("Edit `root/.zed/tasks.json` (local settings)".into()) ); // Test 5: When global default is allow, sensitive and outside-project @@ -2675,39 +2583,26 @@ mod tests { // 5.1: .zed/settings.json is a sensitive path — still prompts let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = cx.update(|cx| { - tool.authorize( - &PathBuf::from(".zed/settings.json"), - "test 5.1", - &stream_tx, - cx, - ) - }); + let _auth = + cx.update(|cx| tool.authorize(&PathBuf::from(".zed/settings.json"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; assert_eq!( event.tool_call.fields.title, - Some("test 5.1 (local settings)".into()) + Some("Edit `.zed/settings.json` (local settings)".into()) ); // 5.2: /etc/hosts is outside the project, but Allow auto-approves let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - cx.update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), "test 5.2", &stream_tx, cx)) + cx.update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), &stream_tx, cx)) .await .unwrap(); assert!(stream_rx.try_recv().is_err()); // 5.3: Normal in-project path with allow — no confirmation needed let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - cx.update(|cx| { - tool.authorize( - &PathBuf::from("root/src/main.rs"), - "test 5.3", - &stream_tx, - cx, - ) - }) - .await - .unwrap(); + cx.update(|cx| tool.authorize(&PathBuf::from("root/src/main.rs"), &stream_tx, cx)) + .await + .unwrap(); assert!(stream_rx.try_recv().is_err()); // 5.4: With Confirm default, non-project paths still prompt @@ -2718,11 +2613,13 @@ mod tests { }); let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = cx - .update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), "test 5.4", &stream_tx, cx)); + let _auth = cx.update(|cx| tool.authorize(&PathBuf::from("/etc/hosts"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; - assert_eq!(event.tool_call.fields.title, Some("test 5.4".into())); + assert_eq!( + event.tool_call.fields.title, + Some("Edit `/etc/hosts`".into()) + ); } #[gpui::test] @@ -2744,14 +2641,8 @@ mod tests { }); let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let authorize_task = cx.update(|cx| { - tool.authorize( - &PathBuf::from("link/new.txt"), - "create through symlink", - &stream_tx, - cx, - ) - }); + let authorize_task = + cx.update(|cx| tool.authorize(&PathBuf::from("link/new.txt"), &stream_tx, cx)); let event = stream_rx.expect_authorization().await; assert!( @@ -2808,7 +2699,6 @@ mod tests { let _authorize_task = cx.update(|cx| { tool.authorize( &PathBuf::from("link_to_external/config.txt"), - "edit through symlink", &stream_tx, cx, ) @@ -2854,7 +2744,6 @@ mod tests { let authorize_task = cx.update(|cx| { tool.authorize( &PathBuf::from("link_to_external/config.txt"), - "edit through symlink", &stream_tx, cx, ) @@ -2911,7 +2800,6 @@ mod tests { .update(|cx| { tool.authorize( &PathBuf::from("link_to_external/config.txt"), - "edit through symlink", &stream_tx, cx, ) @@ -2956,8 +2844,7 @@ mod tests { for (path, should_confirm, description) in test_cases { let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let auth = - cx.update(|cx| tool.authorize(&PathBuf::from(path), "Edit file", &stream_tx, cx)); + let auth = cx.update(|cx| tool.authorize(&PathBuf::from(path), &stream_tx, cx)); if should_confirm { stream_rx.expect_authorization().await; @@ -3033,8 +2920,7 @@ mod tests { for (path, should_confirm, description) in test_cases { let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let auth = - cx.update(|cx| tool.authorize(&PathBuf::from(path), "Edit file", &stream_tx, cx)); + let auth = cx.update(|cx| tool.authorize(&PathBuf::from(path), &stream_tx, cx)); if should_confirm { stream_rx.expect_authorization().await; @@ -3092,8 +2978,7 @@ mod tests { for (path, should_confirm, description) in test_cases { let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let auth = - cx.update(|cx| tool.authorize(&PathBuf::from(path), "Edit file", &stream_tx, cx)); + let auth = cx.update(|cx| tool.authorize(&PathBuf::from(path), &stream_tx, cx)); cx.run_until_parked(); @@ -3134,41 +3019,23 @@ mod tests { // Test .zed path with different modes let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); let _auth = cx.update(|cx| { - tool.authorize( - &PathBuf::from("project/.zed/settings.json"), - "Edit settings", - &stream_tx, - cx, - ) + tool.authorize(&PathBuf::from("project/.zed/settings.json"), &stream_tx, cx) }); stream_rx.expect_authorization().await; // Test outside path with different modes let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - let _auth = cx.update(|cx| { - tool.authorize( - &PathBuf::from("/outside/file.txt"), - "Edit file", - &stream_tx, - cx, - ) - }); + let _auth = + cx.update(|cx| tool.authorize(&PathBuf::from("/outside/file.txt"), &stream_tx, cx)); stream_rx.expect_authorization().await; // Test normal path with different modes let (stream_tx, mut stream_rx) = ToolCallEventStream::test(); - cx.update(|cx| { - tool.authorize( - &PathBuf::from("project/normal.txt"), - "Edit file", - &stream_tx, - cx, - ) - }) - .await - .unwrap(); + cx.update(|cx| tool.authorize(&PathBuf::from("project/normal.txt"), &stream_tx, cx)) + .await + .unwrap(); assert!(stream_rx.try_recv().is_err()); } } @@ -3186,7 +3053,6 @@ mod tests { tool.initial_title( Err(json!({ "path": "src/main.rs", - "display_description": "", })), cx ), @@ -3196,27 +3062,6 @@ mod tests { tool.initial_title( Err(json!({ "path": "", - "display_description": "Fix error handling", - })), - cx - ), - "Fix error handling" - ); - assert_eq!( - tool.initial_title( - Err(json!({ - "path": "src/main.rs", - "display_description": "Fix error handling", - })), - cx - ), - "src/main.rs" - ); - assert_eq!( - tool.initial_title( - Err(json!({ - "path": "", - "display_description": "", })), cx ), @@ -3244,7 +3089,6 @@ mod tests { let edit = cx.update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit file".into(), path: path!("/main.rs").into(), mode: EditFileMode::Write, content: Some("new content".into()), @@ -3274,7 +3118,6 @@ mod tests { let edit = cx.update(|cx| { tool.run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit file".into(), path: path!("/main.rs").into(), mode: EditFileMode::Write, content: Some("dropped content".into()), @@ -3323,7 +3166,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "First edit".into(), path: "root/test.txt".into(), mode: EditFileMode::Edit, content: None, @@ -3348,7 +3190,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Second edit".into(), path: "root/test.txt".into(), mode: EditFileMode::Edit, content: None, @@ -3426,7 +3267,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit after external change".into(), path: "root/test.txt".into(), mode: EditFileMode::Edit, content: None, @@ -3511,7 +3351,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit after external change".into(), path: "root/test.txt".into(), mode: EditFileMode::Edit, content: None, @@ -3596,7 +3435,6 @@ mod tests { .update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit with dirty buffer".into(), path: "root/test.txt".into(), mode: EditFileMode::Edit, content: None, @@ -3652,7 +3490,6 @@ mod tests { // Setup: resolve the buffer sender.send_partial(json!({ - "display_description": "Overlapping edits", "path": "root/file.txt", "mode": "edit" })); @@ -3664,7 +3501,6 @@ mod tests { // in the modified buffer and replaces it with "ZZZ". // Edit 3 exists only to mark edit 2 as "complete" during streaming. sender.send_partial(json!({ - "display_description": "Overlapping edits", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -3677,7 +3513,6 @@ mod tests { // Send the final input with all three edits. sender.send_full(json!({ - "display_description": "Overlapping edits", "path": "root/file.txt", "mode": "edit", "edits": [ @@ -3703,7 +3538,6 @@ mod tests { // Transition to BufferResolved sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write" })); @@ -3711,7 +3545,6 @@ mod tests { // Stream content incrementally sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "line 1\n" @@ -3729,7 +3562,6 @@ mod tests { // Stream more content sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "line 1\nline 2\n" @@ -3739,7 +3571,6 @@ mod tests { // Stream final chunk sender.send_partial(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "line 1\nline 2\nline 3\n" @@ -3752,7 +3583,6 @@ mod tests { // Send final input sender.send_full(json!({ - "display_description": "Create new file", "path": "root/dir/new_file.txt", "mode": "write", "content": "line 1\nline 2\nline 3\n" @@ -3778,13 +3608,11 @@ mod tests { // Transition to BufferResolved sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write" })); @@ -3802,7 +3630,6 @@ mod tests { // Stream first content chunk sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new line 1\n" @@ -3816,7 +3643,6 @@ mod tests { // Send final input sender.send_full(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new line 1\nnew line 2\n" @@ -3849,7 +3675,6 @@ mod tests { // Transition to BufferResolved sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write" })); @@ -3868,7 +3693,6 @@ mod tests { // First content partial replaces old content sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new line 1\n" @@ -3878,7 +3702,6 @@ mod tests { // Subsequent content partials append sender.send_partial(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new line 1\nnew line 2\n" @@ -3891,7 +3714,6 @@ mod tests { // Send final input with complete content sender.send_full(json!({ - "display_description": "Overwrite file", "path": "root/file.txt", "mode": "write", "content": "new line 1\nnew line 2\nnew line 3\n" @@ -3917,7 +3739,6 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit" })); @@ -3929,7 +3750,6 @@ mod tests { // partial 1: old_text = "hello\\" (fixer closes incomplete \n as \\) // partial 2: old_text = "hello\nworld" (fixer corrected the escape) sender.send_partial(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "hello\\"}] @@ -3938,7 +3758,6 @@ mod tests { // Now the fixer corrects it to the real newline. sender.send_partial(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "hello\nworld"}] @@ -3947,7 +3766,6 @@ mod tests { // Send final. sender.send_full(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit", "edits": [{"old_text": "hello\nworld", "new_text": "HELLO\nWORLD"}] @@ -3969,14 +3787,12 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit" })); cx.run_until_parked(); sender.send_full(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "edit", "edits": "[{\"old_text\": \"hello\\nworld\", \"new_text\": \"HELLO\\nWORLD\"}]" @@ -4005,7 +3821,6 @@ mod tests { let task = cx.update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Edit lines".to_string(), path: "root/file.txt".into(), mode: EditFileMode::Edit, content: None, @@ -4049,7 +3864,6 @@ mod tests { let task = cx.update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Overwrite file".to_string(), path: "root/file.txt".into(), mode: EditFileMode::Write, content: Some("completely new content".into()), @@ -4084,20 +3898,17 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "write" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "write", "content": "new_content" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "write", "content": "new_content", "path": "root" @@ -4106,7 +3917,6 @@ mod tests { // Send final. sender.send_full(json!({ - "display_description": "Overwrite file", "mode": "write", "content": "new_content", "path": "root/file.txt" @@ -4130,27 +3940,23 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "edit" })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "edit", "edits": [{"old_text": "old_content"}] })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "edit", "edits": [{"old_text": "old_content", "new_text": "new_content"}] })); cx.run_until_parked(); sender.send_partial(json!({ - "display_description": "Overwrite file", "mode": "edit", "edits": [{"old_text": "old_content", "new_text": "new_content"}], "path": "root" @@ -4159,7 +3965,6 @@ mod tests { // Send final. sender.send_full(json!({ - "display_description": "Overwrite file", "mode": "edit", "edits": [{"old_text": "old_content", "new_text": "new_content"}], "path": "root/file.txt" @@ -4200,7 +4005,6 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_full(json!({ - "display_description": "Remove extra blank lines", "path": "root/file.rs", "mode": "edit", "edits": [{"old_text": old_text, "new_text": new_text}] @@ -4241,7 +4045,6 @@ mod tests { let task = cx.update(|cx| tool.clone().run(input, event_stream, cx)); sender.send_full(json!({ - "display_description": "description", "path": "root/file.rs", "mode": "edit", "edits": [{"old_text": old_text, "new_text": new_text}] @@ -4278,7 +4081,6 @@ mod tests { let task = cx.update(|cx| { tool.clone().run( ToolInput::resolved(EditFileToolInput { - display_description: "Create new file".into(), path: "root/dir/new_file.txt".into(), mode: EditFileMode::Write, content: Some("Hello, World!".into()), @@ -4318,7 +4120,6 @@ mod tests { #[test] fn test_input_deserializes_double_encoded_fields() { let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "\"edit\"", "edits": "[{\"old_text\": \"hello\\nworld\", \"new_text\": \"HELLO\\nWORLD\"}]" @@ -4332,7 +4133,6 @@ mod tests { assert_eq!(edits[0].new_text, "HELLO\nWORLD"); let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "\"edit\"" })) @@ -4340,7 +4140,6 @@ mod tests { assert!(input.edits.is_none()); let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "\"edit\"", "edits": null @@ -4349,7 +4148,6 @@ mod tests { assert!(input.edits.is_none()); let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": "\"edit\"", "edits": "[{\"old_text\": \"hello\\nworld\", \"new_text\": \"HELLO\\nWORLD\"}]" @@ -4363,7 +4161,6 @@ mod tests { assert_eq!(edits[0].new_text.as_deref(), Some("HELLO\nWORLD")); let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt" })) .expect("input should deserialize"); @@ -4371,7 +4168,6 @@ mod tests { assert!(input.edits.is_none()); let input = serde_json::from_value::(json!({ - "display_description": "Edit", "path": "root/file.txt", "mode": null, "edits": null diff --git a/crates/agent/src/tools/tool_permissions.rs b/crates/agent/src/tools/tool_permissions.rs index 4304877cd07..aa541c3e0ef 100644 --- a/crates/agent/src/tools/tool_permissions.rs +++ b/crates/agent/src/tools/tool_permissions.rs @@ -381,7 +381,6 @@ pub fn collect_symlink_escapes<'a>( pub fn authorize_file_edit( tool_name: &str, path: &Path, - display_description: &str, thread: &WeakEntity, event_stream: &ToolCallEventStream, cx: &mut App, @@ -396,7 +395,7 @@ pub fn authorize_file_edit( } let path_owned = path.to_path_buf(); - let display_description = display_description.to_string(); + let title = format!("Edit {}", util::markdown::MarkdownInlineCode(&path_str)); let tool_name = tool_name.to_string(); let thread = thread.clone(); let event_stream = event_stream.clone(); @@ -486,7 +485,7 @@ pub fn authorize_file_edit( vec![path_owned.to_string_lossy().to_string()], ); event_stream.authorize_always_prompt( - format!("{} (local settings)", display_description), + format!("{title} (local settings)"), context, cx, ) @@ -499,11 +498,7 @@ pub fn authorize_file_edit( &tool_name, vec![path_owned.to_string_lossy().to_string()], ); - event_stream.authorize_always_prompt( - format!("{} (settings)", display_description), - context, - cx, - ) + event_stream.authorize_always_prompt(format!("{title} (settings)"), context, cx) }); return authorize.await; } @@ -518,7 +513,7 @@ pub fn authorize_file_edit( &tool_name, vec![path_owned.to_string_lossy().to_string()], ); - event_stream.authorize(&display_description, context, cx) + event_stream.authorize(&title, context, cx) }); authorize.await } From b4a96f9c147d605bc7a3f30e9b37d0d257040e97 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 5 May 2026 14:32:58 +0200 Subject: [PATCH 2/6] eval_cli: Fixes for termbench (#55762) Release Notes: - N/A --- crates/eval_cli/src/main.rs | 2 +- crates/eval_cli/zed_eval/agent.py | 49 +++++++++++++++++++++++-------- 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/crates/eval_cli/src/main.rs b/crates/eval_cli/src/main.rs index bb6cbc883e1..e77e75bc879 100644 --- a/crates/eval_cli/src/main.rs +++ b/crates/eval_cli/src/main.rs @@ -70,7 +70,7 @@ struct Args { workdir: PathBuf, /// Instruction/prompt text. If omitted, read from --instruction-file or stdin. - #[arg(long)] + #[arg(long, allow_hyphen_values = true)] instruction: Option, /// Language model to use, in `provider/model` format. diff --git a/crates/eval_cli/zed_eval/agent.py b/crates/eval_cli/zed_eval/agent.py index 54403e9a253..4543dd9497d 100644 --- a/crates/eval_cli/zed_eval/agent.py +++ b/crates/eval_cli/zed_eval/agent.py @@ -52,19 +52,20 @@ class ZedAgent(BaseInstalledAgent): return "zed" async def _detect_workdir(self, environment: BaseEnvironment) -> str: - """Detect the repo working directory inside the container. + """Detect the working directory inside the container. Checks, in order: 1. Explicit ``EVAL_CLI_WORKDIR`` extra-env override - 2. ``/app`` (SWE-bench Pro) - 3. ``/testbed`` (SWE-bench Verified) - 4. ``/repo`` - 5. First git repo found under ``/`` (max depth 3) + 2. Well-known dirs with a ``.git`` subdirectory (SWE-bench style) + 3. First git repo found under ``/`` (max depth 3) + 4. Well-known dirs that exist at all (terminal-bench style) + 5. The container's default working directory (``pwd``) """ override = self._extra_env.get("EVAL_CLI_WORKDIR") if override: return override + # First: try to find a git repo (SWE-bench, etc.) result = await self.exec_as_agent( environment, command=( @@ -75,13 +76,29 @@ class ZedAgent(BaseInstalledAgent): '| head -1 | sed "s|/.git$||"' ), ) - workdir = result.stdout.strip() - if not workdir: - raise RuntimeError( - "Could not find a git repository in the container. " - "Set EVAL_CLI_WORKDIR explicitly via --ae EVAL_CLI_WORKDIR=/path/to/repo" - ) - return workdir + workdir = (result.stdout or "").strip() + if workdir: + return workdir + + # Fallback: use the first well-known directory that exists, + # even without .git (terminal-bench containers aren't git repos). + result = await self.exec_as_agent( + environment, + command=( + "for d in /app /testbed /repo /root /home; do " + ' if [ -d "$d" ]; then echo "$d"; exit 0; fi; ' + "done; " + "pwd" + ), + ) + workdir = (result.stdout or "").strip() + if workdir: + return workdir + + raise RuntimeError( + "Could not detect a working directory in the container. " + "Set EVAL_CLI_WORKDIR explicitly via --ae EVAL_CLI_WORKDIR=/path/to/repo" + ) async def install(self, environment: BaseEnvironment) -> None: # Detect the package manager and install base dependencies. @@ -426,12 +443,18 @@ class ZedAgent(BaseInstalledAgent): env=env, ) + # Only generate a patch if the workdir is a git repo + # (SWE-bench style). Terminal-bench containers aren't git repos. await self.exec_as_agent( environment, command=( + 'if [ -d ".git" ]; then ' "git add -A && " "git diff --cached HEAD > /logs/agent/patch.diff && " - 'echo "Patch size: $(wc -c < /logs/agent/patch.diff) bytes"' + 'echo "Patch size: $(wc -c < /logs/agent/patch.diff) bytes"; ' + "else " + 'echo "No git repo found, skipping patch generation"; ' + "fi" ), cwd=workdir, ) From 9955c4579d54c168a9e7a54b41ae78f83582a5a0 Mon Sep 17 00:00:00 2001 From: Bennet Bo Fenner Date: Tue, 5 May 2026 14:39:41 +0200 Subject: [PATCH 3/6] agent: Simplify tool schemas for enums (#55763) Previously schemars generated oneOf variants for these enums (because we added inline comments), making the schemas more complicated than they had to be. E.g. `edit_file` `mode` Before: ```json { "mode": { "description": "The mode of operation on the file. Possible values:\n- 'write': Replace the entire contents of the file. If the file doesn't exist, it will be created. Requires 'content' field.\n- 'edit': Make granular edits to an existing file. Requires 'edits' field.\n\nWhen a file already exists or you just created it, prefer editing it as opposed to recreating it from scratch.", "oneOf": [ { "description": "Overwrite the file with new content (replacing any existing content).\nIf the file does not exist, it will be created.", "type": "string", "const": "write" }, { "description": "Make granular edits to an existing file", "type": "string", "const": "edit" } ] } } ``` After: ```json { "mode": { "description": "The mode of operation on the file. Possible values:\n- 'write': Replace the entire contents of the file. If the file doesn't exist, it will be created. Requires 'content' field.\n- 'edit': Make granular edits to an existing file. Requires 'edits' field.\n\nWhen a file already exists or you just created it, prefer editing it as opposed to recreating it from scratch.", "type": "string", "enum": ["write", "edit"] } } ``` Release Notes: - N/A --- crates/agent/src/tools/edit_file_tool.rs | 3 --- crates/agent/src/tools/now_tool.rs | 4 +--- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/crates/agent/src/tools/edit_file_tool.rs b/crates/agent/src/tools/edit_file_tool.rs index 0e6493953c9..234b4ac92f2 100644 --- a/crates/agent/src/tools/edit_file_tool.rs +++ b/crates/agent/src/tools/edit_file_tool.rs @@ -94,10 +94,7 @@ pub struct EditFileToolInput { #[derive(Clone, Copy, Debug, Serialize, Deserialize, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum EditFileMode { - /// Overwrite the file with new content (replacing any existing content). - /// If the file does not exist, it will be created. Write, - /// Make granular edits to an existing file Edit, } diff --git a/crates/agent/src/tools/now_tool.rs b/crates/agent/src/tools/now_tool.rs index f8f4e0d91b5..4032731097c 100644 --- a/crates/agent/src/tools/now_tool.rs +++ b/crates/agent/src/tools/now_tool.rs @@ -12,10 +12,8 @@ use crate::{AgentTool, ToolCallEventStream, ToolInput}; #[serde(rename_all = "snake_case")] #[schemars(inline)] pub enum Timezone { - /// Use UTC for the datetime. #[serde(alias = "UTC", alias = "Utc")] Utc, - /// Use local time for the datetime. #[serde(alias = "LOCAL", alias = "Local")] Local, } @@ -24,7 +22,7 @@ pub enum Timezone { /// Only use this tool when the user specifically asks for it or the current task would benefit from knowing the current datetime. #[derive(Debug, Serialize, Deserialize, JsonSchema)] pub struct NowToolInput { - /// The timezone to use for the datetime. + /// The timezone to use for the datetime. Use `utc` for UTC, or `local` for the system's local time. timezone: Timezone, } From 360f955c283764e59404ee8cf28d32d3678a8f82 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 5 May 2026 14:39:50 +0200 Subject: [PATCH 4/6] agent_servers: Include stderr in ACP startup exit errors (#55757) Previously, we weren't waiting on the status future early enough so we would just hang if we weren't able to start the agent process. I also added the recent stderr logs in there to help the user debug the issue, since it is likely relevant in these cases. image Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Release Notes: - acp: Improve error messages if the ACP agent fails to start. --- crates/acp_thread/src/acp_thread.rs | 3 +- crates/agent_servers/src/acp.rs | 227 ++++++++++++++++++----- crates/agent_ui/src/conversation_view.rs | 16 +- 3 files changed, 197 insertions(+), 49 deletions(-) diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 42c6af38362..a18f9f21e79 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -1177,6 +1177,7 @@ pub enum LoadError { FailedToInstall(SharedString), Exited { status: ExitStatus, + stderr: Option, }, Other(SharedString), } @@ -1195,7 +1196,7 @@ impl Display for LoadError { ) } LoadError::FailedToInstall(msg) => write!(f, "Failed to install: {msg}"), - LoadError::Exited { status } => write!(f, "Server exited with status {status}"), + LoadError::Exited { status, .. } => write!(f, "Server exited with status {status}"), LoadError::Other(msg) => write!(f, "{msg}"), } } diff --git a/crates/agent_servers/src/acp.rs b/crates/agent_servers/src/acp.rs index 832b6afe048..93efddb03d8 100644 --- a/crates/agent_servers/src/acp.rs +++ b/crates/agent_servers/src/acp.rs @@ -20,7 +20,7 @@ use project::{AgentId, Project}; use remote::remote_client::Interactive; use serde::Deserialize; use std::path::PathBuf; -use std::process::Stdio; +use std::process::{ExitStatus, Stdio}; use std::rc::Rc; use std::sync::{Arc, Mutex}; use std::{any::Any, cell::RefCell, collections::VecDeque}; @@ -195,6 +195,34 @@ impl AcpDebugLog { sender.try_send(message.clone()).log_err(); } } + + fn trailing_stderr(&self) -> Option { + let state = self.state.lock().ok()?; + let mut lines = state + .messages + .iter() + .rev() + .take_while(|message| matches!(&message.message, AcpDebugMessageContent::Stderr { .. })) + .filter_map(|message| match &message.message { + AcpDebugMessageContent::Stderr { line } if !line.is_empty() => Some(line.as_ref()), + _ => None, + }) + .collect::>(); + + if lines.is_empty() { + return None; + } + + lines.reverse(); + Some(lines.join("\n")) + } +} + +fn exited_load_error_with_stderr(status: ExitStatus, debug_log: &AcpDebugLog) -> LoadError { + LoadError::Exited { + status, + stderr: debug_log.trailing_stderr().map(SharedString::from), + } } /// Awaits the response to an ACP request from a GPUI foreground task. @@ -714,6 +742,7 @@ impl AcpConnection { log::trace!("Spawned (pid: {})", child.id()); let sessions = Rc::new(RefCell::new(HashMap::default())); + let debug_log = AcpDebugLog::default(); let (release_channel, version): (Option<&str>, String) = cx.update(|cx| { ( @@ -729,7 +758,6 @@ impl AcpConnection { // Set up the foreground dispatch channel for bridging Send handler // closures to the !Send foreground thread. let (dispatch_tx, dispatch_rx) = mpsc::unbounded::(); - let debug_log = AcpDebugLog::default(); let incoming_lines = futures::io::BufReader::new(stdout).lines(); let tapped_incoming = incoming_lines.inspect({ @@ -756,37 +784,6 @@ impl AcpConnection { let transport = Lines::new(tapped_outgoing, tapped_incoming); - // `connect_client_future` installs the production handler set and - // hands us back both the connection-future (to run on a background - // executor) and a oneshot receiver that produces the - // `ConnectionTo` once the transport handshake is ready. - let (connection_tx, connection_rx) = futures::channel::oneshot::channel(); - let connection_future = - connect_client_future("zed", transport, dispatch_tx.clone(), connection_tx); - let io_task = cx.background_spawn(async move { - if let Err(err) = connection_future.await { - log::error!("ACP connection error: {err}"); - } - }); - - let connection: ConnectionTo = connection_rx - .await - .context("Failed to receive ACP connection handle")?; - - // Set up the foreground dispatch loop to process work items from handlers. - let dispatch_context = ClientContext { - sessions: sessions.clone(), - session_list: client_session_list.clone(), - }; - let dispatch_task = cx.spawn({ - let mut dispatch_rx = dispatch_rx; - async move |cx| { - while let Some(work) = dispatch_rx.next().await { - work.run(cx, &dispatch_context); - } - } - }); - let stderr_task = cx.background_spawn({ let debug_log = debug_log.clone(); async move { @@ -804,17 +801,53 @@ impl AcpConnection { } }); - let wait_task = cx.spawn({ - let sessions = sessions.clone(); - let status_fut = child.status(); - async move |cx| { - let status = status_fut.await?; - emit_load_error_to_all_sessions(&sessions, LoadError::Exited { status }, cx); - anyhow::Ok(()) + // `connect_client_future` installs the production handler set and + // hands us back both the connection-future (to run on a background + // executor) and a oneshot receiver that produces the + // `ConnectionTo` once the transport handshake is ready. + let (connection_tx, connection_rx) = futures::channel::oneshot::channel(); + let connection_future = + connect_client_future("zed", transport, dispatch_tx.clone(), connection_tx); + let io_task = cx.background_spawn(async move { + if let Err(err) = connection_future.await { + log::error!("ACP connection error: {err}"); } }); - let response = into_foreground_future( + let connection_rx = async move { + connection_rx + .await + .context("Failed to receive ACP connection handle") + } + .boxed_local(); + let status_fut = child.status().boxed_local(); + let (connection, status_fut) = match futures::future::select(connection_rx, status_fut) + .await + { + futures::future::Either::Left((connection, status_fut)) => (connection?, status_fut), + futures::future::Either::Right((status, _connection_rx)) => match status { + Ok(status) => return Err(exited_load_error_with_stderr(status, &debug_log).into()), + Err(err) => { + return Err(anyhow!("agent server exited before initialization: {err}")); + } + }, + }; + + // Set up the foreground dispatch loop to process work items from handlers. + let dispatch_context = ClientContext { + sessions: sessions.clone(), + session_list: client_session_list.clone(), + }; + let dispatch_task = cx.spawn({ + let mut dispatch_rx = dispatch_rx; + async move |cx| { + while let Some(work) = dispatch_rx.next().await { + work.run(cx, &dispatch_context); + } + } + }); + + let initialize_response = into_foreground_future( connection.send_request( acp::InitializeRequest::new(acp::ProtocolVersion::V1) .client_capabilities( @@ -835,12 +868,38 @@ impl AcpConnection { ), ), ) - .await?; + .map(|response| response.map_err(anyhow::Error::from)) + .boxed_local(); + let (response, status_fut) = match futures::future::select(initialize_response, status_fut) + .await + { + futures::future::Either::Left((response, status_fut)) => (response?, status_fut), + futures::future::Either::Right((status, _initialize_response)) => match status { + Ok(status) => return Err(exited_load_error_with_stderr(status, &debug_log).into()), + Err(err) => { + return Err(anyhow!("agent server exited before initialization: {err}")); + } + }, + }; if response.protocol_version < MINIMUM_SUPPORTED_VERSION { return Err(UnsupportedVersion.into()); } + let wait_task = cx.spawn({ + let sessions = sessions.clone(); + let debug_log = debug_log.clone(); + async move |cx| { + let status = status_fut.await?; + emit_load_error_to_all_sessions( + &sessions, + exited_load_error_with_stderr(status, &debug_log), + cx, + ); + anyhow::Ok(()) + } + }); + let telemetry_id = response .agent_info // Use the one the agent provides if we have one @@ -1881,7 +1940,10 @@ pub mod test_support { while let Ok(status) = exit_rx.recv().await { emit_load_error_to_all_sessions( &connection.sessions, - LoadError::Exited { status }, + LoadError::Exited { + status, + stderr: None, + }, cx, ); } @@ -2373,6 +2435,85 @@ mod tests { assert_eq!(task.label, "Login"); } + #[test] + fn trailing_stderr_only_uses_final_stderr_block() { + let debug_log = AcpDebugLog::default(); + debug_log.record_line(AcpDebugMessageDirection::Stderr, "stale stderr"); + debug_log.record_line( + AcpDebugMessageDirection::Incoming, + r#"{"method":"initialized"}"#, + ); + + assert_eq!(debug_log.trailing_stderr(), None); + + debug_log.record_line(AcpDebugMessageDirection::Stderr, "recent stderr"); + assert_eq!( + debug_log.trailing_stderr().as_deref(), + Some("recent stderr") + ); + } + + #[cfg(not(windows))] + #[gpui::test] + async fn startup_returns_error_when_agent_exits_before_initialization( + cx: &mut gpui::TestAppContext, + ) { + cx.update(|cx| { + let store = settings::SettingsStore::test(cx); + cx.set_global(store); + }); + cx.executor().allow_parking(); + + let temp_dir = tempfile::tempdir().unwrap(); + let project = project::Project::example([temp_dir.path()], &mut cx.to_async()).await; + let agent_server_store = + project.read_with(cx, |project, _| project.agent_server_store().downgrade()); + let command = AgentServerCommand { + path: "/bin/sh".into(), + args: vec![ + "-c".into(), + r#"printf '%s\n' 'npm error code ETARGET' 'npm error notarget No matching version found for @agentclientprotocol/claude-agent-acp@0.32.0 with a date before 4/28/2026, 12:11:38 PM.' >&2; exit 1"#.into(), + ], + env: None, + }; + + let mut async_cx = cx.to_async(); + let startup = AcpConnection::stdio( + AgentId::new("test-agent"), + project, + command, + agent_server_store, + None, + None, + HashMap::default(), + &mut async_cx, + ) + .fuse(); + let timeout = cx + .background_executor + .timer(std::time::Duration::from_secs(5)) + .fuse(); + futures::pin_mut!(startup, timeout); + + let result = futures::select! { + result = startup => result, + _ = timeout => panic!("timed out waiting for failed ACP startup"), + }; + + let Err(error) = result else { + panic!("expected ACP startup to fail"); + }; + let load_error = error + .downcast::() + .expect("startup failure should preserve the typed load error"); + match load_error { + LoadError::Exited { status, .. } => { + assert!(!status.success(), "expected non-zero exit status"); + } + error => panic!("expected exited load error, got: {error:?}"), + }; + } + async fn connect_fake_agent( cx: &mut gpui::TestAppContext, ) -> ( diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index c1a975939f6..6ddac5f3f9f 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -2159,11 +2159,17 @@ impl ConversationView { msg.into(), Some(self.create_copy_button(msg.to_string()).into_any_element()), ), - LoadError::Exited { status } => ( - "Failed to Launch", - format!("Server exited with status {status}").into(), - None, - ), + LoadError::Exited { status, stderr } => { + let mut message = format!("Server exited with status {status}"); + if let Some(stderr) = stderr { + message.push_str("\n"); + message.push_str(stderr); + }; + let action_slot = stderr + .is_some() + .then(|| self.create_copy_button(message.clone()).into_any_element()); + ("Failed to Launch", message.into(), action_slot) + } LoadError::Other(msg) => ( "Failed to Launch", msg.into(), From a2cd962a1ab078c998aa59f4e76ff30be3dcfe0b Mon Sep 17 00:00:00 2001 From: Finn Evers Date: Tue, 5 May 2026 15:01:02 +0200 Subject: [PATCH 5/6] editor: Un-pub `ScrollManager` (#55767) Make this only pub(crate) in preparation for https://github.com/zed-industries/zed/pull/44827 Release Notes: - N/A --- crates/agent_ui/src/entry_view_state.rs | 2 +- crates/agent_ui/src/inline_assistant.rs | 2 +- crates/editor/src/editor.rs | 7 ++++++- crates/editor/src/scroll.rs | 8 ++++++++ crates/vim/src/normal/scroll.rs | 8 ++------ 5 files changed, 18 insertions(+), 9 deletions(-) diff --git a/crates/agent_ui/src/entry_view_state.rs b/crates/agent_ui/src/entry_view_state.rs index 853672142fb..15bd9e89b57 100644 --- a/crates/agent_ui/src/entry_view_state.rs +++ b/crates/agent_ui/src/entry_view_state.rs @@ -449,7 +449,7 @@ fn create_editor_diff( editor.set_show_vertical_scrollbar(false, cx); editor.set_minimap_visibility(MinimapVisibility::Disabled, window, cx); editor.set_soft_wrap_mode(SoftWrap::None, cx); - editor.scroll_manager.set_forbid_vertical_scroll(true); + editor.set_forbid_vertical_scroll(true); editor.set_show_indent_guides(false, cx); editor.set_read_only(true); editor.set_delegate_open_excerpts(true); diff --git a/crates/agent_ui/src/inline_assistant.rs b/crates/agent_ui/src/inline_assistant.rs index cdff9785df7..d442a61e01a 100644 --- a/crates/agent_ui/src/inline_assistant.rs +++ b/crates/agent_ui/src/inline_assistant.rs @@ -1425,7 +1425,7 @@ impl InlineAssistant { editor.set_show_gutter(false, cx); editor.set_offset_content(false, cx); editor.disable_mouse_wheel_zoom(); - editor.scroll_manager.set_forbid_vertical_scroll(true); + editor.set_forbid_vertical_scroll(true); editor.set_read_only(true); editor.set_show_edit_predictions(Some(false), window, cx); editor.highlight_rows::( diff --git a/crates/editor/src/editor.rs b/crates/editor/src/editor.rs index af8d6f6ccc6..d9dd6078c08 100644 --- a/crates/editor/src/editor.rs +++ b/crates/editor/src/editor.rs @@ -1157,7 +1157,12 @@ pub struct Editor { pub display_map: Entity, placeholder_display_map: Option>, pub selections: SelectionsCollection, - pub scroll_manager: ScrollManager, + /// Manages the scroll position for the given editor. + /// + /// Whenever you want to modify the scroll position of the editor, you should + /// usually use the existing available APIs as opposed to directly interacting + /// with the scroll manager. + pub(crate) scroll_manager: ScrollManager, /// When inline assist editors are linked, they all render cursors because /// typing enters text into each of them, even the ones that aren't focused. pub(crate) show_cursor_when_unfocused: bool, diff --git a/crates/editor/src/scroll.rs b/crates/editor/src/scroll.rs index 143a73fd701..f067519e734 100644 --- a/crates/editor/src/scroll.rs +++ b/crates/editor/src/scroll.rs @@ -623,6 +623,14 @@ impl Editor { self.scroll_manager.has_autoscroll_request() } + pub fn set_forbid_vertical_scroll(&mut self, forbid: bool) { + self.scroll_manager.set_forbid_vertical_scroll(forbid); + } + + pub fn scroll_top_display_point(&self, snapshot: &DisplaySnapshot, cx: &App) -> DisplayPoint { + self.scroll_manager.scroll_top_display_point(snapshot, cx) + } + pub fn vertical_scroll_margin(&self) -> usize { self.scroll_manager.vertical_scroll_margin as usize } diff --git a/crates/vim/src/normal/scroll.rs b/crates/vim/src/normal/scroll.rs index 01719cd5932..befaacf31c7 100644 --- a/crates/vim/src/normal/scroll.rs +++ b/crates/vim/src/normal/scroll.rs @@ -109,9 +109,7 @@ impl Vim { self.update_editor(cx, |vim, editor, cx| { let should_move_cursor = editor.newest_selection_on_screen(cx).is_eq(); let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); - let old_top = editor - .scroll_manager - .scroll_top_display_point(&display_snapshot, cx); + let old_top = editor.scroll_top_display_point(&display_snapshot, cx); if editor.scroll_hover(amount, window, cx) { return; @@ -143,9 +141,7 @@ impl Vim { }; let display_snapshot = editor.display_map.update(cx, |map, cx| map.snapshot(cx)); - let top = editor - .scroll_manager - .scroll_top_display_point(&display_snapshot, cx); + let top = editor.scroll_top_display_point(&display_snapshot, cx); let vertical_scroll_margin = EditorSettings::get_global(cx).vertical_scroll_margin; let mut move_cursor = |map: &editor::display_map::DisplaySnapshot, From 7a34bc059e7344e1814c87f994c963b6d8cf5c20 Mon Sep 17 00:00:00 2001 From: Ben Brandt Date: Tue, 5 May 2026 15:40:58 +0200 Subject: [PATCH 6/6] eval_cli: Update eval_cli toolchain and pin harbor (#55768) Move `git_ui` to `agent_ui` test dependencies and bump the eval CLI Docker image to Rust 1.95.0 while pinning the Python `harbor` dependency to 0.6.4 Release Notes: - N/A --- crates/agent_ui/Cargo.toml | 2 +- crates/eval_cli/.gitignore | 1 + crates/eval_cli/Dockerfile | 4 ++-- crates/eval_cli/zed_eval/pyproject.toml | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/crates/agent_ui/Cargo.toml b/crates/agent_ui/Cargo.toml index 5f5a7cc7926..ff34593cbc3 100644 --- a/crates/agent_ui/Cargo.toml +++ b/crates/agent_ui/Cargo.toml @@ -56,7 +56,6 @@ file_icons.workspace = true fs.workspace = true futures.workspace = true git.workspace = true -git_ui.workspace = true fuzzy.workspace = true gpui.workspace = true gpui_tokio.workspace = true @@ -124,6 +123,7 @@ clock = { workspace = true, features = ["test-support"] } db = { workspace = true, features = ["test-support"] } editor = { workspace = true, features = ["test-support"] } eval_utils.workspace = true +git_ui.workspace = true gpui = { workspace = true, "features" = ["test-support"] } http_client = { workspace = true, features = ["test-support"] } indoc.workspace = true diff --git a/crates/eval_cli/.gitignore b/crates/eval_cli/.gitignore index 083ef6e3d35..a6317b892a1 100644 --- a/crates/eval_cli/.gitignore +++ b/crates/eval_cli/.gitignore @@ -1,3 +1,4 @@ **/jobs **/*.egg-info **/__pycache__ +uv.lock diff --git a/crates/eval_cli/Dockerfile b/crates/eval_cli/Dockerfile index 06593a124fe..9782e5982b9 100644 --- a/crates/eval_cli/Dockerfile +++ b/crates/eval_cli/Dockerfile @@ -7,12 +7,12 @@ # Or use the helper script: # crates/eval_cli/script/build-linux -FROM rust:1.94.1 AS builder +FROM rust:1.95.0 AS builder WORKDIR /app # Pre-install the toolchain specified in rust-toolchain.toml so it is cached. -RUN rustup toolchain install 1.94.1 --profile minimal \ +RUN rustup toolchain install 1.95.0 --profile minimal \ --component rustfmt --component clippy --component rust-analyzer --component rust-src \ --target wasm32-wasip2 --target wasm32-unknown-unknown --target x86_64-unknown-linux-musl --target x86_64-unknown-linux-gnu diff --git a/crates/eval_cli/zed_eval/pyproject.toml b/crates/eval_cli/zed_eval/pyproject.toml index 416c025826e..10e72028a5e 100644 --- a/crates/eval_cli/zed_eval/pyproject.toml +++ b/crates/eval_cli/zed_eval/pyproject.toml @@ -3,7 +3,7 @@ name = "zed-eval" version = "0.1.0" description = "Harbor agent wrapper for Zed's eval-cli" requires-python = ">=3.12" -dependencies = ["harbor"] +dependencies = ["harbor==0.6.4"] [build-system] requires = ["setuptools"]