From 69364e484b7782ff4da1571ce021ab07f6c14deb Mon Sep 17 00:00:00 2001 From: ooodc <549496103@qq.com> Date: Wed, 6 May 2026 15:30:59 +0800 Subject: [PATCH] =?UTF-8?q?feat:=20=E6=B7=BB=E5=8A=A0=E6=8A=80=E8=83=BD?= =?UTF-8?q?=E7=A6=81=E7=94=A8=E5=8A=9F=E8=83=BD=EF=BC=8C=E6=94=AF=E6=8C=81?= =?UTF-8?q?=E6=89=B9=E9=87=8F=E7=A6=81=E7=94=A8=E6=8A=80=E8=83=BD=E5=B9=B6?= =?UTF-8?q?=E6=9B=B4=E6=96=B0=E8=BF=90=E8=A1=8C=E6=97=B6=E7=8A=B6=E6=80=81?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 1 + README.md | 34 ++- src/skills/mod.rs | 424 ++++++++++++++++++++++++++++++++++++-- src/tools/skill_manage.rs | 244 +++++++++++++++++++++- 4 files changed, 682 insertions(+), 21 deletions(-) diff --git a/.gitignore b/.gitignore index 0c25edb..92d7bc9 100644 --- a/.gitignore +++ b/.gitignore @@ -8,3 +8,4 @@ Cargo.lock .playwright-cli/ .venv PicoBot.code-workspace +.picobot diff --git a/README.md b/README.md index 7f894c8..20bb661 100644 --- a/README.md +++ b/README.md @@ -410,7 +410,7 @@ description: 用于总结 Rust 项目架构 内置工具： - skill_list：只读列出技能 -- skill_manage：运行时创建、更新、删除、读取和重载技能 +- skill_manage：运行时创建、更新、删除、批量禁用、读取和重载技能 skill_manage 支持的 action： @@ -419,8 +419,40 @@ skill_manage 支持的 action： - create - update - delete +- disable - reload +skill 的启用/禁用状态不会写入 config.json，而是写入独立状态文件： + +- 用户级状态：~/.picobot/skill-state.json +- 项目级状态：.picobot/skill-state.json + +状态文件当前使用最小 JSON 结构： + +```json +{ + "disabled_skills": ["example-skill"] +} +``` + +说明： + +- disable 默认写入项目级状态文件，可通过 tool 参数中的 scope 指定 user 或 project +- disable 只接受 names 数组；即使只禁用 1 个 skill，也需要传单元素数组 +- 一次 disable 调用会批量处理 names 里的所有 skill，并只做一次 reload +- 用户级与项目级状态同时生效，项目运行时会同时读取两者 +- 某个 skill 只要在任一层状态文件中被禁用，就不会出现在 skill_list、skill_activate 和技能索引提示里 + +批量示例： + +```json +{ + "action": "disable", + "scope": "project", + "names": ["lark-calendar", "lark-vc", "lark-minutes"] +} +``` + skills 配置示例： ```json diff --git a/src/skills/mod.rs b/src/skills/mod.rs index 0b5390d..d243878 100644 --- a/src/skills/mod.rs +++ b/src/skills/mod.rs @@ -1,10 +1,18 @@ -use serde::Deserialize; +use serde::{Deserialize, Serialize}; use serde_json::json; -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use std::fs; use std::path::{Path, PathBuf}; use std::sync::RwLock; +#[cfg(test)] +static SKILL_TEST_ENV_LOCK: std::sync::Mutex<()> = std::sync::Mutex::new(()); + +#[cfg(test)] +pub(crate) fn acquire_skill_test_env_lock() -> std::sync::MutexGuard<'static, ()> { + SKILL_TEST_ENV_LOCK.lock().unwrap_or_else(|err| err.into_inner()) +} + use crate::config::SkillsConfig; #[derive(Debug, Clone)] @@ -24,7 +32,7 @@ pub enum SkillSource { ProjectAgent, } -#[derive(Debug, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] pub enum SkillScope { User, Project, @@ -62,6 +70,16 @@ pub struct SkillRuntime { catalog: RwLock, } +#[derive(Debug, Clone)] +pub struct SkillAvailabilityChange { + pub name: String, + pub scope: SkillScope, + pub state_path: PathBuf, + pub changed: bool, + pub disabled_in_scopes: Vec, + pub available: bool, +} + impl Default for SkillRuntime { fn default() -> Self { Self { @@ -220,6 +238,78 @@ impl SkillRuntime { } Ok(dir) } + + pub fn disable_skill( + &self, + scope: SkillScope, + name: &str, + reload: bool, + ) -> Result { + self.set_skill_enabled(scope, name, false, reload) + } + + pub fn enable_skill( + &self, + scope: SkillScope, + name: &str, + reload: bool, + ) -> Result { + self.set_skill_enabled(scope, name, true, reload) + } + + pub fn has_skill_definition(&self, name: &str) -> Result { + validate_skill_name(name)?; + let cwd = std::env::current_dir() + .map_err(|err| format!("failed to get current dir: {}", err))?; + Ok(SkillCatalog::discover_without_state(&self.config, &cwd) + .find_skill(name) + .is_some()) + } + + fn set_skill_enabled( + &self, + scope: SkillScope, + name: &str, + enabled: bool, + reload: bool, + ) -> Result { + validate_skill_name(name)?; + if !self.has_skill_definition(name)? { + return Err(format!("skill '{}' not found", name)); + } + + let state_path = skill_state_path(scope)?; + let mut state = load_skill_state_file(&state_path)?; + let mut disabled: HashSet = state.disabled_skills.into_iter().collect(); + let changed = if enabled { + disabled.remove(name) + } else { + disabled.insert(name.to_string()) + }; + + let mut disabled_skills: Vec = disabled.into_iter().collect(); + disabled_skills.sort(); + state.disabled_skills = disabled_skills; + save_skill_state_file(&state_path, &state)?; + + if reload { + let _ = self.reload()?; + } + + let cwd = std::env::current_dir() + .map_err(|err| format!("failed to get current dir: {}", err))?; + let effective_state = load_skill_disable_state(&cwd); + let disabled_in_scopes = effective_state.disabled_scopes_for(name); + + Ok(SkillAvailabilityChange { + name: name.to_string(), + scope, + state_path, + changed, + available: disabled_in_scopes.is_empty(), + disabled_in_scopes, + }) + } } impl crate::agent::SkillProvider for SkillRuntime { @@ -262,6 +352,20 @@ impl Default for SkillCatalog { impl SkillCatalog { pub fn discover(config: &SkillsConfig) -> Self { + let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); + let disable_state = load_skill_disable_state(&cwd); + Self::discover_with_state(config, &cwd, Some(&disable_state)) + } + + fn discover_without_state(config: &SkillsConfig, cwd: &Path) -> Self { + Self::discover_with_state(config, cwd, None) + } + + fn discover_with_state( + config: &SkillsConfig, + cwd: &Path, + disable_state: Option<&SkillDisableState>, + ) -> Self { if !config.enabled { return Self { max_index_chars: config.max_index_chars, @@ -270,7 +374,6 @@ impl SkillCatalog { }; } - let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let mut merged: HashMap = HashMap::new(); let mut sources_seen = 0usize; @@ -294,6 +397,9 @@ impl SkillCatalog { } let mut skills: Vec = merged.into_values().collect(); + if let Some(disable_state) = disable_state { + skills.retain(|skill| !disable_state.is_disabled(&skill.name)); + } skills.sort_by(|a, b| a.name.cmp(&b.name)); tracing::info!( @@ -399,6 +505,35 @@ impl SkillCatalog { } } +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +struct SkillStateFile { + #[serde(default)] + disabled_skills: Vec, +} + +#[derive(Debug, Clone, Default)] +struct SkillDisableState { + user_disabled: HashSet, + project_disabled: HashSet, +} + +impl SkillDisableState { + fn is_disabled(&self, name: &str) -> bool { + self.user_disabled.contains(name) || self.project_disabled.contains(name) + } + + fn disabled_scopes_for(&self, name: &str) -> Vec { + let mut scopes = Vec::new(); + if self.user_disabled.contains(name) { + scopes.push(SkillScope::User); + } + if self.project_disabled.contains(name) { + scopes.push(SkillScope::Project); + } + scopes + } +} + fn source_order(sources: &[String]) -> Vec { let mut result = Vec::new(); for source in sources { @@ -461,10 +596,18 @@ fn project_agent_skills_root(cwd: &Path) -> PathBuf { cwd.join(".agents").join("skills") } +fn project_skill_state_path(cwd: &Path) -> PathBuf { + cwd.join(".picobot").join("skill-state.json") +} + fn user_skills_root() -> Option { dirs::home_dir().map(|p| p.join(".picobot").join("skills")) } +fn user_skill_state_path() -> Option { + dirs::home_dir().map(|p| p.join(".picobot").join("skill-state.json")) +} + fn user_agent_skills_root() -> Option { dirs::home_dir().map(|p| p.join(".agents").join("skills")) } @@ -495,6 +638,78 @@ fn skill_file_path(scope: SkillScope, name: &str) -> Result { Ok(skill_dir_path(scope, name)?.join("SKILL.md")) } +fn skill_state_path(scope: SkillScope) -> Result { + match scope { + SkillScope::User => user_skill_state_path() + .ok_or_else(|| "failed to resolve home directory".to_string()), + SkillScope::Project => { + let cwd = std::env::current_dir() + .map_err(|err| format!("failed to get current dir: {}", err))?; + Ok(project_skill_state_path(&cwd)) + } + } +} + +fn load_skill_disable_state(cwd: &Path) -> SkillDisableState { + SkillDisableState { + user_disabled: user_skill_state_path() + .map(|path| load_disabled_skill_names(&path)) + .unwrap_or_default(), + project_disabled: load_disabled_skill_names(&project_skill_state_path(cwd)), + } +} + +fn load_disabled_skill_names(path: &Path) -> HashSet { + match load_skill_state_file(path) { + Ok(state) => state + .disabled_skills + .into_iter() + .filter_map(|name| normalize_skill_name(name, path)) + .collect(), + Err(err) => { + tracing::warn!(path = %path.display(), error = %err, "Failed to load skill state file"); + HashSet::new() + } + } +} + +fn normalize_skill_name(name: String, path: &Path) -> Option { + let trimmed = name.trim(); + match validate_skill_name(trimmed) { + Ok(()) => Some(trimmed.to_string()), + Err(err) => { + tracing::warn!(path = %path.display(), skill = %name, error = %err, "Ignoring invalid disabled skill entry"); + None + } + } +} + +fn load_skill_state_file(path: &Path) -> Result { + if !path.exists() { + return Ok(SkillStateFile::default()); + } + + let content = fs::read_to_string(path) + .map_err(|err| format!("failed to read skill state file: {}", err))?; + serde_json::from_str(&content) + .map_err(|err| format!("failed to parse skill state file: {}", err)) +} + +fn save_skill_state_file(path: &Path, state: &SkillStateFile) -> Result<(), String> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent) + .map_err(|err| format!("failed to create skill state directory: {}", err))?; + } + + let content = serde_json::to_string_pretty(state) + .map_err(|err| format!("failed to render skill state file: {}", err))?; + let tmp_path = path.with_extension("json.tmp"); + fs::write(&tmp_path, format!("{}\n", content)) + .map_err(|err| format!("failed to write temporary skill state file: {}", err))?; + fs::rename(&tmp_path, path) + .map_err(|err| format!("failed to persist skill state file: {}", err)) +} + fn render_skill_file(name: &str, description: &str, body: &str) -> Result { if description.trim().is_empty() { return Err("description is required and cannot be empty".to_string()); @@ -615,14 +830,16 @@ fn split_frontmatter(content: &str) -> Option<(&str, &str)> { #[cfg(test)] mod tests { use super::*; - use std::sync::Mutex; - - static CWD_TEST_LOCK: Mutex<()> = Mutex::new(()); + use std::ffi::OsString; struct CurrentDirGuard { previous: PathBuf, } + struct HomeDirGuard { + previous: Option, + } + impl CurrentDirGuard { fn enter(path: &Path) -> Self { let previous = std::env::current_dir().unwrap(); @@ -637,6 +854,33 @@ mod tests { } } + impl HomeDirGuard { + fn enter(path: &Path) -> Self { + let previous = std::env::var_os("HOME"); + unsafe { + std::env::set_var("HOME", path); + } + Self { previous } + } + } + + impl Drop for HomeDirGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => unsafe { + std::env::set_var("HOME", value); + }, + None => unsafe { + std::env::remove_var("HOME"); + }, + } + } + } + + fn acquire_test_lock() -> std::sync::MutexGuard<'static, ()> { + acquire_skill_test_env_lock() + } + #[test] fn test_split_frontmatter() { let input = "---\ndescription: demo\n---\nhello"; @@ -683,9 +927,14 @@ mod tests { #[test] fn test_runtime_create_update_delete_reload() { - let _lock = CWD_TEST_LOCK.lock().unwrap(); + let _lock = acquire_test_lock(); let temp_dir = tempfile::tempdir().unwrap(); - let _guard = CurrentDirGuard::enter(temp_dir.path()); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); let runtime = SkillRuntime::from_config(SkillsConfig { enabled: true, @@ -756,12 +1005,16 @@ mod tests { #[test] fn test_discover_loads_project_agent_skills() { - let _lock = CWD_TEST_LOCK.lock().unwrap(); + let _lock = acquire_test_lock(); let temp_dir = tempfile::tempdir().unwrap(); - let _guard = CurrentDirGuard::enter(temp_dir.path()); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); - let agent_skill_dir = temp_dir - .path() + let agent_skill_dir = project_dir .join(".agents") .join("skills") .join("demo-agent"); @@ -786,11 +1039,16 @@ mod tests { #[test] fn test_discover_prefers_project_agent_on_conflict() { - let _lock = CWD_TEST_LOCK.lock().unwrap(); + let _lock = acquire_test_lock(); let temp_dir = tempfile::tempdir().unwrap(); - let _guard = CurrentDirGuard::enter(temp_dir.path()); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); - let project_skill_dir = temp_dir.path().join(".picobot").join("skills").join("demo"); + let project_skill_dir = project_dir.join(".picobot").join("skills").join("demo"); fs::create_dir_all(&project_skill_dir).unwrap(); fs::write( project_skill_dir.join("SKILL.md"), @@ -798,7 +1056,7 @@ mod tests { ) .unwrap(); - let agent_skill_dir = temp_dir.path().join(".agents").join("skills").join("demo"); + let agent_skill_dir = project_dir.join(".agents").join("skills").join("demo"); fs::create_dir_all(&agent_skill_dir).unwrap(); fs::write( agent_skill_dir.join("SKILL.md"), @@ -817,4 +1075,136 @@ mod tests { assert!(payload.contains("Source: project_agent")); assert!(payload.contains("Agent body")); } + + #[test] + fn test_skill_state_file_roundtrip() { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("skill-state.json"); + let state = SkillStateFile { + disabled_skills: vec!["demo".to_string(), "other".to_string()], + }; + + save_skill_state_file(&path, &state).unwrap(); + + let loaded = load_skill_state_file(&path).unwrap(); + assert_eq!(loaded, state); + } + + #[test] + fn test_discover_filters_disabled_skills_from_sidecar() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let project_skill_dir = project_dir.join(".picobot").join("skills").join("demo"); + fs::create_dir_all(&project_skill_dir).unwrap(); + fs::write( + project_skill_dir.join("SKILL.md"), + "---\ndescription: project version\n---\nProject body", + ) + .unwrap(); + + save_skill_state_file( + &project_dir.join(".picobot").join("skill-state.json"), + &SkillStateFile { + disabled_skills: vec!["demo".to_string()], + }, + ) + .unwrap(); + + let catalog = SkillCatalog::discover(&SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + }); + + assert_eq!(catalog.len(), 0); + assert!(catalog.activation_payload("demo").is_err()); + } + + #[test] + fn test_runtime_disable_and_enable_skill_updates_visibility() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let project_skill_dir = project_dir.join(".picobot").join("skills").join("demo"); + fs::create_dir_all(&project_skill_dir).unwrap(); + fs::write( + project_skill_dir.join("SKILL.md"), + "---\ndescription: project version\n---\nProject body", + ) + .unwrap(); + + let runtime = SkillRuntime::from_config(SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + }); + + let disabled = runtime.disable_skill(SkillScope::Project, "demo", true).unwrap(); + assert!(disabled.changed); + assert_eq!(disabled.disabled_in_scopes, vec![SkillScope::Project]); + assert!(!disabled.available); + assert!(runtime.get_skill("demo").is_none()); + + let enabled = runtime.enable_skill(SkillScope::Project, "demo", true).unwrap(); + assert!(enabled.changed); + assert!(enabled.disabled_in_scopes.is_empty()); + assert!(enabled.available); + assert!(runtime.get_skill("demo").is_some()); + } + + #[test] + fn test_user_scope_disable_overrides_project_scope_enable() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + fs::create_dir_all(&home_dir).unwrap(); + fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let project_skill_dir = project_dir.join(".picobot").join("skills").join("demo"); + fs::create_dir_all(&project_skill_dir).unwrap(); + fs::write( + project_skill_dir.join("SKILL.md"), + "---\ndescription: project version\n---\nProject body", + ) + .unwrap(); + + let runtime = SkillRuntime::from_config(SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + }); + + let user_disabled = runtime.disable_skill(SkillScope::User, "demo", true).unwrap(); + assert_eq!(user_disabled.disabled_in_scopes, vec![SkillScope::User]); + assert!(runtime.get_skill("demo").is_none()); + + let project_enabled = runtime.enable_skill(SkillScope::Project, "demo", true).unwrap(); + assert!(!project_enabled.available); + assert_eq!(project_enabled.disabled_in_scopes, vec![SkillScope::User]); + assert!(runtime.get_skill("demo").is_none()); + + let user_enabled = runtime.enable_skill(SkillScope::User, "demo", true).unwrap(); + assert!(user_enabled.available); + assert!(user_enabled.disabled_in_scopes.is_empty()); + assert!(runtime.get_skill("demo").is_some()); + } } diff --git a/src/tools/skill_manage.rs b/src/tools/skill_manage.rs index e5a93b3..534ac33 100644 --- a/src/tools/skill_manage.rs +++ b/src/tools/skill_manage.rs @@ -32,7 +32,7 @@ impl Tool for SkillManageTool { } fn description(&self) -> &str { - "Manage PicoBot skills stored under .picobot/skills or ~/.picobot/skills, while discovery also reads .agents/skills and ~/.agents/skills. Supports actions: list, get, create, update, delete, reload." + "Manage PicoBot skills stored under .picobot/skills or ~/.picobot/skills, while discovery also reads .agents/skills and ~/.agents/skills. Supports actions: list, get, create, update, delete, disable, reload." } fn parameters_schema(&self) -> serde_json::Value { @@ -41,18 +41,25 @@ impl Tool for SkillManageTool { "properties": { "action": { "type": "string", - "enum": ["list", "get", "create", "update", "delete", "reload"], + "enum": ["list", "get", "create", "update", "delete", "disable", "reload"], "description": "Management action to perform" }, "scope": { "type": "string", "enum": ["project", "user"], - "description": "Writable skill scope for create/update/delete. Defaults to project. .agents discovery sources are read-only here." + "description": "Writable skill scope for create/update/delete/disable. Defaults to project. .agents discovery sources are read-only here, but can still be disabled via sidecar state." }, "name": { "type": "string", "description": "Skill name" }, + "names": { + "type": "array", + "items": { + "type": "string" + }, + "description": "Skill names for batch disable; pass a single-item array to disable one skill" + }, "description": { "type": "string", "description": "Skill description used for discovery" @@ -93,6 +100,10 @@ impl Tool for SkillManageTool { }; let name = args.get("name").and_then(|v| v.as_str()); + let names = match parse_disable_names(&args) { + Ok(names) => names, + Err(err) => return Ok(error_result(&err)), + }; let result = match action { "list" => list_skills_payload(&self.skills), @@ -192,6 +203,30 @@ impl Tool for SkillManageTool { }), Err(err) => return Ok(error_result(&err)), }, + "disable" => { + let targets = &names; + + let mut changes = Vec::new(); + for target in targets { + match self.skills.disable_skill(scope, target, false) { + Ok(change) => changes.push(change), + Err(err) => return Ok(error_result(&err)), + } + } + if reload { + if let Err(err) = self.skills.reload() { + return Ok(error_result(&err)); + } + } + + json!({ + "status": "disabled", + "scope": scope.as_str(), + "count": changes.len(), + "reloaded": reload, + "changes": changes.into_iter().map(skill_change_payload).collect::>(), + }) + } _ => return Ok(error_result("Unsupported action")), }; @@ -242,6 +277,42 @@ fn error_result(message: &str) -> ToolResult { } } +fn parse_disable_names(args: &serde_json::Value) -> Result, String> { + let names = args + .get("names") + .ok_or_else(|| "disable requires names".to_string())? + .as_array() + .ok_or_else(|| "names must be an array of strings".to_string())?; + + let mut parsed = Vec::new(); + for item in names { + let name = item + .as_str() + .ok_or_else(|| "names must be an array of strings".to_string())? + .trim() + .to_string(); + if name.is_empty() { + return Err("names must not contain empty values".to_string()); + } + parsed.push(name); + } + if parsed.is_empty() { + return Err("names must not be empty".to_string()); + } + Ok(parsed) +} + +fn skill_change_payload(change: crate::skills::SkillAvailabilityChange) -> serde_json::Value { + json!({ + "name": change.name, + "scope": change.scope.as_str(), + "path": change.state_path.display().to_string(), + "changed": change.changed, + "available": change.available, + "disabled_in_scopes": change.disabled_in_scopes.into_iter().map(|scope| scope.as_str()).collect::>(), + }) +} + fn list_skills_payload(skills: &Arc) -> serde_json::Value { let skills = skills.list_skills(); json!({ @@ -259,3 +330,170 @@ fn list_skills_payload(skills: &Arc) -> serde_json::Value { })).collect::>() }) } + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::SkillsConfig; + use crate::skills::acquire_skill_test_env_lock; + use std::ffi::OsString; + use std::path::{Path, PathBuf}; + + struct CurrentDirGuard { + previous: PathBuf, + } + + struct HomeDirGuard { + previous: Option, + } + + impl CurrentDirGuard { + fn enter(path: &Path) -> Self { + let previous = std::env::current_dir().unwrap(); + std::env::set_current_dir(path).unwrap(); + Self { previous } + } + } + + impl Drop for CurrentDirGuard { + fn drop(&mut self) { + let _ = std::env::set_current_dir(&self.previous); + } + } + + impl HomeDirGuard { + fn enter(path: &Path) -> Self { + let previous = std::env::var_os("HOME"); + unsafe { + std::env::set_var("HOME", path); + } + Self { previous } + } + } + + impl Drop for HomeDirGuard { + fn drop(&mut self) { + match &self.previous { + Some(value) => unsafe { + std::env::set_var("HOME", value); + }, + None => unsafe { + std::env::remove_var("HOME"); + }, + } + } + } + + fn acquire_test_lock() -> std::sync::MutexGuard<'static, ()> { + acquire_skill_test_env_lock() + } + + #[tokio::test] + async fn test_skill_manage_disable_updates_runtime() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + std::fs::create_dir_all(&home_dir).unwrap(); + std::fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let runtime = Arc::new(SkillRuntime::from_config(SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + })); + runtime + .create_skill(SkillScope::Project, "demo", "demo skill", "body", true) + .unwrap(); + + let tool = SkillManageTool::new(runtime.clone()); + + let disabled = tool + .execute(json!({ + "action": "disable", + "names": ["demo"], + "scope": "project" + })) + .await + .unwrap(); + assert!(disabled.success); + assert!(disabled.output.contains("disabled")); + assert!(runtime.get_skill("demo").is_none()); + } + + #[tokio::test] + async fn test_skill_manage_batch_disable_uses_names_array() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + std::fs::create_dir_all(&home_dir).unwrap(); + std::fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let runtime = Arc::new(SkillRuntime::from_config(SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + })); + runtime + .create_skill(SkillScope::Project, "demo-a", "demo skill a", "body", true) + .unwrap(); + runtime + .create_skill(SkillScope::Project, "demo-b", "demo skill b", "body", true) + .unwrap(); + + let tool = SkillManageTool::new(runtime.clone()); + + let disabled = tool + .execute(json!({ + "action": "disable", + "names": ["demo-a", "demo-b"], + "scope": "project" + })) + .await + .unwrap(); + + assert!(disabled.success); + assert!(disabled.output.contains("\"count\": 2")); + assert!(runtime.get_skill("demo-a").is_none()); + assert!(runtime.get_skill("demo-b").is_none()); + } + + #[tokio::test] + async fn test_skill_manage_disable_requires_names_array() { + let _lock = acquire_test_lock(); + let temp_dir = tempfile::tempdir().unwrap(); + let home_dir = temp_dir.path().join("home"); + let project_dir = temp_dir.path().join("project"); + std::fs::create_dir_all(&home_dir).unwrap(); + std::fs::create_dir_all(&project_dir).unwrap(); + let _home = HomeDirGuard::enter(&home_dir); + let _guard = CurrentDirGuard::enter(&project_dir); + + let runtime = Arc::new(SkillRuntime::from_config(SkillsConfig { + enabled: true, + sources: vec!["project".to_string()], + max_index_chars: 4000, + max_listed_skills: 32, + })); + let tool = SkillManageTool::new(runtime); + + let result = tool + .execute(json!({ + "action": "disable", + "name": "demo", + "scope": "project" + })) + .await + .unwrap(); + + assert!(!result.success); + assert!(result.error.unwrap().contains("disable requires names")); + } +}