From 9f29a9ebb8b30bda7a33dad16afa28d6bc866df3 Mon Sep 17 00:00:00 2001 From: Revantark Date: Mon, 4 May 2026 00:41:53 +0530 Subject: [PATCH] acp_thread: Support absolute file mentions with row fragments (#53229) Sometimes the contents in the agent's thread are direct file links. Codex ACP almost every time uses absolute file paths instead of file://path/to/file. This is resulting in an error described in the #49978 . Self-Review Checklist: - [x] I've reviewed my own diff for quality, security, and reliability - [x] Unsafe blocks (if any) have justifying comments - [x] The content is consistent with the [UI/UX checklist](https://github.com/zed-industries/zed/blob/main/CONTRIBUTING.md#uiux-checklist) - [x] Tests cover the new/changed behavior - [x] Performance impact has been considered and is acceptable Closes #49978 Release Notes: - Fixed absolute path mentions in `acp_thread` so direct file references (including `:line` and `#Lline` formats) now open correctly instead of resolving to invalid/empty files. --------- Co-authored-by: Eric Holk --- crates/acp_thread/src/mention.rs | 190 ++++++++++++++++++++++++++++++- 1 file changed, 189 insertions(+), 1 deletion(-) diff --git a/crates/acp_thread/src/mention.rs b/crates/acp_thread/src/mention.rs index 403b71736c..4d52c202c6 100644 --- a/crates/acp_thread/src/mention.rs +++ b/crates/acp_thread/src/mention.rs @@ -12,7 +12,10 @@ use std::{ use ui::{App, IconName, SharedString}; use url::Url; use urlencoding::decode; -use util::{ResultExt, paths::PathStyle}; +use util::{ + ResultExt, + paths::{PathStyle, PathWithPosition, is_absolute}, +}; #[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize, Hash)] pub enum MentionUri { @@ -65,6 +68,11 @@ pub enum MentionUri { impl MentionUri { pub fn parse(input: &str, path_style: PathStyle) -> Result { + let input = input + .strip_prefix('`') + .and_then(|input| input.strip_suffix('`')) + .unwrap_or(input); + fn parse_line_range(fragment: &str) -> Result> { let range = fragment.strip_prefix("L").unwrap_or(fragment); @@ -92,6 +100,39 @@ impl MentionUri { Ok(start_line..=end_line) } + let parse_absolute_path = |input: &str| -> Result { + let (path_input, fragment) = input + .split_once('#') + .map_or((input, None), |(path, fragment)| (path, Some(fragment))); + + if let Some(fragment) = fragment.and_then(|fragment| parse_line_range(fragment).ok()) { + return Ok(MentionUri::Selection { + abs_path: Some(path_input.into()), + line_range: fragment, + }); + } + + let path_with_position = PathWithPosition::parse_str(path_input); + let abs_path = path_with_position.path; + if let Some(row) = path_with_position.row { + let line = row + .checked_sub(1) + .context("Line numbers should be 1-based")?; + // TODO: Preserve column info too. + Ok(MentionUri::Selection { + abs_path: Some(abs_path), + line_range: line..=line, + }) + } else { + Ok(MentionUri::File { abs_path }) + } + }; + + if is_absolute(input, path_style) && !input.contains("://") { + return parse_absolute_path(input) + .with_context(|| format!("Invalid absolute path mention URI: {input}")); + } + let url = url::Url::parse(input)?; let path = url.path(); match url.scheme() { @@ -737,6 +778,153 @@ mod tests { assert!(MentionUri::parse("zed:///agent/unknown/test", PathStyle::local()).is_err()); } + #[test] + fn test_parse_absolute_file_path() { + let file_path = path!("/path/to/file.rs"); + let parsed = MentionUri::parse(file_path, PathStyle::local()).unwrap(); + match &parsed { + MentionUri::File { abs_path } => { + assert_eq!(abs_path, Path::new(file_path)); + } + _ => panic!("Expected File variant"), + } + } + + #[test] + fn test_parse_absolute_file_path_with_row() { + let file_path = "/path/to/file.rs:42"; + let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + + #[test] + fn test_parse_absolute_file_path_with_fragment_line() { + let file_path = "/path/to/file.rs#L42"; + let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + + #[test] + fn test_parse_absolute_windows_path() { + let file_path = "C:\\Users\\zed\\project\\main.rs"; + let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap(); + match &parsed { + MentionUri::File { abs_path } => { + assert_eq!(abs_path, Path::new("C:\\Users\\zed\\project\\main.rs")); + } + _ => panic!("Expected File variant"), + } + } + + #[test] + fn test_parse_absolute_windows_file_path_with_row() { + let file_path = "C:\\Users\\zed\\project\\main.rs:42"; + let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!( + path.as_ref().unwrap(), + Path::new("C:\\Users\\zed\\project\\main.rs") + ); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + + #[test] + fn test_parse_absolute_windows_file_path_with_fragment_line() { + let file_path = "C:\\Users\\zed\\project\\main.rs#L42"; + let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!( + path.as_ref().unwrap(), + Path::new("C:\\Users\\zed\\project\\main.rs") + ); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + + #[test] + fn test_parse_backticked_absolute_file_path() { + let file_path = "`/path/to/file.rs`"; + let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap(); + match &parsed { + MentionUri::File { abs_path } => { + assert_eq!(abs_path, Path::new("/path/to/file.rs")); + } + _ => panic!("Expected File variant"), + } + } + + #[test] + fn test_parse_backticked_absolute_file_path_with_fragment_line() { + let file_path = "`/path/to/file.rs#L42`"; + let parsed = MentionUri::parse(file_path, PathStyle::Posix).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!(path.as_ref().unwrap(), Path::new("/path/to/file.rs")); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + + #[test] + fn test_parse_backticked_absolute_windows_file_path_with_fragment_line() { + let file_path = "`C:\\Users\\zed\\project\\main.rs#L42`"; + let parsed = MentionUri::parse(file_path, PathStyle::Windows).unwrap(); + match &parsed { + MentionUri::Selection { + abs_path: path, + line_range, + } => { + assert_eq!( + path.as_ref().unwrap(), + Path::new("C:\\Users\\zed\\project\\main.rs") + ); + assert_eq!(line_range.start(), &41); + assert_eq!(line_range.end(), &41); + } + _ => panic!("Expected Selection variant"), + } + } + #[test] fn test_single_line_number() { // https://github.com/zed-industries/zed/issues/46114