diff --git a/crates/acp_thread/src/acp_thread.rs b/crates/acp_thread/src/acp_thread.rs index 0decfc55888..afd8aeda5f3 100644 --- a/crates/acp_thread/src/acp_thread.rs +++ b/crates/acp_thread/src/acp_thread.rs @@ -93,35 +93,6 @@ pub fn subagent_session_info_from_meta(meta: &Option) -> Option) -> Option<&str> { - meta.as_ref() - .and_then(|m| m.get(SKILL_SOURCE_META_KEY)) - .and_then(|v| v.as_str()) -} - -/// Helper to extract skill source label from ACP meta as an owned -/// `SharedString`. Use this when the value needs to outlive the meta -/// reference; otherwise prefer [`skill_source_str_from_meta`]. -pub fn skill_source_from_meta(meta: &Option) -> Option { - skill_source_str_from_meta(meta).map(|s| SharedString::from(s.to_owned())) -} - -/// Helper to create meta tagging an `AvailableCommand` with a skill source. -pub fn meta_with_skill_source(source: &str) -> acp::Meta { - acp::Meta::from_iter([(SKILL_SOURCE_META_KEY.into(), source.into())]) -} - #[derive(Debug)] pub struct UserMessage { pub id: Option, diff --git a/crates/acp_thread/src/mention.rs b/crates/acp_thread/src/mention.rs index 4d52c202c63..6fc2cc50c1f 100644 --- a/crates/acp_thread/src/mention.rs +++ b/crates/acp_thread/src/mention.rs @@ -64,6 +64,11 @@ pub enum MentionUri { MergeConflict { file_path: String, }, + Skill { + name: String, + source: String, + skill_file_path: PathBuf, + }, } impl MentionUri { @@ -261,6 +266,40 @@ impl MentionUri { } else if path.starts_with("/agent/merge-conflict") { let file_path = single_query_param(&url, "path")?.unwrap_or_default(); Ok(Self::MergeConflict { file_path }) + } else if path.starts_with("/agent/skill") { + let mut name = None; + let mut source = None; + let mut skill_file_path = None; + + for (key, value) in url.query_pairs() { + match key.as_ref() { + "name" => { + if name.replace(value.to_string()).is_some() { + bail!("duplicate skill name query parameter"); + } + } + "source" => { + if source.replace(value.to_string()).is_some() { + bail!("duplicate skill source query parameter"); + } + } + "path" => { + if skill_file_path + .replace(PathBuf::from(value.to_string())) + .is_some() + { + bail!("duplicate skill file path query parameter"); + } + } + _ => bail!("invalid query parameter"), + } + } + + Ok(Self::Skill { + name: name.context("missing skill name")?, + source: source.context("missing skill source")?, + skill_file_path: skill_file_path.context("missing skill file path")?, + }) } else { bail!("invalid zed url: {:?}", input); } @@ -303,6 +342,13 @@ impl MentionUri { .. } => selection_name(path.as_deref(), line_range), MentionUri::Fetch { url } => url.to_string(), + MentionUri::Skill { name, source, .. } => { + if source.is_empty() { + format!("{} (global)", name) + } else { + format!("{} ({})", name, source) + } + } } } @@ -337,6 +383,9 @@ impl MentionUri { ) .into(), ), + MentionUri::Skill { + skill_file_path, .. + } => Some(skill_file_path.to_string_lossy().into_owned().into()), _ => None, } } @@ -358,6 +407,7 @@ impl MentionUri { MentionUri::Fetch { .. } => IconName::ToolWeb.path().into(), MentionUri::GitDiff { .. } => IconName::GitBranch.path().into(), MentionUri::MergeConflict { .. } => IconName::GitMergeConflict.path().into(), + MentionUri::Skill { .. } => IconName::Sparkle.path().into(), } } @@ -465,6 +515,19 @@ impl MentionUri { url.query_pairs_mut().append_pair("path", file_path); url } + MentionUri::Skill { + name, + source, + skill_file_path, + } => { + let mut url = Url::parse("zed:///").unwrap(); + url.set_path("/agent/skill"); + url.query_pairs_mut() + .append_pair("name", name) + .append_pair("source", source) + .append_pair("path", &skill_file_path.to_string_lossy()); + url + } } } } @@ -705,6 +768,20 @@ mod tests { assert_eq!(parsed.to_uri().to_string(), rule_uri); } + #[test] + fn test_parse_skill_uri_round_trip() { + let skill_uri = MentionUri::Skill { + name: "rust-best-practices".to_string(), + source: "my-personal-project".to_string(), + skill_file_path: PathBuf::from(path!("/path/to/skills/rust-best-practices/SKILL.md")), + }; + + let serialized = skill_uri.to_uri().to_string(); + let parsed = MentionUri::parse(&serialized, PathStyle::local()).unwrap(); + + assert_eq!(parsed, skill_uri); + } + #[test] fn test_parse_fetch_http_uri() { let http_uri = "http://example.com/path?query=value#fragment"; diff --git a/crates/agent/src/agent.rs b/crates/agent/src/agent.rs index e9cdfdb935f..a8b70c80b47 100644 --- a/crates/agent/src/agent.rs +++ b/crates/agent/src/agent.rs @@ -91,6 +91,25 @@ pub struct SkillLoadingErrorsUpdated { pub errors: Vec, } +#[derive(Clone, Debug)] +pub struct NativeAvailableSkill { + pub name: String, + pub description: String, + pub source: SharedString, + pub skill_file_path: PathBuf, +} + +impl From<&Skill> for NativeAvailableSkill { + fn from(skill: &Skill) -> Self { + Self { + name: skill.name.clone(), + description: skill.description.clone(), + source: skill.source.scope_prefix().to_string().into(), + skill_file_path: skill.skill_file_path.clone(), + } + } +} + struct ProjectState { project: Entity, project_context: Entity, @@ -1266,21 +1285,7 @@ impl NativeAgent { Some(command) }); - // Skills are exposed as slash commands regardless of - // `disable_model_invocation`. The flag controls catalog visibility - // for the model, not user-driven invocation — that's the whole - // point of marking a skill model-disabled. - let skill_commands = state.skills.iter().map(|skill| { - // The meta carries the literal scope prefix the popup - // inserts as `/:`: empty for globals (so - // the inserted text is `/:`) and the worktree root - // name for project-locals. See `SkillSource::scope_prefix`. - acp::AvailableCommand::new(skill.name.clone(), skill.description.clone()).meta( - acp_thread::meta_with_skill_source(skill.source.scope_prefix()), - ) - }); - - mcp_commands.chain(skill_commands).collect() + mcp_commands.collect() } pub fn load_thread( @@ -1721,6 +1726,24 @@ impl NativeAgentConnection { .update(cx, |agent, cx| agent.ensure_skills_scan_started(cx)); } + pub fn available_skills( + &self, + session_id: &acp::SessionId, + cx: &App, + ) -> Vec { + self.0 + .read(cx) + .session_project_state(session_id) + .map(|state| { + state + .skills + .iter() + .map(NativeAvailableSkill::from) + .collect() + }) + .unwrap_or_default() + } + pub fn load_thread( &self, id: acp::SessionId, @@ -3807,7 +3830,7 @@ mod internal_tests { } #[gpui::test] - async fn test_skills_appear_as_slash_commands(cx: &mut TestAppContext) { + async fn test_skills_appear_as_available_skills(cx: &mut TestAppContext) { init_test(cx); cx.update(|cx| { cx.update_flags(true, vec!["skills".to_string()]); @@ -3816,8 +3839,8 @@ mod internal_tests { let skills_dir = global_skills_dir(); // Two skills: one model-invocable (default), one slash-only via - // `disable-model-invocation: true`. Both should still appear as - // slash commands. + // `disable-model-invocation: true`. Both should still appear in + // the slash menu as first-class skills. let visible_dir = skills_dir.join("visible-skill"); fs.create_dir(&visible_dir).await.unwrap(); fs.insert_file( @@ -3841,9 +3864,9 @@ mod internal_tests { cx.update(|cx| NativeAgent::new(thread_store, Templates::new(), None, fs.clone(), cx)); let connection = NativeAgentConnection(agent.clone()); - let _acp_thread = cx + let acp_thread = cx .update(|cx| { - Rc::new(connection).new_session( + Rc::new(connection.clone()).new_session( project.clone(), PathList::new(&[Path::new("/")]), cx, @@ -3854,21 +3877,34 @@ mod internal_tests { cx.run_until_parked(); let project_id = project.entity_id(); + let session_id = acp_thread.read_with(cx, |thread, _cx| thread.session_id().clone()); - // Both skills should be exposed as slash commands. agent.read_with(cx, |agent, cx| { let commands = NativeAgent::build_available_commands_for_project( agent.projects.get(&project_id), cx, ); let names: Vec<&str> = commands.iter().map(|c| c.name.as_str()).collect(); + assert!( + !names.contains(&"visible-skill"), + "skills should not be exposed as ACP slash commands: {names:?}" + ); + assert!( + !names.contains(&"deploy"), + "slash-only skills should not be exposed as ACP slash commands: {names:?}" + ); + }); + + cx.update(|cx| { + let skills = connection.available_skills(&session_id, cx); + let names: Vec<&str> = skills.iter().map(|skill| skill.name.as_str()).collect(); assert!( names.contains(&"visible-skill"), - "visible skill missing from slash commands: {names:?}" + "visible skill missing from available skills: {names:?}" ); assert!( names.contains(&"deploy"), - "slash-only skill missing from slash commands: {names:?}" + "slash-only skill missing from available skills: {names:?}" ); }); @@ -3927,9 +3963,9 @@ mod internal_tests { cx.update(|cx| NativeAgent::new(thread_store, Templates::new(), None, fs.clone(), cx)); let connection = NativeAgentConnection(agent.clone()); - let _acp_thread = cx + let acp_thread = cx .update(|cx| { - Rc::new(connection).new_session( + Rc::new(connection.clone()).new_session( project.clone(), PathList::new(&[Path::new("/project")]), cx, @@ -3940,6 +3976,7 @@ mod internal_tests { cx.run_until_parked(); let project_id = project.entity_id(); + let session_id = acp_thread.read_with(cx, |thread, _cx| thread.session_id().clone()); let worktree_id = project.read_with(cx, |project, cx| { project.worktrees(cx).next().unwrap().read(cx).id() }); @@ -3980,15 +4017,18 @@ mod internal_tests { }); cx.run_until_parked(); - agent.read_with(cx, |agent, cx| { + agent.read_with(cx, |agent, _cx| { let state = agent.projects.get(&project_id).unwrap(); let names: Vec<&str> = state.skills.iter().map(|s| s.name.as_str()).collect(); assert_eq!(names, vec!["my-skill"]); - let commands = NativeAgent::build_available_commands_for_project(Some(state), cx); - let command_names: Vec<&str> = commands.iter().map(|c| c.name.as_str()).collect(); + }); + + cx.update(|cx| { + let skills = connection.available_skills(&session_id, cx); + let skill_names: Vec<&str> = skills.iter().map(|s| s.name.as_str()).collect(); assert!( - command_names.contains(&"my-skill"), - "trusted skill should appear in slash commands: {command_names:?}" + skill_names.contains(&"my-skill"), + "trusted skill should appear in available skills: {skill_names:?}" ); }); } diff --git a/crates/agent/src/thread.rs b/crates/agent/src/thread.rs index c004305779a..b2114fcfb1d 100644 --- a/crates/agent/src/thread.rs +++ b/crates/agent/src/thread.rs @@ -233,6 +233,8 @@ impl UserMessage { const OPEN_DIAGNOSTICS_TAG: &str = ""; const OPEN_DIFFS_TAG: &str = ""; const MERGE_CONFLICT_TAG: &str = ""; + const OPEN_SKILLS_TAG: &str = + "\nThe user has attached the following agent skills:\n"; let mut file_context = OPEN_FILES_TAG.to_string(); let mut directory_context = OPEN_DIRECTORIES_TAG.to_string(); @@ -244,6 +246,7 @@ impl UserMessage { let mut diagnostics_context = OPEN_DIAGNOSTICS_TAG.to_string(); let mut diffs_context = OPEN_DIFFS_TAG.to_string(); let mut merge_conflict_context = MERGE_CONFLICT_TAG.to_string(); + let mut skills_context = OPEN_SKILLS_TAG.to_string(); for chunk in &self.content { let chunk = match chunk { @@ -360,6 +363,14 @@ impl UserMessage { ) .ok(); } + MentionUri::Skill { name, source, .. } => { + let label = if source.is_empty() { + format!("{} (global)", name) + } else { + format!("{} ({})", name, source) + }; + write!(&mut skills_context, "\nSkill: {}\n{}\n", label, content).ok(); + } } language_model::MessageContent::Text(uri.as_link().to_string()) @@ -434,6 +445,13 @@ impl UserMessage { .push(language_model::MessageContent::Text(diagnostics_context)); } + if skills_context.len() > OPEN_SKILLS_TAG.len() { + skills_context.push_str("\n"); + message + .content + .push(language_model::MessageContent::Text(skills_context)); + } + if merge_conflict_context.len() > MERGE_CONFLICT_TAG.len() { merge_conflict_context.push_str("\n"); message diff --git a/crates/agent_ui/src/completion_provider.rs b/crates/agent_ui/src/completion_provider.rs index ceb5c177c84..efade13bfd3 100644 --- a/crates/agent_ui/src/completion_provider.rs +++ b/crates/agent_ui/src/completion_provider.rs @@ -19,8 +19,8 @@ use multi_buffer::ToOffset as _; use ordered_float::OrderedFloat; use project::lsp_store::{CompletionDocumentation, SymbolLocation}; use project::{ - Completion, CompletionDisplayOptions, CompletionIntent, CompletionResponse, DiagnosticSummary, - PathMatchCandidateSet, Project, ProjectPath, Symbol, WorktreeId, + Completion, CompletionDisplayOptions, CompletionGroup, CompletionIntent, CompletionResponse, + DiagnosticSummary, PathMatchCandidateSet, Project, ProjectPath, Symbol, WorktreeId, }; use prompt_store::{PromptStore, UserPromptId}; use rope::Point; @@ -297,18 +297,39 @@ pub struct RulesContextEntry { pub title: SharedString, } +#[derive(Debug, Clone)] +pub struct AvailableSkill { + pub name: Arc, + pub description: Arc, + /// Scope prefix for this skill: empty for global skills, or the + /// worktree root name for project-local skills. + pub source: SharedString, + pub skill_file_path: PathBuf, +} + #[derive(Debug, Clone)] pub struct AvailableCommand { pub name: Arc, pub description: Arc, pub requires_argument: bool, - /// Origin label for this command (e.g. `"global"` or a worktree - /// root name for skills). When present, it's displayed in the - /// autocomplete popup after the command name so users can - /// disambiguate same-named commands from different scopes. pub source: Option, } +#[derive(Debug, Clone)] +enum SlashCompletionCandidate { + Skill(AvailableSkill), + Command(AvailableCommand), +} + +impl SlashCompletionCandidate { + fn name(&self) -> &Arc { + match self { + Self::Skill(skill) => &skill.name, + Self::Command(command) => &command.name, + } + } +} + pub trait PromptCompletionProviderDelegate: Send + Sync + 'static { fn supports_context(&self, mode: PromptContextType, cx: &App) -> bool { self.supported_modes(cx).contains(&mode) @@ -317,6 +338,9 @@ pub trait PromptCompletionProviderDelegate: Send + Sync + 'static { fn supports_images(&self, cx: &App) -> bool; fn available_commands(&self, cx: &App) -> Vec; + fn available_skills(&self, _cx: &App) -> Vec { + Vec::new() + } fn confirm_command(&self, cx: &mut App); /// Called once each time the user opens slash-command autocomplete @@ -374,6 +398,7 @@ impl PromptCompletionProvider { // completion menu will still be shown after "@category " is // inserted confirm: Some(Arc::new(|_, _, _| true)), + group: None, }), PromptContextEntry::Action(action) => { let selection = workspace.update(cx, |workspace, cx| { @@ -431,6 +456,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, } } @@ -470,6 +496,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, } } @@ -536,6 +563,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, }) } @@ -601,6 +629,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, }) } @@ -641,6 +670,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, }) } @@ -686,6 +716,7 @@ impl PromptCompletionProvider { // completion menu will still be shown after "@category " is // inserted confirm: Some(on_action), + group: None, }) } @@ -783,6 +814,7 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, } } @@ -824,31 +856,48 @@ impl PromptCompletionProvider { mention_set, workspace, )), + group: None, } } - fn search_slash_commands(&self, query: String, cx: &mut App) -> Task> { + fn search_slash_commands( + &self, + query: String, + cx: &mut App, + ) -> Task> { // Notify the delegate that slash autocomplete is being // invoked, so it can lazily kick off any work that produces - // additional commands. Whatever it produces won't be visible - // in the current autocomplete pass (we read `available_commands` - // synchronously below), but will appear on the next invocation. + // additional commands or skills. Whatever it produces won't be + // visible in the current autocomplete pass (we read available + // items synchronously below), but will appear on the next + // invocation. self.source.slash_autocomplete_invoked(cx); - let commands = self.source.available_commands(cx); - if commands.is_empty() { + let mut candidates = self + .source + .available_skills(cx) + .into_iter() + .map(SlashCompletionCandidate::Skill) + .collect::>(); + candidates.extend( + self.source + .available_commands(cx) + .into_iter() + .map(SlashCompletionCandidate::Command), + ); + if candidates.is_empty() { return Task::ready(Vec::new()); } cx.spawn(async move |cx| { - let candidates = commands + let string_match_candidates = candidates .iter() .enumerate() - .map(|(id, command)| StringMatchCandidate::new(id, &command.name)) + .map(|(id, candidate)| StringMatchCandidate::new(id, candidate.name())) .collect::>(); let matches = fuzzy::match_strings( - &candidates, + &string_match_candidates, &query, false, true, @@ -860,7 +909,7 @@ impl PromptCompletionProvider { matches .into_iter() - .map(|mat| commands[mat.candidate_id].clone()) + .map(|mat| candidates[mat.candidate_id].clone()) .collect() }) } @@ -1246,6 +1295,7 @@ impl CompletionProvider for PromptCompletio PromptCompletion::SlashCommand(SlashCommandCompletion { command, argument, .. }) => { + let show_section_headers = command.is_none() && argument.is_none(); let search_task = self.search_slash_commands(command.unwrap_or_default(), cx); // Resolve the muted-text highlight up front: the // completion build happens on a background thread where @@ -1255,80 +1305,145 @@ impl CompletionProvider for PromptCompletio .syntax() .highlight_id("variable") .map(HighlightId::new); - cx.background_spawn(async move { - let completions = search_task - .await - .into_iter() - .map(|command| { - // Qualify the inserted text with the skill's - // scope prefix as `/:` when the - // command carries one. The prefix is empty - // for global skills (so the inserted text - // is `/:`) and the worktree root name - // for project-locals (so the inserted text - // is `/:`). The `:` - // separator namespaces skill scopes away - // from MCP server prefixes - // (`/.`), and the empty - // prefix means a worktree literally named - // `global` no longer collides with the - // global source. MCP commands have no - // source meta and keep the bare `/` - // form. - // - // Composed in a single `format!` to avoid - // building an intermediate `qualified_name` - // string just to splice it into the final - // text. - let new_text = match (command.source.as_ref(), argument.as_ref()) { - (Some(source), Some(argument)) => { - format!("/{}:{} {}", source, command.name, argument) - } - (Some(source), None) => { - format!("/{}:{} ", source, command.name) - } - (None, Some(argument)) => { - format!("/{} {}", command.name, argument) - } - (None, None) => format!("/{} ", command.name), - }; - let is_missing_argument = - command.requires_argument && argument.is_none(); - - let label = build_slash_command_label(&command, source_highlight_id); - - Completion { - replace_range: source_range.clone(), - new_text, - label, - documentation: Some(CompletionDocumentation::MultiLinePlainText( - command.description.into(), - )), - source: project::CompletionSource::Custom, - icon_path: None, - match_start: None, - snippet_deduplication_key: None, - insert_text_mode: None, - confirm: Some(Arc::new({ - let source = source.clone(); - move |intent, _window, cx| { - if !is_missing_argument { - cx.defer({ - let source = source.clone(); - move |cx| match intent { - CompletionIntent::Complete - | CompletionIntent::CompleteWithInsert - | CompletionIntent::CompleteWithReplace => { - source.confirm_command(cx); - } - CompletionIntent::Compose => {} - } - }); - } - false + type SkillInfo = ( + String, + SharedString, + Arc bool + Send + Sync>, + ); + let slash_candidates: Task)>> = { + let source = source.clone(); + cx.spawn(async move |_this, cx| { + let candidates = search_task.await; + cx.update(|cx| { + candidates + .into_iter() + .map(|candidate| match &candidate { + SlashCompletionCandidate::Skill(skill) => { + let uri = MentionUri::Skill { + name: skill.name.to_string(), + source: skill.source.to_string(), + skill_file_path: skill.skill_file_path.clone(), + }; + let new_text = format!("{} ", uri.as_link()); + let new_text_len = new_text.len(); + let icon_path = uri.icon_path(cx); + let crease_text: SharedString = uri.name().into(); + let confirm = confirm_completion_callback( + crease_text, + source_range.start, + new_text_len - 1, + uri, + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + ); + (candidate, Some((new_text, icon_path, confirm))) } - })), + SlashCompletionCandidate::Command(_) => (candidate, None), + }) + .collect::)>>() + }) + }) + }; + + cx.background_spawn(async move { + let mut slash_candidates = slash_candidates.await; + slash_candidates.sort_by_key(|(candidate, _)| match candidate { + SlashCompletionCandidate::Skill(_) => 0, + SlashCompletionCandidate::Command(_) => 1, + }); + let completions = slash_candidates + .into_iter() + .map(|(candidate, skill_info)| match candidate { + SlashCompletionCandidate::Skill(skill) => { + let label = build_slash_item_label( + &skill.name, + Some(&skill.source), + source_highlight_id, + ); + let Some((new_text, icon_path, confirm)) = skill_info else { + unreachable!("skill candidates always have confirm callbacks") + }; + Completion { + replace_range: source_range.clone(), + new_text, + label, + documentation: Some( + CompletionDocumentation::MultiLinePlainText( + skill.description.into(), + ), + ), + source: project::CompletionSource::Custom, + icon_path: Some(icon_path), + match_start: None, + snippet_deduplication_key: None, + insert_text_mode: None, + confirm: Some(confirm), + group: show_section_headers.then(|| CompletionGroup { + key: "skills".into(), + label: Some("Skills".into()), + }), + } + } + SlashCompletionCandidate::Command(command) => { + let label = + build_slash_command_label(&command, source_highlight_id); + let new_text = match (command.source.as_ref(), argument.as_ref()) { + (Some(source), Some(argument)) => { + format!("/{}:{} {}", source, command.name, argument) + } + (Some(source), None) => { + format!("/{}:{} ", source, command.name) + } + (None, Some(argument)) => { + format!("/{} {}", command.name, argument) + } + (None, None) => format!("/{} ", command.name), + }; + + let is_missing_argument = + command.requires_argument && argument.is_none(); + + Completion { + replace_range: source_range.clone(), + new_text, + label, + documentation: Some( + CompletionDocumentation::MultiLinePlainText( + command.description.into(), + ), + ), + source: project::CompletionSource::Custom, + icon_path: None, + match_start: None, + snippet_deduplication_key: None, + insert_text_mode: None, + confirm: Some(Arc::new({ + let source = source.clone(); + move |intent, _window, cx| { + if !is_missing_argument { + cx.defer({ + let source = source.clone(); + move |cx| match intent { + CompletionIntent::Complete + | CompletionIntent::CompleteWithInsert + | CompletionIntent::CompleteWithReplace => { + source.confirm_command(cx); + } + CompletionIntent::Compose => {} + } + }); + } + false + } + })), + group: show_section_headers.then(|| CompletionGroup { + key: "agent-commands".into(), + label: Some("Agent Commands".into()), + }), + } } }) .collect(); @@ -1338,8 +1453,6 @@ impl CompletionProvider for PromptCompletio display_options: CompletionDisplayOptions { dynamic_width: true, }, - // Since this does its own filtering (see `filter_completions()` returns false), - // there is no benefit to computing whether this set of completions is incomplete. is_incomplete: true, }]) }) @@ -1367,6 +1480,7 @@ impl CompletionProvider for PromptCompletio } } + let show_section_headers = mode.is_none() && argument.is_none(); let query = argument.unwrap_or_default(); let search_task = self.search_mentions(mode, query, Arc::::default(), cx); @@ -1397,118 +1511,156 @@ impl CompletionProvider for PromptCompletio }; cx.spawn(async move |_, cx| { - let matches = search_task.await; + let mut matches = search_task.await; + if show_section_headers { + matches.sort_by_key(|mat| match mat { + Match::File(FileMatch { + is_recent: true, .. + }) + | Match::RecentThread(_) => 0, + Match::Entry(_) | Match::BranchDiff(_) => 1, + _ => 2, + }); + } let completions = cx.update(|cx| { matches .into_iter() - .filter_map(|mat| match mat { - Match::File(FileMatch { mat, is_recent }) => { - let project_path = ProjectPath { - worktree_id: WorktreeId::from_usize(mat.worktree_id), - path: mat.path.clone(), - }; + .filter_map(|mat| { + let group = if show_section_headers { + match &mat { + Match::File(FileMatch { + is_recent: true, .. + }) + | Match::RecentThread(_) => Some(CompletionGroup { + key: "recent".into(), + label: None, + }), + Match::Entry(_) | Match::BranchDiff(_) => { + Some(CompletionGroup { + key: "context".into(), + label: None, + }) + } + _ => None, + } + } else { + None + }; + let mut completion = match mat { + Match::File(FileMatch { mat, is_recent }) => { + let project_path = ProjectPath { + worktree_id: WorktreeId::from_usize(mat.worktree_id), + path: mat.path.clone(), + }; - // If path is empty, this means we're matching with the root directory itself - // so we use the path_prefix as the name - let path_prefix = if mat.path.is_empty() { - project - .read(cx) - .worktree_for_id(project_path.worktree_id, cx) - .map(|wt| wt.read(cx).root_name().into()) - .unwrap_or_else(|| mat.path_prefix.clone()) - } else { - mat.path_prefix.clone() - }; + // If path is empty, this means we're matching with the root directory itself + // so we use the path_prefix as the name + let path_prefix = if mat.path.is_empty() { + project + .read(cx) + .worktree_for_id(project_path.worktree_id, cx) + .map(|wt| wt.read(cx).root_name().into()) + .unwrap_or_else(|| mat.path_prefix.clone()) + } else { + mat.path_prefix.clone() + }; - Self::completion_for_path( - project_path, - &path_prefix, - is_recent, - mat.is_dir, + Self::completion_for_path( + project_path, + &path_prefix, + is_recent, + mat.is_dir, + source_range.clone(), + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + project.clone(), + label_max_chars, + cx, + ) + } + Match::Symbol(SymbolMatch { symbol, .. }) => { + Self::completion_for_symbol( + symbol, + source_range.clone(), + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + label_max_chars, + cx, + ) + } + Match::Thread(thread) => Some(Self::completion_for_thread( + thread.session_id, + Some(thread.title), source_range.clone(), + false, source.clone(), editor.clone(), mention_set.clone(), workspace.clone(), - project.clone(), - label_max_chars, cx, - ) - } - Match::Symbol(SymbolMatch { symbol, .. }) => { - Self::completion_for_symbol( - symbol, - source_range.clone(), - source.clone(), - editor.clone(), - mention_set.clone(), - workspace.clone(), - label_max_chars, - cx, - ) - } - Match::Thread(thread) => Some(Self::completion_for_thread( - thread.session_id, - Some(thread.title), - source_range.clone(), - false, - source.clone(), - editor.clone(), - mention_set.clone(), - workspace.clone(), - cx, - )), - Match::RecentThread(thread) => Some(Self::completion_for_thread( - thread.session_id, - Some(thread.title), - source_range.clone(), - true, - source.clone(), - editor.clone(), - mention_set.clone(), - workspace.clone(), - cx, - )), - Match::Rules(user_rules) => Some(Self::completion_for_rules( - user_rules, - source_range.clone(), - source.clone(), - editor.clone(), - mention_set.clone(), - workspace.clone(), - cx, - )), - Match::Fetch(url) => Self::completion_for_fetch( - source_range.clone(), - url, - source.clone(), - editor.clone(), - mention_set.clone(), - workspace.clone(), - cx, - ), - Match::Entry(EntryMatch { entry, .. }) => { - Self::completion_for_entry( - entry, - source_range.clone(), - editor.clone(), - mention_set.clone(), - &workspace, - cx, - ) - } - Match::BranchDiff(branch_diff) => { - Some(Self::build_branch_diff_completion( - branch_diff.base_ref, + )), + Match::RecentThread(thread) => { + Some(Self::completion_for_thread( + thread.session_id, + Some(thread.title), + source_range.clone(), + true, + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + cx, + )) + } + Match::Rules(user_rules) => Some(Self::completion_for_rules( + user_rules, source_range.clone(), source.clone(), editor.clone(), mention_set.clone(), workspace.clone(), cx, - )) + )), + Match::Fetch(url) => Self::completion_for_fetch( + source_range.clone(), + url, + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + cx, + ), + Match::Entry(EntryMatch { entry, .. }) => { + Self::completion_for_entry( + entry, + source_range.clone(), + editor.clone(), + mention_set.clone(), + &workspace, + cx, + ) + } + Match::BranchDiff(branch_diff) => { + Some(Self::build_branch_diff_completion( + branch_diff.base_ref, + source_range.clone(), + source.clone(), + editor.clone(), + mention_set.clone(), + workspace.clone(), + cx, + )) + } + }; + if let Some(completion) = &mut completion { + completion.group = group; } + completion }) .collect::>() }); @@ -1655,17 +1807,31 @@ pub struct SlashCommandCompletion { impl SlashCommandCompletion { pub fn try_parse(line: &str, offset_to_line: usize) -> Option { - // If we decide to support commands that are not at the beginning of the prompt, we can remove this check - if !line.starts_with('/') || offset_to_line != 0 { - return None; + let mut last_command_start = None; + for (idx, _) in line.rmatch_indices('/') { + if line[idx + 1..] + .chars() + .next() + .is_some_and(|c| c.is_whitespace()) + { + continue; + } + + if idx > 0 + && line[..idx] + .chars() + .last() + .is_some_and(|c| !c.is_whitespace()) + { + continue; + } + + last_command_start = Some(idx); + break; } - let (prefix, last_command) = line.rsplit_once('/')?; - if prefix.chars().last().is_some_and(|c| !c.is_whitespace()) - || last_command.starts_with(char::is_whitespace) - { - return None; - } + let last_command_start = last_command_start?; + let last_command = &line[last_command_start + 1..]; let mut argument = None; let mut command = None; @@ -1679,7 +1845,7 @@ impl SlashCommandCompletion { }; Some(Self { - source_range: prefix.len() + offset_to_line + source_range: last_command_start + offset_to_line ..line .rfind(|c: char| !c.is_whitespace()) .unwrap_or_else(|| line.len()) @@ -2171,27 +2337,28 @@ pub fn extract_file_name_and_directory( ) } -/// Build the autocomplete-popup label for a slash command, appending -/// the command's origin (a worktree root name for project-local -/// skills) after the name when one is present and non-empty. The -/// suffix is styled with the muted `variable` highlight and excluded -/// from the fuzzy filter range so typing the source doesn't match -/// the entry. -/// -/// Global skills carry an empty source (the literal scope prefix is -/// empty so the popup inserts `/:`), and render with no -/// subtext — the source column is reserved for project-local skills -/// where the worktree name disambiguates same-named entries. fn build_slash_command_label( command: &AvailableCommand, source_highlight_id: Option, ) -> CodeLabel { - let source = command.source.as_ref().filter(|source| !source.is_empty()); + build_slash_item_label(&command.name, command.source.as_ref(), source_highlight_id) +} + +/// Build the autocomplete-popup label for a slash menu item, appending +/// the item origin after the name when one is present and non-empty. +/// The suffix is styled with the muted `variable` highlight and excluded +/// from the fuzzy filter range so typing the source doesn't match the entry. +fn build_slash_item_label( + name: &Arc, + source: Option<&SharedString>, + source_highlight_id: Option, +) -> CodeLabel { + let source = source.filter(|source| !source.is_empty()); let Some(source) = source else { - return CodeLabel::plain(command.name.to_string(), None); + return CodeLabel::plain(name.to_string(), None); }; let mut builder = CodeLabelBuilder::default(); - builder.push_str(&command.name, None); + builder.push_str(name, None); // Two spaces gives a touch of breathing room between the name and // the muted source label. builder.push_str(" ", None); @@ -2203,7 +2370,7 @@ fn build_slash_command_label( // (`filter_completions()` is false), so this is mostly defensive // — but it keeps the displayed filter consistent with what we // actually matched against. - builder.respan_filter_range(Some(&command.name)); + builder.respan_filter_range(Some(name)); builder.build() } @@ -2547,9 +2714,41 @@ mod tests { assert_eq!(SlashCommandCompletion::try_parse("Lorem Ipsum", 0), None); - assert_eq!(SlashCommandCompletion::try_parse("Lorem /", 0), None); + assert_eq!( + SlashCommandCompletion::try_parse("Lorem /", 0), + Some(SlashCommandCompletion { + source_range: 6..7, + command: None, + argument: None, + }) + ); - assert_eq!(SlashCommandCompletion::try_parse("Lorem /help", 0), None); + assert_eq!( + SlashCommandCompletion::try_parse("Lorem /help", 0), + Some(SlashCommandCompletion { + source_range: 6..11, + command: Some("help".to_string()), + argument: None, + }) + ); + + assert_eq!( + SlashCommandCompletion::try_parse("Lorem /help /test", 0), + Some(SlashCommandCompletion { + source_range: 12..17, + command: Some("test".to_string()), + argument: None, + }) + ); + + assert_eq!( + SlashCommandCompletion::try_parse("/help", 10), + Some(SlashCommandCompletion { + source_range: 10..15, + command: Some("help".to_string()), + argument: None, + }) + ); assert_eq!(SlashCommandCompletion::try_parse("Lorem/", 0), None); diff --git a/crates/agent_ui/src/conversation_view.rs b/crates/agent_ui/src/conversation_view.rs index 27225bfea88..394281abad5 100644 --- a/crates/agent_ui/src/conversation_view.rs +++ b/crates/agent_ui/src/conversation_view.rs @@ -78,7 +78,7 @@ use crate::agent_connection_store::{ AgentConnectedState, AgentConnectionEntryEvent, AgentConnectionStore, }; use crate::agent_diff::AgentDiff; -use crate::completion_provider::AgentContextSelection; +use crate::completion_provider::{AgentContextSelection, AvailableSkill}; use crate::entry_view_state::{EntryViewEvent, ViewEvent}; use crate::message_editor::{InputAttempt, MessageEditor, MessageEditorEvent}; use crate::profile_selector::{ProfileProvider, ProfileSelector}; @@ -1025,9 +1025,17 @@ impl ConversationView { cx: &mut Context, ) -> Entity { let agent_id = self.agent.agent_id(); + let connection = thread.read(cx).connection().clone(); + let session_id = thread.read(cx).session_id().clone(); + let available_skills = connection + .clone() + .downcast::() + .map(|native_connection| native_available_skills(&native_connection, &session_id, cx)) + .unwrap_or_default(); let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::new( thread.read(cx).prompt_capabilities(), thread.read(cx).available_commands().to_vec(), + available_skills, ))); let action_log = thread.read(cx).action_log().clone(); @@ -1653,7 +1661,17 @@ impl ConversationView { } AcpThreadEvent::AvailableCommandsUpdated(available_commands) => { if let Some(thread_view) = self.thread_view(&session_id) { - let has_commands = !available_commands.is_empty(); + let available_skills = thread + .read(cx) + .connection() + .clone() + .downcast::() + .map(|native_connection| { + native_available_skills(&native_connection, &session_id, cx) + }) + .unwrap_or_default(); + let has_slash_completions = + !available_commands.is_empty() || !available_skills.is_empty(); let agent_display_name = self .agent_server_store @@ -1662,13 +1680,12 @@ impl ConversationView { .unwrap_or_else(|| self.agent.agent_id().0.to_string().into()); let new_placeholder = - placeholder_text(agent_display_name.as_ref(), has_commands); + placeholder_text(agent_display_name.as_ref(), has_slash_completions); thread_view.update(cx, |thread_view, cx| { - thread_view - .session_capabilities - .write() - .set_available_commands(available_commands.clone()); + let mut session_capabilities = thread_view.session_capabilities.write(); + session_capabilities.set_available_commands(available_commands.clone()); + session_capabilities.set_available_skills(available_skills); thread_view.message_editor.update(cx, |editor, cx| { editor.set_placeholder_text(&new_placeholder, window, cx); }); @@ -2903,9 +2920,29 @@ fn loading_contents_spinner(size: IconSize) -> AnyElement { .into_any_element() } +fn native_available_skills( + native_connection: &agent::NativeAgentConnection, + session_id: &acp::SessionId, + cx: &App, +) -> Vec { + native_connection + .available_skills(session_id, cx) + .into_iter() + .map(|skill| AvailableSkill { + name: skill.name.into(), + description: skill.description.into(), + source: skill.source, + skill_file_path: skill.skill_file_path, + }) + .collect() +} + fn placeholder_text(agent_name: &str, has_commands: bool) -> String { if agent_name == agent::ZED_AGENT_ID.as_ref() { - format!("Message the {} — @ to include context", agent_name) + format!( + "Message the {}, @ to include context, / for commands", + agent_name + ) } else if has_commands { format!( "Message {} — @ to include context, / for commands", diff --git a/crates/agent_ui/src/conversation_view/thread_view.rs b/crates/agent_ui/src/conversation_view/thread_view.rs index 045cdf07f17..fbc43018b14 100644 --- a/crates/agent_ui/src/conversation_view/thread_view.rs +++ b/crates/agent_ui/src/conversation_view/thread_view.rs @@ -393,8 +393,8 @@ impl ThreadView { let session_id = thread.read(cx).session_id().clone(); let parent_session_id = thread.read(cx).parent_session_id().cloned(); - let has_commands = !session_capabilities.read().available_commands().is_empty(); - let placeholder = placeholder_text(agent_display_name.as_ref(), has_commands); + let has_slash_completions = session_capabilities.read().has_slash_completions(); + let placeholder = placeholder_text(agent_display_name.as_ref(), has_slash_completions); let mut should_auto_submit = false; let mut show_external_source_prompt_warning = false; @@ -1029,7 +1029,7 @@ impl ThreadView { .read() .available_commands() .iter() - .any(|command| command.name == "logout"); + .any(|available_command| available_command.name == "logout"); if can_login && !logout_supported { message_editor.update(cx, |editor, cx| editor.clear(window, cx)); self.clear_external_source_prompt_warning(cx); @@ -9409,6 +9409,21 @@ pub(crate) fn open_link( MentionUri::TerminalSelection { .. } => {} MentionUri::GitDiff { .. } => {} MentionUri::MergeConflict { .. } => {} + MentionUri::Skill { + skill_file_path, .. + } => { + workspace + .open_abs_path( + skill_file_path, + workspace::OpenOptions { + focus: Some(true), + ..Default::default() + }, + window, + cx, + ) + .detach_and_log_err(cx); + } }) } else { cx.open_url(&url); diff --git a/crates/agent_ui/src/mention_set.rs b/crates/agent_ui/src/mention_set.rs index a1d4065ea20..17ad11d3288 100644 --- a/crates/agent_ui/src/mention_set.rs +++ b/crates/agent_ui/src/mention_set.rs @@ -140,6 +140,9 @@ impl MentionSet { .. } => self.confirm_mention_for_symbol(abs_path, line_range, cx), MentionUri::Rule { id, .. } => self.confirm_mention_for_rule(id, cx), + MentionUri::Skill { + skill_file_path, .. + } => self.confirm_mention_for_skill(skill_file_path, cx), MentionUri::Diagnostics { include_errors, include_warnings, @@ -289,6 +292,9 @@ impl MentionSet { .. } => self.confirm_mention_for_symbol(abs_path, line_range, cx), MentionUri::Rule { id, .. } => self.confirm_mention_for_rule(id, cx), + MentionUri::Skill { + skill_file_path, .. + } => self.confirm_mention_for_skill(skill_file_path, cx), MentionUri::Diagnostics { include_errors, include_warnings, @@ -440,6 +446,26 @@ impl MentionSet { }) } + fn confirm_mention_for_skill( + &self, + skill_file_path: PathBuf, + cx: &mut Context, + ) -> Task> { + cx.background_spawn(async move { + let content = std::fs::read_to_string(&skill_file_path).map_err(|e| { + anyhow!( + "Failed to read skill file {}: {}", + skill_file_path.display(), + e + ) + })?; + Ok(Mention::Text { + content, + tracked_buffers: Vec::new(), + }) + }) + } + fn confirm_mention_for_rule( &mut self, id: PromptId, diff --git a/crates/agent_ui/src/message_editor.rs b/crates/agent_ui/src/message_editor.rs index 9b9b7a57266..797129f18d0 100644 --- a/crates/agent_ui/src/message_editor.rs +++ b/crates/agent_ui/src/message_editor.rs @@ -3,8 +3,9 @@ use crate::SendImmediately; use crate::{ ChatWithFollow, completion_provider::{ - AgentContextSelection, PromptCompletionProvider, PromptCompletionProviderDelegate, - PromptContextAction, PromptContextType, SlashCommandCompletion, + AgentContextSelection, AvailableCommand, AvailableSkill, PromptCompletionProvider, + PromptCompletionProviderDelegate, PromptContextAction, PromptContextType, + SlashCommandCompletion, }, mention_set::{Mention, MentionImage, MentionSet, insert_crease_for_mention}, }; @@ -47,19 +48,29 @@ use zed_actions::agent::{Chat, PasteRaw}; pub struct SessionCapabilities { prompt_capabilities: acp::PromptCapabilities, available_commands: Vec, + available_skills: Vec, } impl SessionCapabilities { pub fn new( prompt_capabilities: acp::PromptCapabilities, available_commands: Vec, + available_skills: Vec, ) -> Self { Self { prompt_capabilities, available_commands, + available_skills, } } + pub fn from_acp_commands( + prompt_capabilities: acp::PromptCapabilities, + available_commands: Vec, + ) -> Self { + Self::new(prompt_capabilities, available_commands, Vec::new()) + } + pub fn supports_images(&self) -> bool { self.prompt_capabilities.image } @@ -72,6 +83,14 @@ impl SessionCapabilities { &self.available_commands } + pub fn available_skills(&self) -> &[AvailableSkill] { + &self.available_skills + } + + pub fn has_slash_completions(&self) -> bool { + !self.available_commands.is_empty() || !self.available_skills.is_empty() + } + fn supported_modes(&self, has_thread_store: bool) -> Vec { let mut supported = vec![PromptContextType::File, PromptContextType::Symbol]; if self.prompt_capabilities.embedded_context { @@ -88,18 +107,22 @@ impl SessionCapabilities { supported } - pub fn completion_commands(&self) -> Vec { + pub fn completion_commands(&self) -> Vec { self.available_commands .iter() - .map(|cmd| crate::completion_provider::AvailableCommand { - name: cmd.name.clone().into(), - description: cmd.description.clone().into(), - requires_argument: cmd.input.is_some(), - source: acp_thread::skill_source_from_meta(&cmd.meta), + .map(|command| AvailableCommand { + name: command.name.clone().into(), + description: command.description.clone().into(), + requires_argument: command.input.is_some(), + source: None, }) .collect() } + pub fn completion_skills(&self) -> Vec { + self.available_skills.clone() + } + pub fn set_prompt_capabilities(&mut self, prompt_capabilities: acp::PromptCapabilities) { self.prompt_capabilities = prompt_capabilities; } @@ -107,6 +130,10 @@ impl SessionCapabilities { pub fn set_available_commands(&mut self, available_commands: Vec) { self.available_commands = available_commands; } + + pub fn set_available_skills(&mut self, available_skills: Vec) { + self.available_skills = available_skills; + } } pub type SharedSessionCapabilities = Arc>; @@ -128,10 +155,14 @@ impl PromptCompletionProviderDelegate for MessageEditorCompletionDelegate { .supported_modes(self.has_thread_store) } - fn available_commands(&self, _cx: &App) -> Vec { + fn available_commands(&self, _cx: &App) -> Vec { self.session_capabilities.read().completion_commands() } + fn available_skills(&self, _cx: &App) -> Vec { + self.session_capabilities.read().completion_skills() + } + fn slash_autocomplete_invoked(&self, cx: &mut App) { // This may be called synchronously from inside a `MessageEditor` // update (e.g. when pasting a slash command triggers completions), @@ -606,7 +637,7 @@ impl MessageEditor { let command_name = parsed_command.command?; let available_command = available_commands .iter() - .find(|command| command.name == command_name)?; + .find(|available_command| available_command.name == command_name)?; let acp::AvailableCommandInput::Unstructured(acp::UnstructuredCommandInput { mut hint, @@ -717,6 +748,7 @@ impl MessageEditor { fn validate_slash_commands( text: &str, available_commands: &[acp::AvailableCommand], + available_skills: &[AvailableSkill], agent_id: &AgentId, ) -> Result<()> { if let Some(parsed_command) = SlashCommandCompletion::try_parse(text, 0) { @@ -729,7 +761,7 @@ impl MessageEditor { // (`/github.create_pr`), and skills (whose bare name // is registered for the unqualified `/` form). // - // 2. Skill scope qualifier `/:`. The popup + // 2. Trusted native skill scope qualifier `/:`. The popup // inserts this colon-separated form to disambiguate // same-named skills, so the validator splits on the // LAST `:` to recover scope + bare name. Skill @@ -740,23 +772,24 @@ impl MessageEditor { // scope is allowed to be empty: `/:` is the // qualified form for a global skill (see // `SkillSource::scope_prefix`). The validator then - // confirms an available command with that bare - // name has a `zed.skill_source` meta tag whose - // value equals the typed scope (including empty - // for globals). Without this branch, every - // autocomplete pick of a same-named skill would be - // rejected as "not supported" before reaching the - // resolver. + // checks the `available_skills` slice for an entry + // whose `skill.name` matches the bare name and + // whose `skill.source` equals the typed scope + // (including empty for globals). Without this + // branch, every autocomplete pick of a same-named + // skill would be rejected as "not supported" + // before reaching the resolver. let direct_match = available_commands .iter() - .any(|cmd| cmd.name == command_name); + .any(|available_command| available_command.name == command_name) + || available_skills + .iter() + .any(|skill| skill.name.as_ref() == command_name); let scope_match = !direct_match && command_name.rsplit_once(':').is_some_and(|(scope, bare)| { !bare.is_empty() - && available_commands.iter().any(|cmd| { - cmd.name == bare - && acp_thread::skill_source_str_from_meta(&cmd.meta) - == Some(scope) + && available_skills.iter().any(|skill| { + skill.name.as_ref() == bare && skill.source.as_ref() == scope }) }); @@ -765,7 +798,7 @@ impl MessageEditor { "The /{} command is not supported by {}.\n\nAvailable commands: {}", command_name, agent_id, - Self::format_available_commands(available_commands), + Self::format_available_commands(available_commands, available_skills), )); } } @@ -773,25 +806,23 @@ impl MessageEditor { Ok(()) } - /// Render the available-commands list for error messages. Skills + /// Render the available-commands list for error messages. Trusted native skills /// are shown in their qualified `/:` form so users /// see the exact text the popup would insert — otherwise the /// listing would contain confusing duplicates like `/foo, /foo` /// when both a global and a project-local skill share a name. /// Globals carry an empty scope and so render as `/:`. - fn format_available_commands(commands: &[acp::AvailableCommand]) -> String { - if commands.is_empty() { + fn format_available_commands( + commands: &[acp::AvailableCommand], + skills: &[AvailableSkill], + ) -> String { + if commands.is_empty() && skills.is_empty() { return "none".to_string(); } - commands + skills .iter() - .map(|cmd| { - if let Some(scope) = acp_thread::skill_source_str_from_meta(&cmd.meta) { - format!("/{}:{}", scope, cmd.name) - } else { - format!("/{}", cmd.name) - } - }) + .map(|skill| format!("/{}:{}", skill.source, skill.name)) + .chain(commands.iter().map(|command| format!("/{}", command.name))) .collect::>() .join(", ") } @@ -802,16 +833,23 @@ impl MessageEditor { cx: &mut Context, ) -> Task, Vec>)>> { let text = self.editor.read(cx).text(cx); - let available_commands = self - .session_capabilities - .read() - .available_commands() - .to_vec(); + let (available_commands, available_skills) = { + let session_capabilities = self.session_capabilities.read(); + ( + session_capabilities.available_commands().to_vec(), + session_capabilities.available_skills().to_vec(), + ) + }; let agent_id = self.agent_id.clone(); let build_task = self.build_content_blocks(full_mention_content, cx); cx.spawn(async move |_, _cx| { - Self::validate_slash_commands(&text, &available_commands, &agent_id)?; + Self::validate_slash_commands( + &text, + &available_commands, + &available_skills, + &agent_id, + )?; build_task.await }) } @@ -2113,7 +2151,7 @@ mod tests { use util::{path, paths::PathStyle, rel_path::rel_path}; use workspace::{AppState, Item, MultiWorkspace, Workspace}; - use crate::completion_provider::{AgentContextSelection, PromptContextType}; + use crate::completion_provider::{AgentContextSelection, AvailableSkill, PromptContextType}; use crate::{ conversation_view::tests::init_test, mention_set::insert_crease_for_mention, @@ -2122,57 +2160,86 @@ mod tests { }, }; + #[test] + fn test_session_capabilities_keep_commands_and_skills_separate() { + let skill_file_path = PathBuf::from("/tmp/SKILL.md"); + let skill = AvailableSkill { + name: "deploy".into(), + description: "Deploy the app".into(), + source: "".into(), + skill_file_path: skill_file_path.clone(), + }; + let session_capabilities = SessionCapabilities::new( + acp::PromptCapabilities::default(), + vec![acp::AvailableCommand::new("help", "Get help")], + vec![skill], + ); + + assert_eq!(session_capabilities.completion_commands().len(), 1); + let skills = session_capabilities.completion_skills(); + assert_eq!(skills.len(), 1); + assert_eq!(skills[0].name.as_ref(), "deploy"); + assert_eq!(skills[0].skill_file_path, skill_file_path); + } + #[test] fn test_validate_slash_commands_accepts_scope_qualified_skill() { let agent_id = AgentId::from("Zed"); - let make_skill_command = |name: &str, scope: &str| { - acp::AvailableCommand::new(name, "desc").meta(acp_thread::meta_with_skill_source(scope)) + let make_skill = |name: &str, source: &str| AvailableSkill { + name: name.into(), + description: "desc".into(), + source: source.into(), + skill_file_path: PathBuf::from(format!("/tmp/{source}-{name}/SKILL.md")), }; // Global skills carry an empty scope (so the popup inserts // `/:`); project-local skills carry their worktree root // name. The empty-scope encoding means a worktree literally // named `global` no longer collides with the global source. - let commands = vec![ - make_skill_command("deploy", ""), - make_skill_command("deploy", "zed"), - acp::AvailableCommand::new("help", "Get help"), - ]; + let commands = vec![acp::AvailableCommand::new("help", "Get help")]; + let skills = vec![make_skill("deploy", ""), make_skill("deploy", "zed")]; + let no_skills = Vec::new(); // Bare name still works (current behavior — the resolver // applies project-overrides-global for unqualified commands). - MessageEditor::validate_slash_commands("/deploy", &commands, &agent_id) + MessageEditor::validate_slash_commands("/deploy", &commands, &skills, &agent_id) .expect("bare /deploy should validate when a skill named `deploy` exists"); + MessageEditor::validate_slash_commands("/zed:deploy", &commands, &no_skills, &agent_id) + .expect_err("scope-qualified skills should require a first-class available skill"); // Scope-qualified forms both validate, each pointing at the // matching source. `/:` is the qualified form for a // global skill; `/:` is the qualified form // for a project-local skill. - MessageEditor::validate_slash_commands("/:deploy", &commands, &agent_id) + MessageEditor::validate_slash_commands("/:deploy", &commands, &skills, &agent_id) .expect("/:deploy should validate when a global skill named `deploy` exists"); - MessageEditor::validate_slash_commands("/zed:deploy", &commands, &agent_id).expect( + MessageEditor::validate_slash_commands("/zed:deploy", &commands, &skills, &agent_id).expect( "/zed:deploy should validate when a project skill named `deploy` exists in the `zed` worktree", ); // Hand-typed `/global:` is NOT an alias for `/:`. // It looks for a project-local skill from a worktree named // `global`, and fails when no such worktree skill exists. - MessageEditor::validate_slash_commands("/global:deploy", &commands, &agent_id).expect_err( - "/global:deploy should fail when no worktree named `global` has a `deploy` skill", - ); + MessageEditor::validate_slash_commands("/global:deploy", &commands, &skills, &agent_id) + .expect_err( + "/global:deploy should fail when no worktree named `global` has a `deploy` skill", + ); // The `:` separator is what distinguishes a skill scope from // an MCP server prefix — the dotted form `/zed.deploy` is an // MCP-style lookup, which doesn't match here. - MessageEditor::validate_slash_commands("/zed.deploy", &commands, &agent_id) + MessageEditor::validate_slash_commands("/zed.deploy", &commands, &skills, &agent_id) .expect_err("/zed.deploy (dotted) should be treated as an MCP-style prefix and fail"); // Wrong scope is rejected so the resolver doesn't silently // fall through when the user meant a skill. `zed:help` looks // like a skill scope qualifier but no skill named `help` // exists in the `zed` worktree (it's an MCP command). - let err = MessageEditor::validate_slash_commands("/zed:help", &commands, &agent_id) - .expect_err("/zed:help should fail — `help` is an MCP command, not a worktree skill"); + let err = + MessageEditor::validate_slash_commands("/zed:help", &commands, &skills, &agent_id) + .expect_err( + "/zed:help should fail — `help` is an MCP command, not a worktree skill", + ); let err_message = err.to_string(); assert!( err_message.contains("/zed:help"), @@ -2393,7 +2460,7 @@ mod tests { let project = Project::test(fs.clone(), ["/test".as_ref()], cx).await; let thread_store = None; - let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::new( + let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::from_acp_commands( acp::PromptCapabilities::default(), vec![], ))); @@ -2555,7 +2622,7 @@ mod tests { let mut cx = VisualTestContext::from_window(window.into(), cx); let thread_store = None; - let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::new( + let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::from_acp_commands( acp::PromptCapabilities::default(), vec![ acp::AvailableCommand::new("quick-math", "2 + 2 = 4 - 1 = 3"), @@ -2729,7 +2796,7 @@ mod tests { let mut cx = VisualTestContext::from_window(window.into(), cx); - let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::new( + let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::from_acp_commands( acp::PromptCapabilities::default(), vec![acp::AvailableCommand::new("hello", "Say hello")], ))); @@ -2884,7 +2951,7 @@ mod tests { } let thread_store = cx.new(|cx| ThreadStore::new(cx)); - let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::new( + let session_capabilities = Arc::new(RwLock::new(SessionCapabilities::from_acp_commands( acp::PromptCapabilities::default(), vec![], ))); diff --git a/crates/agent_ui/src/ui/mention_crease.rs b/crates/agent_ui/src/ui/mention_crease.rs index 49b294f3a77..4d4b282cf0b 100644 --- a/crates/agent_ui/src/ui/mention_crease.rs +++ b/crates/agent_ui/src/ui/mention_crease.rs @@ -180,6 +180,11 @@ fn open_mention_uri( MentionUri::Rule { id, .. } => { open_rule(workspace, id, window, cx); } + MentionUri::Skill { + skill_file_path, .. + } => { + open_skill_file(workspace, skill_file_path, window, cx); + } MentionUri::Fetch { url } => { cx.open_url(url.as_str()); } @@ -192,6 +197,25 @@ fn open_mention_uri( }); } +fn open_skill_file( + workspace: &mut Workspace, + skill_file_path: PathBuf, + window: &mut Window, + cx: &mut Context, +) { + workspace + .open_abs_path( + skill_file_path, + OpenOptions { + focus: Some(true), + ..Default::default() + }, + window, + cx, + ) + .detach_and_log_err(cx); +} + fn open_file( workspace: &mut Workspace, abs_path: PathBuf, diff --git a/crates/debugger_ui/src/session/running/console.rs b/crates/debugger_ui/src/session/running/console.rs index 5177fb259e7..746c765722e 100644 --- a/crates/debugger_ui/src/session/running/console.rs +++ b/crates/debugger_ui/src/session/running/console.rs @@ -676,6 +676,7 @@ impl ConsoleQueryBarCompletionProvider { confirm: None, source: project::CompletionSource::Custom, insert_text_mode: None, + group: None, }) }) .collect::>(); @@ -787,6 +788,7 @@ impl ConsoleQueryBarCompletionProvider { confirm: None, source: project::CompletionSource::Dap { sort_text }, insert_text_mode: None, + group: None, } }) .collect(); diff --git a/crates/edit_prediction_ui/src/rate_prediction_modal.rs b/crates/edit_prediction_ui/src/rate_prediction_modal.rs index e4ee608293d..8299f86054f 100644 --- a/crates/edit_prediction_ui/src/rate_prediction_modal.rs +++ b/crates/edit_prediction_ui/src/rate_prediction_modal.rs @@ -1400,6 +1400,7 @@ impl editor::CompletionProvider for FeedbackCompletionProvider { snippet_deduplication_key: None, insert_text_mode: None, confirm: None, + group: None, }) .collect(); diff --git a/crates/editor/src/code_completion_tests.rs b/crates/editor/src/code_completion_tests.rs index b3d05e23e57..cf8023cad8e 100644 --- a/crates/editor/src/code_completion_tests.rs +++ b/crates/editor/src/code_completion_tests.rs @@ -486,6 +486,7 @@ impl CompletionBuilder { confirm: None, match_start: None, snippet_deduplication_key: None, + group: None, } } } diff --git a/crates/editor/src/code_context_menus.rs b/crates/editor/src/code_context_menus.rs index e2cdf9c1203..e63f60f4e73 100644 --- a/crates/editor/src/code_context_menus.rs +++ b/crates/editor/src/code_context_menus.rs @@ -13,7 +13,7 @@ use markdown::{CopyButtonVisibility, Markdown, MarkdownElement}; use multi_buffer::Anchor; use ordered_float::OrderedFloat; use project::lsp_store::CompletionDocumentation; -use project::{CodeAction, Completion, TaskSourceKind}; +use project::{CodeAction, Completion, CompletionGroup, TaskSourceKind}; use project::{CompletionDisplayOptions, CompletionSource}; use task::DebugScenario; use task::TaskContext; @@ -29,8 +29,8 @@ use std::{ }; use task::ResolvedTask; use ui::{ - Color, IntoElement, ListItem, Pixels, Popover, ScrollAxes, Scrollbars, Styled, Tooltip, - WithScrollbar, prelude::*, + Divider, ListItem, ListSubHeader, Popover, ScrollAxes, Scrollbars, Tooltip, WithScrollbar, + prelude::*, }; use util::ResultExt; @@ -68,6 +68,26 @@ const MARKDOWN_CACHE_AFTER_ITEMS: usize = 2; const RESOLVE_BEFORE_ITEMS: usize = 4; const RESOLVE_AFTER_ITEMS: usize = 4; +#[derive(Clone, Debug)] +pub enum CompletionMenuEntry { + Match(StringMatch), + Divider, + GroupHeader(SharedString), +} + +impl CompletionMenuEntry { + pub fn as_match(&self) -> Option<&StringMatch> { + match self { + CompletionMenuEntry::Match(m) => Some(m), + CompletionMenuEntry::Divider | CompletionMenuEntry::GroupHeader(_) => None, + } + } + + pub fn is_selectable(&self) -> bool { + matches!(self, CompletionMenuEntry::Match(_)) + } +} + pub enum CodeContextMenu { Completions(CompletionsMenu), CodeActions(CodeActionsMenu), @@ -235,7 +255,7 @@ pub struct CompletionsMenu { /// String match candidate for each completion, grouped by `match_start`. match_candidates: Arc<[(Option, Vec)]>, /// Entries displayed in the menu, which is a filtered and sorted subset of `match_candidates`. - pub entries: Rc>>, + pub entries: Rc>>, pub selected_item: usize, filter_task: Task<()>, cancel_filter: Arc, @@ -376,6 +396,7 @@ impl CompletionsMenu { confirm: None, insert_text_mode: None, source: CompletionSource::Custom, + group: None, }) .collect(); @@ -390,11 +411,13 @@ impl CompletionsMenu { let entries = choices .iter() .enumerate() - .map(|(id, completion)| StringMatch { - candidate_id: id, - score: 1., - positions: vec![], - string: completion.clone(), + .map(|(id, completion)| { + CompletionMenuEntry::Match(StringMatch { + candidate_id: id, + score: 1., + positions: vec![], + string: completion.clone(), + }) }) .collect(); Self { @@ -430,12 +453,20 @@ impl CompletionsMenu { window: &mut Window, cx: &mut Context, ) { - let index = if self.scroll_handle.y_flipped() { - self.entries.borrow().len() - 1 + let entries = self.entries.borrow(); + if entries.is_empty() { + return; + } + let start = if self.scroll_handle.y_flipped() { + entries.len() - 1 } else { 0 }; - self.update_selection_index(index, provider, window, cx); + drop(entries); + let index = self.find_selectable_entry(start, !self.scroll_handle.y_flipped()); + if let Some(index) = index { + self.update_selection_index(index, provider, window, cx); + } } fn select_last( @@ -444,12 +475,20 @@ impl CompletionsMenu { window: &mut Window, cx: &mut Context, ) { - let index = if self.scroll_handle.y_flipped() { + let entries = self.entries.borrow(); + if entries.is_empty() { + return; + } + let start = if self.scroll_handle.y_flipped() { 0 } else { - self.entries.borrow().len() - 1 + entries.len() - 1 }; - self.update_selection_index(index, provider, window, cx); + drop(entries); + let index = self.find_selectable_entry(start, self.scroll_handle.y_flipped()); + if let Some(index) = index { + self.update_selection_index(index, provider, window, cx); + } } fn select_prev( @@ -494,18 +533,70 @@ impl CompletionsMenu { } fn prev_match_index(&self) -> usize { - if self.selected_item > 0 { + let entries = self.entries.borrow(); + let len = entries.len(); + if len == 0 { + return 0; + } + let mut index = if self.selected_item > 0 { self.selected_item - 1 } else { - self.entries.borrow().len() - 1 + len - 1 + }; + let start = index; + loop { + if entries[index].is_selectable() { + return index; + } + index = if index > 0 { index - 1 } else { len - 1 }; + if index == start { + return self.selected_item; + } } } fn next_match_index(&self) -> usize { - if self.selected_item + 1 < self.entries.borrow().len() { + let entries = self.entries.borrow(); + let len = entries.len(); + if len == 0 { + return 0; + } + let mut index = if self.selected_item + 1 < len { self.selected_item + 1 } else { 0 + }; + let start = index; + loop { + if entries[index].is_selectable() { + return index; + } + index = if index + 1 < len { index + 1 } else { 0 }; + if index == start { + return self.selected_item; + } + } + } + + fn find_selectable_entry(&self, start: usize, forward: bool) -> Option { + let entries = self.entries.borrow(); + let len = entries.len(); + if len == 0 { + return None; + } + let mut index = start; + loop { + if entries[index].is_selectable() { + return Some(index); + } + if forward { + index = if index + 1 < len { index + 1 } else { 0 }; + } else { + index = if index > 0 { index - 1 } else { len - 1 }; + } + if index == start { + return None; + } } } @@ -520,7 +611,7 @@ impl CompletionsMenu { if let Some(provider) = provider { let entries = self.entries.borrow(); let entry = if self.selected_item < entries.len() { - Some(&entries[self.selected_item]) + entries[self.selected_item].as_match() } else { None }; @@ -590,12 +681,19 @@ impl CompletionsMenu { // This filtering doesn't happen if the completions are currently being updated. let completions = self.completions.borrow(); let candidate_ids = entry_indices - .map(|i| entries[i].candidate_id) + .filter_map(|i| entries[i].as_match().map(|m| m.candidate_id)) .filter(|i| completions[*i].documentation.is_none()); // Current selection is always resolved even if it already has documentation, to handle // out-of-spec language servers that return more results later. - let selected_candidate_id = entries[self.selected_item].candidate_id; + let Some(selected_candidate_id) = entries[self.selected_item] + .as_match() + .map(|m| m.candidate_id) + else { + drop(entries); + drop(completions); + return; + }; let candidate_ids = iter::once(selected_candidate_id) .chain(candidate_ids.filter(|id| *id != selected_candidate_id)) .collect::>(); @@ -658,7 +756,7 @@ impl CompletionsMenu { if index >= entries.len() { return None; } - let candidate_id = entries[index].candidate_id; + let candidate_id = entries[index].as_match()?.candidate_id; let completions = self.completions.borrow(); match &completions[candidate_id].documentation { Some(CompletionDocumentation::MultiLineMarkdown(source)) if !source.is_empty() => self @@ -767,7 +865,7 @@ impl CompletionsMenu { } pub fn visible(&self) -> bool { - !self.entries.borrow().is_empty() + self.entries.borrow().iter().any(|e| e.as_match().is_some()) } fn origin(&self) -> ContextMenuOrigin { @@ -792,6 +890,7 @@ impl CompletionsMenu { .borrow() .iter() .enumerate() + .filter_map(|(ix, entry)| entry.as_match().map(|m| (ix, m))) .max_by_key(|(_, mat)| { let completion = &completions[mat.candidate_id]; let documentation = &completion.documentation; @@ -828,8 +927,23 @@ impl CompletionsMenu { entries.borrow()[range] .iter() .enumerate() - .map(|(ix, mat)| { + .map(|(ix, entry)| { let item_ix = start_ix + ix; + + let Some(mat) = entry.as_match() else { + return match entry { + CompletionMenuEntry::GroupHeader(label) => div() + .child(ListSubHeader::new(label.clone()).inset(true)) + .into_any_element(), + CompletionMenuEntry::Divider => h_flex() + .flex_1() + .size_full() + .child(Divider::horizontal()) + .into_any_element(), + CompletionMenuEntry::Match(_) => unreachable!(), + }; + }; + let completion = &completions_guard[mat.candidate_id]; let documentation = if show_completion_documentation { &completion.documentation @@ -1033,6 +1147,7 @@ impl CompletionsMenu { ) .end_slot::