Re-enable `open` tool (#29707)

Cargo.lock 🔗

@@ -732,6 +732,7 @@ dependencies = [
  "serde_json",
  "settings",
  "task",
+ "tempfile",
  "terminal",
  "terminal_view",
  "tree-sitter-rust",

assets/settings/default.json 🔗

@@ -671,6 +671,7 @@
           "now": true,
           "find_path": true,
           "read_file": true,
+          "open": true,
           "grep": true,
           "thinking": true,
           "web_search": true

crates/assistant_tools/Cargo.toml 🔗

@@ -55,6 +55,7 @@ rand.workspace = true
 pretty_assertions.workspace = true
 settings = { workspace = true, features = ["test-support"] }
 task = { workspace = true, features = ["test-support"]}
+tempfile.workspace = true
 tree-sitter-rust.workspace = true
 workspace = { workspace = true, features = ["test-support"] }
 unindent.workspace = true

crates/assistant_tools/src/open_tool.rs 🔗

@@ -6,7 +6,7 @@ use language_model::{LanguageModelRequestMessage, LanguageModelToolSchemaFormat}
 use project::Project;
 use schemars::JsonSchema;
 use serde::{Deserialize, Serialize};
-use std::sync::Arc;
+use std::{path::PathBuf, sync::Arc};
 use ui::IconName;
 use util::markdown::MarkdownEscaped;
 
@@ -50,7 +50,7 @@ impl Tool for OpenTool {
         self: Arc<Self>,
         input: serde_json::Value,
         _messages: &[LanguageModelRequestMessage],
-        _project: Entity<Project>,
+        project: Entity<Project>,
         _action_log: Entity<ActionLog>,
         _window: Option<AnyWindowHandle>,
         cx: &mut App,
@@ -60,11 +60,107 @@ impl Tool for OpenTool {
             Err(err) => return Task::ready(Err(anyhow!(err))).into(),
         };
 
+        // 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, project, cx);
+
         cx.background_spawn(async move {
-            open::that(&input.path_or_url).context("Failed to open URL or file path")?;
+            match abs_path {
+                Some(path) => open::that(path),
+                None => open::that(&input.path_or_url),
+            }
+            .context("Failed to open URL or file path")?;
 
             Ok(format!("Successfully opened {}", input.path_or_url))
         })
         .into()
     }
 }
+
+fn to_absolute_path(
+    potential_path: &str,
+    project: Entity<Project>,
+    cx: &mut App,
+) -> Option<PathBuf> {
+    let project = project.read(cx);
+    project
+        .find_project_path(PathBuf::from(potential_path), cx)
+        .and_then(|project_path| project.absolute_path(&project_path, cx))
+}
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+    use gpui::TestAppContext;
+    use project::{FakeFs, Project};
+    use settings::SettingsStore;
+    use std::path::Path;
+    use tempfile::TempDir;
+
+    #[gpui::test]
+    async fn test_to_absolute_path(cx: &mut TestAppContext) {
+        init_test(cx);
+        let temp_dir = TempDir::new().expect("Failed to create temp directory");
+        let temp_path = temp_dir.path().to_string_lossy().to_string();
+
+        let fs = FakeFs::new(cx.executor());
+        fs.insert_tree(
+            &temp_path,
+            serde_json::json!({
+                "src": {
+                    "main.rs": "fn main() {}",
+                    "lib.rs": "pub fn lib_fn() {}"
+                },
+                "docs": {
+                    "readme.md": "# Project Documentation"
+                }
+            }),
+        )
+        .await;
+
+        // Use the temp_path as the root directory, not just its filename
+        let project = Project::test(fs.clone(), [temp_dir.path()], cx).await;
+
+        // Test cases where the function should return Some
+        cx.update(|cx| {
+            // Project-relative paths should return Some
+            // Create paths using the last segment of the temp path to simulate a project-relative path
+            let root_dir_name = Path::new(&temp_path)
+                .file_name()
+                .unwrap_or_else(|| std::ffi::OsStr::new("temp"))
+                .to_string_lossy();
+
+            assert!(
+                to_absolute_path(&format!("{root_dir_name}/src/main.rs"), project.clone(), cx)
+                    .is_some(),
+                "Failed to resolve main.rs path"
+            );
+
+            assert!(
+                to_absolute_path(
+                    &format!("{root_dir_name}/docs/readme.md",),
+                    project.clone(),
+                    cx,
+                )
+                .is_some(),
+                "Failed to resolve readme.md path"
+            );
+
+            // External URL should return None
+            let result = to_absolute_path("https://example.com", project.clone(), cx);
+            assert_eq!(result, None, "External URLs should return None");
+
+            // Path outside project
+            let result = to_absolute_path("../invalid/path", project.clone(), cx);
+            assert_eq!(result, None, "Paths outside the project should return None");
+        });
+    }
+
+    fn init_test(cx: &mut TestAppContext) {
+        cx.update(|cx| {
+            let settings_store = SettingsStore::test(cx);
+            cx.set_global(settings_store);
+            language::init(cx);
+            Project::init_settings(cx);
+        });
+    }
+}

Change summary

Detailed changes

Cargo.lock 🔗

assets/settings/default.json 🔗

crates/assistant_tools/Cargo.toml 🔗

crates/assistant_tools/src/open_tool.rs 🔗