feat: 添加技能禁用功能,支持批量禁用技能并更新运行时状态
This commit is contained in:
parent
b239083eb5
commit
69364e484b
1
.gitignore
vendored
1
.gitignore
vendored
@ -8,3 +8,4 @@ Cargo.lock
|
|||||||
.playwright-cli/
|
.playwright-cli/
|
||||||
.venv
|
.venv
|
||||||
PicoBot.code-workspace
|
PicoBot.code-workspace
|
||||||
|
.picobot
|
||||||
|
|||||||
34
README.md
34
README.md
@ -410,7 +410,7 @@ description: 用于总结 Rust 项目架构
|
|||||||
内置工具:
|
内置工具:
|
||||||
|
|
||||||
- skill_list:只读列出技能
|
- skill_list:只读列出技能
|
||||||
- skill_manage:运行时创建、更新、删除、读取和重载技能
|
- skill_manage:运行时创建、更新、删除、批量禁用、读取和重载技能
|
||||||
|
|
||||||
skill_manage 支持的 action:
|
skill_manage 支持的 action:
|
||||||
|
|
||||||
@ -419,8 +419,40 @@ skill_manage 支持的 action:
|
|||||||
- create
|
- create
|
||||||
- update
|
- update
|
||||||
- delete
|
- delete
|
||||||
|
- disable
|
||||||
- reload
|
- 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 配置示例:
|
skills 配置示例:
|
||||||
|
|
||||||
```json
|
```json
|
||||||
|
|||||||
@ -1,10 +1,18 @@
|
|||||||
use serde::Deserialize;
|
use serde::{Deserialize, Serialize};
|
||||||
use serde_json::json;
|
use serde_json::json;
|
||||||
use std::collections::HashMap;
|
use std::collections::{HashMap, HashSet};
|
||||||
use std::fs;
|
use std::fs;
|
||||||
use std::path::{Path, PathBuf};
|
use std::path::{Path, PathBuf};
|
||||||
use std::sync::RwLock;
|
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;
|
use crate::config::SkillsConfig;
|
||||||
|
|
||||||
#[derive(Debug, Clone)]
|
#[derive(Debug, Clone)]
|
||||||
@ -24,7 +32,7 @@ pub enum SkillSource {
|
|||||||
ProjectAgent,
|
ProjectAgent,
|
||||||
}
|
}
|
||||||
|
|
||||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)]
|
||||||
pub enum SkillScope {
|
pub enum SkillScope {
|
||||||
User,
|
User,
|
||||||
Project,
|
Project,
|
||||||
@ -62,6 +70,16 @@ pub struct SkillRuntime {
|
|||||||
catalog: RwLock<SkillCatalog>,
|
catalog: RwLock<SkillCatalog>,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone)]
|
||||||
|
pub struct SkillAvailabilityChange {
|
||||||
|
pub name: String,
|
||||||
|
pub scope: SkillScope,
|
||||||
|
pub state_path: PathBuf,
|
||||||
|
pub changed: bool,
|
||||||
|
pub disabled_in_scopes: Vec<SkillScope>,
|
||||||
|
pub available: bool,
|
||||||
|
}
|
||||||
|
|
||||||
impl Default for SkillRuntime {
|
impl Default for SkillRuntime {
|
||||||
fn default() -> Self {
|
fn default() -> Self {
|
||||||
Self {
|
Self {
|
||||||
@ -220,6 +238,78 @@ impl SkillRuntime {
|
|||||||
}
|
}
|
||||||
Ok(dir)
|
Ok(dir)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
pub fn disable_skill(
|
||||||
|
&self,
|
||||||
|
scope: SkillScope,
|
||||||
|
name: &str,
|
||||||
|
reload: bool,
|
||||||
|
) -> Result<SkillAvailabilityChange, String> {
|
||||||
|
self.set_skill_enabled(scope, name, false, reload)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn enable_skill(
|
||||||
|
&self,
|
||||||
|
scope: SkillScope,
|
||||||
|
name: &str,
|
||||||
|
reload: bool,
|
||||||
|
) -> Result<SkillAvailabilityChange, String> {
|
||||||
|
self.set_skill_enabled(scope, name, true, reload)
|
||||||
|
}
|
||||||
|
|
||||||
|
pub fn has_skill_definition(&self, name: &str) -> Result<bool, String> {
|
||||||
|
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<SkillAvailabilityChange, String> {
|
||||||
|
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<String> = state.disabled_skills.into_iter().collect();
|
||||||
|
let changed = if enabled {
|
||||||
|
disabled.remove(name)
|
||||||
|
} else {
|
||||||
|
disabled.insert(name.to_string())
|
||||||
|
};
|
||||||
|
|
||||||
|
let mut disabled_skills: Vec<String> = 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 {
|
impl crate::agent::SkillProvider for SkillRuntime {
|
||||||
@ -262,6 +352,20 @@ impl Default for SkillCatalog {
|
|||||||
|
|
||||||
impl SkillCatalog {
|
impl SkillCatalog {
|
||||||
pub fn discover(config: &SkillsConfig) -> Self {
|
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 {
|
if !config.enabled {
|
||||||
return Self {
|
return Self {
|
||||||
max_index_chars: config.max_index_chars,
|
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<String, Skill> = HashMap::new();
|
let mut merged: HashMap<String, Skill> = HashMap::new();
|
||||||
let mut sources_seen = 0usize;
|
let mut sources_seen = 0usize;
|
||||||
|
|
||||||
@ -294,6 +397,9 @@ impl SkillCatalog {
|
|||||||
}
|
}
|
||||||
|
|
||||||
let mut skills: Vec<Skill> = merged.into_values().collect();
|
let mut skills: Vec<Skill> = 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));
|
skills.sort_by(|a, b| a.name.cmp(&b.name));
|
||||||
|
|
||||||
tracing::info!(
|
tracing::info!(
|
||||||
@ -399,6 +505,35 @@ impl SkillCatalog {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)]
|
||||||
|
struct SkillStateFile {
|
||||||
|
#[serde(default)]
|
||||||
|
disabled_skills: Vec<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
#[derive(Debug, Clone, Default)]
|
||||||
|
struct SkillDisableState {
|
||||||
|
user_disabled: HashSet<String>,
|
||||||
|
project_disabled: HashSet<String>,
|
||||||
|
}
|
||||||
|
|
||||||
|
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<SkillScope> {
|
||||||
|
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<SkillSource> {
|
fn source_order(sources: &[String]) -> Vec<SkillSource> {
|
||||||
let mut result = Vec::new();
|
let mut result = Vec::new();
|
||||||
for source in sources {
|
for source in sources {
|
||||||
@ -461,10 +596,18 @@ fn project_agent_skills_root(cwd: &Path) -> PathBuf {
|
|||||||
cwd.join(".agents").join("skills")
|
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<PathBuf> {
|
fn user_skills_root() -> Option<PathBuf> {
|
||||||
dirs::home_dir().map(|p| p.join(".picobot").join("skills"))
|
dirs::home_dir().map(|p| p.join(".picobot").join("skills"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn user_skill_state_path() -> Option<PathBuf> {
|
||||||
|
dirs::home_dir().map(|p| p.join(".picobot").join("skill-state.json"))
|
||||||
|
}
|
||||||
|
|
||||||
fn user_agent_skills_root() -> Option<PathBuf> {
|
fn user_agent_skills_root() -> Option<PathBuf> {
|
||||||
dirs::home_dir().map(|p| p.join(".agents").join("skills"))
|
dirs::home_dir().map(|p| p.join(".agents").join("skills"))
|
||||||
}
|
}
|
||||||
@ -495,6 +638,78 @@ fn skill_file_path(scope: SkillScope, name: &str) -> Result<PathBuf, String> {
|
|||||||
Ok(skill_dir_path(scope, name)?.join("SKILL.md"))
|
Ok(skill_dir_path(scope, name)?.join("SKILL.md"))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fn skill_state_path(scope: SkillScope) -> Result<PathBuf, String> {
|
||||||
|
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<String> {
|
||||||
|
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<String> {
|
||||||
|
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<SkillStateFile, String> {
|
||||||
|
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<String, String> {
|
fn render_skill_file(name: &str, description: &str, body: &str) -> Result<String, String> {
|
||||||
if description.trim().is_empty() {
|
if description.trim().is_empty() {
|
||||||
return Err("description is required and cannot be empty".to_string());
|
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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::*;
|
use super::*;
|
||||||
use std::sync::Mutex;
|
use std::ffi::OsString;
|
||||||
|
|
||||||
static CWD_TEST_LOCK: Mutex<()> = Mutex::new(());
|
|
||||||
|
|
||||||
struct CurrentDirGuard {
|
struct CurrentDirGuard {
|
||||||
previous: PathBuf,
|
previous: PathBuf,
|
||||||
}
|
}
|
||||||
|
|
||||||
|
struct HomeDirGuard {
|
||||||
|
previous: Option<OsString>,
|
||||||
|
}
|
||||||
|
|
||||||
impl CurrentDirGuard {
|
impl CurrentDirGuard {
|
||||||
fn enter(path: &Path) -> Self {
|
fn enter(path: &Path) -> Self {
|
||||||
let previous = std::env::current_dir().unwrap();
|
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]
|
#[test]
|
||||||
fn test_split_frontmatter() {
|
fn test_split_frontmatter() {
|
||||||
let input = "---\ndescription: demo\n---\nhello";
|
let input = "---\ndescription: demo\n---\nhello";
|
||||||
@ -683,9 +927,14 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_runtime_create_update_delete_reload() {
|
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 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 {
|
let runtime = SkillRuntime::from_config(SkillsConfig {
|
||||||
enabled: true,
|
enabled: true,
|
||||||
@ -756,12 +1005,16 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_discover_loads_project_agent_skills() {
|
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 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
|
let agent_skill_dir = project_dir
|
||||||
.path()
|
|
||||||
.join(".agents")
|
.join(".agents")
|
||||||
.join("skills")
|
.join("skills")
|
||||||
.join("demo-agent");
|
.join("demo-agent");
|
||||||
@ -786,11 +1039,16 @@ mod tests {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_discover_prefers_project_agent_on_conflict() {
|
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 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::create_dir_all(&project_skill_dir).unwrap();
|
||||||
fs::write(
|
fs::write(
|
||||||
project_skill_dir.join("SKILL.md"),
|
project_skill_dir.join("SKILL.md"),
|
||||||
@ -798,7 +1056,7 @@ mod tests {
|
|||||||
)
|
)
|
||||||
.unwrap();
|
.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::create_dir_all(&agent_skill_dir).unwrap();
|
||||||
fs::write(
|
fs::write(
|
||||||
agent_skill_dir.join("SKILL.md"),
|
agent_skill_dir.join("SKILL.md"),
|
||||||
@ -817,4 +1075,136 @@ mod tests {
|
|||||||
assert!(payload.contains("Source: project_agent"));
|
assert!(payload.contains("Source: project_agent"));
|
||||||
assert!(payload.contains("Agent body"));
|
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());
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -32,7 +32,7 @@ impl Tool for SkillManageTool {
|
|||||||
}
|
}
|
||||||
|
|
||||||
fn description(&self) -> &str {
|
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 {
|
fn parameters_schema(&self) -> serde_json::Value {
|
||||||
@ -41,18 +41,25 @@ impl Tool for SkillManageTool {
|
|||||||
"properties": {
|
"properties": {
|
||||||
"action": {
|
"action": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"enum": ["list", "get", "create", "update", "delete", "reload"],
|
"enum": ["list", "get", "create", "update", "delete", "disable", "reload"],
|
||||||
"description": "Management action to perform"
|
"description": "Management action to perform"
|
||||||
},
|
},
|
||||||
"scope": {
|
"scope": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"enum": ["project", "user"],
|
"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": {
|
"name": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Skill name"
|
"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": {
|
"description": {
|
||||||
"type": "string",
|
"type": "string",
|
||||||
"description": "Skill description used for discovery"
|
"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 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 {
|
let result = match action {
|
||||||
"list" => list_skills_payload(&self.skills),
|
"list" => list_skills_payload(&self.skills),
|
||||||
@ -192,6 +203,30 @@ impl Tool for SkillManageTool {
|
|||||||
}),
|
}),
|
||||||
Err(err) => return Ok(error_result(&err)),
|
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::<Vec<_>>(),
|
||||||
|
})
|
||||||
|
}
|
||||||
_ => return Ok(error_result("Unsupported action")),
|
_ => 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<Vec<String>, 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::<Vec<_>>(),
|
||||||
|
})
|
||||||
|
}
|
||||||
|
|
||||||
fn list_skills_payload(skills: &Arc<SkillRuntime>) -> serde_json::Value {
|
fn list_skills_payload(skills: &Arc<SkillRuntime>) -> serde_json::Value {
|
||||||
let skills = skills.list_skills();
|
let skills = skills.list_skills();
|
||||||
json!({
|
json!({
|
||||||
@ -259,3 +330,170 @@ fn list_skills_payload(skills: &Arc<SkillRuntime>) -> serde_json::Value {
|
|||||||
})).collect::<Vec<_>>()
|
})).collect::<Vec<_>>()
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[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<OsString>,
|
||||||
|
}
|
||||||
|
|
||||||
|
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"));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user