Fix agent hyperlinks do not open files (#56283)

## Summary

Fix agent message hyperlinks that point to Windows file paths but are
not parsed as openable project paths.

This covers links like:

```md
[Cargo.toml](</C:/Projects/Example Workspace/Cargo.toml:2>)
[filename.ext](C:\Projects\Example%20Workspace\path\to\filename.ext:42)
[AGENTS.md](</c/Projects/Example Workspace/AGENTS.md>)
```

## Problem

Agent responses can emit Markdown hyperlinks whose targets are Windows
paths rather than `file://` URLs. Some of those targets include a
leading slash before the drive (`/C:/...`), Git Bash/MSYS-style drive
prefixes (`/c/...`), percent-escaped spaces, or line suffixes. These
were not normalized before mention parsing, so clicking the hyperlink
could do nothing instead of opening the file.

## Solution

- Normalize hyperlink path targets before parsing them as `MentionUri`
paths.
- Decode percent escapes in bare path targets so `%20` becomes a literal
space before path/line parsing.
- Convert Windows-compatible hyperlink paths such as `/C:/...` and
`/c/...` into native Windows paths.
- Generate file resource links from `find_path_tool` through
`MentionUri::to_uri()` instead of hand-building `file://` strings.

## Result

Before: clicking agent path hyperlinks did not open the referenced file.

After:  


https://github.com/user-attachments/assets/6c7fad77-4a1e-4497-a4f9-4a4fdf86d527


## Validation

- `cargo test -p acp_thread test_parse_windows --features test-support`
- `cargo fmt --check`

## Follow-up changes

Additions on top of the original work above:

- Moved the hyperlink heuristics into a dedicated
`MentionUri::parse_hyperlink` entry point. `MentionUri::parse` stays
strict, so canonical mention URIs round-trip verbatim and other callers
(message editor, thread deserialization, resource links) are unaffected.
- Percent escapes that decode to path separators (`%2F`, `%5C`) are left
encoded, so decoding can never change which directories a path
traverses.
- Bare paths with escapes are ambiguous (a file may literally be named
`a%20b.rs`): `open_link` prefers the decoded interpretation and falls
back to `MentionUri::parse_hyperlink_literal` when the decoded path
doesn't resolve in the project but the literal one does.
- Links to files outside the project's worktrees now open, gated by an
async existence check through the project `Fs` (correct for remote
projects; broken links no longer create empty buffers or add worktrees).
`open_link` and the mention-crease open path are unified into one
`open_abs_path_at_point`, which now also places the cursor for
out-of-project selection/symbol links.
- `grep_tool` resource links also go through `MentionUri::to_uri()` now,
fixing malformed `file://C:\...` URIs and unencoded spaces.
- Added tests: percent-escape disambiguation, out-of-project link
opening, drive-letter normalization, UNC paths, and literal-path parsing
(`cargo test -p acp_thread mention`, `cargo test -p agent_ui open_link`,
`cargo test -p agent grep_tool`); manually verified the link spellings
above on Windows.

Release Notes:

- Fixed agent path hyperlinks on Windows when paths contain spaces or
shell-style drive prefixes.

---------

Co-authored-by: Martin Ye <martin@zed.dev>
This commit is contained in:
di404andMartin Ye authored and GitHub committed 2026-07-05 03:54:54 +00:00
1 parent 6701fe7652
commit dba0fdc9aa
6 files changed
+770 -134

No files matched your search

+557 -59
View File
@@ -83,33 +83,6 @@ impl MentionUri {
.and_then(|input| input.strip_suffix('`'))
.unwrap_or(input);
fn parse_line_range(fragment: &str) -> Result<RangeInclusive<u32>> {
let range = fragment.strip_prefix("L").unwrap_or(fragment);
let (start, end) = if let Some((start, end)) = range.split_once(":") {
(start, end)
} else if let Some((start, end)) = range.split_once("-") {
// Also handle L10-20 or L10-L20 format
(start, end.strip_prefix("L").unwrap_or(end))
} else {
// Single line number like L1872 - treat as a range of one line
(range, range)
};
let start_line = start
.parse::<u32>()
.context("Parsing line range start")?
.checked_sub(1)
.context("Line numbers should be 1-based")?;
let end_line = end
.parse::<u32>()
.context("Parsing line range end")?
.checked_sub(1)
.context("Line numbers should be 1-based")?;
Ok(start_line..=end_line)
}
let parse_column =
|input: Option<String>| -> Option<u32> { input?.parse::<u32>().ok()?.checked_sub(1) };
let validate_query_params = |url: &Url, allowed: &[&str]| -> Result<()> {
@@ -121,37 +94,6 @@ impl MentionUri {
Ok(())
};
let parse_absolute_path = |input: &str| -> Result<Self> {
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,
column: None,
});
}
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")?;
Ok(MentionUri::Selection {
abs_path: Some(abs_path),
line_range: line..=line,
column: path_with_position
.column
.map(|column| column.saturating_sub(1)),
})
} 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}"));
@@ -168,7 +110,10 @@ impl MentionUri {
};
let decoded = decode(trimmed).unwrap_or(Cow::Borrowed(trimmed));
let normalized: Cow<str> = if path_style.is_windows() {
Cow::Owned(decoded.replace('/', "\\"))
match to_native_windows_path(&decoded) {
Some(native) => Cow::Owned(native),
None => decoded,
}
} else {
decoded
};
@@ -337,6 +282,56 @@ impl MentionUri {
}
}
/// Parses a hyperlink target from agent-authored Markdown.
///
/// Unlike [`MentionUri::parse`] — which stays strict so canonical mention
/// URIs round-trip verbatim — bare path targets are normalized first:
/// percent escapes are decoded (see [`decode_path_escapes`]) and
/// Windows-compatible spellings like `/C:/foo` or `/c/foo` become native
/// paths (see [`to_native_windows_path`]).
pub fn parse_hyperlink(input: &str, path_style: PathStyle) -> Result<Self> {
if let Some(target) = bare_path_target(input, path_style) {
return parse_hyperlink_path(target, path_style, DecodePercentEscapes::Yes)
.with_context(|| format!("Invalid hyperlink path target: {input}"));
}
Self::parse(input, path_style)
}
/// Returns the literal (un-decoded) interpretation of a bare-path
/// hyperlink target, for files whose names literally contain an escape
/// sequence (e.g. `a%20b.rs`). Returns `None` when this wouldn't differ
/// from [`MentionUri::parse_hyperlink`], including for URLs, whose
/// escapes are unambiguous.
pub fn parse_hyperlink_literal(input: &str, path_style: PathStyle) -> Option<Self> {
let target = bare_path_target(input, path_style)?;
let (path_input, _) = split_path_fragment(target);
if !matches!(decode_path_escapes(path_input), Cow::Owned(_)) {
return None;
}
parse_hyperlink_path(target, path_style, DecodePercentEscapes::No).ok()
}
/// The absolute path this mention refers to, if it refers to one.
pub fn abs_path(&self) -> Option<&Path> {
match self {
MentionUri::File { abs_path }
| MentionUri::Directory { abs_path }
| MentionUri::Symbol { abs_path, .. } => Some(abs_path),
MentionUri::Selection { abs_path, .. } => abs_path.as_deref(),
MentionUri::Skill {
skill_file_path, ..
} => Some(skill_file_path),
MentionUri::PastedImage { .. }
| MentionUri::Thread { .. }
| MentionUri::Rule { .. }
| MentionUri::Diagnostics { .. }
| MentionUri::Fetch { .. }
| MentionUri::TerminalSelection { .. }
| MentionUri::GitDiff { .. }
| MentionUri::MergeConflict { .. } => None,
}
}
pub fn name(&self) -> String {
match self {
MentionUri::File { abs_path, .. } | MentionUri::Directory { abs_path, .. } => abs_path
@@ -599,6 +594,217 @@ impl fmt::Display for MentionLink<'_> {
}
}
#[derive(Clone, Copy, PartialEq, Eq)]
enum DecodePercentEscapes {
Yes,
No,
}
fn parse_line_range(fragment: &str) -> Result<RangeInclusive<u32>> {
let range = fragment.strip_prefix("L").unwrap_or(fragment);
let (start, end) = if let Some((start, end)) = range.split_once(":") {
(start, end)
} else if let Some((start, end)) = range.split_once("-") {
// Also handle L10-20 or L10-L20 format
(start, end.strip_prefix("L").unwrap_or(end))
} else {
// Single line number like L1872 - treat as a range of one line
(range, range)
};
let start_line = start
.parse::<u32>()
.context("Parsing line range start")?
.checked_sub(1)
.context("Line numbers should be 1-based")?;
let end_line = end
.parse::<u32>()
.context("Parsing line range end")?
.checked_sub(1)
.context("Line numbers should be 1-based")?;
Ok(start_line..=end_line)
}
/// Returns the mention target as a bare absolute path (not a URL), with the
/// backticks agents sometimes add stripped.
fn bare_path_target(input: &str, path_style: PathStyle) -> Option<&str> {
let input = input
.strip_prefix('`')
.and_then(|input| input.strip_suffix('`'))
.unwrap_or(input);
(is_absolute(input, path_style) && !input.contains("://")).then_some(input)
}
fn split_path_fragment(input: &str) -> (&str, Option<&str>) {
input
.split_once('#')
.map_or((input, None), |(path, fragment)| (path, Some(fragment)))
}
fn parse_absolute_path(input: &str) -> Result<MentionUri> {
let (path_input, fragment) = split_path_fragment(input);
absolute_path_mention(path_input, fragment)
}
/// Like [`parse_absolute_path`], but normalizes hyperlink spellings first.
fn parse_hyperlink_path(
input: &str,
path_style: PathStyle,
decode_escapes: DecodePercentEscapes,
) -> Result<MentionUri> {
let (path_input, fragment) = split_path_fragment(input);
let path_input = normalize_path_mention(path_input, path_style, decode_escapes);
absolute_path_mention(&path_input, fragment)
}
fn absolute_path_mention(path_input: &str, fragment: Option<&str>) -> Result<MentionUri> {
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,
column: None,
});
}
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")?;
Ok(MentionUri::Selection {
abs_path: Some(abs_path),
line_range: line..=line,
column: path_with_position
.column
.map(|column| column.saturating_sub(1)),
})
} else {
Ok(MentionUri::File { abs_path })
}
}
fn normalize_path_mention(
input: &str,
path_style: PathStyle,
decode_escapes: DecodePercentEscapes,
) -> Cow<'_, str> {
let decoded = match decode_escapes {
DecodePercentEscapes::Yes => decode_path_escapes(input),
DecodePercentEscapes::No => Cow::Borrowed(input),
};
if !path_style.is_windows() {
return decoded;
}
match to_native_windows_path(&decoded) {
Some(native) => Cow::Owned(native),
None => decoded,
}
}
/// Decodes percent escapes in a path, leaving separator escapes (`%2F`,
/// `%5C`) encoded so decoding can't change which directories the path
/// traverses. Invalid sequences and non-UTF-8 results leave the input
/// unchanged. Returns `Cow::Owned` iff decoding changed the input
/// (`parse_hyperlink_literal` relies on this).
fn decode_path_escapes(input: &str) -> Cow<'_, str> {
fn hex_digit(byte: u8) -> Option<u8> {
match byte {
b'0'..=b'9' => Some(byte - b'0'),
b'a'..=b'f' => Some(byte - b'a' + 10),
b'A'..=b'F' => Some(byte - b'A' + 10),
_ => None,
}
}
if !input.contains('%') {
return Cow::Borrowed(input);
}
let bytes = input.as_bytes();
let mut decoded = Vec::with_capacity(bytes.len());
let mut index = 0;
while index < bytes.len() {
if bytes[index] == b'%'
&& let Some(high) = bytes.get(index + 1).copied().and_then(hex_digit)
&& let Some(low) = bytes.get(index + 2).copied().and_then(hex_digit)
{
let byte = (high << 4) | low;
if byte != b'/' && byte != b'\\' {
decoded.push(byte);
index += 3;
continue;
}
}
decoded.push(bytes[index]);
index += 1;
}
if decoded == bytes {
return Cow::Borrowed(input);
}
match String::from_utf8(decoded) {
Ok(decoded) => Cow::Owned(decoded),
Err(_) => Cow::Borrowed(input),
}
}
/// Converts Windows-compatible path spellings into a native Windows path,
/// normalizing separators to backslashes and drive letters to uppercase so
/// parsed paths compare equal to worktree paths. Returns `None` when the
/// input needs no changes.
fn to_native_windows_path(path: &str) -> Option<String> {
fn join_drive(drive: char, rest: &str) -> String {
format!(
"{}:\\{}",
drive.to_ascii_uppercase(),
rest.replace('/', "\\")
)
}
if let Some(rest) = path.strip_prefix('/') {
// URL-style path with a leading slash before the drive: `/C:/foo`.
let mut chars = rest.chars();
if let (Some(drive), Some(':'), Some('/' | '\\')) =
(chars.next(), chars.next(), chars.next())
&& drive.is_ascii_alphabetic()
{
return Some(join_drive(drive, chars.as_str()));
}
// MSYS/Git Bash style: `/c/foo`. Lowercase-only, since that's what
// those shells emit and uppercase risks misreading real directories.
let mut chars = rest.chars();
if let (Some(drive), Some('/' | '\\')) = (chars.next(), chars.next())
&& drive.is_ascii_lowercase()
{
return Some(join_drive(drive, chars.as_str()));
}
}
// A native path with a drive prefix: uppercase the drive and normalize
// separators, e.g. `c:/foo` or `c:\foo`.
let mut chars = path.chars();
if let (Some(drive), Some(':')) = (chars.next(), chars.next())
&& drive.is_ascii_alphabetic()
{
if drive.is_ascii_uppercase() && !path.contains('/') {
return None;
}
return Some(format!(
"{}:{}",
drive.to_ascii_uppercase(),
chars.as_str().replace('/', "\\")
));
}
if path.contains('/') {
return Some(path.replace('/', "\\"));
}
None
}
fn default_include_errors() -> bool {
true
}
@@ -727,6 +933,298 @@ mod tests {
}
}
#[test]
fn test_parse_file_uri_with_spaces() {
let parsed =
MentionUri::parse("file:///C:/path%20with%20space/file.rs", PathStyle::Windows)
.unwrap();
match parsed {
MentionUri::File { abs_path } => {
assert_eq!(abs_path, PathBuf::from("C:\\path with space\\file.rs"));
}
other => panic!("Expected File variant, got {other:?}"),
}
assert_eq!(
MentionUri::File {
abs_path: PathBuf::from("C:\\path with space\\file.rs")
}
.to_uri()
.to_string(),
"file:///C:/path%20with%20space/file.rs"
);
}
#[test]
fn test_parse_windows_drive_path_with_leading_slash_and_line() {
let parsed = MentionUri::parse_hyperlink(
"/C:/Projects/Example Workspace/Cargo.toml:2",
PathStyle::Windows,
)
.unwrap();
match parsed {
MentionUri::Selection {
abs_path: Some(abs_path),
line_range,
..
} => {
assert_eq!(
abs_path,
PathBuf::from("C:\\Projects\\Example Workspace\\Cargo.toml")
);
assert_eq!(line_range, 1..=1);
}
other => panic!("Expected Selection variant, got {other:?}"),
}
}
#[test]
fn test_parse_windows_path_with_percent_escaped_spaces_and_line() {
let parsed = MentionUri::parse_hyperlink(
"C:\\Projects\\Example%20Workspace\\path\\to\\filename.ext:42",
PathStyle::Windows,
)
.unwrap();
match parsed {
MentionUri::Selection {
abs_path: Some(abs_path),
line_range,
..
} => {
assert_eq!(
abs_path,
PathBuf::from("C:\\Projects\\Example Workspace\\path\\to\\filename.ext")
);
assert_eq!(line_range, 41..=41);
}
other => panic!("Expected Selection variant, got {other:?}"),
}
}
#[test]
fn test_parse_windows_compat_path_with_spaces() {
let parsed = MentionUri::parse_hyperlink(
"/c/Projects/Example Workspace/AGENTS.md",
PathStyle::Windows,
)
.unwrap();
match parsed {
MentionUri::File { abs_path } => {
assert_eq!(
abs_path,
PathBuf::from("C:\\Projects\\Example Workspace\\AGENTS.md")
);
}
other => panic!("Expected File variant, got {other:?}"),
}
}
#[test]
fn test_parse_windows_drive_path_with_leading_slash_and_fragment_line() {
let parsed =
MentionUri::parse_hyperlink("/C:/Projects/Cargo.toml#L4", PathStyle::Windows).unwrap();
match parsed {
MentionUri::Selection {
abs_path: Some(abs_path),
line_range,
..
} => {
assert_eq!(abs_path, PathBuf::from("C:\\Projects\\Cargo.toml"));
assert_eq!(line_range, 3..=3);
}
other => panic!("Expected Selection variant, got {other:?}"),
}
}
#[test]
fn test_windows_drive_path_with_leading_slash_round_trips() {
let parsed = MentionUri::parse_hyperlink("/C:/dir/file.rs", PathStyle::Windows).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("C:\\dir\\file.rs")
}
);
let uri = parsed.to_uri().to_string();
assert_eq!(uri, "file:///C:/dir/file.rs");
assert_eq!(MentionUri::parse(&uri, PathStyle::Windows).unwrap(), parsed);
}
#[test]
fn test_parse_windows_unc_path() {
let parsed =
MentionUri::parse_hyperlink("//server/share/dir/file.rs", PathStyle::Windows).unwrap();
match parsed {
MentionUri::File { abs_path } => {
assert_eq!(abs_path, PathBuf::from("\\\\server\\share\\dir\\file.rs"));
}
other => panic!("Expected File variant, got {other:?}"),
}
}
#[test]
fn test_parse_windows_drive_letters_are_uppercased() {
for input in [
"file:///c:/foo/bar.rs",
"/c:/foo/bar.rs",
"/c/foo/bar.rs",
"c:\\foo\\bar.rs",
"c:/foo/bar.rs",
] {
let parsed = MentionUri::parse_hyperlink(input, PathStyle::Windows).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("C:\\foo\\bar.rs")
},
"input: {input}"
);
}
}
#[test]
fn test_msys_style_paths_require_lowercase_drive() {
// Uppercase `/C/foo` is more likely a real directory than a drive.
let parsed = MentionUri::parse_hyperlink("/C/Users/readme.md", PathStyle::Windows).unwrap();
match parsed {
MentionUri::File { abs_path } => {
assert_eq!(abs_path, PathBuf::from("\\C\\Users\\readme.md"));
}
other => panic!("Expected File variant, got {other:?}"),
}
}
#[test]
fn test_posix_paths_are_not_rewritten_as_windows_drives() {
let parsed =
MentionUri::parse_hyperlink("/c/Projects/AGENTS.md", PathStyle::Posix).unwrap();
match parsed {
MentionUri::File { abs_path } => {
assert_eq!(abs_path, PathBuf::from("/c/Projects/AGENTS.md"));
}
other => panic!("Expected File variant, got {other:?}"),
}
}
#[test]
fn test_hyperlink_percent_escapes_are_decoded() {
let parsed = MentionUri::parse_hyperlink("/tmp/a%20b.rs", PathStyle::Posix).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("/tmp/a b.rs")
}
);
// Invalid escape sequences pass through unchanged.
let parsed =
MentionUri::parse_hyperlink("C:\\dir\\100%_done.txt", PathStyle::Windows).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("C:\\dir\\100%_done.txt")
}
);
// Separator escapes stay encoded (no introduced path traversal).
let parsed = MentionUri::parse_hyperlink("/tmp/a%2Fb.rs", PathStyle::Posix).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("/tmp/a%2Fb.rs")
}
);
let parsed =
MentionUri::parse_hyperlink("/tmp/..%2F..%2Fsecret", PathStyle::Posix).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("/tmp/..%2F..%2Fsecret")
}
);
}
#[test]
fn test_parse_keeps_bare_path_targets_verbatim() {
let parsed = MentionUri::parse("/tmp/a%20b.rs", PathStyle::Posix).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("/tmp/a%20b.rs")
}
);
let parsed = MentionUri::parse("/c/Projects/AGENTS.md", PathStyle::Windows).unwrap();
assert_eq!(
parsed,
MentionUri::File {
abs_path: PathBuf::from("/c/Projects/AGENTS.md")
}
);
}
#[test]
fn test_parse_hyperlink_literal_keeps_percent_escapes() {
let literal =
MentionUri::parse_hyperlink_literal("/tmp/a%20b.rs", PathStyle::Posix).unwrap();
assert_eq!(
literal,
MentionUri::File {
abs_path: PathBuf::from("/tmp/a%20b.rs")
}
);
// Line suffixes still parse.
let literal =
MentionUri::parse_hyperlink_literal("/tmp/a%20b.rs:42", PathStyle::Posix).unwrap();
assert_eq!(
literal,
MentionUri::Selection {
abs_path: Some(PathBuf::from("/tmp/a%20b.rs")),
line_range: 41..=41,
column: None,
}
);
// Windows normalization still applies.
let literal =
MentionUri::parse_hyperlink_literal("/C:/dir/a%20b.rs", PathStyle::Windows).unwrap();
assert_eq!(
literal,
MentionUri::File {
abs_path: PathBuf::from("C:\\dir\\a%20b.rs")
}
);
}
#[test]
fn test_parse_hyperlink_literal_returns_none_when_unambiguous() {
// No percent escapes: identical to `parse_hyperlink`.
assert_eq!(
MentionUri::parse_hyperlink_literal("/tmp/a b.rs", PathStyle::Posix),
None
);
// Invalid escape sequences are also left alone by `parse_hyperlink`.
assert_eq!(
MentionUri::parse_hyperlink_literal("/tmp/100%_done.txt", PathStyle::Posix),
None
);
// Separator escapes are never decoded, so they're not ambiguous.
assert_eq!(
MentionUri::parse_hyperlink_literal("/tmp/a%2Fb.rs", PathStyle::Posix),
None
);
// URLs are spec-encoded, not ambiguous.
assert_eq!(
MentionUri::parse_hyperlink_literal("file:///tmp/a%20b.rs", PathStyle::Posix),
None
);
// Relative paths are not bare-path mentions.
assert_eq!(
MentionUri::parse_hyperlink_literal("tmp/a%20b.rs", PathStyle::Posix),
None
);
}
#[test]
fn test_to_directory_uri_without_slash() {
let uri = MentionUri::Directory {
+5 -1
View File
@@ -1,4 +1,5 @@
use crate::{AgentTool, ToolCallEventStream, ToolInput};
use acp_thread::MentionUri;
use agent_client_protocol::schema::v1 as acp;
use anyhow::{Result, anyhow};
use futures::FutureExt as _;
@@ -155,10 +156,13 @@ impl AgentTool for FindPathTool {
paginated_matches
.iter()
.map(|path| {
let uri = MentionUri::File {
abs_path: path.clone(),
};
acp::ToolCallContent::Content(acp::Content::new(
acp::ContentBlock::ResourceLink(acp::ResourceLink::new(
path.to_string_lossy(),
format!("file://{}", path.display()),
uri.to_uri().to_string(),
)),
))
})
+25 -9
View File
@@ -1,4 +1,5 @@
use crate::{AgentTool, ToolCallEventStream, ToolInput};
use acp_thread::MentionUri;
use agent_client_protocol::schema::v1 as acp;
use anyhow::Result;
use futures::{FutureExt as _, StreamExt};
@@ -327,10 +328,15 @@ impl AgentTool for GrepTool {
output.push_str("\n```\n");
if let Some(abs_path) = &abs_path {
let uri = MentionUri::Selection {
abs_path: Some(abs_path.clone()),
line_range: range.start.row..=end_row,
column: None,
};
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),
uri.to_uri().to_string(),
)),
)));
locations.push(
@@ -393,6 +399,7 @@ mod tests {
use project::{FakeFs, Project};
use serde_json::json;
use settings::SettingsStore;
use std::path::PathBuf;
use unindent::Unindent;
use util::path;
@@ -611,21 +618,30 @@ mod tests {
.collect::<Vec<_>>();
assert_eq!(links.len(), 2, "expected one resource link per match");
let alpha_uri = format!("file://{}#L1", path!("/root/src/alpha.txt"));
let selection_uri = |abs_path: &str| {
MentionUri::Selection {
abs_path: Some(PathBuf::from(abs_path)),
line_range: 0..=0,
column: None,
}
.to_uri()
.to_string()
};
let alpha_uri = selection_uri(path!("/root/src/alpha.txt"));
assert!(
links.iter().any(|link| {
link.name.replace('\\', "/") == "root/src/alpha.txt#L1"
&& link.uri.replace('\\', "/") == alpha_uri.replace('\\', "/")
link.name.replace('\\', "/") == "root/src/alpha.txt#L1" && link.uri == alpha_uri
}),
"missing clickable link for alpha.txt, got: {links:?}"
);
let beta_uri = format!("file://{}#L1", path!("/root/beta.txt"));
let beta_uri = selection_uri(path!("/root/beta.txt"));
assert!(
links.iter().any(|link| {
link.name.replace('\\', "/") == "root/beta.txt#L1"
&& link.uri.replace('\\', "/") == beta_uri.replace('\\', "/")
}),
links
.iter()
.any(|link| link.name.replace('\\', "/") == "root/beta.txt#L1"
&& link.uri == beta_uri),
"missing clickable link for beta.txt, got: {links:?}"
);
+42 -14
View File
@@ -65,7 +65,7 @@ use serde::{Deserialize, Serialize};
use settings::{LanguageModelSelection, Settings as _, SettingsStore, SidebarSide};
use std::any::TypeId;
use std::path::{Path, PathBuf};
use workspace::Workspace;
use workspace::{OpenOptions, Workspace};
use crate::agent_configuration::ManageProfilesModal;
pub use crate::agent_connection_store::{ActiveAcpConnection, AgentConnectionStore};
@@ -118,40 +118,68 @@ pub(crate) fn resolve_agent_image(
None
}
/// Opens `abs_path` in the workspace, moving the cursor to `point` when one
/// is given. Paths outside every worktree are only opened when a file exists
/// there, so broken agent links don't create empty buffers.
pub(crate) fn open_abs_path_at_point(
workspace: &mut Workspace,
abs_path: PathBuf,
point: Point,
point: Option<Point>,
window: &mut Window,
cx: &mut Context<Workspace>,
) -> bool {
let project = workspace.project();
let Some(path) = project.update(cx, |project, cx| project.find_project_path(abs_path, cx))
else {
return false;
};
let item = workspace.open_path(path, None, true, window, cx);
) {
let project_path = workspace
.project()
.update(cx, |project, cx| project.find_project_path(&abs_path, cx));
let fs = workspace.project().read(cx).fs().clone();
let workspace = cx.weak_entity();
window
.spawn(cx, async move |cx| {
let Some(editor) = item.await?.downcast::<Editor>() else {
let item = if let Some(project_path) = project_path {
workspace
.update_in(cx, |workspace, window, cx| {
workspace.open_path(project_path, None, true, window, cx)
})?
.await?
} else {
let metadata = fs.metadata(&abs_path).await?;
anyhow::ensure!(
metadata.is_some_and(|metadata| !metadata.is_dir),
"no file found at path {abs_path:?}"
);
workspace
.update_in(cx, |workspace, window, cx| {
workspace.open_abs_path(
abs_path,
OpenOptions {
focus: Some(true),
..Default::default()
},
window,
cx,
)
})?
.await?
};
let Some(point) = point else {
return Ok(());
};
let Some(editor) = item.downcast::<Editor>() else {
return Ok(());
};
let range = point..point;
editor
.update_in(cx, |editor, window, cx| {
editor.change_selections(
SelectionEffects::scroll(Autoscroll::center()),
window,
cx,
|selections| selections.select_ranges([range]),
|selections| selections.select_ranges([point..point]),
);
})
.ok();
anyhow::Ok(())
})
.detach_and_log_err(cx);
true
}
pub const DEFAULT_THREAD_TITLE: &str = "New Agent Thread";
@@ -11931,19 +11931,31 @@ pub(crate) fn open_link(
return;
};
if let Some(mention) = MentionUri::parse(&url, workspace.read(cx).path_style(cx)).log_err() {
let path_style = workspace.read(cx).path_style(cx);
if let Some(mention) = MentionUri::parse_hyperlink(&url, path_style).log_err() {
// Percent escapes in bare paths are ambiguous: prefer the decoded
// interpretation, falling back to the literal one (e.g. a file
// actually named `a%20b.rs`) only when the decoded path doesn't
// resolve in the project but the literal one does.
let resolves_in_project = |mention: &MentionUri, cx: &App| {
mention.abs_path().is_some_and(|abs_path| {
let project = workspace.read(cx).project().read(cx);
project
.find_project_path(abs_path, cx)
.is_some_and(|path| project.entry_for_path(&path, cx).is_some())
})
};
let mention = match MentionUri::parse_hyperlink_literal(&url, path_style) {
Some(literal)
if !resolves_in_project(&mention, cx) && resolves_in_project(&literal, cx) =>
{
literal
}
_ => mention,
};
workspace.update(cx, |workspace, cx| match mention {
MentionUri::File { abs_path } => {
let project = workspace.project();
let Some(path) =
project.update(cx, |project, cx| project.find_project_path(abs_path, cx))
else {
return;
};
workspace
.open_path(path, None, true, window, cx)
.detach_and_log_err(cx);
open_abs_path_at_point(workspace, abs_path, None, window, cx);
}
MentionUri::PastedImage { .. } => {}
MentionUri::Directory { abs_path } => {
@@ -11967,7 +11979,7 @@ pub(crate) fn open_link(
open_abs_path_at_point(
workspace,
path,
Point::new(*line_range.start(), 0),
Some(Point::new(*line_range.start(), 0)),
window,
cx,
);
@@ -11980,7 +11992,7 @@ pub(crate) fn open_link(
open_abs_path_at_point(
workspace,
path,
Point::new(*line_range.start(), column.unwrap_or(0)),
Some(Point::new(*line_range.start(), column.unwrap_or(0))),
window,
cx,
);
@@ -12067,6 +12079,7 @@ mod tests {
use super::*;
use project::{FakeFs, Project};
use serde_json::json;
use std::path::Path;
use util::path;
use workspace::MultiWorkspace;
@@ -12171,6 +12184,118 @@ mod tests {
assert!(*active.path == *"src/main.rs");
});
}
#[gpui::test]
async fn test_open_link_percent_escape_disambiguation(cx: &mut gpui::TestAppContext) {
crate::test_support::init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(
path!("/project"),
json!({
"a%20b.rs": "literal",
"a b.rs": "decoded",
"c d.rs": "",
"e%20f.rs": "",
}),
)
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
let (multi_workspace, cx) =
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
let workspace_weak = workspace.downgrade();
let open_link_and_active_path = |url: String, cx: &mut gpui::VisualTestContext| {
multi_workspace.update_in(cx, |_, window, cx| {
open_link(url.into(), &workspace_weak, window, cx);
});
cx.run_until_parked();
workspace.read_with(cx, |workspace, cx| {
workspace
.active_item(cx)
.and_then(|item| item.project_path(cx))
.expect("file should be open")
.path
})
};
// Both interpretations exist: the decoded one wins.
let path = open_link_and_active_path(path!("/project/a%20b.rs").to_string(), cx);
assert_eq!(*path, *"a b.rs");
// Only the decoded file exists.
let path = open_link_and_active_path(path!("/project/c%20d.rs").to_string(), cx);
assert_eq!(*path, *"c d.rs");
// Only the literally-named file exists: fall back to it.
let path = open_link_and_active_path(path!("/project/e%20f.rs").to_string(), cx);
assert_eq!(*path, *"e%20f.rs");
}
#[gpui::test]
async fn test_open_link_out_of_project_path(cx: &mut gpui::TestAppContext) {
crate::test_support::init_test(cx);
let fs = FakeFs::new(cx.executor());
fs.insert_tree(path!("/project"), json!({"src": {"main.rs": ""}}))
.await;
fs.insert_tree(path!("/outside"), json!({"notes.md": "one\ntwo\nthree\n"}))
.await;
let project = Project::test(fs, [path!("/project").as_ref()], cx).await;
let (multi_workspace, cx) =
cx.add_window_view(|window, cx| MultiWorkspace::test_new(project.clone(), window, cx));
let workspace = multi_workspace.read_with(cx, |mw, _| mw.workspace().clone());
let workspace_weak = workspace.downgrade();
// A nonexistent out-of-project path opens nothing, not even an
// empty buffer.
multi_workspace.update_in(cx, |_, window, cx| {
open_link(
path!("/outside/missing.md").to_string().into(),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
workspace.read_with(cx, |workspace, cx| {
assert!(
workspace.active_item(cx).is_none(),
"nothing should open for a nonexistent path"
);
});
// An existing out-of-project file opens at the linked line.
multi_workspace.update_in(cx, |_, window, cx| {
open_link(
format!("{}:2", path!("/outside/notes.md")).into(),
&workspace_weak,
window,
cx,
);
});
cx.run_until_parked();
let editor = workspace.read_with(cx, |workspace, cx| {
let item = workspace.active_item(cx).expect("file should be open");
let project_path = item.project_path(cx).expect("item should have a path");
let abs_path = workspace
.project()
.read(cx)
.absolute_path(&project_path, cx);
assert_eq!(
abs_path.as_deref(),
Some(Path::new(path!("/outside/notes.md")))
);
item.downcast::<Editor>().expect("should be an editor")
});
editor.update_in(cx, |editor, window, cx| {
let snapshot = editor.snapshot(window, cx);
assert_eq!(editor.selections.newest::<Point>(&snapshot).head().row, 1);
});
}
}
const FAST_MODE_WARNING_NAMESPACE: &str = "fast-mode-warning-dismissed";
+3 -38
View File
@@ -160,14 +160,14 @@ fn open_mention_uri(
workspace.update(cx, |workspace, cx| match mention_uri {
MentionUri::File { abs_path } => {
open_file(workspace, abs_path, None, window, cx);
open_abs_path_at_point(workspace, abs_path, None, window, cx);
}
MentionUri::Symbol {
abs_path,
line_range,
..
} => {
open_file(
open_abs_path_at_point(
workspace,
abs_path,
Some(Point::new(*line_range.start(), 0)),
@@ -180,7 +180,7 @@ fn open_mention_uri(
line_range,
column,
} => {
open_file(
open_abs_path_at_point(
workspace,
abs_path,
Some(Point::new(*line_range.start(), column.unwrap_or(0))),
@@ -339,41 +339,6 @@ fn open_skill_content_buffer(
workspace.add_item(pane, Box::new(editor), None, true, true, window, cx);
}
fn open_file(
workspace: &mut Workspace,
abs_path: PathBuf,
point: Option<Point>,
window: &mut Window,
cx: &mut Context<Workspace>,
) {
if let Some(point) = point {
if open_abs_path_at_point(workspace, abs_path.clone(), point, window, cx) {
return;
}
}
let project = workspace.project();
if let Some(project_path) =
project.update(cx, |project, cx| project.find_project_path(&abs_path, cx))
{
workspace
.open_path(project_path, None, true, window, cx)
.detach_and_log_err(cx);
} else if abs_path.exists() {
workspace
.open_abs_path(
abs_path,
OpenOptions {
focus: Some(true),
..Default::default()
},
window,
cx,
)
.detach_and_log_err(cx);
}
}
fn reveal_in_project_panel(
workspace: &mut Workspace,
abs_path: PathBuf,