From 95ba01eba8e2cb8b16fe65263ceafef8f09cf4ed Mon Sep 17 00:00:00 2001 From: Ben Gao Date: Wed, 27 May 2026 21:43:45 +0800 Subject: [PATCH 1/3] fix(skills): align skills API with TUI command behavior - Use discover_for_workspace_and_dir() instead of SkillRegistry::discover() to search all skill directories (workspace + global), matching TUI /skills - Add is_bundled field to SkillEntry for built-in skill identification - Add directories field to SkillsResponse showing all search paths - Use skill.path instead of constructing path from skills_dir + name - Update set_skill_enabled to use the same discovery logic --- crates/tui/src/runtime_api.rs | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/crates/tui/src/runtime_api.rs b/crates/tui/src/runtime_api.rs index 20110cc4..90c1156e 100644 --- a/crates/tui/src/runtime_api.rs +++ b/crates/tui/src/runtime_api.rs @@ -40,7 +40,6 @@ use crate::runtime_threads::{ }; use crate::session_manager::{SavedSession, SessionManager, SessionMetadata, default_sessions_dir}; use crate::skill_state::SkillStateStore; -use crate::skills::SkillRegistry; use crate::task_manager::{ NewTaskRequest, SharedTaskManager, TaskManager, TaskManagerConfig, TaskRecord, TaskSummary, }; @@ -261,11 +260,13 @@ struct SkillEntry { description: String, path: PathBuf, enabled: bool, + is_bundled: bool, } #[derive(Debug, Serialize)] struct SkillsResponse { directory: PathBuf, + directories: Vec, warnings: Vec, skills: Vec, } @@ -906,20 +907,24 @@ async fn list_skills( State(state): State, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = SkillRegistry::discover(&skills_dir); + let registry = + crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); let skill_state = state.skill_state.lock().await; + let directories = crate::skills::skills_directories(&state.workspace); let skills = registry .list() .iter() .map(|skill| SkillEntry { name: skill.name.clone(), description: skill.description.clone(), - path: skills_dir.join(&skill.name).join("SKILL.md"), + path: skill.path.clone(), enabled: skill_state.is_enabled(&skill.name), + is_bundled: crate::skills::is_bundled_skill_name(&skill.name), }) .collect(); Ok(Json(SkillsResponse { directory: skills_dir, + directories, warnings: registry.warnings().to_vec(), skills, })) @@ -931,12 +936,12 @@ async fn set_skill_enabled( Json(req): Json, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = SkillRegistry::discover(&skills_dir); + let registry = + crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); let exists = registry.list().iter().any(|skill| skill.name == name); if !exists { return Err(ApiError::not_found(format!( - "skill '{name}' not found under {}", - skills_dir.display() + "skill '{name}' not found" ))); } From ab81d1a2e1d114e376ea4d1cdf3866f4c98702ba Mon Sep 17 00:00:00 2001 From: Hunter B Date: Sat, 30 May 2026 22:30:51 -0700 Subject: [PATCH 2/3] fix(runtime): report custom skills search directories --- crates/tui/src/runtime_api.rs | 50 ++++++++++++++++++++++++++++++----- 1 file changed, 43 insertions(+), 7 deletions(-) diff --git a/crates/tui/src/runtime_api.rs b/crates/tui/src/runtime_api.rs index ffa7fd77..553d308a 100644 --- a/crates/tui/src/runtime_api.rs +++ b/crates/tui/src/runtime_api.rs @@ -4,7 +4,7 @@ use std::collections::HashSet; use std::convert::Infallible; use std::fs; use std::net::SocketAddr; -use std::path::PathBuf; +use std::path::{Path as FsPath, PathBuf}; use std::process::Command; use std::sync::Arc; use std::time::Duration; @@ -961,10 +961,9 @@ async fn list_skills( State(state): State, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = - crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); + let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); let skill_state = state.skill_state.lock().await; - let directories = crate::skills::skills_directories(&state.workspace); + let directories = skills_search_directories(&state.workspace, &skills_dir); let skills = registry .list() .iter() @@ -990,12 +989,12 @@ async fn set_skill_enabled( Json(req): Json, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = - crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); + let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); let exists = registry.list().iter().any(|skill| skill.name == name); if !exists { return Err(ApiError::not_found(format!( - "skill '{name}' not found" + "skill '{name}' not found in searched directories: {}", + format_skill_search_paths(&skills_search_directories(&state.workspace, &skills_dir)) ))); } @@ -1771,6 +1770,25 @@ fn resolve_skills_dir(config: &Config, workspace: &std::path::Path) -> PathBuf { config.skills_dir() } +fn skills_search_directories(workspace: &FsPath, skills_dir: &FsPath) -> Vec { + let mut directories = crate::skills::skills_directories(workspace); + if skills_dir.is_dir() && !directories.iter().any(|path| path == skills_dir) { + directories.push(skills_dir.to_path_buf()); + } + directories +} + +fn format_skill_search_paths(directories: &[PathBuf]) -> String { + if directories.is_empty() { + return "".to_string(); + } + directories + .iter() + .map(|path| path.display().to_string()) + .collect::>() + .join(", ") +} + fn load_mcp_config_or_default(path: &std::path::Path) -> Result { crate::mcp::load_config(path) .map_err(|e| ApiError::internal(format!("Failed to load MCP config: {e:#}"))) @@ -3890,6 +3908,24 @@ mod tests { assert_eq!(resolved, expected); } + #[test] + fn skills_search_directories_includes_custom_skills_dir() { + let tmp = tempfile::tempdir().expect("tempdir"); + let workspace = tmp.path().join("workspace"); + let custom_skills = tmp.path().join("custom-skills"); + fs::create_dir_all(&workspace).expect("create workspace"); + fs::create_dir_all(&custom_skills).expect("create custom skills"); + + let directories = skills_search_directories(&workspace, &custom_skills); + + assert!( + directories.iter().any(|dir| dir == &custom_skills), + "custom skills_dir must be reported when discovery searches it" + ); + let message = format_skill_search_paths(&directories); + assert!(message.contains("custom-skills")); + } + /// A `skills` symlink that points outside the workspace must NOT be /// returned as the resolved skills directory. Containment check ensures /// the canonicalized candidate stays under the canonicalized workspace From 39fc14b948d677808a5928e27f4ab6c3bc907391 Mon Sep 17 00:00:00 2001 From: Hunter B Date: Sat, 30 May 2026 22:42:47 -0700 Subject: [PATCH 3/3] fix(runtime): identify bundled skill entries by path --- crates/tui/src/runtime_api.rs | 81 ++++++++++++++++++++++++++++++++--- crates/tui/src/skills/mod.rs | 4 ++ 2 files changed, 80 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/runtime_api.rs b/crates/tui/src/runtime_api.rs index 553d308a..3b2de2f6 100644 --- a/crates/tui/src/runtime_api.rs +++ b/crates/tui/src/runtime_api.rs @@ -961,9 +961,8 @@ async fn list_skills( State(state): State, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); + let (registry, directories) = discover_skills_for_runtime_api(&state.workspace, &skills_dir); let skill_state = state.skill_state.lock().await; - let directories = skills_search_directories(&state.workspace, &skills_dir); let skills = registry .list() .iter() @@ -972,7 +971,7 @@ async fn list_skills( description: skill.description.clone(), path: skill.path.clone(), enabled: skill_state.is_enabled(&skill.name), - is_bundled: crate::skills::is_bundled_skill_name(&skill.name), + is_bundled: skill_entry_is_bundled(skill, &skills_dir), }) .collect(); Ok(Json(SkillsResponse { @@ -989,12 +988,12 @@ async fn set_skill_enabled( Json(req): Json, ) -> Result, ApiError> { let skills_dir = resolve_skills_dir(&state.config, &state.workspace); - let registry = crate::skills::discover_for_workspace_and_dir(&state.workspace, &skills_dir); + let (registry, directories) = discover_skills_for_runtime_api(&state.workspace, &skills_dir); let exists = registry.list().iter().any(|skill| skill.name == name); if !exists { return Err(ApiError::not_found(format!( "skill '{name}' not found in searched directories: {}", - format_skill_search_paths(&skills_search_directories(&state.workspace, &skills_dir)) + format_skill_search_paths(&directories) ))); } @@ -1778,6 +1777,31 @@ fn skills_search_directories(workspace: &FsPath, skills_dir: &FsPath) -> Vec (crate::skills::SkillRegistry, Vec) { + let directories = skills_search_directories(workspace, skills_dir); + let registry = crate::skills::discover_from_directories(directories.clone()); + (registry, directories) +} + +fn skill_entry_is_bundled(skill: &crate::skills::Skill, skills_dir: &FsPath) -> bool { + if !crate::skills::is_bundled_skill_name(&skill.name) { + return false; + } + + let expected_path = skills_dir.join(&skill.name).join("SKILL.md"); + paths_refer_to_same_file(&skill.path, &expected_path) +} + +fn paths_refer_to_same_file(left: &FsPath, right: &FsPath) -> bool { + match (fs::canonicalize(left), fs::canonicalize(right)) { + (Ok(left), Ok(right)) => left == right, + _ => left == right, + } +} + fn format_skill_search_paths(directories: &[PathBuf]) -> String { if directories.is_empty() { return "".to_string(); @@ -3926,6 +3950,53 @@ mod tests { assert!(message.contains("custom-skills")); } + #[test] + fn skill_entry_is_bundled_requires_configured_bundle_path() { + let tmp = tempfile::tempdir().expect("tempdir"); + let bundled_skills_dir = tmp.path().join("bundled-skills"); + let bundled_skill_path = bundled_skills_dir.join("delegate").join("SKILL.md"); + let override_skill_path = tmp + .path() + .join("workspace") + .join(".agents") + .join("skills") + .join("delegate") + .join("SKILL.md"); + fs::create_dir_all(bundled_skill_path.parent().expect("bundled parent")) + .expect("create bundled skill dir"); + fs::create_dir_all(override_skill_path.parent().expect("override parent")) + .expect("create override skill dir"); + fs::write( + &bundled_skill_path, + "---\nname: delegate\ndescription: bundled\n---\n", + ) + .expect("write bundled skill"); + fs::write( + &override_skill_path, + "---\nname: delegate\ndescription: override\n---\n", + ) + .expect("write override skill"); + + let bundled_skill = crate::skills::Skill { + name: "delegate".to_string(), + description: String::new(), + body: String::new(), + path: bundled_skill_path, + }; + let override_skill = crate::skills::Skill { + name: "delegate".to_string(), + description: String::new(), + body: String::new(), + path: override_skill_path, + }; + + assert!(skill_entry_is_bundled(&bundled_skill, &bundled_skills_dir)); + assert!(!skill_entry_is_bundled( + &override_skill, + &bundled_skills_dir + )); + } + /// A `skills` symlink that points outside the workspace must NOT be /// returned as the resolved skills directory. Containment check ensures /// the canonicalized candidate stays under the canonicalized workspace diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index d962d32e..d2c2f6ad 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -580,6 +580,10 @@ fn discover_for_workspace_dirs_and_dir(mut dirs: Vec, skills_dir: &Path dirs.push(skills_dir.to_path_buf()); } + discover_from_directories(dirs) +} + +pub(crate) fn discover_from_directories(dirs: impl IntoIterator) -> SkillRegistry { let mut merged = SkillRegistry::default(); for dir in dirs { let registry = SkillRegistry::discover(&dir);