Work around when the model quotes paths

crates/assistant2/src/active_thread.rs

@@ -4,8 +4,8 @@ use collections::HashMap;
 use editor::{Editor, MultiBuffer};
 use gpui::{
     list, AbsoluteLength, AnyElement, App, ClickEvent, DefiniteLength, EdgesRefinement, Empty,
-    Entity, Focusable, Length, ListAlignment, ListOffset, ListState, StyleRefinement, Subscription,
+    Entity, Focusable, FontWeight, Length, ListAlignment, ListOffset, ListState, StyleRefinement,
-    Task, TextStyleRefinement, UnderlineStyle,
+    Subscription, Task, TextStyleRefinement, UnderlineStyle,
 };
 use language::{Buffer, LanguageRegistry};
 use language_model::{LanguageModelRegistry, LanguageModelToolUseId, Role};
@@ -29,7 +29,8 @@ pub struct ActiveThread {
     messages: Vec<MessageId>,
     list_state: ListState,
     rendered_messages_by_id: HashMap<MessageId, Entity<Markdown>>,
-    rendered_scripting_tool_uses: HashMap<LanguageModelToolUseId, Entity<Markdown>>,
+    rendered_scripting_tool_uses:
+        HashMap<LanguageModelToolUseId, (Entity<Markdown>, Option<String>)>,
     editing_message: Option<(MessageId, EditMessageState)>,
     expanded_tool_uses: HashMap<LanguageModelToolUseId, bool>,
     last_error: Option<ThreadError>,
@@ -204,7 +205,7 @@ impl ActiveThread {
                     right: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels(8.)))),
                     bottom: Some(DefiniteLength::Absolute(AbsoluteLength::Pixels(Pixels(8.)))),
                 },
-                background: Some(colors.editor_background.into()),
+                background: Some(colors.terminal_background.into()), 
                 border_color: Some(colors.border_variant),
                 border_widths: EdgesRefinement {
                     top: Some(AbsoluteLength::Pixels(Pixels(1.))),
@@ -267,15 +268,19 @@ impl ActiveThread {
             return;
         }
 
-        let lua_script = serde_json::from_value::<ScriptingToolInput>(tool_input)
+        let tool_input_result = serde_json::from_value::<ScriptingToolInput>(tool_input);
-            .map(|input| input.lua_script)
+
-            .unwrap_or_default();
+        let (lua_script, summary) = if let Ok(input) = tool_input_result {
+            (input.lua_script, input.summary)
+        } else {
+            (String::new(), None)
+        };
 
         let lua_script =
             self.render_markdown(format!("```lua\n{lua_script}\n```").into(), window, cx);
 
         self.rendered_scripting_tool_uses
-            .insert(tool_use_id, lua_script);
+            .insert(tool_use_id, (lua_script, summary));
     }
 
     fn handle_thread_event(
@@ -852,9 +857,14 @@ impl ActiveThread {
                         return parent;
                     }
 
-                    let lua_script_markdown =
+                    let scripting_tool_result =
                         self.rendered_scripting_tool_uses.get(&tool_use.id).cloned();
 
+                    let (lua_script_markdown, summary) = match scripting_tool_result {
+                        Some((markdown, summary)) => (Some(markdown), summary),
+                        None => (None, None),
+                    };
+
                     parent.child(
                         v_flex()
                             .child(
@@ -864,7 +874,22 @@ impl ActiveThread {
                                     .px_2p5()
                                     .border_b_1()
                                     .border_color(cx.theme().colors().border)
-                                    .child(Label::new("Input:"))
+                                    .child(v_flex().when_some(
+                                        summary.clone(),
+                                        |parent, summary| {
+                                            parent.child(
+                                                v_flex()
+                                                    .gap_0p5()
+                                                    .child(
+                                                        Label::new("Summary:")
+                                                            .weight(FontWeight::SEMIBOLD),
+                                                    )
+                                                    .child(Label::new(summary))
+                                                    .pb_1(),
+                                            )
+                                        },
+                                    ))
+                                    .child(Label::new("Input:").weight(FontWeight::SEMIBOLD))
                                     .map(|parent| {
                                         if let Some(markdown) = lua_script_markdown {
                                             parent.child(markdown)

crates/scripting_tool/src/scripting_session.rs

@@ -252,7 +252,7 @@ impl ScriptingSession {
 
         let path = match Self::parse_abs_path_in_root_dir(&root_dir, &path_str) {
             Ok(path) => path,
-            Err(err) => return Ok((None, format!("{err}"))),
+            Err(err) => return Ok((None, err.to_string())),
         };
 
         let project_path = ProjectPath {
@@ -816,20 +816,30 @@ impl ScriptingSession {
     }
 
     fn parse_abs_path_in_root_dir(root_dir: &Path, path_str: &str) -> anyhow::Result<PathBuf> {
-        let path = Path::new(&path_str);
+        // Sometimes the model produces a path that has quotation marks around it.
-        if path.is_absolute() {
+        // If we encounter that, trim off the quotation marks.
-            // Check if path starts with root_dir prefix without resolving symlinks
+        let path = if path_str.starts_with('"') && path_str.ends_with('"') {
-            if path.starts_with(&root_dir) {
+            Path::new(&path_str[1..path_str.len() - 1]).canonicalize()
-                Ok(path.to_path_buf())
+        } else {
+            Path::new(&path_str).canonicalize()
+        };
+
+        // Get the canonical absolute path (including resolving symlinks)
+        // and then make that path relative to the root_dir if possible.
+        if let Ok(absolute) = path {
+            if let Ok(relative) = absolute.strip_prefix(&root_dir) {
+                debug_assert!(
+                    relative.is_relative(),
+                    "{relative:?} should have been a relative path, but was absolute."
+                );
+                Ok(relative.to_path_buf())
             } else {
-                Err(anyhow!(
+                // todo-sandbox: In the future, sandbox this by prompting the user for
-                    "Error: Absolute path {} is outside the current working directory",
+                // accessing a file outside the project.
-                    path_str
+                Ok(absolute.to_path_buf())
-                ))
             }
         } else {
-            // TODO: Does use of `../` break sandbox - is path canonicalization needed?
+            Err(anyhow!("Invalid path: {path_str}"))
-            Ok(root_dir.join(path))
         }
     }
 }

crates/scripting_tool/src/scripting_tool.rs

@@ -8,6 +8,7 @@ use serde::Deserialize;
 #[derive(Debug, Deserialize, JsonSchema)]
 pub struct ScriptingToolInput {
     pub lua_script: String,
+    pub summary: Option<String>,
 }
 
 pub struct ScriptingTool;

crates/scripting_tool/src/scripting_tool_description.txt

@@ -20,3 +20,6 @@ contents of every file in the code base (aside from gitignored files), then
 returns an array of tables with two fields: "path" (the path to the file that
 had the matches) and "matches" (an array of strings, with each string being a
 match that was found within the file).
+
+
+Please also include an extremely terse English summary of what your Lua script does in the "summary" field. This summary will be displayed alongside your script to provide context about what the script is doing. There isn't much room in the display, so make it extremely concise.

crates/worktree/src/worktree.rs

@@ -3007,7 +3007,10 @@ impl Snapshot {
 
     pub fn entry_for_path(&self, path: impl AsRef<Path>) -> Option<&Entry> {
         let path = path.as_ref();
-        debug_assert!(path.is_relative());
+        debug_assert!(
+            path.is_relative(),
+            "{path:?} was not relative, but entry_for_path should only be given relative paths."
+        );
         self.traverse_from_path(true, true, true, path)
             .entry()
             .and_then(|entry| {