oak-team/oak-gpui
2
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

acp: Fix @mentions when remoting from Windows to Linux (#38882)

Browse Source 
Closes #38620

`Url::from_file_path` and `Url::from_directory_path` assume the path
style of the target they were compiled for, so we can't use them in
general. So, switch from `file://` to encoding the absolute path (for
mentions that have one) as a query parameter, which works no matter the
platforms. We'll still parse the old `file://` mention URIs for
compatibility with thread history.

Release Notes:

- windows: Fixed a crash when using `@mentions` in agent threads when
remoting from Windows to Linux or WSL.
This commit is contained in:
Cole Miller
2025-09-25 16:23:45 -04:00
committed by GitHub
parent 236006b6b3
commit ecc35fcd9a
3 changed files with 123 additions and 65 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/acp_thread/src/mention.rs
+76 -33
View File
@@ -126,6 +126,39 @@ impl MentionUri {
abs_path: None,
line_range,
})
} else if let Some(name) = path.strip_prefix("/agent/symbol/") {
let fragment = url
.fragment()
.context("Missing fragment for untitled buffer selection")?;
let line_range = parse_line_range(fragment)?;
let path =
single_query_param(&url, "path")?.context("Missing path for symbol")?;
Ok(Self::Symbol {
name: name.to_string(),
abs_path: path.into(),
line_range,
})
} else if path.starts_with("/agent/file") {
let path =
single_query_param(&url, "path")?.context("Missing path for file")?;
Ok(Self::File {
abs_path: path.into(),
})
} else if path.starts_with("/agent/directory") {
let path =
single_query_param(&url, "path")?.context("Missing path for directory")?;
Ok(Self::Directory {
abs_path: path.into(),
})
} else if path.starts_with("/agent/selection") {
let fragment = url.fragment().context("Missing fragment for selection")?;
let line_range = parse_line_range(fragment)?;
let path =
single_query_param(&url, "path")?.context("Missing path for selection")?;
Ok(Self::Selection {
abs_path: Some(path.into()),
line_range,
})
} else {
bail!("invalid zed url: {:?}", input);
}
@@ -180,20 +213,29 @@ impl MentionUri {
pub fn to_uri(&self) -> Url {
match self {
MentionUri::File { abs_path } => {
Url::from_file_path(abs_path).expect("mention path should be absolute")
let mut url = Url::parse("zed:///").unwrap();
url.set_path("/agent/file");
url.query_pairs_mut()
.append_pair("path", &abs_path.to_string_lossy());
url
}
MentionUri::PastedImage => Url::parse("zed:///agent/pasted-image").unwrap(),
MentionUri::Directory { abs_path } => {
Url::from_directory_path(abs_path).expect("mention path should be absolute")
let mut url = Url::parse("zed:///").unwrap();
url.set_path("/agent/directory");
url.query_pairs_mut()
.append_pair("path", &abs_path.to_string_lossy());
url
}
MentionUri::Symbol {
abs_path,
name,
line_range,
} => {
let mut url =
Url::from_file_path(abs_path).expect("mention path should be absolute");
url.query_pairs_mut().append_pair("symbol", name);
let mut url = Url::parse("zed:///").unwrap();
url.set_path(&format!("/agent/symbol/{name}"));
url.query_pairs_mut()
.append_pair("path", &abs_path.to_string_lossy());
url.set_fragment(Some(&format!(
"L{}:{}",
line_range.start() + 1,
@@ -202,15 +244,16 @@ impl MentionUri {
url
}
MentionUri::Selection {
abs_path: path,
abs_path,
line_range,
} => {
let mut url = if let Some(path) = path {
Url::from_file_path(path).expect("mention path should be absolute")
let mut url = Url::parse("zed:///").unwrap();
if let Some(abs_path) = abs_path {
url.set_path("/agent/selection");
url.query_pairs_mut()
.append_pair("path", &abs_path.to_string_lossy());
} else {
let mut url = Url::parse("zed:///").unwrap();
url.set_path("/agent/untitled-buffer");
url
};
url.set_fragment(Some(&format!(
"L{}:{}",
@@ -295,37 +338,32 @@ mod tests {
#[test]
fn test_parse_file_uri() {
let file_uri = uri!("file:///path/to/file.rs");
let parsed = MentionUri::parse(file_uri).unwrap();
let old_uri = uri!("file:///path/to/file.rs");
let parsed = MentionUri::parse(old_uri).unwrap();
match &parsed {
MentionUri::File { abs_path } => {
assert_eq!(abs_path.to_str().unwrap(), path!("/path/to/file.rs"));
}
_ => panic!("Expected File variant"),
}
assert_eq!(parsed.to_uri().to_string(), file_uri);
let new_uri = parsed.to_uri().to_string();
assert!(new_uri.starts_with("zed:///agent/file"));
assert_eq!(MentionUri::parse(&new_uri).unwrap(), parsed);
}
#[test]
fn test_parse_directory_uri() {
let file_uri = uri!("file:///path/to/dir/");
let parsed = MentionUri::parse(file_uri).unwrap();
let old_uri = uri!("file:///path/to/dir/");
let parsed = MentionUri::parse(old_uri).unwrap();
match &parsed {
MentionUri::Directory { abs_path } => {
assert_eq!(abs_path.to_str().unwrap(), path!("/path/to/dir/"));
}
_ => panic!("Expected Directory variant"),
}
assert_eq!(parsed.to_uri().to_string(), file_uri);
}
#[test]
fn test_to_directory_uri_with_slash() {
let uri = MentionUri::Directory {
abs_path: PathBuf::from(path!("/path/to/dir/")),
};
let expected = uri!("file:///path/to/dir/");
assert_eq!(uri.to_uri().to_string(), expected);
let new_uri = parsed.to_uri().to_string();
assert!(new_uri.starts_with("zed:///agent/directory"));
assert_eq!(MentionUri::parse(&new_uri).unwrap(), parsed);
}
#[test]
@@ -333,14 +371,15 @@ mod tests {
let uri = MentionUri::Directory {
abs_path: PathBuf::from(path!("/path/to/dir")),
};
let expected = uri!("file:///path/to/dir/");
assert_eq!(uri.to_uri().to_string(), expected);
let uri_string = uri.to_uri().to_string();
assert!(uri_string.starts_with("zed:///agent/directory"));
assert_eq!(MentionUri::parse(&uri_string).unwrap(), uri);
}
#[test]
fn test_parse_symbol_uri() {
let symbol_uri = uri!("file:///path/to/file.rs?symbol=MySymbol#L10:20");
let parsed = MentionUri::parse(symbol_uri).unwrap();
let old_uri = uri!("file:///path/to/file.rs?symbol=MySymbol#L10:20");
let parsed = MentionUri::parse(old_uri).unwrap();
match &parsed {
MentionUri::Symbol {
abs_path: path,
@@ -354,13 +393,15 @@ mod tests {
}
_ => panic!("Expected Symbol variant"),
}
assert_eq!(parsed.to_uri().to_string(), symbol_uri);
let new_uri = parsed.to_uri().to_string();
assert!(new_uri.starts_with("zed:///agent/symbol/MySymbol"));
assert_eq!(MentionUri::parse(&new_uri).unwrap(), parsed);
}
#[test]
fn test_parse_selection_uri() {
let selection_uri = uri!("file:///path/to/file.rs#L5:15");
let parsed = MentionUri::parse(selection_uri).unwrap();
let old_uri = uri!("file:///path/to/file.rs#L5:15");
let parsed = MentionUri::parse(old_uri).unwrap();
match &parsed {
MentionUri::Selection {
abs_path: path,
@@ -375,7 +416,9 @@ mod tests {
}
_ => panic!("Expected Selection variant"),
}
assert_eq!(parsed.to_uri().to_string(), selection_uri);
let new_uri = parsed.to_uri().to_string();
assert!(new_uri.starts_with("zed:///agent/selection"));
assert_eq!(MentionUri::parse(&new_uri).unwrap(), parsed);
}
#[test]
crates/agent2/src/agent.rs
+9 -5
View File
@@ -1205,7 +1205,7 @@ mod tests {
use acp_thread::{AgentConnection, AgentModelGroupName, AgentModelInfo, MentionUri};
use fs::FakeFs;
use gpui::TestAppContext;
use indoc::indoc;
use indoc::formatdoc;
use language_model::fake_provider::FakeLanguageModel;
use serde_json::json;
use settings::SettingsStore;
@@ -1502,13 +1502,17 @@ mod tests {
summary_model.end_last_completion_stream();
send.await.unwrap();
let uri = MentionUri::File {
abs_path: path!("/a/b.md").into(),
}
.to_uri();
acp_thread.read_with(cx, |thread, cx| {
assert_eq!(
thread.to_markdown(cx),
indoc! {"
formatdoc! {"
## User
What does [@b.md](file:///a/b.md) mean?
What does [@b.md]({uri}) mean?
## Assistant
@@ -1544,10 +1548,10 @@ mod tests {
acp_thread.read_with(cx, |thread, cx| {
assert_eq!(
thread.to_markdown(cx),
indoc! {"
formatdoc! {"
## User
What does [@b.md](file:///a/b.md) mean?
What does [@b.md]({uri}) mean?
## Assistant
crates/agent_ui/src/acp/message_editor.rs
+38 -27
View File
@@ -48,7 +48,7 @@ use std::{
use text::OffsetRangeExt;
use theme::ThemeSettings;
use ui::{ButtonLike, TintColor, Toggleable, prelude::*};
use util::{ResultExt, debug_panic, paths::PathStyle, rel_path::RelPath};
use util::{ResultExt, debug_panic, rel_path::RelPath};
use workspace::{Workspace, notifications::NotifyResultExt as _};
use zed_actions::agent::Chat;
@@ -108,11 +108,6 @@ impl MessageEditor {
available_commands.clone(),
));
let mention_set = MentionSet::default();
// TODO: fix mentions when remoting with mixed path styles.
let host_and_guest_paths_differ = project
.read(cx)
.remote_client()
.is_some_and(|client| client.read(cx).path_style() != PathStyle::local());
let editor = cx.new(|cx| {
let buffer = cx.new(|cx| Buffer::local("", cx).with_language(Arc::new(language), cx));
let buffer = cx.new(|cx| MultiBuffer::singleton(buffer, cx));
@@ -122,9 +117,7 @@ impl MessageEditor {
editor.set_show_indent_guides(false, cx);
editor.set_soft_wrap();
editor.set_use_modal_editing(true);
if !host_and_guest_paths_differ {
editor.set_completion_provider(Some(completion_provider.clone()));
}
editor.set_completion_provider(Some(completion_provider.clone()));
editor.set_context_menu_options(ContextMenuOptions {
min_entries_visible: 12,
max_entries_visible: 12,
@@ -1600,7 +1593,7 @@ mod tests {
use serde_json::json;
use text::Point;
use ui::{App, Context, IntoElement, Render, SharedString, Window};
use util::{path, paths::PathStyle, rel_path::rel_path, uri};
use util::{path, paths::PathStyle, rel_path::rel_path};
use workspace::{AppState, Item, Workspace};
use crate::acp::{
@@ -2266,7 +2259,11 @@ mod tests {
editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx);
});
let url_one = uri!("file:///dir/a/one.txt");
let url_one = MentionUri::File {
abs_path: path!("/dir/a/one.txt").into(),
}
.to_uri()
.to_string();
editor.update(&mut cx, |editor, cx| {
let text = editor.text(cx);
assert_eq!(text, format!("Lorem [@one.txt]({url_one}) "));
@@ -2371,7 +2368,11 @@ mod tests {
.into_values()
.collect::<Vec<_>>();
let url_eight = uri!("file:///dir/b/eight.txt");
let url_eight = MentionUri::File {
abs_path: path!("/dir/b/eight.txt").into(),
}
.to_uri()
.to_string();
{
let [_, (uri, Mention::Text { content, .. })] = contents.as_slice() else {
@@ -2470,6 +2471,12 @@ mod tests {
editor.confirm_completion(&editor::actions::ConfirmCompletion::default(), window, cx);
});
let symbol = MentionUri::Symbol {
abs_path: path!("/dir/a/one.txt").into(),
name: "MySymbol".into(),
line_range: 0..=0,
};
let contents = message_editor
.update(&mut cx, |message_editor, cx| {
message_editor.mention_set().contents(
@@ -2489,12 +2496,7 @@ mod tests {
panic!("Unexpected mentions");
};
pretty_assertions::assert_eq!(content, "1");
pretty_assertions::assert_eq!(
uri,
&format!("{url_one}?symbol=MySymbol#L1:1")
.parse::<MentionUri>()
.unwrap()
);
pretty_assertions::assert_eq!(uri, &symbol);
}
cx.run_until_parked();
@@ -2502,7 +2504,10 @@ mod tests {
editor.read_with(&cx, |editor, cx| {
assert_eq!(
editor.text(cx),
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) ")
format!(
"Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) ",
symbol.to_uri(),
)
);
});
@@ -2512,7 +2517,7 @@ mod tests {
editor.update(&mut cx, |editor, cx| {
assert_eq!(
editor.text(cx),
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) @file x.png")
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) @file x.png", symbol.to_uri())
);
assert!(editor.has_visible_completions_menu());
assert_eq!(current_completion_labels(editor), &[format!("x.png dir{slash}")]);
@@ -2541,7 +2546,10 @@ mod tests {
editor.read_with(&cx, |editor, cx| {
assert_eq!(
editor.text(cx),
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) ")
format!(
"Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) ",
symbol.to_uri()
)
);
});
@@ -2551,7 +2559,7 @@ mod tests {
editor.update(&mut cx, |editor, cx| {
assert_eq!(
editor.text(cx),
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) @file x.png")
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) @file x.png", symbol.to_uri())
);
assert!(editor.has_visible_completions_menu());
assert_eq!(current_completion_labels(editor), &[format!("x.png dir{slash}")]);
@@ -2566,11 +2574,14 @@ mod tests {
// Mention was removed
editor.read_with(&cx, |editor, cx| {
assert_eq!(
editor.text(cx),
format!("Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({url_one}?symbol=MySymbol#L1:1) ")
);
});
assert_eq!(
editor.text(cx),
format!(
"Lorem [@one.txt]({url_one}) Ipsum [@eight.txt]({url_eight}) [@MySymbol]({}) ",
symbol.to_uri()
)
);
});
// Now getting the contents succeeds, because the invalid mention was removed
let contents = message_editor