From 5872276511c7b4e3ac2cac47b037da4c7996d9c6 Mon Sep 17 00:00:00 2001 From: Richard Feldman Date: Wed, 30 Apr 2025 22:33:52 -0400 Subject: [PATCH] Re-enable `open` tool (#29707) Release Notes: - Added `open` tool for opening files or URLs. --- Cargo.lock | 1 + assets/settings/default.json | 1 + crates/assistant_tools/Cargo.toml | 1 + crates/assistant_tools/src/open_tool.rs | 102 +++++++++++++++++++++++- 4 files changed, 102 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 3a18bc678aabce4690b9f2268f3d2f50def73758..c1b165309ec99da16a177ab679e0ac24577a970a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -732,6 +732,7 @@ dependencies = [ "serde_json", "settings", "task", + "tempfile", "terminal", "terminal_view", "tree-sitter-rust", diff --git a/assets/settings/default.json b/assets/settings/default.json index 692ac57d607cf58b5d996c01b86062264360bda4..41dac08ca8fc4dbb6450107c58672eca57e07e71 100644 --- a/assets/settings/default.json +++ b/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 diff --git a/crates/assistant_tools/Cargo.toml b/crates/assistant_tools/Cargo.toml index a394ed2c34fd9ecebd1ad914851d42d9a7a66953..78464a8a23b26bc561e99ce7bf4ba5c8a9f6bfa7 100644 --- a/crates/assistant_tools/Cargo.toml +++ b/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 diff --git a/crates/assistant_tools/src/open_tool.rs b/crates/assistant_tools/src/open_tool.rs index 88894ef79fbe6cfbdeb6ac9547dace8c2daafce1..2df0dda9052d38686323621c3e631260e0b5949f 100644 --- a/crates/assistant_tools/src/open_tool.rs +++ b/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, input: serde_json::Value, _messages: &[LanguageModelRequestMessage], - _project: Entity, + project: Entity, _action_log: Entity, _window: Option, 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, + cx: &mut App, +) -> Option { + 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); + }); + } +}