Fix issues in EditFilesTool, ListDirectoryTool and BashTool (#26647)

Release Notes:

- N/A

assets/prompts/assistant_system_prompt.hbs

@@ -11,8 +11,8 @@ You should only perform actions that modify the user’s system if explicitly re
 
 Be concise and direct in your responses.
 
-The user has opened a project that contains the following top-level directories/files:
+The user has opened a project that contains the following root directories/files:
 
-{{#each worktree_root_names}}
+{{#each worktrees}}
-- {{this}}
+- {{root_name}} (absolute path: {{abs_path}})
 {{/each}}

crates/assistant2/src/thread.rs

@@ -13,7 +13,7 @@ use language_model::{
     Role, StopReason,
 };
 use project::Project;
-use prompt_store::PromptBuilder;
+use prompt_store::{AssistantSystemPromptWorktree, PromptBuilder};
 use scripting_tool::{ScriptingSession, ScriptingTool};
 use serde::{Deserialize, Serialize};
 use util::{post_inc, ResultExt, TryFutureExt as _};
@@ -384,8 +384,14 @@ impl Thread {
         let worktree_root_names = self
             .project
             .read(cx)
-            .worktree_root_names(cx)
+            .visible_worktrees(cx)
-            .map(ToString::to_string)
+            .map(|worktree| {
+                let worktree = worktree.read(cx);
+                AssistantSystemPromptWorktree {
+                    root_name: worktree.root_name().into(),
+                    abs_path: worktree.abs_path(),
+                }
+            })
             .collect::<Vec<_>>();
         let system_prompt = self
             .prompt_builder

crates/assistant_tools/src/bash_tool.rs

@@ -1,4 +1,4 @@
-use anyhow::{anyhow, Result};
+use anyhow::{anyhow, Context as _, Result};
 use assistant_tool::Tool;
 use gpui::{App, Entity, Task};
 use language_model::LanguageModelRequestMessage;
@@ -6,11 +6,14 @@ use project::Project;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
 use std::sync::Arc;
+use util::command::new_smol_command;
 
 #[derive(Debug, Serialize, Deserialize, JsonSchema)]
 pub struct BashToolInput {
     /// The bash command to execute as a one-liner.
     command: String,
+    /// Working directory for the command. This must be one of the root directories of the project.
+    working_directory: String,
 }
 
 pub struct BashTool;
@@ -33,7 +36,7 @@ impl Tool for BashTool {
         self: Arc<Self>,
         input: serde_json::Value,
         _messages: &[LanguageModelRequestMessage],
-        _project: Entity<Project>,
+        project: Entity<Project>,
         cx: &mut App,
     ) -> Task<Result<String>> {
         let input: BashToolInput = match serde_json::from_value(input) {
@@ -41,23 +44,34 @@ impl Tool for BashTool {
             Err(err) => return Task::ready(Err(anyhow!(err))),
         };
 
+        let Some(worktree) = project
+            .read(cx)
+            .worktree_for_root_name(&input.working_directory, cx)
+        else {
+            return Task::ready(Err(anyhow!("Working directory not found in the project")));
+        };
+        let working_directory = worktree.read(cx).abs_path();
+
         cx.spawn(|_| async move {
-            // Add 2>&1 to merge stderr into stdout for proper interleaving
+            // Add 2>&1 to merge stderr into stdout for proper interleaving.
             let command = format!("{} 2>&1", input.command);
 
-            // Spawn a blocking task to execute the command
+            let output = new_smol_command("bash")
-            let output = futures::executor::block_on(async {
+                .arg("-c")
-                std::process::Command::new("bash")
+                .arg(&command)
-                    .arg("-c")
+                .current_dir(working_directory)
-                    .arg(&command)
+                .output()
-                    .output()
+                .await
-                    .map_err(|err| anyhow!("Failed to execute bash command: {}", err))
+                .context("Failed to execute bash command")?;
-            })?;
 
             let output_string = String::from_utf8_lossy(&output.stdout).to_string();
 
             if output.status.success() {
-                Ok(output_string)
+                if output_string.is_empty() {
+                    Ok("Command executed successfully.".to_string())
+                } else {
+                    Ok(output_string)
+                }
             } else {
                 Ok(format!(
                     "Command failed with exit code {}\n{}",

crates/assistant_tools/src/bash_tool/description.md

@@ -1 +1,5 @@
-Executes a bash one-liner and returns the combined output. This tool spawns a bash process, combines stdout and stderr into one interleaved stream as they are produced (preserving the order of writes), and captures that stream into a string which is returned. Use this tool when you need to run shell commands to get information about the system or process files.
+Executes a bash one-liner and returns the combined output.
+
+This tool spawns a bash process, combines stdout and stderr into one interleaved stream as they are produced (preserving the order of writes), and captures that stream into a string which is returned.
+
+Remember that each invocation of this tool will spawn a new bash process, so you can't rely on any state from previous invocations.

crates/assistant_tools/src/edit_files_tool.rs

@@ -20,17 +20,40 @@ use util::ResultExt;
 
 #[derive(Debug, Serialize, Deserialize, JsonSchema)]
 pub struct EditFilesToolInput {
-    /// High-level edit instructions. These will be interpreted by a smaller model,
+    /// High-level edit instructions. These will be interpreted by a smaller
-    /// so explain the edits you want that model to make and to which files need changing.
+    /// model, so explain the changes you want that model to make and which
-    /// The description should be concise and clear. We will show this description to the user
+    /// file paths need changing.
-    /// as well.
+    ///
+    /// The description should be concise and clear. We will show this
+    /// description to the user as well.
+    ///
+    /// WARNING: When specifying which file paths need changing, you MUST
+    /// start each path with one of the project's root directories.
+    ///
+    /// WARNING: NEVER include code blocks or snippets in edit instructions.
+    /// Only provide natural language descriptions of the changes needed! The tool will
+    /// reject any instructions that contain code blocks or snippets.
+    ///
+    /// The following examples assume we have two root directories in the project:
+    /// - root-1
+    /// - root-2
     ///
     /// <example>
-    /// If you want to rename a function you can say "Rename the function 'foo' to 'bar'".
+    /// If you want to introduce a new quit function to kill the process, your
+    /// instructions should be: "Add a new `quit` function to
+    /// `root-1/src/main.rs` to kill the process".
+    ///
+    /// Notice how the file path starts with root-1. Without that, the path
+    /// would be ambiguous and the call would fail!
     /// </example>
     ///
     /// <example>
-    /// If you want to add a new function you can say "Add a new method to the `User` struct that prints the age".
+    /// If you want to change documentation to always start with a capital
+    /// letter, your instructions should be: "In `root-2/db.js`,
+    /// `root-2/inMemory.js` and `root-2/sql.js`, change all the documentation
+    /// to start with a capital letter".
+    ///
+    /// Notice how we never specify code snippets in the instructions!
     /// </example>
     pub edit_instructions: String,
 }
@@ -212,7 +235,7 @@ impl EditFilesTool {
                 project
                     .update(&mut cx, |project, cx| {
                         if let Some(file) = buffer.read(&cx).file() {
-                            let _ = writeln!(&mut answer, "{}", &file.path().display());
+                            let _ = writeln!(&mut answer, "{}", &file.full_path(cx).display());
                         }
 
                         project.save_buffer(buffer, cx)

crates/assistant_tools/src/edit_files_tool/description.md

@@ -1,4 +1,4 @@
-Edit files in the current project.
+Edit files in the current project by specifying instructions in natural language.
 
 When using this tool, you should suggest one coherent edit that can be made to the codebase.
 

crates/assistant_tools/src/edit_files_tool/edit_prompt.md

@@ -106,7 +106,7 @@ Every *SEARCH/REPLACE block* must use this format:
 7. The end of the replace block: >>>>>>> REPLACE
 8. The closing fence: ```
 
-Use the *FULL* file path, as shown to you by the user.
+Use the *FULL* file path, as shown to you by the user. Make sure to include the project's root directory name at the start of the path. *NEVER* specify the absolute path of the file!
 
 Every *SEARCH* section must *EXACTLY MATCH* the existing file content, character for character, including all comments, docstrings, etc.
 If the file contains code or other data wrapped/escaped in json/xml/quotes or other containers, you need to propose edits to the literal contents of the file, including the container markup.

crates/assistant_tools/src/list_directory_tool.rs

@@ -12,10 +12,10 @@ pub struct ListDirectoryToolInput {
     /// The relative path of the directory to list.
     ///
     /// This path should never be absolute, and the first component
-    /// of the path should always be a top-level directory in a project.
+    /// of the path should always be a root directory in a project.
     ///
     /// <example>
-    /// If the project has the following top-level directories:
+    /// If the project has the following root directories:
     ///
     /// - directory1
     /// - directory2
@@ -24,7 +24,7 @@ pub struct ListDirectoryToolInput {
     /// </example>
     ///
     /// <example>
-    /// If the project has the following top-level directories:
+    /// If the project has the following root directories:
     ///
     /// - foo
     /// - bar
@@ -72,8 +72,18 @@ impl Tool for ListDirectoryTool {
             return Task::ready(Err(anyhow!("Directory not found in the project")));
         };
         let path = input.path.strip_prefix(worktree_root_name).unwrap();
+        let worktree = worktree.read(cx);
+
+        let Some(entry) = worktree.entry_for_path(path) else {
+            return Task::ready(Err(anyhow!("Path not found: {}", input.path.display())));
+        };
+
+        if !entry.is_dir() {
+            return Task::ready(Err(anyhow!("{} is a file.", input.path.display())));
+        }
+
         let mut output = String::new();
-        for entry in worktree.read(cx).child_entries(path) {
+        for entry in worktree.child_entries(path) {
             writeln!(
                 output,
                 "{}",
@@ -83,6 +93,9 @@ impl Tool for ListDirectoryTool {
             )
             .unwrap();
         }
+        if output.is_empty() {
+            return Task::ready(Ok(format!("{} is empty.", input.path.display())));
+        }
         Task::ready(Ok(output))
     }
 }

crates/assistant_tools/src/path_search_tool.rs

@@ -13,7 +13,7 @@ pub struct PathSearchToolInput {
     /// The glob to search all project paths for.
     ///
     /// <example>
-    /// If the project has the following top-level directories:
+    /// If the project has the following root directories:
     ///
     /// - directory1/a/something.txt
     /// - directory2/a/things.txt

crates/assistant_tools/src/read_file_tool.rs

@@ -14,10 +14,10 @@ pub struct ReadFileToolInput {
     /// The relative path of the file to read.
     ///
     /// This path should never be absolute, and the first component
-    /// of the path should always be a top-level directory in a project.
+    /// of the path should always be a root directory in a project.
     ///
     /// <example>
-    /// If the project has the following top-level directories:
+    /// If the project has the following root directories:
     ///
     /// - directory1
     /// - directory2

crates/prompt_store/src/prompts.rs

@@ -7,13 +7,24 @@ use handlebars::{Handlebars, RenderError};
 use language::{BufferSnapshot, LanguageName, Point};
 use parking_lot::Mutex;
 use serde::Serialize;
-use std::{ops::Range, path::PathBuf, sync::Arc, time::Duration};
+use std::{
+    ops::Range,
+    path::{Path, PathBuf},
+    sync::Arc,
+    time::Duration,
+};
 use text::LineEnding;
 use util::ResultExt;
 
 #[derive(Serialize)]
 pub struct AssistantSystemPromptContext {
-    pub worktree_root_names: Vec<String>,
+    pub worktrees: Vec<AssistantSystemPromptWorktree>,
+}
+
+#[derive(Serialize)]
+pub struct AssistantSystemPromptWorktree {
+    pub root_name: String,
+    pub abs_path: Arc<Path>,
 }
 
 #[derive(Serialize)]
@@ -223,11 +234,9 @@ impl PromptBuilder {
 
     pub fn generate_assistant_system_prompt(
         &self,
-        worktree_root_names: Vec<String>,
+        worktrees: Vec<AssistantSystemPromptWorktree>,
     ) -> Result<String, RenderError> {
-        let prompt = AssistantSystemPromptContext {
+        let prompt = AssistantSystemPromptContext { worktrees };
-            worktree_root_names,
-        };
         self.handlebars
             .lock()
             .render("assistant_system_prompt", &prompt)