Fully support all mention kinds (#36134)

Feature parity with the agent1 @mention kinds:
- File
- Symbols
- Selections
- Threads
- Rules
- Fetch


Release Notes:

- N/A

---------

Co-authored-by: Cole Miller <cole@zed.dev>

crates/acp_thread/Cargo.toml

@@ -18,6 +18,7 @@ test-support = ["gpui/test-support", "project/test-support"]
 [dependencies]
 action_log.workspace = true
 agent-client-protocol.workspace = true
+agent.workspace = true
 anyhow.workspace = true
 buffer_diff.workspace = true
 collections.workspace = true
@@ -28,6 +29,7 @@ itertools.workspace = true
 language.workspace = true
 markdown.workspace = true
 project.workspace = true
+prompt_store.workspace = true
 serde.workspace = true
 serde_json.workspace = true
 settings.workspace = true

crates/acp_thread/src/acp_thread.rs

@@ -399,7 +399,7 @@ impl ContentBlock {
             }
         }
 
-        let new_content = self.extract_content_from_block(block);
+        let new_content = self.block_string_contents(block);
 
         match self {
             ContentBlock::Empty => {
@@ -409,7 +409,7 @@ impl ContentBlock {
                 markdown.update(cx, |markdown, cx| markdown.append(&new_content, cx));
             }
             ContentBlock::ResourceLink { resource_link } => {
-                let existing_content = Self::resource_link_to_content(&resource_link.uri);
+                let existing_content = Self::resource_link_md(&resource_link.uri);
                 let combined = format!("{}\n{}", existing_content, new_content);
 
                 *self = Self::create_markdown_block(combined, language_registry, cx);
@@ -417,14 +417,6 @@ impl ContentBlock {
         }
     }
 
-    fn resource_link_to_content(uri: &str) -> String {
-        if let Some(uri) = MentionUri::parse(&uri).log_err() {
-            uri.to_link()
-        } else {
-            uri.to_string().clone()
-        }
-    }
-
     fn create_markdown_block(
         content: String,
         language_registry: &Arc<LanguageRegistry>,
@@ -436,11 +428,11 @@ impl ContentBlock {
         }
     }
 
-    fn extract_content_from_block(&self, block: acp::ContentBlock) -> String {
+    fn block_string_contents(&self, block: acp::ContentBlock) -> String {
         match block {
             acp::ContentBlock::Text(text_content) => text_content.text.clone(),
             acp::ContentBlock::ResourceLink(resource_link) => {
-                Self::resource_link_to_content(&resource_link.uri)
+                Self::resource_link_md(&resource_link.uri)
             }
             acp::ContentBlock::Resource(acp::EmbeddedResource {
                 resource:
@@ -449,13 +441,21 @@ impl ContentBlock {
                         ..
                     }),
                 ..
-            }) => Self::resource_link_to_content(&uri),
+            }) => Self::resource_link_md(&uri),
             acp::ContentBlock::Image(_)
             | acp::ContentBlock::Audio(_)
             | acp::ContentBlock::Resource(_) => String::new(),
         }
     }
 
