diff --git a/crates/agent/src/tools/grep_tool.rs b/crates/agent/src/tools/grep_tool.rs index 32d872f657..381639e3e2 100644 --- a/crates/agent/src/tools/grep_tool.rs +++ b/crates/agent/src/tools/grep_tool.rs @@ -5,7 +5,7 @@ use futures::{FutureExt as _, StreamExt}; use gpui::{App, Entity, SharedString, Task}; use language::{OffsetRangeExt, ParseStatus, Point}; use project::{ - Project, SearchResults, WorktreeSettings, + Project, ProjectPath, SearchResults, WorktreeSettings, search::{SearchQuery, SearchResult}, }; use schemars::JsonSchema; @@ -13,7 +13,7 @@ use serde::{Deserialize, Serialize}; use settings::Settings; use std::{cmp, fmt::Write, sync::Arc}; use util::RangeExt; -use util::markdown::MarkdownInlineCode; +use util::markdown::{MarkdownCodeBlock, MarkdownInlineCode}; use util::paths::PathMatcher; /// Searches the contents of files in the project with a regular expression @@ -178,6 +178,8 @@ impl AgentTool for GrepTool { futures::pin_mut!(rx); let mut output = String::new(); + let mut content = Vec::new(); + let mut locations = Vec::new(); let mut skips_remaining = input.offset; let mut matches_found = 0; let mut has_more_matches = false; @@ -203,25 +205,39 @@ impl AgentTool for GrepTool { continue; } - let (Some(path), mut parse_status) = buffer.read_with(cx, |buffer, cx| { - (buffer.file().map(|file| file.full_path(cx)), buffer.parse_status()) - }) else { + let (Some((path, project_path)), mut parse_status) = + buffer.read_with(cx, |buffer, cx| { + ( + buffer.file().map(|file| { + ( + file.full_path(cx), + ProjectPath { + worktree_id: file.worktree_id(cx), + path: file.path().clone(), + }, + ) + }), + buffer.parse_status(), + ) + }) + else { continue; }; // Check if this file should be excluded based on its worktree settings - if let Ok(Some(project_path)) = project.read_with(cx, |project, cx| { - project.find_project_path(&path, cx) + if cx.update(|cx| { + let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx); + worktree_settings.is_path_excluded(&project_path.path) + || worktree_settings.is_path_private(&project_path.path) }) { - if cx.update(|cx| { - let worktree_settings = WorktreeSettings::get(Some((&project_path).into()), cx); - worktree_settings.is_path_excluded(&project_path.path) - || worktree_settings.is_path_private(&project_path.path) - }) { - continue; - } + continue; } + let abs_path = project + .read_with(cx, |project, cx| project.absolute_path(&project_path, cx)) + .ok() + .flatten(); + while *parse_status.borrow() != ParseStatus::Idle { parse_status.changed().await.map_err(|e| e.to_string())?; } @@ -298,18 +314,41 @@ impl AgentTool for GrepTool { .ok(); } - if range.start.row == end_row { - writeln!(output, "L{}", range.start.row + 1) - .ok(); + let line_label = if range.start.row == end_row { + format!("L{}", range.start.row + 1) } else { - writeln!(output, "L{}-{}", range.start.row + 1, end_row + 1) - .ok(); - } + format!("L{}-{}", range.start.row + 1, end_row + 1) + }; + writeln!(output, "{line_label}").ok(); + let snippet: String = snapshot.text_for_range(range.clone()).collect(); output.push_str("```\n"); - output.extend(snapshot.text_for_range(range)); + output.push_str(&snippet); output.push_str("\n```\n"); + if let Some(abs_path) = &abs_path { + content.push(acp::ToolCallContent::Content(acp::Content::new( + acp::ContentBlock::ResourceLink(acp::ResourceLink::new( + format!("{}#{}", path.display(), line_label), + format!("file://{}#{}", abs_path.display(), line_label), + )), + ))); + locations.push( + acp::ToolCallLocation::new(abs_path).line(Some(range.start.row)), + ); + } + // Use a fence longer than any backtick run in the snippet so + // matches containing code fences don't break the rendering. + content.push(acp::ToolCallContent::Content(acp::Content::new( + acp::ContentBlock::Text(acp::TextContent::new( + MarkdownCodeBlock { + tag: "", + text: &snippet, + } + .to_string(), + )), + ))); + if let Some(ancestor_range) = ancestor_range && end_row < ancestor_range.end.row { let remaining_lines = ancestor_range.end.row - end_row; @@ -321,6 +360,14 @@ impl AgentTool for GrepTool { } } + if !content.is_empty() { + event_stream.update_fields( + acp::ToolCallUpdateFields::new() + .content(content) + .locations(locations), + ); + } + if matches_found == 0 { Ok("No matches found".into()) } else if has_more_matches { @@ -508,6 +555,158 @@ mod tests { ); } + // The grep tool streams a clickable `file://` ResourceLink and a tool-call + // location for every match so each result opens the file at the matched line + // in the agent panel. The model-facing text output stays link-free. + #[gpui::test] + async fn test_grep_results_are_clickable_file_links(cx: &mut TestAppContext) { + init_test(cx); + cx.executor().allow_parking(); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/root"), + json!({ + "src": { + "alpha.txt": "the needle is in alpha", + }, + "beta.txt": "the needle is in beta", + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + + let tool = Arc::new(GrepTool { project }); + let (event_stream, mut events) = ToolCallEventStream::test(); + let input = GrepToolInput { + regex: "needle".to_string(), + include_pattern: None, + offset: 0, + case_sensitive: false, + }; + let task = cx.update(|cx| tool.run(ToolInput::resolved(input), event_stream, cx)); + let output = task.await.expect("grep tool should succeed"); + let update = events.expect_update_fields().await; + + // Model-facing output is unchanged: matches are rendered as markdown, but + // the clickable `file://` URIs only live in the tool-call UI content. + assert!(output.contains("## Matches in")); + assert!( + !output.contains("file://"), + "model-facing output should not embed file:// links, got:\n{output}" + ); + + // Pull the ResourceLink blocks (the clickable links) out of the content. + let content = update.content.expect("expected content blocks"); + let links = content + .iter() + .filter_map(|block| match block { + acp::ToolCallContent::Content(inner) => match &inner.content { + acp::ContentBlock::ResourceLink(link) => Some(link), + _ => None, + }, + _ => None, + }) + .collect::>(); + assert_eq!(links.len(), 2, "expected one resource link per match"); + + let alpha_uri = format!("file://{}#L1", path!("/root/src/alpha.txt")); + assert!( + links.iter().any(|link| { + link.name.replace('\\', "/") == "root/src/alpha.txt#L1" + && link.uri.replace('\\', "/") == alpha_uri.replace('\\', "/") + }), + "missing clickable link for alpha.txt, got: {links:?}" + ); + + let beta_uri = format!("file://{}#L1", path!("/root/beta.txt")); + assert!( + links.iter().any(|link| { + link.name.replace('\\', "/") == "root/beta.txt#L1" + && link.uri.replace('\\', "/") == beta_uri.replace('\\', "/") + }), + "missing clickable link for beta.txt, got: {links:?}" + ); + + // Each match also reports a location so the panel can reveal the file at + // the matched (0-based) row. + let locations = update.locations.expect("expected locations"); + assert_eq!(locations.len(), 2); + assert!( + locations.iter().any(|location| { + location.path.to_string_lossy().replace('\\', "/") + == path!("/root/src/alpha.txt").replace('\\', "/") + && location.line == Some(0) + }), + "missing location for alpha.txt, got: {locations:?}" + ); + assert!( + locations.iter().any(|location| { + location.path.to_string_lossy().replace('\\', "/") + == path!("/root/beta.txt").replace('\\', "/") + && location.line == Some(0) + }), + "missing location for beta.txt, got: {locations:?}" + ); + } + + // Snippets that themselves contain a ``` code fence (e.g. matches inside + // markdown) must be wrapped in a longer fence so they don't break out of the + // surrounding code block when rendered in the agent panel. + #[gpui::test] + async fn test_grep_snippet_fence_outlives_inner_backticks(cx: &mut TestAppContext) { + init_test(cx); + cx.executor().allow_parking(); + + let fs = FakeFs::new(cx.executor()); + fs.insert_tree( + path!("/root"), + json!({ + "doc.md": "before\n```\nNEEDLE inside fence\n```\nafter", + }), + ) + .await; + + let project = Project::test(fs.clone(), [path!("/root").as_ref()], cx).await; + + let tool = Arc::new(GrepTool { project }); + let (event_stream, mut events) = ToolCallEventStream::test(); + let input = GrepToolInput { + regex: "NEEDLE".to_string(), + include_pattern: None, + offset: 0, + case_sensitive: false, + }; + let task = cx.update(|cx| tool.run(ToolInput::resolved(input), event_stream, cx)); + task.await.expect("grep tool should succeed"); + let update = events.expect_update_fields().await; + + // Find the snippet text block emitted alongside the clickable link. + let content = update.content.expect("expected content blocks"); + let snippet = content + .iter() + .find_map(|block| match block { + acp::ToolCallContent::Content(inner) => match &inner.content { + acp::ContentBlock::Text(text) => Some(text.text.as_str()), + _ => None, + }, + _ => None, + }) + .expect("expected a snippet text block in the tool-call content"); + + // The snippet embeds a three-backtick fence, so the wrapping fence must be + // at least four backticks long to avoid breaking out of the code block. + assert!( + snippet.contains("NEEDLE inside fence"), + "snippet should contain the matched line, got:\n{snippet}" + ); + assert!( + snippet.starts_with("````\n"), + "snippet should be wrapped in a fence longer than the inner ```, got:\n{snippet}" + ); + } + /// Helper function to set up a syntax test environment async fn setup_syntax_test(cx: &mut TestAppContext) -> Entity { use unindent::Unindent; diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 8028b9bde3..b938a2572a 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -9215,7 +9215,12 @@ impl ThreadView { }; let label: SharedString = if let Some(abs_path) = is_file { - if let Some(project_path) = project + // Split off an optional `#L` fragment so the path still resolves. + let (abs_path, fragment) = abs_path + .split_once('#') + .map_or((abs_path, None), |(path, fragment)| (path, Some(fragment))); + + let path_label = if let Some(project_path) = project .read(cx) .project_path_for_absolute_path(&Path::new(abs_path), cx) && let Some(worktree) = project @@ -9227,9 +9232,13 @@ impl ThreadView { .full_path(&project_path.path) .to_string_lossy() .to_string() - .into() } else { - abs_path.to_string().into() + abs_path.to_string() + }; + + match fragment { + Some(fragment) => format!("{path_label}#{fragment}").into(), + None => path_label.into(), } } else { uri.clone()