diff --git a/Cargo.lock b/Cargo.lock index c6adf75f..ac335595 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10464,6 +10464,7 @@ dependencies = [ "gpui_platform", "image", "llm", + "lsp-types", "open", "rust-embed", "sandbox", diff --git a/crates/code_assistant/src/app/gpui.rs b/crates/code_assistant/src/app/gpui.rs index 2b007afa..eb526a09 100644 --- a/crates/code_assistant/src/app/gpui.rs +++ b/crates/code_assistant/src/app/gpui.rs @@ -92,6 +92,9 @@ pub fn run(config: AgentRunConfig) -> Result<()> { } } + // Populate the skill catalog for the `/skill` input-area popup. + gui_for_thread.refresh_skills(session_id.clone()); + let project_manager = Box::new(DefaultProjectManager::new()); let command_executor = Box::new(GpuiTerminalCommandExecutor::new(session_id.clone())); @@ -160,6 +163,9 @@ pub fn run(config: AgentRunConfig) -> Result<()> { error!("Failed to send UI event: {}", e); } } + + // Populate the skill catalog for the `/skill` input-area popup. + gui_for_thread.refresh_skills(session_id.clone()); } else { info!("No existing sessions found - showing empty state (no session view)"); // In GPUI mode, don't auto-create a session. The user can diff --git a/crates/code_assistant_core/resources/skills/samples/skill-creator/SKILL.md b/crates/code_assistant_core/resources/skills/samples/skill-creator/SKILL.md index edfdd9c8..28d6adbd 100644 --- a/crates/code_assistant_core/resources/skills/samples/skill-creator/SKILL.md +++ b/crates/code_assistant_core/resources/skills/samples/skill-creator/SKILL.md @@ -48,7 +48,13 @@ Frontmatter rules (enforced by the loader): at most 64 characters, and must **equal the directory name**. - `description` is required, at most 1024 characters. Write it as a routing signal: say *when* to use the skill, not just what it does. -- Other keys (license, etc.) are allowed and ignored for now. +- `disable-model-invocation` (optional, default `false`): when `true`, the + skill is hidden from the model-facing catalog and the `list_skills` tool so + the model never auto-invokes it. It stays loadable via `read_skill` (e.g. + when a user explicitly activates it) and visible in the settings UI. Use this + for skills that should only run on explicit user request. +- Other keys (license, allowed-tools, metadata, etc.) are allowed and ignored + for now. ## 3. Add resources (optional) diff --git a/crates/code_assistant_core/src/backend.rs b/crates/code_assistant_core/src/backend.rs index 373113c1..60373b38 100644 --- a/crates/code_assistant_core/src/backend.rs +++ b/crates/code_assistant_core/src/backend.rs @@ -1,6 +1,9 @@ use crate::config::{save_project, DefaultProjectManager}; use crate::persistence::{ChatMetadata, DraftAttachment, SessionModelConfig}; use crate::session::SessionManager; +use crate::skills::{ + discover_session_catalog, load_skill_payload, render_skill_invocation_message, SkillsConfig, +}; use crate::types::Project; use crate::ui::UserInterface; use crate::utils::content::content_blocks_from; @@ -53,6 +56,23 @@ pub enum BackendEvent { session_id: String, }, + /// List the skills available to a session (across project / user / system + /// scopes), for the input-area skill picker. Includes skills flagged + /// `disable-model-invocation`, since a user may invoke any of them. + ListSkills { + session_id: String, + }, + + /// User-initiated ("explicit") skill activation: load the skill's body and + /// inject it directly as a synthetic user message, then run the agent. No + /// `read_skill` round-trip is needed. + InvokeSkill { + session_id: String, + /// Scope token: the session's project name, or `:config:` / `:system:`. + scope: String, + name: String, + }, + // Model management SwitchModel { session_id: String, @@ -152,12 +172,30 @@ pub enum BackendEvent { }, } +/// A single entry in the input-area skill picker. +#[derive(Debug, Clone)] +pub struct SkillCatalogEntry { + pub name: String, + pub description: String, + /// Scope token to pass back in [`BackendEvent::InvokeSkill`] (the project + /// name, or `:config:` / `:system:`). + pub scope_token: String, + /// Human-readable scope label (`project` / `user` / `system`). + pub scope_label: String, +} + // Response from backend to UI #[derive(Debug, Clone)] pub enum BackendResponse { SessionCreated { session_id: String, }, + + /// The skills available to a session, for the input-area picker. + SkillsListed { + session_id: String, + skills: Vec, + }, #[allow(dead_code)] SessionDeleted { session_id: String, @@ -342,6 +380,26 @@ pub async fn handle_backend_events( Some(handle_request_pending_message_edit(&multi_session_manager, &session_id).await) } + BackendEvent::ListSkills { session_id } => { + Some(handle_list_skills(&multi_session_manager, &session_id).await) + } + + BackendEvent::InvokeSkill { + session_id, + scope, + name, + } => { + handle_invoke_skill( + &multi_session_manager, + &session_id, + &scope, + &name, + runtime_options.as_ref(), + &ui, + ) + .await + } + BackendEvent::SwitchModel { session_id, model_name, @@ -985,6 +1043,98 @@ async fn handle_request_pending_message_edit( } } +/// List the skills available to a session, deduped across scopes with the same +/// precedence as the system-prompt catalog (project > user > system). Includes +/// `disable-model-invocation` skills since a user may invoke any of them. +async fn handle_list_skills( + multi_session_manager: &Arc>, + session_id: &str, +) -> BackendResponse { + let project_name = { + let manager = multi_session_manager.lock().await; + match manager.get_session(session_id) { + Some(session) => session.session.config.initial_project.clone(), + None => { + return BackendResponse::Error { + message: format!("Session {session_id} not found"), + } + } + } + }; + + let config = SkillsConfig::load(); + let pm = DefaultProjectManager::new(); + + let skills: Vec = discover_session_catalog(&pm, &project_name, &config) + .into_iter() + .map(|(skill, scope_token)| SkillCatalogEntry { + name: skill.name, + description: skill.description, + scope_label: skill.scope.label().to_string(), + scope_token, + }) + .collect(); + + BackendResponse::SkillsListed { + session_id: session_id.to_string(), + skills, + } +} + +/// Handle a user-initiated skill activation: load the body and inject it as a +/// synthetic user message, then run the agent (reusing the normal user-message +/// path). The activation is recorded in the session's `active_skills` so the +/// compaction reminder can re-surface it if the body is later dropped. +async fn handle_invoke_skill( + multi_session_manager: &Arc>, + session_id: &str, + scope: &str, + name: &str, + runtime_options: &BackendRuntimeOptions, + ui: &Arc, +) -> Option { + debug!("InvokeSkill `{name}` (scope `{scope}`) for session {session_id}"); + + let config = SkillsConfig::load(); + let pm = DefaultProjectManager::new(); + let payload = match load_skill_payload(&pm, scope, name, &config) { + Ok(payload) => payload, + Err(e) => { + error!("Failed to load skill `{name}` for {session_id}: {e}"); + return Some(BackendResponse::Error { + message: format!("Failed to load skill `{name}`: {e}"), + }); + } + }; + + let message = render_skill_invocation_message(&payload); + + // Record the activation (deduped) so compaction can remind the model if the + // injected body is summarised away. + { + let mut manager = multi_session_manager.lock().await; + if let Some(session) = manager.get_session_mut(session_id) { + if !session.session.active_skills.iter().any(|s| s == name) { + session.session.active_skills.push(name.to_string()); + } + } + if let Err(e) = manager.save_session(session_id) { + warn!("Failed to persist active_skills for {session_id}: {e}"); + } + } + + handle_send_user_message( + multi_session_manager, + session_id, + &message, + &[], + None, + runtime_options, + ui, + ) + .await +} + async fn handle_switch_model( multi_session_manager: &Arc>, session_id: &str, diff --git a/crates/code_assistant_core/src/skills/config.rs b/crates/code_assistant_core/src/skills/config.rs index d1b68367..20af094f 100644 --- a/crates/code_assistant_core/src/skills/config.rs +++ b/crates/code_assistant_core/src/skills/config.rs @@ -130,6 +130,7 @@ mod tests { skill_md: PathBuf::from(skill_md), dir: PathBuf::from(skill_md).parent().unwrap().to_path_buf(), scope: SkillScope::Project, + disable_model_invocation: false, } } diff --git a/crates/code_assistant_core/src/skills/invoke.rs b/crates/code_assistant_core/src/skills/invoke.rs new file mode 100644 index 00000000..e1711f24 --- /dev/null +++ b/crates/code_assistant_core/src/skills/invoke.rs @@ -0,0 +1,254 @@ +//! Loading a skill's body for invocation, shared by the `read_skill` tool and +//! the user-initiated ("explicit") invocation path. +//! +//! Two consumers need the same resolution + read + truncation logic: +//! - the `read_skill` tool (model-initiated, progressive disclosure), and +//! - the UI invocation path (terminal `/skill`, GPUI completion, ACP command), +//! which injects the body directly as a synthetic user message so no extra +//! model round-trip is needed. + +use crate::config::ProjectManager; +use crate::skills::config::SkillsConfig; +use crate::skills::loader::discover_scope_skills_filtered; +use crate::skills::manifest::parse_skill_content; +use anyhow::{anyhow, Result}; +use std::path::PathBuf; + +/// Cap on the returned skill body to keep context size reasonable. +pub const MAX_BODY_LEN: usize = 64 * 1024; + +/// A resolved, read-from-disk skill body plus the metadata needed to render it +/// and to address its bundled resources. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SkillPayload { + /// The scope token the skill was loaded from (a project name, or + /// `:config:` / `:system:`) — pass it back to `read_files` for resources. + pub scope_token: String, + /// Human-readable scope label (`project` / `user` / `system`). + pub scope_label: String, + pub name: String, + /// The skill's directory, relative to the scope's sandbox root, so the + /// paths referenced in the body resolve directly with `read_files`. + pub dir: PathBuf, + /// The skill body (possibly truncated to [`MAX_BODY_LEN`]). + pub body: String, +} + +/// Resolve `name` in `scope_token` (a project name, or `:config:` / `:system:`) +/// and read its `SKILL.md` body. Honors the skills master switch and the +/// disabled list, but **not** `disable-model-invocation` — a user may load any +/// discoverable skill explicitly. +pub fn load_skill_payload( + project_manager: &dyn ProjectManager, + scope_token: &str, + name: &str, + config: &SkillsConfig, +) -> Result { + if !config.enabled { + return Err(anyhow!( + "Skills are disabled in this configuration (skills.json). \ + Enable them to load skills." + )); + } + + let resolved = discover_scope_skills_filtered(project_manager, scope_token, config) + .map_err(|e| anyhow!("Failed to resolve scope {}: {}", scope_token, e))?; + + let skill = resolved + .skills + .into_iter() + .find(|s| s.name == name) + .ok_or_else(|| { + anyhow!( + "No skill named `{}` was found in scope `{}`", + name, + scope_token + ) + })?; + + // Express the skill directory relative to the scope's sandbox root so the + // body's relative resource references resolve directly via read_files with + // the same scope token. + let dir = skill + .dir + .strip_prefix(&resolved.sandbox_root) + .unwrap_or(skill.dir.as_path()) + .to_path_buf(); + + let content = std::fs::read_to_string(&skill.skill_md) + .map_err(|e| anyhow!("Failed to read skill `{}`: {}", name, e))?; + let (_manifest, mut body) = parse_skill_content(&content)?; + + if body.len() > MAX_BODY_LEN { + // Truncate on a char boundary to avoid panicking on multi-byte text. + let mut end = MAX_BODY_LEN; + while end > 0 && !body.is_char_boundary(end) { + end -= 1; + } + body.truncate(end); + body.push_str("\n\n[... skill truncated to keep context size reasonable ...]"); + } + + Ok(SkillPayload { + scope_token: scope_token.to_string(), + scope_label: resolved.scope.label().to_string(), + name: skill.name, + dir, + body, + }) +} + +/// Render the skill body with the standard header describing its scope and how +/// to reach its bundled resources. This is the verbatim text the `read_skill` +/// tool returns, and the core of the synthetic invocation message. +pub fn render_skill_body_with_header(payload: &SkillPayload) -> String { + let dir = payload.dir.to_string_lossy().replace('\\', "/"); + format!( + "# Skill: {name} ({scope_label})\n\n\ + Bundled resources live under `{dir}/`; the paths referenced below are relative to that \ + directory. Read a resource with `read_files` (project `{scope}`, path \ + `{dir}/`) or run a bundled script with `execute_command`.\n\n\ + ---\n\n\ + {body}", + name = payload.name, + scope_label = payload.scope_label, + dir = dir, + scope = payload.scope_token, + body = payload.body, + ) +} + +/// Render the synthetic user message used when a user explicitly activates a +/// skill from the UI. The full body is embedded inline so the model can act on +/// it immediately, without a `read_skill` round-trip. +pub fn render_skill_invocation_message(payload: &SkillPayload) -> String { + format!( + "The user activated the **{name}** skill. Its full instructions are included below — \ + follow them for the current task. You do not need to call `read_skill` for this skill \ + again.\n\n\ + {body}\n\n\ + ---\n\n\ + Apply this skill to the user's request. Explore the skill's bundled resources (as \ + described above) only as needed.", + name = payload.name, + body = render_skill_body_with_header(payload), + ) +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::mocks::{MockExplorer, MockProjectManager}; + use std::collections::HashMap; + use std::fs; + use tempfile::tempdir; + + fn pm_for_root(project: &str, root: &std::path::Path) -> MockProjectManager { + let explorer = MockExplorer::new(HashMap::new(), None).with_root(root.to_path_buf()); + MockProjectManager::default().with_project_path( + project, + root.to_path_buf(), + Box::new(explorer), + ) + } + + fn write_skill(root: &std::path::Path, name: &str, description: &str, body: &str) { + let dir = root.join(".agents").join("skills").join(name); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {description}\n---\n{body}"), + ) + .unwrap(); + } + + #[test] + fn loads_payload_with_relative_dir() { + let dir = tempdir().unwrap(); + write_skill(dir.path(), "demo", "Demo skill.", "Step 1."); + let pm = pm_for_root("proj", dir.path()); + + let payload = + load_skill_payload(&pm, "proj", "demo", &SkillsConfig::default()).expect("loads"); + assert_eq!(payload.name, "demo"); + assert_eq!(payload.scope_token, "proj"); + assert_eq!(payload.scope_label, "project"); + assert_eq!(payload.dir, PathBuf::from(".agents/skills/demo")); + assert_eq!(payload.body, "Step 1."); + } + + #[test] + fn errors_when_missing() { + let dir = tempdir().unwrap(); + write_skill(dir.path(), "demo", "Demo.", "body"); + let pm = pm_for_root("proj", dir.path()); + + let err = load_skill_payload(&pm, "proj", "nope", &SkillsConfig::default()).unwrap_err(); + assert!(err.to_string().contains("No skill named")); + } + + #[test] + fn errors_when_disabled() { + let dir = tempdir().unwrap(); + write_skill(dir.path(), "demo", "Demo.", "body"); + let pm = pm_for_root("proj", dir.path()); + let config = SkillsConfig { + enabled: false, + ..Default::default() + }; + let err = load_skill_payload(&pm, "proj", "demo", &config).unwrap_err(); + assert!(err.to_string().contains("disabled")); + } + + #[test] + fn loads_model_invocation_disabled_skill() { + // Explicit user invocation may load a skill the model can't auto-invoke. + let dir = tempdir().unwrap(); + let skill_dir = dir.path().join(".agents").join("skills").join("internal"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: internal\ndescription: Hidden.\ndisable-model-invocation: true\n---\nBody.", + ) + .unwrap(); + let pm = pm_for_root("proj", dir.path()); + + let payload = + load_skill_payload(&pm, "proj", "internal", &SkillsConfig::default()).expect("loads"); + assert_eq!(payload.name, "internal"); + assert_eq!(payload.body, "Body."); + } + + #[test] + fn body_header_mentions_scope_and_resources() { + let payload = SkillPayload { + scope_token: "proj".to_string(), + scope_label: "project".to_string(), + name: "demo".to_string(), + dir: PathBuf::from(".agents/skills/demo"), + body: "Do the thing.".to_string(), + }; + let rendered = render_skill_body_with_header(&payload); + assert!(rendered.contains("# Skill: demo (project)")); + assert!(rendered.contains(".agents/skills/demo/")); + assert!(rendered.contains("project `proj`")); + assert!(rendered.contains("Do the thing.")); + } + + #[test] + fn invocation_message_embeds_body_and_framing() { + let payload = SkillPayload { + scope_token: ":config:".to_string(), + scope_label: "user".to_string(), + name: "security-review".to_string(), + dir: PathBuf::from("security-review"), + body: "Audit auth.".to_string(), + }; + let rendered = render_skill_invocation_message(&payload); + assert!(rendered.contains("activated the **security-review** skill")); + assert!(rendered.contains("# Skill: security-review (user)")); + assert!(rendered.contains("Audit auth.")); + // Tells the model not to call read_skill again. + assert!(rendered.contains("read_skill")); + } +} diff --git a/crates/code_assistant_core/src/skills/loader.rs b/crates/code_assistant_core/src/skills/loader.rs index 604a254e..767ab60e 100644 --- a/crates/code_assistant_core/src/skills/loader.rs +++ b/crates/code_assistant_core/src/skills/loader.rs @@ -11,12 +11,22 @@ use crate::config::{explorer_for_scope, ProjectManager, SCOPE_CONFIG, SCOPE_SYSTEM}; use crate::skills::config::SkillsConfig; use crate::skills::manifest::parse_skill_content; + use anyhow::Result; -use std::collections::HashMap; +use std::collections::{HashMap, VecDeque}; use std::fs; use std::path::{Path, PathBuf}; use tracing::warn; +/// Deepest a `SKILL.md`-containing directory may sit below a skills root. A +/// skill normally lives at depth 1 (a direct child of the root); we tolerate a +/// few levels of organizational nesting (e.g. `/category//`). +const MAX_SKILL_DEPTH: usize = 4; + +/// Upper bound on directories visited per root, guarding against pathological +/// trees (and symlink cycles, alongside the depth cap). +const MAX_DIRS_PER_ROOT: usize = 2000; + /// The file name that marks a directory as a skill. const SKILL_FILE: &str = "SKILL.md"; @@ -63,6 +73,31 @@ pub struct Skill { pub dir: PathBuf, /// The scope this skill was discovered in. pub scope: SkillScope, + /// When `true`, the skill is hidden from the model-facing catalog and the + /// `list_skills` tool (the model must not auto-invoke it), but it remains + /// loadable via `read_skill` and visible in the settings UI. Maps to the + /// `disable-model-invocation` frontmatter field. + pub disable_model_invocation: bool, +} + +impl Skill { + /// Whether this skill should be advertised to the model (system-prompt + /// catalog and the `list_skills` tool). Skills with + /// `disable-model-invocation: true` are hidden from the model but stay + /// loadable via `read_skill`. + pub fn is_model_invocable(&self) -> bool { + !self.disable_model_invocation + } +} + +/// Filter a slice of skills down to those that may be advertised to the model, +/// preserving order. Skills flagged `disable-model-invocation` are dropped. +pub fn model_invocable(skills: &[Skill]) -> Vec { + skills + .iter() + .filter(|s| s.is_model_invocable()) + .cloned() + .collect() } /// The skills found in a single scope, with the sandbox root `read_files` @@ -117,6 +152,38 @@ pub fn discover_all_skills(project_root: &Path) -> Vec { discover_all_skills_filtered(project_root, &SkillsConfig::load()) } +/// Discover the skills available to a session as `(skill, scope_token)` pairs, +/// where `scope_token` is what callers pass back to load the skill (the +/// `project_name`, or `:config:` / `:system:`). Deduped across scopes with the +/// same precedence as the catalog (project > user > system) and sorted by name. +/// +/// Unlike [`discover_all_skills`] (which takes a path), this resolves each +/// scope through the `project_manager`, so it agrees with how `read_skill` and +/// the invocation path resolve scope tokens. Includes skills flagged +/// `disable-model-invocation`, since a user may explicitly invoke any of them. +pub fn discover_session_catalog( + project_manager: &dyn ProjectManager, + project_name: &str, + config: &SkillsConfig, +) -> Vec<(Skill, String)> { + let mut seen = std::collections::HashSet::new(); + let mut out: Vec<(Skill, String)> = Vec::new(); + for token in [project_name, SCOPE_CONFIG, SCOPE_SYSTEM] { + let resolved = match discover_scope_skills_filtered(project_manager, token, config) { + Ok(resolved) => resolved, + Err(_) => continue, + }; + for skill in resolved.skills { + // First scope to define a name wins (project > user > system). + if seen.insert(skill.name.clone()) { + out.push((skill, token.to_string())); + } + } + } + out.sort_by(|a, b| a.0.name.cmp(&b.0.name)); + out +} + /// Discover the shared user (`:config:`) and bundled system (`:system:`) /// skills across both roots, **unfiltered** and deduped by precedence. /// @@ -152,41 +219,56 @@ pub fn discover_all_skills_filtered(project_root: &Path, config: &SkillsConfig) config.filter_skills(discovered) } -/// Discover skills directly under `skills_root` — each immediate subdirectory -/// containing a `SKILL.md` — tagging them with `scope`. Skills that fail to -/// parse (or whose `name` does not match the directory name) are skipped with -/// a warning. Results are sorted by name. +/// Discover skills under `skills_root`, tagging them with `scope`. A directory +/// is a skill when it contains a `SKILL.md`; discovery recurses through +/// non-skill directories (bounded by [`MAX_SKILL_DEPTH`] and +/// [`MAX_DIRS_PER_ROOT`]) so skills may be organized one or more levels deep +/// (e.g. `/category//SKILL.md`). +/// +/// Once a `SKILL.md` is found, that subtree is not descended into further (a +/// skill's bundled `references/`, `scripts/`, etc. may themselves contain a +/// `SKILL.md` that is not a separate skill). Hidden directories (e.g. the +/// `.system` cache under the user root) are skipped at every level. Skills that +/// fail to parse (or whose `name` does not match the directory name) are +/// skipped with a warning. Results are sorted by name. pub(crate) fn discover_skills_in(skills_root: &Path, scope: SkillScope) -> Vec { - let entries = match fs::read_dir(skills_root) { - Ok(entries) => entries, - // A missing skills directory is the common case, not an error. - Err(_) => return Vec::new(), - }; - let mut skills = Vec::new(); - for entry in entries.flatten() { - let dir = entry.path(); - if !dir.is_dir() { - continue; + let mut visited = 0usize; + let mut queue: VecDeque<(PathBuf, usize)> = VecDeque::new(); + + // Seed with the immediate children of the root (depth 1). + enqueue_subdirs(skills_root, 1, &mut queue); + + while let Some((dir, depth)) = queue.pop_front() { + visited += 1; + if visited > MAX_DIRS_PER_ROOT { + warn!( + "Reached the skill scan cap ({}) under {}; some skills may be skipped", + MAX_DIRS_PER_ROOT, + skills_root.display() + ); + break; } - // Skip hidden directories (e.g. the `.system` cache under the user root). - let is_hidden = dir - .file_name() - .and_then(|n| n.to_str()) - .map(|n| n.starts_with('.')) - .unwrap_or(true); - if is_hidden { + + // Skip hidden directories at any level (e.g. the `.system` cache under + // the user root, or VCS/metadata dirs nested under an org folder). + if is_hidden_dir(&dir) { continue; } let skill_md = dir.join(SKILL_FILE); - if !skill_md.is_file() { + if skill_md.is_file() { + match load_skill(&dir, &skill_md, scope) { + Ok(skill) => skills.push(skill), + Err(e) => warn!("Skipping skill at {}: {:#}", dir.display(), e), + } + // Do not descend into a skill's own subtree. continue; } - match load_skill(&dir, &skill_md, scope) { - Ok(skill) => skills.push(skill), - Err(e) => warn!("Skipping skill at {}: {:#}", dir.display(), e), + // Not a skill directory: descend, bounded by depth. + if depth < MAX_SKILL_DEPTH { + enqueue_subdirs(&dir, depth + 1, &mut queue); } } @@ -194,6 +276,29 @@ pub(crate) fn discover_skills_in(skills_root: &Path, scope: SkillScope) -> Vec bool { + dir.file_name() + .and_then(|n| n.to_str()) + .map(|n| n.starts_with('.')) + .unwrap_or(true) +} + +/// Push every immediate subdirectory of `parent` onto `queue` at `depth`. +/// A missing/unreadable directory is treated as empty. +fn enqueue_subdirs(parent: &Path, depth: usize, queue: &mut VecDeque<(PathBuf, usize)>) { + let entries = match fs::read_dir(parent) { + Ok(entries) => entries, + Err(_) => return, + }; + for entry in entries.flatten() { + let path = entry.path(); + if path.is_dir() { + queue.push_back((path, depth)); + } + } +} + /// Discover skills across the given `(root, scope)` pairs and resolve name /// collisions by precedence (lowest [`SkillScope::rank`] wins). Sorted by name. fn discover_across_roots(roots: &[(PathBuf, SkillScope)]) -> Vec { @@ -239,6 +344,7 @@ fn load_skill(dir: &Path, skill_md: &Path, scope: SkillScope) -> Result { skill_md: skill_md.to_path_buf(), dir: dir.to_path_buf(), scope, + disable_model_invocation: manifest.disable_model_invocation, }) } @@ -301,6 +407,76 @@ mod tests { assert_eq!(names, vec!["real"]); } + #[test] + fn discovers_nested_skill_one_level_deep() { + let dir = tempdir().unwrap(); + // /category/my-skill/SKILL.md — one organizational level deep. + write_skill( + &dir.path().join("category"), + "my-skill", + "my-skill", + "Nested.", + ); + + let skills = discover_skills_in(dir.path(), SkillScope::Project); + let names: Vec<_> = skills.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["my-skill"]); + } + + #[test] + fn does_not_descend_into_a_skill_subtree() { + let dir = tempdir().unwrap(); + // A real skill ... + write_skill(dir.path(), "outer", "outer", "Outer skill."); + // ... whose bundled resources happen to contain another SKILL.md must + // not surface as a second skill. + let nested = dir.path().join("outer").join("references").join("inner"); + fs::create_dir_all(&nested).unwrap(); + fs::write( + nested.join(SKILL_FILE), + "---\nname: inner\ndescription: Not a real skill.\n---\nbody", + ) + .unwrap(); + + let skills = discover_skills_in(dir.path(), SkillScope::Project); + let names: Vec<_> = skills.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["outer"]); + } + + #[test] + fn respects_max_scan_depth() { + let dir = tempdir().unwrap(); + // Depth 4 (a/b/c/) is discovered; depth 5 is not. + write_skill( + &dir.path().join("a").join("b").join("c"), + "deep-ok", + "deep-ok", + "Reachable.", + ); + write_skill( + &dir.path().join("a2").join("b").join("c").join("d"), + "too-deep", + "too-deep", + "Beyond the depth cap.", + ); + + let skills = discover_skills_in(dir.path(), SkillScope::Project); + let names: Vec<_> = skills.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["deep-ok"]); + } + + #[test] + fn skips_hidden_directories_when_recursing() { + let dir = tempdir().unwrap(); + // A skill inside a hidden organizational directory is skipped at depth. + write_skill(&dir.path().join(".hidden"), "ghost", "ghost", "Hidden."); + write_skill(dir.path(), "real", "real", "Fine."); + + let skills = discover_skills_in(dir.path(), SkillScope::Project); + let names: Vec<_> = skills.iter().map(|s| s.name.as_str()).collect(); + assert_eq!(names, vec!["real"]); + } + #[test] fn higher_precedence_scope_shadows_lower() { let project = tempdir().unwrap(); diff --git a/crates/code_assistant_core/src/skills/manifest.rs b/crates/code_assistant_core/src/skills/manifest.rs index 1ce43a78..4f0de5af 100644 --- a/crates/code_assistant_core/src/skills/manifest.rs +++ b/crates/code_assistant_core/src/skills/manifest.rs @@ -19,6 +19,12 @@ const MAX_DESCRIPTION_LEN: usize = 1024; pub struct SkillManifest { pub name: String, pub description: String, + /// When `true`, the skill is hidden from the model-facing catalog and the + /// `list_skills` tool: the model must not auto-invoke it. It remains + /// loadable via `read_skill` (e.g. when a user explicitly activates it) + /// and visible in the settings UI. Maps to the Anthropic Agent Skills + /// `disable-model-invocation` frontmatter field. + pub disable_model_invocation: bool, } /// Raw frontmatter shape. All fields are optional so deserialization never @@ -30,6 +36,9 @@ struct SkillFrontmatter { name: Option, #[serde(default)] description: Option, + /// Anthropic spec: when set, the model must not auto-invoke this skill. + #[serde(default, rename = "disable-model-invocation")] + disable_model_invocation: Option, } /// Parse the contents of a `SKILL.md` file into its validated [`SkillManifest`] @@ -43,11 +52,19 @@ pub fn parse_skill_content(content: &str) -> Result<(SkillManifest, String)> { let name = parsed.name.unwrap_or_default().trim().to_string(); let description = parsed.description.unwrap_or_default().trim().to_string(); + let disable_model_invocation = parsed.disable_model_invocation.unwrap_or(false); validate_name(&name)?; validate_description(&description)?; - Ok((SkillManifest { name, description }, body)) + Ok(( + SkillManifest { + name, + description, + disable_model_invocation, + }, + body, + )) } /// Split a `SKILL.md` into its frontmatter and body. @@ -124,6 +141,29 @@ mod tests { assert_eq!(manifest.description, "A multi-line\ndescription."); } + #[test] + fn disable_model_invocation_defaults_to_false() { + let content = "---\nname: my-skill\ndescription: ok\n---\nbody"; + let (manifest, _body) = parse_skill_content(content).expect("should parse"); + assert!(!manifest.disable_model_invocation); + } + + #[test] + fn parses_disable_model_invocation_true() { + let content = + "---\nname: my-skill\ndescription: ok\ndisable-model-invocation: true\n---\nbody"; + let (manifest, _body) = parse_skill_content(content).expect("should parse"); + assert!(manifest.disable_model_invocation); + } + + #[test] + fn parses_disable_model_invocation_false() { + let content = + "---\nname: my-skill\ndescription: ok\ndisable-model-invocation: false\n---\nbody"; + let (manifest, _body) = parse_skill_content(content).expect("should parse"); + assert!(!manifest.disable_model_invocation); + } + #[test] fn tolerates_unknown_frontmatter_keys() { let content = "---\nname: my-skill\ndescription: ok\nlicense: Apache-2.0\nallowed-tools: read_files\n---\nbody"; diff --git a/crates/code_assistant_core/src/skills/mod.rs b/crates/code_assistant_core/src/skills/mod.rs index f25e828d..c0e21233 100644 --- a/crates/code_assistant_core/src/skills/mod.rs +++ b/crates/code_assistant_core/src/skills/mod.rs @@ -12,15 +12,21 @@ pub mod bundled; pub mod config; +pub mod invoke; pub mod loader; pub mod manifest; pub mod render; pub use bundled::install_system_skills; pub use config::{skills_config_path, SkillsConfig}; +pub use invoke::{ + load_skill_payload, render_skill_body_with_header, render_skill_invocation_message, + SkillPayload, MAX_BODY_LEN, +}; pub use loader::{ discover_all_skills, discover_all_skills_filtered, discover_config_and_system_skills, - discover_scope_skills, discover_scope_skills_filtered, ScopeSkills, Skill, SkillScope, + discover_scope_skills, discover_scope_skills_filtered, discover_session_catalog, + model_invocable, ScopeSkills, Skill, SkillScope, }; pub use manifest::{parse_skill_content, SkillManifest}; pub use render::render_skills_section; diff --git a/crates/code_assistant_core/src/skills/render.rs b/crates/code_assistant_core/src/skills/render.rs index aa99cf48..c438a707 100644 --- a/crates/code_assistant_core/src/skills/render.rs +++ b/crates/code_assistant_core/src/skills/render.rs @@ -17,6 +17,9 @@ const MAX_SHOWN: usize = 20; /// reflect which skills have been loaded), so the system prompt stays stable /// across turns and remains eligible for provider prompt caching. pub fn render_skills_section(project: &str, skills: &[Skill]) -> Option { + // Skills flagged `disable-model-invocation` are never advertised to the + // model; they remain loadable via `read_skill` (e.g. user-initiated). + let skills = crate::skills::model_invocable(skills); if skills.is_empty() { return None; } @@ -67,6 +70,7 @@ mod tests { skill_md: PathBuf::from(format!(".agents/skills/{name}/SKILL.md")), dir: PathBuf::from(format!(".agents/skills/{name}")), scope: crate::skills::SkillScope::Project, + disable_model_invocation: false, } } @@ -100,6 +104,39 @@ mod tests { assert!(rendered.contains(":system:")); } + #[test] + fn hides_model_invocation_disabled_skills() { + let mut hidden = skill("internal-only", "User-invocable only."); + hidden.disable_model_invocation = true; + let visible = skill("public", "Normal skill."); + + let rendered = render_skills_section("p", &[hidden, visible]).expect("should render"); + assert!(rendered.contains("- public (project): Normal skill.")); + assert!(!rendered.contains("internal-only")); + } + + #[test] + fn renders_nothing_when_all_skills_are_model_disabled() { + let mut hidden = skill("internal-only", "User-invocable only."); + hidden.disable_model_invocation = true; + assert!(render_skills_section("p", &[hidden]).is_none()); + } + + #[test] + fn overflow_counts_only_visible_skills() { + // One hidden skill plus exactly MAX_SHOWN visible ones must not report + // overflow (the hidden one is filtered before counting). + let mut skills: Vec = (0..MAX_SHOWN) + .map(|i| skill(&format!("skill-{i:02}"), "desc")) + .collect(); + let mut hidden = skill("hidden", "desc"); + hidden.disable_model_invocation = true; + skills.push(hidden); + + let rendered = render_skills_section("p", &skills).expect("should render"); + assert!(!rendered.contains("more skill(s) are not shown")); + } + #[test] fn renders_overflow_marker() { let skills: Vec = (0..(MAX_SHOWN + 3)) diff --git a/crates/code_assistant_core/src/tools/impls/list_skills.rs b/crates/code_assistant_core/src/tools/impls/list_skills.rs index 947cd1ce..160da34f 100644 --- a/crates/code_assistant_core/src/tools/impls/list_skills.rs +++ b/crates/code_assistant_core/src/tools/impls/list_skills.rs @@ -139,6 +139,9 @@ impl Tool for ListSkillsTool { let skills = resolved .skills .into_iter() + // Skills flagged `disable-model-invocation` are hidden from the + // model-facing catalog; they remain loadable via `read_skill`. + .filter(|s| s.is_model_invocable()) .filter(|s| match &query { Some(q) => { s.name.to_lowercase().contains(q) || s.description.to_lowercase().contains(q) @@ -187,6 +190,19 @@ mod tests { .unwrap(); } + fn write_model_disabled_skill(root: &std::path::Path, name: &str, description: &str) { + let dir = root.join(".agents").join("skills").join(name); + fs::create_dir_all(&dir).unwrap(); + fs::write( + dir.join("SKILL.md"), + format!( + "---\nname: {name}\ndescription: {description}\n\ + disable-model-invocation: true\n---\nbody" + ), + ) + .unwrap(); + } + async fn run(fixture: &mut ToolTestFixture, params: serde_json::Value) -> String { let mut context = fixture.context(); let registry = crate::tools::test_registry(); @@ -227,6 +243,19 @@ mod tests { assert!(!output.contains("security-review")); } + #[tokio::test] + async fn hides_model_invocation_disabled_skills() { + let dir = tempdir().unwrap(); + write_skill(dir.path(), "visible", "Shown to the model."); + write_model_disabled_skill(dir.path(), "internal-only", "Hidden from the model."); + + let mut fixture = fixture_for_root("my-project", dir.path()); + let output = run(&mut fixture, json!({ "project": "my-project" })).await; + + assert!(output.contains("- visible: Shown to the model.")); + assert!(!output.contains("internal-only")); + } + #[tokio::test] async fn reports_empty_catalog() { let dir = tempdir().unwrap(); diff --git a/crates/code_assistant_core/src/tools/impls/read_skill.rs b/crates/code_assistant_core/src/tools/impls/read_skill.rs index b189fabc..09bc405a 100644 --- a/crates/code_assistant_core/src/tools/impls/read_skill.rs +++ b/crates/code_assistant_core/src/tools/impls/read_skill.rs @@ -1,16 +1,15 @@ -use crate::skills::{discover_scope_skills_filtered, parse_skill_content, SkillsConfig}; +use crate::skills::{ + load_skill_payload, render_skill_body_with_header, SkillPayload, SkillsConfig, +}; use crate::tools::core::{ capabilities, Render, ResourcesTracker, Tool, ToolContext, ToolResult, ToolSpec, }; use crate::tools::ToolServicesAccess; -use anyhow::{anyhow, Result}; +use anyhow::Result; use serde::{Deserialize, Serialize}; use serde_json::json; use std::path::PathBuf; -/// Cap on the returned skill body to keep context size reasonable. -const MAX_BODY_LEN: usize = 64 * 1024; - // Input type for the read_skill tool #[derive(Deserialize, Serialize)] pub struct ReadSkillInput { @@ -35,26 +34,25 @@ pub struct ReadSkillOutput { pub body: String, } +impl ReadSkillOutput { + fn payload(&self) -> SkillPayload { + SkillPayload { + scope_token: self.scope.clone(), + scope_label: self.scope_label.clone(), + name: self.name.clone(), + dir: self.dir.clone(), + body: self.body.clone(), + } + } +} + impl Render for ReadSkillOutput { fn status(&self) -> String { format!("Loaded skill '{}'", self.name) } fn render(&self, _tracker: &mut ResourcesTracker) -> String { - let dir = self.dir.to_string_lossy().replace('\\', "/"); - format!( - "# Skill: {name} ({scope_label})\n\n\ - Bundled resources live under `{dir}/`; the paths referenced below are relative to \ - that directory. Read a resource with `read_files` (project `{scope}`, path \ - `{dir}/`) or run a bundled script with `execute_command`.\n\n\ - ---\n\n\ - {body}", - name = self.name, - scope_label = self.scope_label, - dir = dir, - scope = self.scope, - body = self.body, - ) + render_skill_body_with_header(&self.payload()) } } @@ -117,60 +115,19 @@ impl Tool for ReadSkillTool { input: &mut Self::Input, ) -> Result { let config = SkillsConfig::load(); - if !config.enabled { - return Err(anyhow!( - "Skills are disabled in this configuration (skills.json). \ - Enable them to load skills." - )); - } - - // Resolve the requested scope (project name or :config:/:system:) to - // its skills and sandbox root. - let resolved = - discover_scope_skills_filtered(context.project_manager(), &input.project, &config) - .map_err(|e| anyhow!("Failed to resolve scope {}: {}", input.project, e))?; - - let skill = resolved - .skills - .into_iter() - .find(|s| s.name == input.name) - .ok_or_else(|| { - anyhow!( - "No skill named `{}` was found in scope `{}`", - input.name, - input.project - ) - })?; - - // Express the skill directory relative to the scope's sandbox root so - // the body's relative resource references resolve directly via - // read_files with the same scope token. - let dir = skill - .dir - .strip_prefix(&resolved.sandbox_root) - .unwrap_or(skill.dir.as_path()) - .to_path_buf(); - - let content = std::fs::read_to_string(&skill.skill_md) - .map_err(|e| anyhow!("Failed to read skill `{}`: {}", input.name, e))?; - let (_manifest, mut body) = parse_skill_content(&content)?; - - if body.len() > MAX_BODY_LEN { - // Truncate on a char boundary to avoid panicking on multi-byte text. - let mut end = MAX_BODY_LEN; - while end > 0 && !body.is_char_boundary(end) { - end -= 1; - } - body.truncate(end); - body.push_str("\n\n[... skill truncated to keep context size reasonable ...]"); - } + let payload = load_skill_payload( + context.project_manager(), + &input.project, + &input.name, + &config, + )?; Ok(ReadSkillOutput { - scope: input.project.clone(), - scope_label: resolved.scope.label().to_string(), - name: skill.name, - dir, - body, + scope: payload.scope_token, + scope_label: payload.scope_label, + name: payload.name, + dir: payload.dir, + body: payload.body, }) } } @@ -235,6 +192,36 @@ mod tests { Ok(()) } + #[tokio::test] + async fn loads_model_invocation_disabled_skill() -> Result<()> { + // A skill hidden from the model catalog must still be loadable via + // read_skill (e.g. when the user explicitly activates it). + let dir = tempdir().unwrap(); + let skill_dir = dir.path().join(".agents").join("skills").join("internal"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: internal\ndescription: Hidden.\ndisable-model-invocation: true\n---\nBody here.", + ) + .unwrap(); + + let mut fixture = fixture_for_root("my-project", dir.path()); + let mut context = fixture.context(); + + let registry = crate::tools::test_registry(); + let tool = registry.get("read_skill").expect("read_skill registered"); + + let mut params = json!({ "project": "my-project", "name": "internal" }); + let result = tool.invoke(&mut context, &mut params).await?; + + assert!(result.is_success()); + let mut tracker = ResourcesTracker::new(); + let output = result.as_render().render(&mut tracker); + assert!(output.contains("# Skill: internal (project)")); + assert!(output.contains("Body here.")); + Ok(()) + } + #[tokio::test] async fn errors_when_skill_missing() -> Result<()> { let dir = tempdir().unwrap(); diff --git a/crates/ui_acp/src/agent.rs b/crates/ui_acp/src/agent.rs index d49a724d..f61b2bc1 100644 --- a/crates/ui_acp/src/agent.rs +++ b/crates/ui_acp/src/agent.rs @@ -11,16 +11,34 @@ use crate::{ACPTerminalCommandExecutor, ACPUserUI, AcpProjectManager, ClientConn use code_assistant_core::config::{DefaultProjectManager, ProjectManager}; use code_assistant_core::persistence::SessionModelConfig; use code_assistant_core::session::{SessionConfig, SessionManager}; +use code_assistant_core::skills::{ + discover_session_catalog, load_skill_payload, render_skill_invocation_message, SkillsConfig, +}; use code_assistant_core::ui::UserInterface; use command_executor::{CommandExecutor, DefaultCommandExecutor}; use llm::factory::create_llm_client_from_model; use llm::provider_config::ConfigurationSystem; -use tokio::sync::mpsc; +use tokio::sync::{mpsc, oneshot}; /// Config option id used for the model selector exposed via session config /// options. This is what the client echoes back in `session/set_config_option`. const MODEL_CONFIG_ID: &str = "model"; +/// If the prompt is a bare `/` slash command (the form clients send when +/// the user runs an advertised command), return ``. Only the first text +/// block is considered, and only when it is a single `/word` with no extra text. +fn slash_command_token(prompt: &[acp::ContentBlock]) -> Option { + let text = prompt.iter().find_map(|block| match block { + acp::ContentBlock::Text(t) => Some(t.text.trim().to_string()), + _ => None, + })?; + let rest = text.strip_prefix('/')?; + if rest.is_empty() || rest.contains(char::is_whitespace) { + return None; + } + Some(rest.to_string()) +} + /// Pending session that hasn't been persisted yet (deferred until first prompt). #[derive(Clone)] struct PendingSession { @@ -238,6 +256,39 @@ impl AgentState { Ok(acp::AuthenticateResponse::new()) } + /// Build the ACP `AvailableCommand`s advertising the session's skills, so + /// clients (e.g. Zed) can expose them as slash commands. Resolved through + /// `project_manager` so project/user/system scopes are deduped consistently. + fn skill_commands( + project_manager: &dyn ProjectManager, + project_name: &str, + ) -> Vec { + let config = SkillsConfig::load(); + discover_session_catalog(project_manager, project_name, &config) + .into_iter() + .map(|(skill, _scope_token)| { + acp::AvailableCommand::new( + skill.name.clone(), + format!("[{}] {}", skill.scope.label(), skill.description), + ) + }) + .collect() + } + + /// Push an `available_commands_update` for `session_id` to the client. + fn send_available_commands( + &self, + session_id: &acp::SessionId, + commands: Vec, + ) { + let update = acp::SessionUpdate::AvailableCommandsUpdate( + acp::AvailableCommandsUpdate::new(commands), + ); + let notification = acp::SessionNotification::new(session_id.clone(), update); + let (ack_tx, _ack_rx) = oneshot::channel(); + let _ = self.session_update_tx.send((notification, ack_tx)); + } + pub async fn handle_new_session( &self, arguments: acp::NewSessionRequest, @@ -258,6 +309,8 @@ impl AgentState { let session_model_config = SessionModelConfig::new(selected_model_name); + let initial_project = session_config.initial_project.clone(); + { let mut pending = self.pending_sessions.lock().await; pending.insert( @@ -269,6 +322,15 @@ impl AgentState { ); } + // Advertise the available skills as slash commands. Project-scoped + // resolution is best-effort here (the session isn't materialized yet); + // user/system skills always resolve, and run_prompt re-advertises with + // full project resolution on the first turn. + let commands = Self::skill_commands(&DefaultProjectManager::new(), &initial_project); + if !commands.is_empty() { + self.send_available_commands(&acp::SessionId::new(session_id.clone()), commands); + } + tracing::info!("ACP: Created pending session: {}", session_id); { @@ -301,7 +363,7 @@ impl AgentState { } // Replay message history as session/update events - let (tool_syntax, messages, base_path, stored_model_config) = { + let (tool_syntax, messages, base_path, stored_model_config, initial_project) = { let manager = self.session_manager.lock().await; let session_instance = manager .get_session(&arguments.session_id.0) @@ -312,9 +374,16 @@ impl AgentState { session_instance.session.get_active_messages_cloned(), session_instance.session.config.init_path.clone(), session_instance.session.model_config.clone(), + session_instance.session.config.initial_project.clone(), ) }; + // Advertise the session's skills as slash commands. + let commands = Self::skill_commands(&DefaultProjectManager::new(), &initial_project); + if !commands.is_empty() { + self.send_available_commands(&arguments.session_id, commands); + } + let context_limit = stored_model_config .as_ref() .and_then(|cfg| Self::model_context_limit(&cfg.model_name)); @@ -673,6 +742,14 @@ impl AgentState { .and_then(|session| session.session.config.init_path.clone()) }; + let initial_project = { + let manager = self.session_manager.lock().await; + manager + .get_session(&arguments.session_id.0) + .map(|session| session.session.config.initial_project.clone()) + .unwrap_or_default() + }; + // Resolve model config first so we can compute the context-window limit // before building the UI. let session_model_config = { @@ -709,7 +786,11 @@ impl AgentState { let ui: Arc = acp_ui.clone(); - let content_blocks = + // Detect a `/skill` slash command before converting (clients send the + // advertised command name as prompt text). + let skill_command = slash_command_token(&arguments.prompt); + + let mut content_blocks = convert_prompt_to_content_blocks(arguments.prompt, base_path.as_deref()); // Create LLM client @@ -751,6 +832,53 @@ impl AgentState { Box::new(DefaultProjectManager::new()) }; + // Refresh the advertised skill commands using the real project manager + // (so project-scoped skills resolve), then translate an explicit + // `/skill` invocation into a synthetic user message with the body + // inlined — no `read_skill` round-trip needed. + { + let commands = Self::skill_commands(project_manager.as_ref(), &initial_project); + if !commands.is_empty() { + self.send_available_commands(&arguments.session_id, commands); + } + } + if let Some(name) = skill_command { + let config = SkillsConfig::load(); + if let Some((skill, scope_token)) = + discover_session_catalog(project_manager.as_ref(), &initial_project, &config) + .into_iter() + .find(|(s, _)| s.name == name) + { + match load_skill_payload( + project_manager.as_ref(), + &scope_token, + &skill.name, + &config, + ) { + Ok(payload) => { + let message = render_skill_invocation_message(&payload); + content_blocks = vec![llm::ContentBlock::new_text(message)]; + // Record the activation so compaction can remind the model. + let mut manager = self.session_manager.lock().await; + if let Some(session) = manager.get_session_mut(&arguments.session_id.0) { + if !session + .session + .active_skills + .iter() + .any(|s| s == &skill.name) + { + session.session.active_skills.push(skill.name.clone()); + } + } + let _ = manager.save_session(&arguments.session_id.0); + } + Err(e) => { + tracing::warn!("ACP: failed to load skill `{name}`: {e}"); + } + } + } + } + let command_executor: Box = if terminal_supported { tracing::info!( "ACP: Using ACPTerminalCommandExecutor for session {}", @@ -918,3 +1046,46 @@ impl AgentState { } } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn detects_bare_slash_command() { + let prompt = vec![acp::ContentBlock::Text(acp::TextContent::new( + "/pdf-extraction", + ))]; + assert_eq!( + slash_command_token(&prompt), + Some("pdf-extraction".to_string()) + ); + } + + #[test] + fn trims_whitespace_around_command() { + let prompt = vec![acp::ContentBlock::Text(acp::TextContent::new( + " /review ", + ))]; + assert_eq!(slash_command_token(&prompt), Some("review".to_string())); + } + + #[test] + fn ignores_non_command_prompts() { + // Ordinary message. + let prompt = vec![acp::ContentBlock::Text(acp::TextContent::new( + "hello there", + ))]; + assert_eq!(slash_command_token(&prompt), None); + // Slash with trailing text is not a bare command token. + let prompt = vec![acp::ContentBlock::Text(acp::TextContent::new( + "/skill do it", + ))]; + assert_eq!(slash_command_token(&prompt), None); + // Bare slash. + let prompt = vec![acp::ContentBlock::Text(acp::TextContent::new("/"))]; + assert_eq!(slash_command_token(&prompt), None); + // Empty prompt. + assert_eq!(slash_command_token(&[]), None); + } +} diff --git a/crates/ui_gpui/Cargo.toml b/crates/ui_gpui/Cargo.toml index e7951326..f70039df 100644 --- a/crates/ui_gpui/Cargo.toml +++ b/crates/ui_gpui/Cargo.toml @@ -17,6 +17,9 @@ terminal_view = { path = "../terminal_view" } gpui = "0.2.2" gpui_platform = { version = "0.1", features = ["font-kit"] } gpui-component = "0.5.2" +# Must match the version gpui-component uses so CompletionProvider trait types +# (CompletionItem, CompletionResponse, …) are identical. +lsp-types = "0.97" rust-embed = { version = "8.9", features = ["include-exclude"] } tokio = { version = "1.48", features = ["full"] } diff --git a/crates/ui_gpui/src/app/backend.rs b/crates/ui_gpui/src/app/backend.rs index b5e4d12b..893b1f04 100644 --- a/crates/ui_gpui/src/app/backend.rs +++ b/crates/ui_gpui/src/app/backend.rs @@ -20,9 +20,11 @@ impl Gpui { let _ = sender.try_send(BackendEvent::ListSessions); // Load the newly created session to connect it to the UI let _ = sender.try_send(BackendEvent::LoadSession { - session_id, + session_id: session_id.clone(), edit_until_node_id: None, }); + // Refresh the skill catalog for the `/skill` picker. + let _ = sender.try_send(BackendEvent::ListSkills { session_id }); } } @@ -60,6 +62,15 @@ impl Gpui { *self.chat_sessions.lock().unwrap() = sessions.clone(); self.push_event(UiEvent::UpdateChatList { sessions }); } + BackendResponse::SkillsListed { session_id, skills } => { + debug!( + "Received BackendResponse::SkillsListed for {} ({} skills)", + session_id, + skills.len() + ); + // Cache for the `/skill` input-area completion + invocation. + *self.skills.lock().unwrap() = skills; + } BackendResponse::Error { message } => { warn!("Backend error: {}", message); // Display the error to the user @@ -325,9 +336,10 @@ impl Gpui { if let Some(sender) = self.backend_event_sender.lock().unwrap().as_ref() { let _ = sender.try_send(BackendEvent::ListSessions); let _ = sender.try_send(BackendEvent::LoadSession { - session_id, + session_id: session_id.clone(), edit_until_node_id: None, }); + let _ = sender.try_send(BackendEvent::ListSkills { session_id }); } } BackendResponse::ProjectPersisted { project_name } => { diff --git a/crates/ui_gpui/src/input/mod.rs b/crates/ui_gpui/src/input/mod.rs index 2568a72d..275a624b 100644 --- a/crates/ui_gpui/src/input/mod.rs +++ b/crates/ui_gpui/src/input/mod.rs @@ -1,6 +1,7 @@ pub mod attachment; pub mod model_selector; pub mod sandbox_selector; +pub mod skill_completion; pub mod worktree_selector; use super::shared::file_icons; @@ -28,6 +29,9 @@ pub enum InputAreaEvent { /// If set, this message creates a new branch from this parent node branch_parent_id: Option, }, + /// User submitted a bare `/` that matches a known skill. + /// Carries the scope token and skill name to activate. + SkillInvoked { scope: String, name: String }, /// Content changed (for draft saving) ContentChanged { content: String, @@ -100,6 +104,13 @@ impl InputArea { .placeholder("Type your message...") }); + // Wire the `/skill` autocomplete provider onto the input's LSP slot. + text_input.update(cx, |state, _cx| { + state.lsp.completion_provider = Some(std::rc::Rc::new( + skill_completion::SkillCompletionProvider::new(), + )); + }); + // Subscribe to text input events let input_subscription = cx.subscribe_in(&text_input, window, Self::on_input_event); @@ -294,6 +305,27 @@ impl InputArea { fn on_enter(&mut self, action: &Enter, window: &mut Window, cx: &mut Context) { if !action.secondary { let current_text = self.text_input.read(cx).value().to_string(); + + // Slash-command handling: classify the input against the skill + // catalog so Enter does the intuitive thing. + let skills = cx.global::().skills(); + match skill_completion::slash_completion_state(¤t_text, &skills) { + skill_completion::SlashState::MenuOpen => { + // The completion menu is open. Let Enter propagate to the + // inner input so it confirms the highlighted skill (which + // inserts `/`); do not submit or insert a newline. + return; + } + skill_completion::SlashState::Invoke { scope, name } => { + cx.emit(InputAreaEvent::ClearDraftRequested); + cx.emit(InputAreaEvent::SkillInvoked { scope, name }); + self.clear(window, cx); + cx.stop_propagation(); + return; + } + skill_completion::SlashState::None => {} + } + // Don't submit empty messages if current_text.trim().is_empty() && self.attachments.is_empty() { // Stop propagation so InputState::enter() doesn't insert a newline diff --git a/crates/ui_gpui/src/input/skill_completion.rs b/crates/ui_gpui/src/input/skill_completion.rs new file mode 100644 index 00000000..85ecbb09 --- /dev/null +++ b/crates/ui_gpui/src/input/skill_completion.rs @@ -0,0 +1,230 @@ +//! A `CompletionProvider` for the input composer that autocompletes skills. +//! +//! When the current line starts with `/`, this provider offers the +//! session's skills (read from the [`crate::Gpui`] global, populated via +//! `BackendEvent::ListSkills`). Selecting one replaces the typed `/...` with +//! `/`. On submit, [`super::InputArea::on_enter`] recognizes a +//! lone `/` that matches a known skill and translates it into a +//! skill invocation (see [`skill_invocation_from_input`]). + +use std::time::Duration; + +use anyhow::Result; +use code_assistant_core::backend::SkillCatalogEntry; +use gpui::{Context, Task, Window}; +use gpui_component::input::{InputState, RopeExt}; +use gpui_component::Rope; +use lsp_types::{ + CompletionContext, CompletionItem, CompletionItemKind, CompletionResponse, CompletionTextEdit, + TextEdit, +}; + +use crate::Gpui; + +/// Completion provider that suggests skills after a leading `/`. +#[derive(Default)] +pub struct SkillCompletionProvider; + +impl SkillCompletionProvider { + pub fn new() -> Self { + Self + } +} + +/// Extract the current line's text up to `offset` (cursor) as a string. +fn line_prefix(text: &Rope, offset: usize) -> (usize, String) { + let full = text.to_string(); + let offset = offset.min(full.len()); + let line_start = full[..offset].rfind('\n').map(|i| i + 1).unwrap_or(0); + (line_start, full[line_start..offset].to_string()) +} + +impl gpui_component::input::CompletionProvider for SkillCompletionProvider { + fn completions( + &self, + text: &Rope, + offset: usize, + _trigger: CompletionContext, + _window: &mut Window, + cx: &mut Context, + ) -> Task> { + let (line_start, prefix) = line_prefix(text, offset); + + // Only offer skill completions on a line that begins with '/'. + if !prefix.starts_with('/') { + return Task::ready(Ok(CompletionResponse::Array(vec![]))); + } + + // The query is the text after the leading '/'. Skill names are a single + // token ([a-z0-9-]); anything with a space is not a skill query. + let query = prefix[1..].to_lowercase(); + if query.contains(char::is_whitespace) { + return Task::ready(Ok(CompletionResponse::Array(vec![]))); + } + + let skills = cx.global::().skills(); + + // Replace the whole typed `/...` token with `/` on accept. + let start = text.offset_to_position(line_start); + let end = text.offset_to_position(offset); + + let items: Vec = skills + .iter() + .filter(|s| { + query.is_empty() + || s.name.to_lowercase().contains(&query) + || s.description.to_lowercase().contains(&query) + }) + .map(|s| CompletionItem { + label: s.name.clone(), + kind: Some(CompletionItemKind::SNIPPET), + detail: Some(format!("({}) {}", s.scope_label, s.description)), + filter_text: Some(s.name.clone()), + text_edit: Some(CompletionTextEdit::Edit(TextEdit { + range: lsp_types::Range { start, end }, + new_text: format!("/{}", s.name), + })), + ..Default::default() + }) + .collect(); + + Task::ready(Ok(CompletionResponse::Array(items))) + } + + fn is_completion_trigger( + &self, + _offset: usize, + _new_text: &str, + _cx: &mut Context, + ) -> bool { + // Be permissive: `completions` gates on the leading-'/' line prefix and + // returns an empty list (which hides the menu) outside a slash context. + true + } + + fn inline_completion_debounce(&self) -> Duration { + Duration::from_millis(0) + } +} + +/// What pressing Enter on the composer should do, given the current input and +/// the available skills. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SlashState { + /// The input is a complete `/` — activate it. + Invoke { scope: String, name: String }, + /// The input is a `/` for which the completion menu is showing at + /// least one entry. Enter should confirm the highlighted entry (handled by + /// the inner input's completion menu), not submit the message. + MenuOpen, + /// Not a slash-completion context — submit as an ordinary message. + None, +} + +/// Whether a `/` line would show at least one completion entry. Mirrors +/// the filter in [`SkillCompletionProvider::completions`] so callers can tell +/// when the completion menu is open. +fn query_matches(query: &str, skills: &[SkillCatalogEntry]) -> bool { + let q = query.to_lowercase(); + skills.iter().any(|s| { + q.is_empty() + || s.name.to_lowercase().contains(&q) + || s.description.to_lowercase().contains(&q) + }) +} + +/// Classify the composer input for Enter handling (see [`SlashState`]). +pub fn slash_completion_state(input: &str, skills: &[SkillCatalogEntry]) -> SlashState { + let trimmed = input.trim(); + let Some(rest) = trimmed.strip_prefix('/') else { + return SlashState::None; + }; + // A slash command is a single bare token (skill names never contain spaces). + if rest.contains(char::is_whitespace) { + return SlashState::None; + } + // A fully-typed, known skill name activates immediately. + if let Some(s) = skills.iter().find(|s| s.name == rest) { + return SlashState::Invoke { + scope: s.scope_token.clone(), + name: s.name.clone(), + }; + } + // Otherwise, if the menu is showing matches, Enter belongs to the menu. + if query_matches(rest, skills) { + SlashState::MenuOpen + } else { + SlashState::None + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn entry(name: &str, scope_token: &str) -> SkillCatalogEntry { + SkillCatalogEntry { + name: name.to_string(), + description: "desc".to_string(), + scope_token: scope_token.to_string(), + scope_label: "project".to_string(), + } + } + + #[test] + fn exact_name_invokes() { + let skills = vec![entry("pdf-extraction", "proj"), entry("review", ":config:")]; + assert_eq!( + slash_completion_state("/review", &skills), + SlashState::Invoke { + scope: ":config:".to_string(), + name: "review".to_string() + } + ); + assert_eq!( + slash_completion_state(" /pdf-extraction ", &skills), + SlashState::Invoke { + scope: "proj".to_string(), + name: "pdf-extraction".to_string() + } + ); + } + + #[test] + fn prefix_query_keeps_menu_open() { + let skills = vec![entry("pdf-extraction", "proj"), entry("review", ":config:")]; + // A bare slash shows all skills. + assert_eq!(slash_completion_state("/", &skills), SlashState::MenuOpen); + // A partial token that matches at least one skill keeps the menu open. + assert_eq!(slash_completion_state("/pd", &skills), SlashState::MenuOpen); + assert_eq!( + slash_completion_state("/rev", &skills), + SlashState::MenuOpen + ); + } + + #[test] + fn ignores_non_skill_input() { + let skills = vec![entry("pdf-extraction", "proj")]; + // Ordinary message. + assert_eq!( + slash_completion_state("hello there", &skills), + SlashState::None + ); + // Slash but no matching skill → submit as a normal message. + assert_eq!(slash_completion_state("/zzz", &skills), SlashState::None); + // Slash with trailing text is not a bare skill token. + assert_eq!( + slash_completion_state("/pdf-extraction now", &skills), + SlashState::None + ); + } + + #[test] + fn line_prefix_extracts_current_line() { + let rope = Rope::from("first line\n/sec"); + let (start, prefix) = line_prefix(&rope, rope.to_string().len()); + assert_eq!(prefix, "/sec"); + assert_eq!(start, "first line\n".len()); + } +} diff --git a/crates/ui_gpui/src/lib.rs b/crates/ui_gpui/src/lib.rs index 8774788c..0285912e 100644 --- a/crates/ui_gpui/src/lib.rs +++ b/crates/ui_gpui/src/lib.rs @@ -118,6 +118,11 @@ pub struct Gpui { /// Incremented each time config files (providers.json / models.json) change on disk. /// Components compare their locally cached generation with this to know when to reload. config_generation: Arc, + + /// Skills available to the current session, cached for the `/skill` + /// input-area completion and submit-time invocation. Refreshed on session + /// load via `BackendEvent::ListSkills` / `BackendResponse::SkillsListed`. + skills: Arc>>, } /// State for a pending message edit (for branching) @@ -416,6 +421,8 @@ impl Gpui { )), config_generation: Arc::new(std::sync::atomic::AtomicU64::new(0)), + + skills: Arc::new(Mutex::new(Vec::new())), } } @@ -664,6 +671,21 @@ impl Gpui { (event_rx, response_tx) } + /// Snapshot of the skills available to the current session. + pub(crate) fn skills(&self) -> Vec { + self.skills.lock().unwrap().clone() + } + + /// Request the skill catalog for `session_id` from the backend. The + /// response (`BackendResponse::SkillsListed`) populates the cached catalog + /// used by the `/skill` input-area completion. Used by startup paths that + /// connect a session without going through the in-app `LoadSession` flow. + pub fn refresh_skills(&self, session_id: String) { + if let Some(sender) = self.backend_event_sender.lock().unwrap().as_ref() { + let _ = sender.try_send(BackendEvent::ListSkills { session_id }); + } + } + // Helper to add an event to the queue fn push_event(&self, event: UiEvent) { let sender = self.event_sender.lock().unwrap().clone(); diff --git a/crates/ui_gpui/src/main_screen/mod.rs b/crates/ui_gpui/src/main_screen/mod.rs index 9c76e12c..58839087 100644 --- a/crates/ui_gpui/src/main_screen/mod.rs +++ b/crates/ui_gpui/src/main_screen/mod.rs @@ -457,6 +457,21 @@ impl MainScreen { } } + InputAreaEvent::SkillInvoked { scope, name } => { + if let Some(session_id) = &self.current_session_id { + let gpui = cx + .try_global::() + .expect("Failed to obtain Gpui global"); + if let Some(sender) = gpui.backend_event_sender.lock().unwrap().as_ref() { + let _ = sender.try_send(BackendEvent::InvokeSkill { + session_id: session_id.clone(), + scope: scope.clone(), + name: name.clone(), + }); + } + } + } + InputAreaEvent::ContentChanged { content, attachments, @@ -680,6 +695,10 @@ impl MainScreen { session_id: session_id.clone(), edit_until_node_id, }); + // Refresh the skill catalog for the `/skill` picker. + let _ = sender.try_send(BackendEvent::ListSkills { + session_id: session_id.clone(), + }); } SessionSidebarEvent::NewSessionRequested { name, diff --git a/crates/ui_terminal/src/app.rs b/crates/ui_terminal/src/app.rs index 9b6dc2f3..0ccbeb3b 100644 --- a/crates/ui_terminal/src/app.rs +++ b/crates/ui_terminal/src/app.rs @@ -49,8 +49,12 @@ async fn handle_slash_prefix_changed( // If the stack is empty (or the top is not the root command list), // open the root command list popup. Sub-popups remain on the stack // and ignore the prefix change (the top-of-stack popup decides). + if state.popup_stack.depth() == 0 { - state.popup_stack.push(Box::new(CommandListPopup::new())); + let skills = state.skills.clone(); + state + .popup_stack + .push(Box::new(CommandListPopup::with_skills(skills))); } state.popup_stack.set_query(&q); } @@ -186,6 +190,67 @@ async fn handle_command_result( let session_id = app_state.lock().await.current_session_id.clone(); session_id.map(|session_id| BackendEvent::CompactContext { session_id }) } + + CommandResult::OpenSkillPicker => { + // Open the skill picker built from the cached catalog. If no skills + // are available, show an informational message instead. + let mut state = app_state.lock().await; + if state.skills.is_empty() { + state.set_info_message(Some( + "No skills are available for this session.".to_string(), + )); + } else { + let skills = state.skills.clone(); + state.popup_stack.push(Box::new( + crate::slash_popup::SkillPickerPopup::from_entries(skills), + )); + } + None + } + CommandResult::InvokeSkill { scope, name } => { + let session_id = app_state.lock().await.current_session_id.clone(); + let Some(session_id) = session_id else { + app_state + .lock() + .await + .set_info_message(Some("No active session to activate a skill".to_string())); + return None; + }; + + // Resolve the scope token: use the explicit one from the picker, or + // look it up in the cached catalog by name (inline `/skill `). + let resolved_scope = match scope { + Some(scope) => Some(scope), + None => app_state + .lock() + .await + .skills + .iter() + .find(|s| s.name == name) + .map(|s| s.scope_token.clone()), + }; + + match resolved_scope { + Some(scope) => { + app_state + .lock() + .await + .set_info_message(Some(format!("Activating skill: {name}"))); + Some(BackendEvent::InvokeSkill { + session_id, + scope, + name, + }) + } + None => { + app_state + .lock() + .await + .set_info_message(Some(format!("No skill named '{name}' was found"))); + None + } + } + } CommandResult::InvalidCommand(error) => { app_state .lock() @@ -482,6 +547,30 @@ async fn event_loop( .await; } } + + KeyEventResult::OpenSkillPicker => { + // Inline `/skill` (popup not active): reuse the + // command-result handler to open the picker. + let _ = handle_command_result( + crate::commands::CommandResult::OpenSkillPicker, + &app_state, + &renderer, + &backend_event_tx, + ) + .await; + } + KeyEventResult::InvokeSkill { scope, name } => { + if let Some(event) = handle_command_result( + crate::commands::CommandResult::InvokeSkill { scope, name }, + &app_state, + &renderer, + &backend_event_tx, + ) + .await + { + let _ = backend_event_tx.send(event).await; + } + } KeyEventResult::SlashPrefixChanged(query) => { handle_slash_prefix_changed( query, @@ -766,6 +855,11 @@ impl TerminalTuiApp { // Kick off a session list refresh (optional but useful) let _ = backend_event_tx.try_send(BackendEvent::ListSessions); + // Fetch the skill catalog for the `/skill` picker. + let _ = backend_event_tx.try_send(BackendEvent::ListSkills { + session_id: session_id.clone(), + }); + // Spawn a background task to process UI events from display fragments { let terminal_ui_clone = terminal_ui.clone(); @@ -790,6 +884,13 @@ impl TerminalTuiApp { }) .await; } + BackendResponse::SkillsListed { + session_id: _, + skills, + } => { + // Cache the catalog for the `/skill` picker. + app_state_clone.lock().await.skills = skills; + } BackendResponse::PendingMessageUpdated { session_id: _, message, diff --git a/crates/ui_terminal/src/commands.rs b/crates/ui_terminal/src/commands.rs index 2e5ee48a..8dd20e46 100644 --- a/crates/ui_terminal/src/commands.rs +++ b/crates/ui_terminal/src/commands.rs @@ -50,6 +50,11 @@ pub fn all_commands() -> &'static [SlashCommand] { aliases: &[], description: "Summarize and compact the conversation context", }, + SlashCommand { + name: "skill", + aliases: &[], + description: "Activate a skill: /skill (or pick from the list)", + }, ] } @@ -72,10 +77,17 @@ pub enum CommandResult { InvalidCommand(String), /// Toggle plan rendering mode TogglePlan, + /// Clear conversation context ClearContext, /// Compact (summarise) conversation context CompactContext, + /// Open the skill picker popup. + OpenSkillPicker, + /// Activate a skill by name. `scope` is the scope token (project name, or + /// `:config:` / `:system:`); `None` means resolve it from the cached + /// catalog by name. + InvokeSkill { scope: Option, name: String }, } /// Process slash commands in terminal UI @@ -110,6 +122,16 @@ impl CommandProcessor { "plan" => CommandResult::TogglePlan, "clear" => CommandResult::ClearContext, "compact" => CommandResult::CompactContext, + "skill" => { + if parts.len() > 1 { + CommandResult::InvokeSkill { + scope: None, + name: parts[1..].join(" "), + } + } else { + CommandResult::OpenSkillPicker + } + } _ => CommandResult::InvalidCommand(format!("Unknown command: /{}", parts[0])), } } diff --git a/crates/ui_terminal/src/input.rs b/crates/ui_terminal/src/input.rs index 91d5f566..ab848117 100644 --- a/crates/ui_terminal/src/input.rs +++ b/crates/ui_terminal/src/input.rs @@ -34,10 +34,16 @@ pub enum KeyEventResult { ShowCurrentModel, /// Toggle plan rendering mode TogglePlan, + /// Clear conversation context ClearContext, /// Compact (summarise) conversation context CompactContext, + /// Open the skill picker popup. + OpenSkillPicker, + /// Activate a skill. `scope` is the scope token, or `None` to resolve it + /// from the cached catalog by name. + InvokeSkill { scope: Option, name: String }, /// The slash-prefix on the current input line changed. /// /// `Some("")` means the user just typed `/` (open the popup at root). @@ -189,6 +195,10 @@ impl InputManager { CommandResult::TogglePlan => KeyEventResult::TogglePlan, CommandResult::ClearContext => KeyEventResult::ClearContext, CommandResult::CompactContext => KeyEventResult::CompactContext, + CommandResult::OpenSkillPicker => KeyEventResult::OpenSkillPicker, + CommandResult::InvokeSkill { scope, name } => { + KeyEventResult::InvokeSkill { scope, name } + } CommandResult::InvalidCommand(error) => { KeyEventResult::ShowInfo(format!("Error: {error}")) } diff --git a/crates/ui_terminal/src/slash_popup/command_list.rs b/crates/ui_terminal/src/slash_popup/command_list.rs index 116175e6..ff6542f7 100644 --- a/crates/ui_terminal/src/slash_popup/command_list.rs +++ b/crates/ui_terminal/src/slash_popup/command_list.rs @@ -1,7 +1,9 @@ //! Root popup that lists all known slash commands. use crate::commands::{all_commands, CommandResult}; +use crate::slash_popup::skill_picker::SkillPickerPopup; use crate::slash_popup::{PopupAction, PopupRow, SlashPopup}; +use code_assistant_core::backend::SkillCatalogEntry; pub struct CommandListPopup { /// All rows the popup knows about, before filtering. @@ -14,6 +16,8 @@ pub struct CommandListPopup { visible_indices: Vec, /// Highlighted row inside `visible_rows`. selected: usize, + /// Cached skill catalog used to build the `/skill` sub-popup. + skills: Vec, } impl Default for CommandListPopup { @@ -24,6 +28,12 @@ impl Default for CommandListPopup { impl CommandListPopup { pub fn new() -> Self { + Self::with_skills(Vec::new()) + } + + /// Construct the command list with a cached skill catalog so activating + /// `/skill` can open a populated picker without a backend round-trip. + pub fn with_skills(skills: Vec) -> Self { let mut all_rows = Vec::new(); let mut all_names = Vec::new(); for cmd in all_commands() { @@ -42,6 +52,7 @@ impl CommandListPopup { visible_rows, visible_indices, selected: 0, + skills, } } @@ -55,7 +66,7 @@ impl CommandListPopup { /// Returns true if the command should open a sub-popup when activated without /// arguments (instead of running immediately). fn command_has_submenu(name: &str) -> bool { - matches!(name, "model") + matches!(name, "model" | "skill") } /// Build the [`PopupAction`] for activating a slash command by name. @@ -119,6 +130,12 @@ impl SlashPopup for CommandListPopup { fn activate(&self) -> PopupAction { match self.selected_name() { + // The skill picker needs the session-scoped catalog, which the + // static `dispatch_command` table can't provide; build it here from + // the cached entries instead. + Some("skill") => PopupAction::Push(Box::new(SkillPickerPopup::from_entries( + self.skills.clone(), + ))), Some(name) => dispatch_command(name), None => PopupAction::Continue, } @@ -190,7 +207,7 @@ mod tests { fn non_submenu_commands_are_not_marked() { let popup = CommandListPopup::new(); for row in popup.rows() { - if row.label != "/model" { + if row.label != "/model" && row.label != "/skill" { assert!( !row.has_submenu, "{} should not be marked as having a sub-menu", @@ -200,6 +217,13 @@ mod tests { } } + #[test] + fn skill_command_opens_a_submenu() { + let popup = CommandListPopup::new(); + let skill_row = popup.rows().iter().find(|r| r.label == "/skill").unwrap(); + assert!(skill_row.has_submenu); + } + #[test] fn enter_on_clear_commits_clear_context() { let mut stack = PopupStack::new(); diff --git a/crates/ui_terminal/src/slash_popup/mod.rs b/crates/ui_terminal/src/slash_popup/mod.rs index aad2497a..11198025 100644 --- a/crates/ui_terminal/src/slash_popup/mod.rs +++ b/crates/ui_terminal/src/slash_popup/mod.rs @@ -17,9 +17,11 @@ pub mod command_list; pub mod model_picker; +pub mod skill_picker; pub use command_list::CommandListPopup; pub use model_picker::ModelPickerPopup; +pub use skill_picker::SkillPickerPopup; use crate::commands::CommandResult; use crossterm::event::{KeyCode, KeyEvent}; diff --git a/crates/ui_terminal/src/slash_popup/skill_picker.rs b/crates/ui_terminal/src/slash_popup/skill_picker.rs new file mode 100644 index 00000000..f8ad36cd --- /dev/null +++ b/crates/ui_terminal/src/slash_popup/skill_picker.rs @@ -0,0 +1,182 @@ +//! Sub-popup that lets the user pick a skill to activate. +//! +//! Pushed by [`super::command_list::CommandListPopup`] when `/skill` is +//! activated. Unlike the model picker (which reads global config), the skills +//! are session-scoped and supplied from [`crate::state::AppState::skills`], +//! which is populated from a [`code_assistant_core::backend::BackendEvent::ListSkills`] +//! request. + +use crate::commands::CommandResult; +use crate::slash_popup::{PopupAction, PopupRow, SlashPopup}; +use code_assistant_core::backend::SkillCatalogEntry; + +pub struct SkillPickerPopup { + /// All skill entries (parallel to `all_rows`), used to dispatch on activate. + all_entries: Vec, + /// All rows, before filtering. + all_rows: Vec, + /// Currently visible rows. + visible_rows: Vec, + /// Indices into `all_entries`/`all_rows` for the visible rows. + visible_indices: Vec, + /// Highlighted row in `visible_rows`. + selected: usize, +} + +impl SkillPickerPopup { + pub fn from_entries(entries: Vec) -> Self { + let all_rows: Vec = entries + .iter() + .map(|e| PopupRow { + label: e.name.clone(), + description: format!("({}) {}", e.scope_label, e.description), + has_submenu: false, + }) + .collect(); + let visible_indices = (0..all_rows.len()).collect::>(); + let visible_rows = all_rows.clone(); + Self { + all_entries: entries, + all_rows, + visible_rows, + visible_indices, + selected: 0, + } + } + + /// The entry for the currently highlighted visible row. + fn selected_entry(&self) -> Option<&SkillCatalogEntry> { + let idx = *self.visible_indices.get(self.selected)?; + self.all_entries.get(idx) + } +} + +impl SlashPopup for SkillPickerPopup { + fn title(&self) -> &str { + "Activate skill" + } + + fn set_query(&mut self, query: &str) { + // Substring match (case-insensitive) on name + description so partial + // typing surfaces results. + self.visible_rows.clear(); + self.visible_indices.clear(); + let q = query.to_lowercase(); + for (i, entry) in self.all_entries.iter().enumerate() { + if q.is_empty() + || entry.name.to_lowercase().contains(&q) + || entry.description.to_lowercase().contains(&q) + { + self.visible_rows.push(self.all_rows[i].clone()); + self.visible_indices.push(i); + } + } + if self.visible_rows.is_empty() { + self.selected = 0; + } else if self.selected >= self.visible_rows.len() { + self.selected = self.visible_rows.len() - 1; + } + } + + fn rows(&self) -> &[PopupRow] { + &self.visible_rows + } + + fn selected(&self) -> usize { + self.selected + } + + fn move_selection(&mut self, delta: i32) { + let len = self.visible_rows.len() as i32; + if len == 0 { + return; + } + self.selected = (self.selected as i32 + delta).rem_euclid(len) as usize; + } + + fn activate(&self) -> PopupAction { + match self.selected_entry() { + Some(entry) => PopupAction::Commit(CommandResult::InvokeSkill { + scope: Some(entry.scope_token.clone()), + name: entry.name.clone(), + }), + None => PopupAction::Continue, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::slash_popup::PopupStack; + use crossterm::event::{KeyCode, KeyEvent, KeyModifiers}; + + fn key(code: KeyCode) -> KeyEvent { + KeyEvent::new(code, KeyModifiers::NONE) + } + + fn entry(name: &str, scope_token: &str, scope_label: &str, desc: &str) -> SkillCatalogEntry { + SkillCatalogEntry { + name: name.to_string(), + description: desc.to_string(), + scope_token: scope_token.to_string(), + scope_label: scope_label.to_string(), + } + } + + #[test] + fn lists_entries_with_scope_in_description() { + let popup = SkillPickerPopup::from_entries(vec![ + entry("pdf", "proj", "project", "Extract PDFs."), + entry("review", ":config:", "user", "Audit auth."), + ]); + let labels: Vec<&str> = popup.rows().iter().map(|r| r.label.as_str()).collect(); + assert_eq!(labels, vec!["pdf", "review"]); + assert!(popup.rows()[0].description.contains("(project)")); + assert!(popup.rows()[1].description.contains("(user)")); + } + + #[test] + fn enter_commits_invoke_skill_with_scope_token() { + let mut stack = PopupStack::new(); + stack.push(Box::new(SkillPickerPopup::from_entries(vec![ + entry("pdf", "proj", "project", "Extract PDFs."), + entry("review", ":config:", "user", "Audit auth."), + ]))); + stack.handle_key(key(KeyCode::Down)); // move to "review" + let result = stack.handle_key(key(KeyCode::Enter)); + assert!(matches!( + result, + Some(CommandResult::InvokeSkill { ref scope, ref name }) + if scope.as_deref() == Some(":config:") && name == "review" + )); + assert!(!stack.is_active()); + } + + #[test] + fn filter_matches_name_and_description() { + let mut popup = SkillPickerPopup::from_entries(vec![ + entry( + "pdf-extraction", + "proj", + "project", + "Extract text from PDFs.", + ), + entry( + "security-review", + ":config:", + "user", + "Audit auth and crypto.", + ), + ]); + popup.set_query("auth"); + let labels: Vec<&str> = popup.rows().iter().map(|r| r.label.as_str()).collect(); + assert_eq!(labels, vec!["security-review"]); + } + + #[test] + fn empty_catalog_activate_is_noop() { + let popup = SkillPickerPopup::from_entries(vec![]); + assert!(matches!(popup.activate(), PopupAction::Continue)); + } +} diff --git a/crates/ui_terminal/src/state.rs b/crates/ui_terminal/src/state.rs index d3595827..49359cd2 100644 --- a/crates/ui_terminal/src/state.rs +++ b/crates/ui_terminal/src/state.rs @@ -1,4 +1,5 @@ use crate::slash_popup::PopupStack; +use code_assistant_core::backend::SkillCatalogEntry; use code_assistant_core::persistence::ChatMetadata; use code_assistant_core::session::instance::SessionActivityState; use code_assistant_core::types::PlanState; @@ -25,6 +26,8 @@ pub struct AppState { pub current_model: Option, pub info_message: Option, pub current_sandbox_policy: Option, + /// Skills available to the current session, cached for the `/skill` picker. + pub skills: Vec, /// Slash-command popup stack. Empty stack ↔ no popup visible. pub popup_stack: PopupStack, } @@ -46,6 +49,7 @@ impl AppState { current_model: None, info_message: None, current_sandbox_policy: None, + skills: Vec::new(), popup_stack: PopupStack::new(), } }