+    fn resource_link_md(uri: &str) -> String {
+        if let Some(uri) = MentionUri::parse(&uri).log_err() {
+            uri.as_link().to_string()
+        } else {
+            uri.to_string()
+        }
+    }
+
     fn to_markdown<'a>(&'a self, cx: &'a App) -> &'a str {
         match self {
             ContentBlock::Empty => "",

crates/acp_thread/src/mention.rs

@@ -1,13 +1,40 @@
-use agent_client_protocol as acp;
-use anyhow::{Result, bail};
-use std::path::PathBuf;
+use agent::ThreadId;
+use anyhow::{Context as _, Result, bail};
+use prompt_store::{PromptId, UserPromptId};
+use std::{
+    fmt,
+    ops::Range,
+    path::{Path, PathBuf},
+};
+use url::Url;
 
 #[derive(Clone, Debug, PartialEq, Eq)]
 pub enum MentionUri {
     File(PathBuf),
-    Symbol(PathBuf, String),
-    Thread(acp::SessionId),
-    Rule(String),
+    Symbol {
+        path: PathBuf,
+        name: String,
+        line_range: Range<u32>,
+    },
+    Thread {
+        id: ThreadId,
+        name: String,
+    },
+    TextThread {
+        path: PathBuf,
+        name: String,
+    },
+    Rule {
+        id: PromptId,
+        name: String,
+    },
+    Selection {
+        path: PathBuf,
+        line_range: Range<u32>,
+    },
+    Fetch {
+        url: Url,
+    },
 }
 
 impl MentionUri {
@@ -17,7 +44,34 @@ impl MentionUri {
         match url.scheme() {
             "file" => {
                 if let Some(fragment) = url.fragment() {
-                    Ok(Self::Symbol(path.into(), fragment.into()))
+                    let range = fragment
+                        .strip_prefix("L")
+                        .context("Line range must start with \"L\"")?;
+                    let (start, end) = range
+                        .split_once(":")
+                        .context("Line range must use colon as separator")?;
+                    let line_range = start
+                        .parse::<u32>()
+                        .context("Parsing line range start")?
+                        .checked_sub(1)
+                        .context("Line numbers should be 1-based")?
+                        ..end
+                            .parse::<u32>()
+                            .context("Parsing line range end")?
+                            .checked_sub(1)
+                            .context("Line numbers should be 1-based")?;
+                    if let Some(name) = single_query_param(&url, "symbol")? {
+                        Ok(Self::Symbol {
+                            name,
+                            path: path.into(),
+                            line_range,
+                        })
+                    } else {
+                        Ok(Self::Selection {
+                            path: path.into(),
+                            line_range,
+                        })
+                    }
                 } else {
                     let file_path =
                         PathBuf::from(format!("{}{}", url.host_str().unwrap_or(""), path));
@@ -26,100 +80,292 @@ impl MentionUri {
                 }
             }
             "zed" => {
-                if let Some(thread) = path.strip_prefix("/agent/thread/") {
-                    Ok(Self::Thread(acp::SessionId(thread.into())))
-                } else if let Some(rule) = path.strip_prefix("/agent/rule/") {
-                    Ok(Self::Rule(rule.into()))
+                if let Some(thread_id) = path.strip_prefix("/agent/thread/") {
+                    let name = single_query_param(&url, "name")?.context("Missing thread name")?;
+                    Ok(Self::Thread {
+                        id: thread_id.into(),
+                        name,
+                    })
+                } else if let Some(path) = path.strip_prefix("/agent/text-thread/") {
+                    let name = single_query_param(&url, "name")?.context("Missing thread name")?;
+                    Ok(Self::TextThread {
+                        path: path.into(),
+                        name,
+                    })
+                } else if let Some(rule_id) = path.strip_prefix("/agent/rule/") {
+                    let name = single_query_param(&url, "name")?.context("Missing rule name")?;
+                    let rule_id = UserPromptId(rule_id.parse()?);
+                    Ok(Self::Rule {
+                        id: rule_id.into(),
+                        name,
+                    })
                 } else {
                     bail!("invalid zed url: {:?}", input);
                 }
             }
+            "http" | "https" => Ok(MentionUri::Fetch { url }),
             other => bail!("unrecognized scheme {:?}", other),
         }
     }
 
-    pub fn name(&self) -> String {
+    fn name(&self) -> String {
         match self {
-            MentionUri::File(path) => path.file_name().unwrap().to_string_lossy().into_owned(),
-            MentionUri::Symbol(_path, name) => name.clone(),
-            MentionUri::Thread(thread) => thread.to_string(),
-            MentionUri::Rule(rule) => rule.clone(),
+            MentionUri::File(path) => path
+                .file_name()
+                .unwrap_or_default()
+                .to_string_lossy()
+                .into_owned(),
+            MentionUri::Symbol { name, .. } => name.clone(),
+            MentionUri::Thread { name, .. } => name.clone(),
+            MentionUri::TextThread { name, .. } => name.clone(),
+            MentionUri::Rule { name, .. } => name.clone(),
+            MentionUri::Selection {
+                path, line_range, ..
+            } => selection_name(path, line_range),
+            MentionUri::Fetch { url } => url.to_string(),
         }
     }
 
-    pub fn to_link(&self) -> String {
-        let name = self.name();
-        let uri = self.to_uri();
-        format!("[{name}]({uri})")
+    pub fn as_link<'a>(&'a self) -> MentionLink<'a> {
+        MentionLink(self)
     }
 
-    pub fn to_uri(&self) -> String {
+    pub fn to_uri(&self) -> Url {
         match self {
             MentionUri::File(path) => {
-                format!("file://{}", path.display())
+                let mut url = Url::parse("file:///").unwrap();
+                url.set_path(&path.to_string_lossy());
+                url
             }
-            MentionUri::Symbol(path, name) => {
-                format!("file://{}#{}", path.display(), name)
+            MentionUri::Symbol {
+                path,
+                name,
+                line_range,
+            } => {
+                let mut url = Url::parse("file:///").unwrap();
+                url.set_path(&path.to_string_lossy());
+                url.query_pairs_mut().append_pair("symbol", name);
+                url.set_fragment(Some(&format!(
+                    "L{}:{}",
+                    line_range.start + 1,
+                    line_range.end + 1
+                )));
+                url
             }
-            MentionUri::Thread(thread) => {
-                format!("zed:///agent/thread/{}", thread.0)
+            MentionUri::Selection { path, line_range } => {
+                let mut url = Url::parse("file:///").unwrap();
+                url.set_path(&path.to_string_lossy());
+                url.set_fragment(Some(&format!(
+                    "L{}:{}",
+                    line_range.start + 1,
+                    line_range.end + 1
+                )));
+                url
             }
-            MentionUri::Rule(rule) => {
-                format!("zed:///agent/rule/{}", rule)
+            MentionUri::Thread { name, id } => {
+                let mut url = Url::parse("zed:///").unwrap();
+                url.set_path(&format!("/agent/thread/{id}"));
+                url.query_pairs_mut().append_pair("name", name);
+                url
             }
+            MentionUri::TextThread { path, name } => {
+                let mut url = Url::parse("zed:///").unwrap();
+                url.set_path(&format!("/agent/text-thread/{}", path.to_string_lossy()));
+                url.query_pairs_mut().append_pair("name", name);
+                url
+            }
+            MentionUri::Rule { name, id } => {
+                let mut url = Url::parse("zed:///").unwrap();
+                url.set_path(&format!("/agent/rule/{id}"));
+                url.query_pairs_mut().append_pair("name", name);
+                url
+            }
+            MentionUri::Fetch { url } => url.clone(),
         }
     }
 }
 
+pub struct MentionLink<'a>(&'a MentionUri);
+
+impl fmt::Display for MentionLink<'_> {
+    fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+        write!(f, "[@{}]({})", self.0.name(), self.0.to_uri())
+    }
+}
+
+fn single_query_param(url: &Url, name: &'static str) -> Result<Option<String>> {
+    let pairs = url.query_pairs().collect::<Vec<_>>();
+    match pairs.as_slice() {
+        [] => Ok(None),
+        [(k, v)] => {
+            if k != name {
+                bail!("invalid query parameter")
+            }
+
+            Ok(Some(v.to_string()))
+        }
+        _ => bail!("too many query pairs"),
+    }
+}
+
+pub fn selection_name(path: &Path, line_range: &Range<u32>) -> String {
+    format!(
+        "{} ({}:{})",
+        path.file_name().unwrap_or_default().display(),
+        line_range.start + 1,
+        line_range.end + 1
+    )
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;
 
     #[test]
-    fn test_mention_uri_parse_and_display() {
-        // Test file URI
+    fn test_parse_file_uri() {
         let file_uri = "file:///path/to/file.rs";
         let parsed = MentionUri::parse(file_uri).unwrap();
         match &parsed {
             MentionUri::File(path) => assert_eq!(path.to_str().unwrap(), "/path/to/file.rs"),
             _ => panic!("Expected File variant"),
         }
-        assert_eq!(parsed.to_uri(), file_uri);
+        assert_eq!(parsed.to_uri().to_string(), file_uri);
+    }
 
-        // Test symbol URI
-        let symbol_uri = "file:///path/to/file.rs#MySymbol";
+    #[test]
+    fn test_parse_symbol_uri() {
+        let symbol_uri = "file:///path/to/file.rs?symbol=MySymbol#L10:20";
         let parsed = MentionUri::parse(symbol_uri).unwrap();
         match &parsed {
-            MentionUri::Symbol(path, symbol) => {
+            MentionUri::Symbol {
+                path,
+                name,
+                line_range,
+            } => {
                 assert_eq!(path.to_str().unwrap(), "/path/to/file.rs");
-                assert_eq!(symbol, "MySymbol");
+                assert_eq!(name, "MySymbol");
+                assert_eq!(line_range.start, 9);
+                assert_eq!(line_range.end, 19);
             }
             _ => panic!("Expected Symbol variant"),
         }
-        assert_eq!(parsed.to_uri(), symbol_uri);
+        assert_eq!(parsed.to_uri().to_string(), symbol_uri);
+    }
 
-        // Test thread URI
-        let thread_uri = "zed:///agent/thread/session123";
+    #[test]
+    fn test_parse_selection_uri() {
+        let selection_uri = "file:///path/to/file.rs#L5:15";
+        let parsed = MentionUri::parse(selection_uri).unwrap();
+        match &parsed {
+            MentionUri::Selection { path, line_range } => {
+                assert_eq!(path.to_str().unwrap(), "/path/to/file.rs");
+                assert_eq!(line_range.start, 4);
+                assert_eq!(line_range.end, 14);
+            }
+            _ => panic!("Expected Selection variant"),
+        }
+        assert_eq!(parsed.to_uri().to_string(), selection_uri);
+    }
+
+    #[test]
+    fn test_parse_thread_uri() {
+        let thread_uri = "zed:///agent/thread/session123?name=Thread+name";
         let parsed = MentionUri::parse(thread_uri).unwrap();
         match &parsed {
-            MentionUri::Thread(session_id) => assert_eq!(session_id.0.as_ref(), "session123"),
+            MentionUri::Thread {
+                id: thread_id,
+                name,
+            } => {
+                assert_eq!(thread_id.to_string(), "session123");
+                assert_eq!(name, "Thread name");
+            }
             _ => panic!("Expected Thread variant"),
         }
-        assert_eq!(parsed.to_uri(), thread_uri);
+        assert_eq!(parsed.to_uri().to_string(), thread_uri);
+    }
 
-        // Test rule URI
-        let rule_uri = "zed:///agent/rule/my_rule";
+    #[test]
+    fn test_parse_rule_uri() {
+        let rule_uri = "zed:///agent/rule/d8694ff2-90d5-4b6f-be33-33c1763acd52?name=Some+rule";
         let parsed = MentionUri::parse(rule_uri).unwrap();
         match &parsed {
-            MentionUri::Rule(rule) => assert_eq!(rule, "my_rule"),
+            MentionUri::Rule { id, name } => {
+                assert_eq!(id.to_string(), "d8694ff2-90d5-4b6f-be33-33c1763acd52");
+                assert_eq!(name, "Some rule");
+            }
             _ => panic!("Expected Rule variant"),
         }
-        assert_eq!(parsed.to_uri(), rule_uri);
+        assert_eq!(parsed.to_uri().to_string(), rule_uri);
+    }
 
-        // Test invalid scheme
-        assert!(MentionUri::parse("http://example.com").is_err());
+    #[test]
+    fn test_parse_fetch_http_uri() {
+        let http_uri = "http://example.com/path?query=value#fragment";
+        let parsed = MentionUri::parse(http_uri).unwrap();
+        match &parsed {
+            MentionUri::Fetch { url } => {
+                assert_eq!(url.to_string(), http_uri);
+            }
+            _ => panic!("Expected Fetch variant"),
+        }
+        assert_eq!(parsed.to_uri().to_string(), http_uri);
+    }
 
-        // Test invalid zed path
+    #[test]
+    fn test_parse_fetch_https_uri() {
+        let https_uri = "https://example.com/api/endpoint";
+        let parsed = MentionUri::parse(https_uri).unwrap();
+        match &parsed {
+            MentionUri::Fetch { url } => {
+                assert_eq!(url.to_string(), https_uri);
+            }
+            _ => panic!("Expected Fetch variant"),
+        }
+        assert_eq!(parsed.to_uri().to_string(), https_uri);
+    }
+
+    #[test]
+    fn test_invalid_scheme() {
+        assert!(MentionUri::parse("ftp://example.com").is_err());
+        assert!(MentionUri::parse("ssh://example.com").is_err());
+        assert!(MentionUri::parse("unknown://example.com").is_err());
+    }
+
+    #[test]
+    fn test_invalid_zed_path() {
         assert!(MentionUri::parse("zed:///invalid/path").is_err());
+        assert!(MentionUri::parse("zed:///agent/unknown/test").is_err());
+    }
+
+    #[test]
+    fn test_invalid_line_range_format() {
+        // Missing L prefix
+        assert!(MentionUri::parse("file:///path/to/file.rs#10:20").is_err());
+
+        // Missing colon separator
+        assert!(MentionUri::parse("file:///path/to/file.rs#L1020").is_err());
+
+        // Invalid numbers
+        assert!(MentionUri::parse("file:///path/to/file.rs#L10:abc").is_err());
+        assert!(MentionUri::parse("file:///path/to/file.rs#Labc:20").is_err());
+    }
+
+    #[test]
+    fn test_invalid_query_parameters() {
+        // Invalid query parameter name
+        assert!(MentionUri::parse("file:///path/to/file.rs#L10:20?invalid=test").is_err());
+
+        // Too many query parameters
+        assert!(
+            MentionUri::parse("file:///path/to/file.rs#L10:20?symbol=test&another=param").is_err()
+        );
+    }
+
+    #[test]
+    fn test_zero_based_line_numbers() {
+        // Test that 0-based line numbers are rejected (should be 1-based)
+        assert!(MentionUri::parse("file:///path/to/file.rs#L0:10").is_err());
+        assert!(MentionUri::parse("file:///path/to/file.rs#L1:0").is_err());
+        assert!(MentionUri::parse("file:///path/to/file.rs#L0:0").is_err());
     }
 }