diff --git a/Cargo.toml b/Cargo.toml index da7f482a4..d79b3a1e7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -77,6 +77,7 @@ dirs = "5.0" dark-light = "1.1" dunce = "1" filetime = "0.2" +fs2 = "0.4" zip = "0.6" # plugin load flate2 = "1.0" toml = "0.8" diff --git a/src/apps/desktop/src/api/skill_api.rs b/src/apps/desktop/src/api/skill_api.rs index e9b3eda75..f4dfe8f3a 100644 --- a/src/apps/desktop/src/api/skill_api.rs +++ b/src/apps/desktop/src/api/skill_api.rs @@ -17,13 +17,12 @@ use tokio::time::{timeout, Duration}; use crate::api::app_state::AppState; use bitfun_core::agentic::tools::implementations::skills::mode_overrides::{ - get_disabled_mode_skills_from_document, load_project_mode_skills_document_local, - load_user_mode_skill_overrides, project_mode_skills_path_for_remote, + load_project_mode_skills_document_local, project_mode_skills_path_for_remote, save_project_mode_skills_document_local, set_disabled_mode_skills_in_document, set_mode_skill_disabled_in_document, set_user_mode_skill_state, }; use bitfun_core::agentic::tools::implementations::skills::{ - default_profiles::{is_enabled_by_default_for_mode, is_skill_enabled_for_mode}, + resolver::resolve_skill_default_enabled_for_mode, ModeSkillInfo, SkillData, SkillInfo, SkillLocation, SkillRegistry, }; use bitfun_core::agentic::workspace::RemoteWorkspaceFs; @@ -191,87 +190,26 @@ async fn get_mode_skill_infos_for_workspace_input( mode_id: &str, workspace_path: Option<&str>, ) -> Result, String> { - let all_skills = get_all_skills_for_workspace_input(state, registry, workspace_path).await?; - let user_overrides = load_user_mode_skill_overrides(mode_id) - .await - .map_err(|e| format!("Failed to load user skill overrides: {}", e))?; - - let (disabled_project, resolved_skills): (HashSet, Vec) = if let Some(( - remote_root, - entry, - )) = - resolve_remote_workspace(state, workspace_path).await? - { + if let Some((remote_root, entry)) = resolve_remote_workspace(state, workspace_path).await? { let remote_fs = state .get_remote_file_service_async() .await .map_err(|e| format!("Remote file service not available: {}", e))?; let remote_workspace_fs = RemoteWorkspaceFs::new(entry.connection_id.clone(), remote_fs.clone()); - let project_config_path = project_mode_skills_path_for_remote(&remote_root); - let project_config = if remote_fs - .exists(&entry.connection_id, &project_config_path) - .await - .map_err(|e| format!("Failed to check remote project skill overrides: {}", e))? - { - let content = remote_fs - .read_file(&entry.connection_id, &project_config_path) - .await - .map_err(|e| format!("Failed to read remote project skill overrides: {}", e))?; - let content = String::from_utf8(content).map_err(|e| { - format!("Remote project skill overrides are not valid UTF-8: {}", e) - })?; - serde_json::from_str::(&content) - .map_err(|e| format!("Invalid remote project skill overrides JSON: {}", e))? - } else { - serde_json::json!({}) - }; - - ( - get_disabled_mode_skills_from_document(&project_config, mode_id) - .into_iter() - .collect(), - registry - .get_resolved_skills_for_remote_workspace( - &remote_workspace_fs, - &remote_root, - Some(mode_id), - ) - .await, - ) + Ok(registry + .get_mode_skill_infos_for_remote_workspace(&remote_workspace_fs, &remote_root, mode_id) + .await) + } else if let Some(workspace_root) = workspace_root_from_input(workspace_path) { + Ok(registry + .get_mode_skill_infos_for_workspace(Some(&workspace_root), mode_id) + .await) } else { - let workspace_root = workspace_root_from_input(workspace_path) - .ok_or_else(|| "Project-level skill overrides require an open workspace".to_string())?; - let project_config = load_project_mode_skills_document_local(&workspace_root) - .await - .map_err(|e| format!("Failed to load project mode skills: {}", e))?; - ( - get_disabled_mode_skills_from_document(&project_config, mode_id) - .into_iter() - .collect(), - registry - .get_resolved_skills_for_workspace(Some(&workspace_root), Some(mode_id)) - .await, - ) - }; - - let resolved_keys: HashSet = - resolved_skills.into_iter().map(|skill| skill.key).collect(); - - Ok(all_skills - .into_iter() - .map(|skill| { - let disabled_by_mode = - !is_skill_enabled_for_mode(&skill, mode_id, &user_overrides, &disabled_project); - let selected_for_runtime = resolved_keys.contains(&skill.key); - - ModeSkillInfo { - skill, - disabled_by_mode, - selected_for_runtime, - } - }) - .collect()) + // Mode-scoped built-in and user-level skills should still be available even + // when no project workspace is open. In that case there are simply no + // project-level overrides to apply. + Ok(registry.get_mode_skill_infos_for_workspace(None, mode_id).await) + } } fn normalize_skill_key_list(keys: Vec) -> Vec { @@ -306,7 +244,7 @@ async fn persist_user_mode_skill_selection( .filter(|skill| skill.level == SkillLocation::User) { let should_enable = enabled_keys.contains(&skill.key); - let default_enabled = is_enabled_by_default_for_mode(skill, mode_id); + let default_enabled = resolve_skill_default_enabled_for_mode(skill, mode_id); if default_enabled && !should_enable { disabled_user_skills.push(skill.key.clone()); @@ -492,7 +430,7 @@ pub async fn set_mode_skill_disabled( } .ok_or_else(|| format!("Skill '{}' not found", skill_key))?; - let default_enabled = is_enabled_by_default_for_mode(&skill_info, &mode_id); + let default_enabled = resolve_skill_default_enabled_for_mode(&skill_info, &mode_id); set_user_mode_skill_state(&mode_id, &skill_key, !disabled, default_enabled) .await .map_err(|e| format!("Failed to update user skill override: {}", e))?; diff --git a/src/crates/core/Cargo.toml b/src/crates/core/Cargo.toml index 6b7073691..05698f006 100644 --- a/src/crates/core/Cargo.toml +++ b/src/crates/core/Cargo.toml @@ -51,6 +51,7 @@ notify = { workspace = true } dirs = { workspace = true } dunce = { workspace = true } filetime = { workspace = true } +fs2 = { workspace = true } zip = { workspace = true } flate2 = { workspace = true } include_dir = { workspace = true } @@ -143,3 +144,6 @@ schannel = "0.1" default = ["ssh-remote"] tauri-support = ["tauri"] # Optional tauri support ssh-remote = ["russh", "russh-sftp", "russh-keys", "shellexpand", "ssh_config"] # russh-keys pure-Rust crypto backend (no openssl) + +[build-dependencies] +sha2 = { workspace = true } diff --git a/src/crates/core/build.rs b/src/crates/core/build.rs index d75f42959..370802399 100644 --- a/src/crates/core/build.rs +++ b/src/crates/core/build.rs @@ -1,6 +1,10 @@ fn main() { emit_rerun_if_changed(std::path::Path::new("builtin_skills")); + if let Err(e) = build_embedded_builtin_skills_metadata() { + eprintln!("Warning: Failed to embed built-in skills metadata: {}", e); + } + // Run the build script to embed prompts data if let Err(e) = build_embedded_prompts() { eprintln!("Warning: Failed to embed prompts data: {}", e); @@ -12,6 +16,66 @@ fn main() { } } +fn build_embedded_builtin_skills_metadata() -> Result<(), Box> { + use sha2::{Digest, Sha256}; + use std::fs; + use std::io::Write; + use std::path::{Path, PathBuf}; + + fn collect_files(root: &Path, current: &Path, out: &mut Vec) -> std::io::Result<()> { + for entry in fs::read_dir(current)? { + let entry = entry?; + let path = entry.path(); + if path.is_dir() { + collect_files(root, &path, out)?; + } else if path.is_file() { + out.push(path.strip_prefix(root).unwrap_or(&path).to_path_buf()); + } + } + + Ok(()) + } + + let manifest_dir = std::env::var("CARGO_MANIFEST_DIR")?; + let builtin_root = Path::new(&manifest_dir).join("builtin_skills"); + + println!("cargo:rerun-if-changed={}", builtin_root.display()); + emit_rerun_if_changed(&builtin_root); + + let mut files = Vec::new(); + if builtin_root.exists() { + collect_files(&builtin_root, &builtin_root, &mut files)?; + } + files.sort(); + + let mut hasher = Sha256::new(); + for relative_path in files { + let normalized = relative_path.to_string_lossy().replace('\\', "/"); + hasher.update(normalized.as_bytes()); + hasher.update([0]); + hasher.update(fs::read(builtin_root.join(&relative_path))?); + hasher.update([0xff]); + } + let bundle_hash = format!("{:x}", hasher.finalize()); + + let out_dir = std::env::var("OUT_DIR")?; + let dest_path = Path::new(&out_dir).join("embedded_builtin_skills.rs"); + let mut file = fs::File::create(&dest_path)?; + + writeln!( + file, + "// Embedded built-in skills metadata (auto-generated by build.rs)" + )?; + writeln!(file, "// Do not edit manually.")?; + writeln!( + file, + "pub const BUILTIN_SKILLS_BUNDLE_HASH: &str = \"{}\";", + bundle_hash + )?; + + Ok(()) +} + fn build_embedded_prompts() -> Result<(), Box> { // Embed prompts data embed_agents_prompt_data() diff --git a/src/crates/core/builtin_skills/SUPERPOWERS_LICENSE.txt b/src/crates/core/builtin_skills/SUPERPOWERS_LICENSE.txt deleted file mode 100644 index abf039032..000000000 --- a/src/crates/core/builtin_skills/SUPERPOWERS_LICENSE.txt +++ /dev/null @@ -1,21 +0,0 @@ -MIT License - -Copyright (c) 2025 Jesse Vincent - -Permission is hereby granted, free of charge, to any person obtaining a copy -of this software and associated documentation files (the "Software"), to deal -in the Software without restriction, including without limitation the rights -to use, copy, modify, merge, publish, distribute, sublicense, and/or sell -copies of the Software, and to permit persons to whom the Software is -furnished to do so, subject to the following conditions: - -The above copyright notice and this permission notice shall be included in all -copies or substantial portions of the Software. - -THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR -IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, -FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE -AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER -LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, -OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE -SOFTWARE. diff --git a/src/crates/core/src/agentic/tools/implementations/skills/builtin.rs b/src/crates/core/src/agentic/tools/implementations/skills/builtin.rs index 6c124c609..21dcda1c5 100644 --- a/src/crates/core/src/agentic/tools/implementations/skills/builtin.rs +++ b/src/crates/core/src/agentic/tools/implementations/skills/builtin.rs @@ -1,19 +1,65 @@ //! Built-in skills shipped with BitFun. //! -//! These skills are embedded into the `bitfun-core` binary and installed into the user skills -//! directory on demand and kept in sync with bundled versions. +//! These skills are embedded into the `bitfun-core` binary and installed into a +//! managed `.system` directory under the user skills root on demand. use crate::infrastructure::get_path_manager_arc; use crate::util::errors::BitFunResult; +use fs2::FileExt; use include_dir::{include_dir, Dir}; -use log::{debug, error}; +use log::{debug, error, warn}; +use serde::{Deserialize, Serialize}; use std::collections::HashSet; +use std::fs::OpenOptions; use std::path::{Path, PathBuf}; use std::sync::OnceLock; +use std::time::{SystemTime, UNIX_EPOCH}; use tokio::fs; +use tokio::task; static BUILTIN_SKILLS_DIR: Dir = include_dir!("$CARGO_MANIFEST_DIR/builtin_skills"); static BUILTIN_SKILL_DIR_NAMES: OnceLock> = OnceLock::new(); +include!(concat!(env!("OUT_DIR"), "/embedded_builtin_skills.rs")); + +const BUILTIN_SKILLS_MANIFEST_FILE_NAME: &str = ".manifest.json"; +const BUILTIN_SKILLS_INSTALL_LOCK_FILE_NAME: &str = ".system.install.lock"; +const BUILTIN_SKILLS_STAGING_PREFIX: &str = ".system.tmp"; +const LEGACY_BUILTIN_SKILL_DIR_NAMES: &[&str] = &[ + // Historical bundled "Superpowers" skills removed in 2026-04. + "brainstorming", + "dispatching-parallel-agents", + "executing-plans", + "finishing-a-development-branch", + "receiving-code-review", + "requesting-code-review", + "subagent-driven-development", + "systematic-debugging", + "test-driven-development", + "using-git-worktrees", + "using-superpowers", + "verification-before-completion", + "writing-plans", + // Earlier built-in skill bundled before the Superpowers set. + "skill-creator", +]; +const LEGACY_BUILTIN_ROOT_FILES: &[&str] = &["SUPERPOWERS_LICENSE.txt"]; + +#[derive(Debug, Clone, Serialize, Deserialize)] +struct BuiltinSkillsManifest { + bundle_hash: String, +} + +struct BuiltinSkillsInstallLock { + file: std::fs::File, +} + +impl Drop for BuiltinSkillsInstallLock { + fn drop(&mut self) { + if let Err(error) = self.file.unlock() { + warn!("Failed to unlock built-in skills install lock: {}", error); + } + } +} fn collect_builtin_skill_dir_names() -> HashSet { BUILTIN_SKILLS_DIR @@ -35,61 +81,213 @@ pub fn builtin_skill_dir_names() -> &'static HashSet { BUILTIN_SKILL_DIR_NAMES.get_or_init(collect_builtin_skill_dir_names) } +pub fn builtin_skills_bundle_hash() -> &'static str { + BUILTIN_SKILLS_BUNDLE_HASH +} + pub fn is_builtin_skill_dir_name(dir_name: &str) -> bool { builtin_skill_dir_names().contains(dir_name) } -pub fn builtin_skill_group_key(dir_name: &str) -> Option<&'static str> { - match dir_name { - "docx" | "pdf" | "pptx" | "xlsx" => Some("office"), - "find-skills" | "writing-skills" => Some("meta"), - "agent-browser" => Some("computer-use"), - _ if dir_name.starts_with("gstack-") => Some("team"), - _ => None, +fn builtin_skills_manifest_path(root: &Path) -> PathBuf { + root.join(BUILTIN_SKILLS_MANIFEST_FILE_NAME) +} + +fn builtin_skills_install_lock_path(root: &Path) -> PathBuf { + root.join(BUILTIN_SKILLS_INSTALL_LOCK_FILE_NAME) +} + +fn builtin_skills_staging_root(parent: &Path) -> PathBuf { + let timestamp = SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_default() + .as_nanos(); + parent.join(format!( + "{}.{}.{}", + BUILTIN_SKILLS_STAGING_PREFIX, + std::process::id(), + timestamp + )) +} + +async fn read_installed_manifest(root: &Path) -> BitFunResult> { + let path = builtin_skills_manifest_path(root); + match fs::read_to_string(&path).await { + Ok(content) => match serde_json::from_str::(&content) { + Ok(manifest) => Ok(Some(manifest)), + Err(error) => { + warn!( + "Invalid built-in skills manifest at {}: {}", + path.display(), + error + ); + Ok(None) + } + }, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(None), + Err(error) => Err(error.into()), } } -pub fn is_team_skill(dir_name: &str) -> bool { - builtin_skill_group_key(dir_name) == Some("team") +async fn write_installed_manifest(root: &Path) -> BitFunResult<()> { + let path = builtin_skills_manifest_path(root); + let manifest = BuiltinSkillsManifest { + bundle_hash: builtin_skills_bundle_hash().to_string(), + }; + let content = serde_json::to_vec_pretty(&manifest)?; + fs::write(path, content).await?; + Ok(()) } -pub async fn ensure_builtin_skills_installed() -> BitFunResult<()> { - let pm = get_path_manager_arc(); - let dest_root = pm.user_skills_dir(); +async fn remove_existing_path(path: &Path) -> BitFunResult<()> { + let Ok(metadata) = fs::symlink_metadata(path).await else { + return Ok(()); + }; - // Create user skills directory if needed. - if let Err(e) = fs::create_dir_all(&dest_root).await { - error!( - "Failed to create user skills directory: path={}, error={}", - dest_root.display(), - e - ); - return Err(e.into()); + if metadata.is_dir() { + fs::remove_dir_all(path).await?; + } else { + fs::remove_file(path).await?; } + Ok(()) +} + +async fn cleanup_legacy_builtin_dirs(legacy_root: &Path) -> BitFunResult<()> { + for dir_name in builtin_skill_dir_names() { + let path = legacy_root.join(dir_name); + remove_existing_path(&path).await?; + } + + for dir_name in LEGACY_BUILTIN_SKILL_DIR_NAMES { + let path = legacy_root.join(dir_name); + remove_existing_path(&path).await?; + } + + for file_name in LEGACY_BUILTIN_ROOT_FILES { + let path = legacy_root.join(file_name); + remove_existing_path(&path).await?; + } + + Ok(()) +} + +async fn acquire_install_lock(legacy_root: &Path) -> BitFunResult { + let lock_path = builtin_skills_install_lock_path(legacy_root); + + // Use an OS-backed advisory file lock so parallel test processes and app + // instances serialize built-in skill installation across the shared + // `.system` directory. + let file = task::spawn_blocking(move || -> BitFunResult { + let file = OpenOptions::new() + .create(true) + .read(true) + .write(true) + .open(&lock_path)?; + file.lock_exclusive()?; + Ok(file) + }) + .await + .map_err(|error| { + crate::util::errors::BitFunError::io(format!( + "Failed to join built-in skills install lock task: {}", + error + )) + })??; + + Ok(BuiltinSkillsInstallLock { file }) +} + +async fn install_builtin_skills_to_staging(staging_root: &Path) -> BitFunResult<(usize, usize)> { let mut installed = 0usize; let mut updated = 0usize; + for skill_dir in BUILTIN_SKILLS_DIR.dirs() { let rel = skill_dir.path(); if rel.components().count() != 1 { continue; } - let stats = sync_dir(skill_dir, &dest_root).await?; + let stats = sync_dir(skill_dir, staging_root).await?; installed += stats.installed; updated += stats.updated; } - if installed > 0 || updated > 0 { - debug!( - "Built-in skills synchronized: installed={}, updated={}, dest_root={}", - installed, - updated, - dest_root.display() + write_installed_manifest(staging_root).await?; + Ok((installed, updated)) +} + +pub async fn ensure_builtin_skills_installed() -> BitFunResult<()> { + let pm = get_path_manager_arc(); + let legacy_root = pm.user_skills_dir(); + let dest_root = pm.builtin_skills_dir(); + + // Create the parent user skills directory before taking the shared install + // lock so every contender points at the same stable path. + if let Err(e) = fs::create_dir_all(&legacy_root).await { + error!( + "Failed to create user skills directory: path={}, error={}", + legacy_root.display(), + e ); + return Err(e.into()); } - Ok(()) + let _install_lock = acquire_install_lock(&legacy_root).await?; + let system_dir_preexisting = fs::symlink_metadata(&dest_root).await.is_ok(); + + if !system_dir_preexisting { + cleanup_legacy_builtin_dirs(&legacy_root).await?; + } + + if let Some(manifest) = read_installed_manifest(&dest_root).await? { + if manifest.bundle_hash == builtin_skills_bundle_hash() { + return Ok(()); + } + } + + let staging_root = builtin_skills_staging_root(&legacy_root); + if let Err(error) = fs::remove_dir_all(&staging_root).await { + if error.kind() != std::io::ErrorKind::NotFound { + return Err(error.into()); + } + } + fs::create_dir_all(&staging_root).await?; + + let publish_result = async { + let (installed, updated) = install_builtin_skills_to_staging(&staging_root).await?; + + if let Err(error) = fs::remove_dir_all(&dest_root).await { + if error.kind() != std::io::ErrorKind::NotFound { + return Err(error.into()); + } + } + fs::rename(&staging_root, &dest_root).await?; + + if installed > 0 || updated > 0 { + debug!( + "Built-in skills synchronized: installed={}, updated={}, dest_root={}", + installed, + updated, + dest_root.display() + ); + } + + Ok(()) + } + .await; + + if let Err(error) = fs::remove_dir_all(&staging_root).await { + if error.kind() != std::io::ErrorKind::NotFound { + warn!( + "Failed to remove built-in skills staging directory {}: {}", + staging_root.display(), + error + ); + } + } + + publish_result } #[derive(Default)] @@ -165,27 +363,3 @@ async fn desired_file_content( ) -> BitFunResult> { Ok(file.contents().to_vec()) } - -#[cfg(test)] -mod tests { - use super::builtin_skill_group_key; - - #[test] - fn builtin_skill_groups_match_expected_sets() { - assert_eq!(builtin_skill_group_key("docx"), Some("office")); - assert_eq!(builtin_skill_group_key("pdf"), Some("office")); - assert_eq!(builtin_skill_group_key("pptx"), Some("office")); - assert_eq!(builtin_skill_group_key("xlsx"), Some("office")); - assert_eq!(builtin_skill_group_key("find-skills"), Some("meta")); - assert_eq!(builtin_skill_group_key("writing-skills"), Some("meta")); - assert_eq!( - builtin_skill_group_key("agent-browser"), - Some("computer-use") - ); - assert_eq!(builtin_skill_group_key("unknown-skill"), None); - assert_eq!(builtin_skill_group_key("gstack-review"), Some("team")); - assert_eq!(builtin_skill_group_key("gstack-ship"), Some("team")); - assert_eq!(builtin_skill_group_key("gstack-qa"), Some("team")); - assert_eq!(builtin_skill_group_key("gstack-cso"), Some("team")); - } -} diff --git a/src/crates/core/src/agentic/tools/implementations/skills/catalog.rs b/src/crates/core/src/agentic/tools/implementations/skills/catalog.rs new file mode 100644 index 000000000..65f96a743 --- /dev/null +++ b/src/crates/core/src/agentic/tools/implementations/skills/catalog.rs @@ -0,0 +1,217 @@ +//! Built-in skill catalog. +//! +//! This module is the single source of truth for built-in skill identity and +//! grouping metadata. Runtime policy code should depend on this catalog instead +//! of scattering string matches across multiple files. + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum BuiltinSkillId { + AgentBrowser, + Docx, + FindSkills, + GstackAutoplan, + GstackCso, + GstackDesignConsultation, + GstackDesignReview, + GstackDocumentRelease, + GstackInvestigate, + GstackOfficeHours, + GstackPlanCeoReview, + GstackPlanDesignReview, + GstackPlanEngReview, + GstackQa, + GstackQaOnly, + GstackRetro, + GstackReview, + GstackShip, + Pdf, + Pptx, + WritingSkills, + Xlsx, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum BuiltinSkillGroup { + Office, + Meta, + ComputerUse, + Team, +} + +impl BuiltinSkillGroup { + pub fn as_str(self) -> &'static str { + match self { + Self::Office => "office", + Self::Meta => "meta", + Self::ComputerUse => "computer-use", + Self::Team => "team", + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct BuiltinSkillSpec { + pub id: BuiltinSkillId, + pub dir_name: &'static str, + pub group: BuiltinSkillGroup, +} + +const BUILTIN_SKILL_SPECS: &[BuiltinSkillSpec] = &[ + BuiltinSkillSpec { + id: BuiltinSkillId::AgentBrowser, + dir_name: "agent-browser", + group: BuiltinSkillGroup::ComputerUse, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::Docx, + dir_name: "docx", + group: BuiltinSkillGroup::Office, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::FindSkills, + dir_name: "find-skills", + group: BuiltinSkillGroup::Meta, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackAutoplan, + dir_name: "gstack-autoplan", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackCso, + dir_name: "gstack-cso", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackDesignConsultation, + dir_name: "gstack-design-consultation", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackDesignReview, + dir_name: "gstack-design-review", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackDocumentRelease, + dir_name: "gstack-document-release", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackInvestigate, + dir_name: "gstack-investigate", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackOfficeHours, + dir_name: "gstack-office-hours", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackPlanCeoReview, + dir_name: "gstack-plan-ceo-review", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackPlanDesignReview, + dir_name: "gstack-plan-design-review", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackPlanEngReview, + dir_name: "gstack-plan-eng-review", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackQa, + dir_name: "gstack-qa", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackQaOnly, + dir_name: "gstack-qa-only", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackRetro, + dir_name: "gstack-retro", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackReview, + dir_name: "gstack-review", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::GstackShip, + dir_name: "gstack-ship", + group: BuiltinSkillGroup::Team, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::Pdf, + dir_name: "pdf", + group: BuiltinSkillGroup::Office, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::Pptx, + dir_name: "pptx", + group: BuiltinSkillGroup::Office, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::WritingSkills, + dir_name: "writing-skills", + group: BuiltinSkillGroup::Meta, + }, + BuiltinSkillSpec { + id: BuiltinSkillId::Xlsx, + dir_name: "xlsx", + group: BuiltinSkillGroup::Office, + }, +]; + +pub fn builtin_skill_spec(dir_name: &str) -> Option<&'static BuiltinSkillSpec> { + BUILTIN_SKILL_SPECS + .iter() + .find(|spec| spec.dir_name == dir_name) +} + +pub fn builtin_skill_group(dir_name: &str) -> Option { + builtin_skill_spec(dir_name).map(|spec| spec.group) +} + +pub fn builtin_skill_group_key(dir_name: &str) -> Option<&'static str> { + builtin_skill_group(dir_name).map(BuiltinSkillGroup::as_str) +} + +#[cfg(test)] +mod tests { + use super::{builtin_skill_group, builtin_skill_group_key, BUILTIN_SKILL_SPECS}; + use crate::agentic::tools::implementations::skills::builtin::builtin_skill_dir_names; + use std::collections::HashSet; + + #[test] + fn builtin_skill_groups_match_expected_sets() { + assert_eq!(builtin_skill_group_key("docx"), Some("office")); + assert_eq!(builtin_skill_group_key("pdf"), Some("office")); + assert_eq!(builtin_skill_group_key("pptx"), Some("office")); + assert_eq!(builtin_skill_group_key("xlsx"), Some("office")); + assert_eq!(builtin_skill_group_key("find-skills"), Some("meta")); + assert_eq!(builtin_skill_group_key("writing-skills"), Some("meta")); + assert_eq!(builtin_skill_group_key("agent-browser"), Some("computer-use")); + assert_eq!(builtin_skill_group_key("gstack-review"), Some("team")); + assert_eq!(builtin_skill_group("unknown-skill"), None); + } + + #[test] + fn catalog_covers_all_embedded_builtin_skills() { + let known: HashSet<&'static str> = BUILTIN_SKILL_SPECS.iter().map(|spec| spec.dir_name).collect(); + + for dir_name in builtin_skill_dir_names() { + assert!( + known.contains(dir_name.as_str()), + "Missing built-in skill catalog entry for '{}'", + dir_name + ); + } + } +} diff --git a/src/crates/core/src/agentic/tools/implementations/skills/default_profiles.rs b/src/crates/core/src/agentic/tools/implementations/skills/default_profiles.rs deleted file mode 100644 index b597381e6..000000000 --- a/src/crates/core/src/agentic/tools/implementations/skills/default_profiles.rs +++ /dev/null @@ -1,184 +0,0 @@ -//! Default built-in skill profiles per mode. - -use super::builtin::is_team_skill; -use super::mode_overrides::UserModeSkillOverrides; -use super::types::{SkillInfo, SkillLocation}; -use std::collections::HashSet; - -#[derive(Debug, Clone, Copy, PartialEq, Eq)] -struct BuiltinSkillProfile { - /// Baseline state for built-in skills in this mode. - default_enabled: bool, - /// Built-in skill directory names whose state differs from `default_enabled`. - overridden_skills: &'static [&'static str], -} - -const ENABLE_ALL_BUILTINS: BuiltinSkillProfile = BuiltinSkillProfile { - default_enabled: true, - overridden_skills: &[], -}; - -const DISABLE_ALL_BUILTINS: BuiltinSkillProfile = BuiltinSkillProfile { - default_enabled: false, - overridden_skills: &[], -}; - -const AGENTIC_PROFILE: BuiltinSkillProfile = BuiltinSkillProfile { - default_enabled: true, - overridden_skills: &["docx", "pdf", "pptx", "xlsx"], -}; - -const COWORK_PROFILE: BuiltinSkillProfile = BuiltinSkillProfile { - default_enabled: false, - overridden_skills: &[ - "docx", - "pdf", - "pptx", - "xlsx", - "find-skills", - "writing-skills", - ], -}; - -fn builtin_profile_for_mode(mode_id: &str) -> BuiltinSkillProfile { - match mode_id { - "Plan" | "debug" => DISABLE_ALL_BUILTINS, - "agentic" => AGENTIC_PROFILE, - "Cowork" => COWORK_PROFILE, - _ => ENABLE_ALL_BUILTINS, - } -} - -pub fn is_enabled_by_default_for_mode(skill: &SkillInfo, mode_id: &str) -> bool { - if skill.level != SkillLocation::User || !skill.is_builtin { - return true; - } - - // Team (gstack-*) skills are only enabled in Team mode - if is_team_skill(&skill.dir_name) { - return mode_id == "Team"; - } - - let profile = builtin_profile_for_mode(mode_id); - if profile.overridden_skills.contains(&skill.dir_name.as_str()) { - !profile.default_enabled - } else { - profile.default_enabled - } -} - -pub fn is_skill_enabled_for_mode( - skill: &SkillInfo, - mode_id: &str, - user_overrides: &UserModeSkillOverrides, - disabled_project_skills: &HashSet, -) -> bool { - match skill.level { - SkillLocation::Project => !disabled_project_skills.contains(&skill.key), - SkillLocation::User => { - let default_enabled = is_enabled_by_default_for_mode(skill, mode_id); - - if default_enabled { - !user_overrides.disabled_skills.contains(&skill.key) - } else { - user_overrides.enabled_skills.contains(&skill.key) - } - } - } -} - -#[cfg(test)] -mod tests { - use super::{is_enabled_by_default_for_mode, is_skill_enabled_for_mode}; - use crate::agentic::tools::implementations::skills::mode_overrides::UserModeSkillOverrides; - use crate::agentic::tools::implementations::skills::types::{SkillInfo, SkillLocation}; - use std::collections::HashSet; - - fn builtin_skill(dir_name: &str) -> SkillInfo { - SkillInfo { - key: format!("user::bitfun::{}", dir_name), - name: dir_name.to_string(), - description: String::new(), - path: format!("/tmp/{}", dir_name), - level: SkillLocation::User, - source_slot: "bitfun".to_string(), - dir_name: dir_name.to_string(), - is_builtin: true, - group_key: None, - is_shadowed: false, - shadowed_by_key: None, - } - } - - fn custom_user_skill(dir_name: &str) -> SkillInfo { - SkillInfo { - key: format!("user::bitfun::{}", dir_name), - name: dir_name.to_string(), - description: String::new(), - path: format!("/tmp/{}", dir_name), - level: SkillLocation::User, - source_slot: "bitfun".to_string(), - dir_name: dir_name.to_string(), - is_builtin: false, - group_key: None, - is_shadowed: false, - shadowed_by_key: None, - } - } - - #[test] - fn builtin_defaults_follow_mode_profiles() { - let pdf = builtin_skill("pdf"); - let browser = builtin_skill("agent-browser"); - - assert!(!is_enabled_by_default_for_mode(&pdf, "agentic")); - assert!(is_enabled_by_default_for_mode(&browser, "agentic")); - assert!(is_enabled_by_default_for_mode(&pdf, "Cowork")); - assert!(!is_enabled_by_default_for_mode(&browser, "Cowork")); - assert!(!is_enabled_by_default_for_mode(&pdf, "Plan")); - assert!(!is_enabled_by_default_for_mode(&browser, "debug")); - } - - #[test] - fn non_builtin_user_skills_remain_enabled_by_default() { - let custom = custom_user_skill("my-custom-skill"); - assert!(is_enabled_by_default_for_mode(&custom, "agentic")); - assert!(is_enabled_by_default_for_mode(&custom, "Plan")); - } - - #[test] - fn team_skills_only_enabled_in_team_mode() { - let review = builtin_skill("gstack-review"); - let ship = builtin_skill("gstack-ship"); - - assert!(is_enabled_by_default_for_mode(&review, "Team")); - assert!(is_enabled_by_default_for_mode(&ship, "Team")); - - assert!(!is_enabled_by_default_for_mode(&review, "agentic")); - assert!(!is_enabled_by_default_for_mode(&ship, "agentic")); - assert!(!is_enabled_by_default_for_mode(&review, "Plan")); - assert!(!is_enabled_by_default_for_mode(&review, "Cowork")); - } - - #[test] - fn user_overrides_apply_on_top_of_defaults() { - let pdf = builtin_skill("pdf"); - let mut overrides = UserModeSkillOverrides::default(); - let disabled_project = HashSet::new(); - - assert!(!is_skill_enabled_for_mode( - &pdf, - "agentic", - &overrides, - &disabled_project, - )); - - overrides.enabled_skills.push(pdf.key.clone()); - assert!(is_skill_enabled_for_mode( - &pdf, - "agentic", - &overrides, - &disabled_project, - )); - } -} diff --git a/src/crates/core/src/agentic/tools/implementations/skills/mod.rs b/src/crates/core/src/agentic/tools/implementations/skills/mod.rs index c65852e6e..89f135407 100644 --- a/src/crates/core/src/agentic/tools/implementations/skills/mod.rs +++ b/src/crates/core/src/agentic/tools/implementations/skills/mod.rs @@ -3,13 +3,15 @@ //! Provides Skill registry, loading, and configuration management functionality pub mod builtin; -pub mod default_profiles; +pub mod catalog; pub mod mode_overrides; +pub mod policy; pub mod registry; +pub mod resolver; pub mod types; pub use registry::SkillRegistry; -pub use types::{ModeSkillInfo, SkillData, SkillInfo, SkillLocation}; +pub use types::{ModeSkillInfo, ModeSkillStateReason, SkillData, SkillInfo, SkillLocation}; /// Get global Skill registry instance pub fn get_skill_registry() -> &'static SkillRegistry { diff --git a/src/crates/core/src/agentic/tools/implementations/skills/policy.rs b/src/crates/core/src/agentic/tools/implementations/skills/policy.rs new file mode 100644 index 000000000..34c05c738 --- /dev/null +++ b/src/crates/core/src/agentic/tools/implementations/skills/policy.rs @@ -0,0 +1,192 @@ +//! Mode-aware built-in skill policy. +//! +//! The policy layer answers a narrow question: given a built-in skill and a +//! mode identifier, should that skill be enabled by default before any user or +//! project override is applied? + +use super::catalog::{builtin_skill_spec, BuiltinSkillGroup, BuiltinSkillId, BuiltinSkillSpec}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillModeId { + Agentic, + Cowork, + Plan, + Debug, + Team, + Claw, + ComputerUse, + DeepResearch, + Other, +} + +impl SkillModeId { + pub fn parse(mode_id: &str) -> Self { + match mode_id.trim() { + "agentic" => Self::Agentic, + "Cowork" => Self::Cowork, + "Plan" => Self::Plan, + "debug" => Self::Debug, + "Team" => Self::Team, + "Claw" => Self::Claw, + "ComputerUse" => Self::ComputerUse, + "DeepResearch" => Self::DeepResearch, + _ => Self::Other, + } + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PolicyEffect { + Enable, + Disable, +} + +impl PolicyEffect { + pub fn is_enabled(self) -> bool { + matches!(self, Self::Enable) + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SkillSelector { + Builtin(BuiltinSkillId), + Group(BuiltinSkillGroup), +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct SkillPolicyRule { + pub selector: SkillSelector, + pub effect: PolicyEffect, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ModeSkillPolicy { + pub builtin_default: PolicyEffect, + pub rules: &'static [SkillPolicyRule], +} + +const DISABLE_OFFICE: SkillPolicyRule = SkillPolicyRule { + selector: SkillSelector::Group(BuiltinSkillGroup::Office), + effect: PolicyEffect::Disable, +}; + +const DISABLE_TEAM: SkillPolicyRule = SkillPolicyRule { + selector: SkillSelector::Group(BuiltinSkillGroup::Team), + effect: PolicyEffect::Disable, +}; + +const ENABLE_OFFICE: SkillPolicyRule = SkillPolicyRule { + selector: SkillSelector::Group(BuiltinSkillGroup::Office), + effect: PolicyEffect::Enable, +}; + +const ENABLE_META: SkillPolicyRule = SkillPolicyRule { + selector: SkillSelector::Group(BuiltinSkillGroup::Meta), + effect: PolicyEffect::Enable, +}; + +// Open-ended modes should only surface the lightweight metadata helpers by +// default. The rest of the built-ins remain opt-in. +const OPEN_META_ONLY_POLICY: ModeSkillPolicy = ModeSkillPolicy { + builtin_default: PolicyEffect::Disable, + rules: &[ENABLE_META], +}; + +const PLAN_POLICY: ModeSkillPolicy = ModeSkillPolicy { + builtin_default: PolicyEffect::Disable, + rules: &[], +}; + +const DEBUG_POLICY: ModeSkillPolicy = PLAN_POLICY; + +const AGENTIC_POLICY: ModeSkillPolicy = ModeSkillPolicy { + builtin_default: PolicyEffect::Enable, + rules: &[DISABLE_OFFICE, DISABLE_TEAM], +}; + +const COWORK_POLICY: ModeSkillPolicy = ModeSkillPolicy { + builtin_default: PolicyEffect::Disable, + rules: &[ENABLE_OFFICE, ENABLE_META], +}; + +// Team mode keeps the broad built-in toolkit except for office helpers. +// Office skills remain exclusive to Cowork's default profile so document +// handling does not show up by default in other modes. +const TEAM_POLICY: ModeSkillPolicy = ModeSkillPolicy { + builtin_default: PolicyEffect::Enable, + rules: &[DISABLE_OFFICE], +}; + +pub fn policy_for_mode(mode_id: &str) -> ModeSkillPolicy { + match SkillModeId::parse(mode_id) { + SkillModeId::Plan => PLAN_POLICY, + SkillModeId::Debug => DEBUG_POLICY, + SkillModeId::Agentic | SkillModeId::Claw => AGENTIC_POLICY, + SkillModeId::Cowork => COWORK_POLICY, + SkillModeId::Team => TEAM_POLICY, + SkillModeId::ComputerUse + | SkillModeId::DeepResearch + | SkillModeId::Other => OPEN_META_ONLY_POLICY, + } +} + +fn selector_matches(selector: SkillSelector, spec: &BuiltinSkillSpec) -> bool { + match selector { + SkillSelector::Builtin(skill_id) => spec.id == skill_id, + SkillSelector::Group(group) => spec.group == group, + } +} + +pub fn resolve_builtin_default_effect( + spec: &BuiltinSkillSpec, + mode_id: &str, +) -> PolicyEffect { + let policy = policy_for_mode(mode_id); + let mut current = policy.builtin_default; + + // Rules are applied in declaration order. Later rules can intentionally + // override broader earlier rules, which keeps profile definitions explicit + // without introducing another priority system. + for rule in policy.rules { + if selector_matches(rule.selector, spec) { + current = rule.effect; + } + } + + current +} + +pub fn resolve_builtin_default_enabled(dir_name: &str, mode_id: &str) -> Option { + builtin_skill_spec(dir_name).map(|spec| resolve_builtin_default_effect(spec, mode_id).is_enabled()) +} + +#[cfg(test)] +mod tests { + use super::{resolve_builtin_default_enabled, PolicyEffect, SkillModeId}; + + #[test] + fn builtin_defaults_follow_mode_policies() { + assert_eq!(SkillModeId::parse("agentic"), SkillModeId::Agentic); + assert_eq!(SkillModeId::parse("debug"), SkillModeId::Debug); + assert_eq!(SkillModeId::parse("something-else"), SkillModeId::Other); + + assert_eq!(resolve_builtin_default_enabled("pdf", "agentic"), Some(false)); + assert_eq!(resolve_builtin_default_enabled("agent-browser", "agentic"), Some(true)); + assert_eq!(resolve_builtin_default_enabled("pdf", "Cowork"), Some(true)); + assert_eq!(resolve_builtin_default_enabled("agent-browser", "Cowork"), Some(false)); + assert_eq!(resolve_builtin_default_enabled("gstack-review", "Team"), Some(true)); + assert_eq!(resolve_builtin_default_enabled("pdf", "Team"), Some(false)); + assert_eq!(resolve_builtin_default_enabled("find-skills", "DeepResearch"), Some(true)); + assert_eq!(resolve_builtin_default_enabled("pdf", "DeepResearch"), Some(false)); + assert_eq!(resolve_builtin_default_enabled("agent-browser", "Claw"), Some(true)); + assert_eq!(resolve_builtin_default_enabled("pdf", "Claw"), Some(false)); + assert_eq!(resolve_builtin_default_enabled("pdf", "Other"), Some(false)); + } + + #[test] + fn unknown_builtins_return_none() { + assert_eq!(resolve_builtin_default_enabled("not-real", "agentic"), None); + assert!(PolicyEffect::Enable.is_enabled()); + assert!(!PolicyEffect::Disable.is_enabled()); + } +} diff --git a/src/crates/core/src/agentic/tools/implementations/skills/registry.rs b/src/crates/core/src/agentic/tools/implementations/skills/registry.rs index 540e410c9..0bbd6bb8b 100644 --- a/src/crates/core/src/agentic/tools/implementations/skills/registry.rs +++ b/src/crates/core/src/agentic/tools/implementations/skills/registry.rs @@ -2,15 +2,14 @@ //! //! Manages skill discovery, mode-specific filtering, and loading. -use super::builtin::{ - builtin_skill_group_key, ensure_builtin_skills_installed, is_builtin_skill_dir_name, -}; -use super::default_profiles::{is_enabled_by_default_for_mode, is_skill_enabled_for_mode}; +use super::builtin::ensure_builtin_skills_installed; +use super::catalog::builtin_skill_group_key; use super::mode_overrides::{ UserModeSkillOverrides, load_disabled_mode_skills_local, load_disabled_mode_skills_remote, load_user_mode_skill_overrides, }; -use super::types::{SkillData, SkillInfo, SkillLocation}; +use super::resolver::{resolve_skill_default_enabled_for_mode, resolve_skill_state_for_mode}; +use super::types::{ModeSkillInfo, SkillData, SkillInfo, SkillLocation}; use crate::agentic::workspace::WorkspaceFileSystem; use crate::infrastructure::get_path_manager_arc; use crate::util::errors::{BitFunError, BitFunResult}; @@ -26,6 +25,9 @@ static SKILL_REGISTRY: OnceLock = OnceLock::new(); const USER_PREFIX: &str = "user"; const PROJECT_PREFIX: &str = "project"; +const BITFUN_USER_SLOT: &str = "bitfun"; +const BITFUN_SYSTEM_SLOT: &str = "bitfun-system"; +const BITFUN_SYSTEM_DIR_NAME: &str = ".system"; /// Project-level skill roots under a workspace. const PROJECT_SKILL_SLOTS: &[(&str, &str, &str)] = &[ @@ -57,6 +59,7 @@ struct SkillRootEntry { level: SkillLocation, slot: &'static str, priority: usize, + is_builtin: bool, } #[derive(Debug, Clone)] @@ -73,12 +76,15 @@ struct SkillCandidate { } impl SkillCandidate { - fn from_data(mut data: SkillData, slot: &str, key_prefix: &str, priority: usize) -> Self { + fn from_data( + mut data: SkillData, + slot: &str, + key_prefix: &str, + priority: usize, + is_builtin: bool, + ) -> Self { data.source_slot = slot.to_string(); data.key = build_skill_key(key_prefix, slot, &data.dir_name); - let is_builtin = data.location == SkillLocation::User - && slot == "bitfun" - && is_builtin_skill_dir_name(&data.dir_name); let group_key = if is_builtin { builtin_skill_group_key(&data.dir_name).map(str::to_string) } else { @@ -184,6 +190,26 @@ fn resolve_visible_skills(candidates: Vec) -> Vec { .collect() } +fn filter_candidates_for_mode( + candidates: Vec, + mode_id: &str, + user_overrides: &UserModeSkillOverrides, + disabled_project_skills: &HashSet, +) -> Vec { + candidates + .into_iter() + .filter(|candidate| { + resolve_skill_state_for_mode( + &candidate.info, + mode_id, + user_overrides, + disabled_project_skills, + ) + .effective_enabled + }) + .collect() +} + /// Annotate each candidate with shadowing information. /// For every skill that has a higher-priority (lower number) skill with the same name, /// set `is_shadowed = true` and `shadowed_by_key` to the winner's key. @@ -242,6 +268,7 @@ impl SkillRegistry { level: SkillLocation::Project, slot, priority, + is_builtin: false, }); } priority += 1; @@ -257,13 +284,14 @@ impl SkillRegistry { level: SkillLocation::User, slot, priority, + is_builtin: false, }); } priority += 1; } } - // BitFun's own user skills dir sits between home slots and config slots. + // BitFun's own user-defined skills sit between home slots and config slots. // This lets other agent directories (e.g. ~/.claude/skills) take precedence // while still keeping config-level overrides after BitFun defaults. let path_manager = get_path_manager_arc(); @@ -272,8 +300,21 @@ impl SkillRegistry { entries.push(SkillRootEntry { path: bitfun_skills, level: SkillLocation::User, - slot: "bitfun", + slot: BITFUN_USER_SLOT, + priority, + is_builtin: false, + }); + } + priority += 1; + + let builtin_skills = path_manager.builtin_skills_dir(); + if builtin_skills.exists() && builtin_skills.is_dir() { + entries.push(SkillRootEntry { + path: builtin_skills, + level: SkillLocation::User, + slot: BITFUN_SYSTEM_SLOT, priority, + is_builtin: true, }); } priority += 1; @@ -287,6 +328,7 @@ impl SkillRegistry { level: SkillLocation::User, slot, priority, + is_builtin: false, }); } priority += 1; @@ -316,6 +358,10 @@ impl SkillRegistry { continue; }; + if entry.slot == BITFUN_USER_SLOT && dir_name == BITFUN_SYSTEM_DIR_NAME { + continue; + } + let skill_md_path = path.join("SKILL.md"); if !skill_md_path.exists() { continue; @@ -339,6 +385,7 @@ impl SkillRegistry { entry.slot, key_prefix, entry.priority, + entry.is_builtin, )); } Err(error) => { @@ -421,6 +468,7 @@ impl SkillRegistry { entry.slot, PROJECT_PREFIX, entry.priority, + false, )); } Err(error) => { @@ -471,17 +519,7 @@ impl SkillRegistry { .into_iter() .collect(); - candidates - .into_iter() - .filter(|candidate| { - is_skill_enabled_for_mode( - &candidate.info, - mode_id, - &user_overrides, - &disabled_project, - ) - }) - .collect() + filter_candidates_for_mode(candidates, mode_id, &user_overrides, &disabled_project) } async fn apply_mode_filters_for_remote_workspace( @@ -506,15 +544,38 @@ impl SkillRegistry { .into_iter() .collect(); - candidates + filter_candidates_for_mode(candidates, mode_id, &user_overrides, &disabled_project) + } + + fn build_mode_skill_infos( + all_skills: Vec, + resolved_skills: Vec, + mode_id: &str, + user_overrides: &UserModeSkillOverrides, + disabled_project_skills: &HashSet, + ) -> Vec { + let resolved_keys: HashSet = + resolved_skills.into_iter().map(|skill| skill.key).collect(); + + all_skills .into_iter() - .filter(|candidate| { - is_skill_enabled_for_mode( - &candidate.info, + .map(|skill| { + let state = resolve_skill_state_for_mode( + &skill, mode_id, - &user_overrides, - &disabled_project, - ) + user_overrides, + disabled_project_skills, + ); + let selected_for_runtime = resolved_keys.contains(&skill.key); + + ModeSkillInfo { + skill, + default_enabled: state.default_enabled, + effective_enabled: state.effective_enabled, + disabled_by_mode: !state.effective_enabled, + selected_for_runtime, + state_reason: state.reason, + } }) .collect() } @@ -539,7 +600,7 @@ impl SkillRegistry { if info.level == SkillLocation::User && info.is_builtin && info.group_key.as_deref() == Some("team") - && !is_enabled_by_default_for_mode(&info, mode_id) + && !resolve_skill_default_enabled_for_mode(&info, mode_id) { return Ok(info); } @@ -680,6 +741,78 @@ impl SkillRegistry { resolve_visible_skills(filtered) } + pub async fn get_mode_skill_infos_for_workspace( + &self, + workspace_root: Option<&Path>, + mode_id: &str, + ) -> Vec { + let candidates = self.scan_skill_candidates_for_workspace(workspace_root).await; + let all_skills = sort_skills(annotate_shadowed_skills(candidates.clone())); + let user_overrides = load_user_mode_skill_overrides(mode_id) + .await + .unwrap_or_else(|_| UserModeSkillOverrides::default()); + let disabled_project = match workspace_root { + Some(root) => load_disabled_mode_skills_local(root, mode_id) + .await + .unwrap_or_default(), + None => Vec::new(), + }; + let disabled_project: HashSet = dedupe_preserving_order(disabled_project) + .into_iter() + .collect(); + let filtered = filter_candidates_for_mode( + candidates, + mode_id, + &user_overrides, + &disabled_project, + ); + let resolved = resolve_visible_skills(filtered); + + Self::build_mode_skill_infos( + all_skills, + resolved, + mode_id, + &user_overrides, + &disabled_project, + ) + } + + pub async fn get_mode_skill_infos_for_remote_workspace( + &self, + fs: &dyn WorkspaceFileSystem, + remote_root: &str, + mode_id: &str, + ) -> Vec { + let candidates = self + .scan_skill_candidates_for_remote_workspace(fs, remote_root) + .await; + let all_skills = sort_skills(annotate_shadowed_skills(candidates.clone())); + let user_overrides = load_user_mode_skill_overrides(mode_id) + .await + .unwrap_or_else(|_| UserModeSkillOverrides::default()); + let disabled_project = load_disabled_mode_skills_remote(fs, remote_root, mode_id) + .await + .unwrap_or_default(); + let disabled_project: HashSet = dedupe_preserving_order(disabled_project) + .into_iter() + .collect(); + let filtered = filter_candidates_for_mode( + candidates, + mode_id, + &user_overrides, + &disabled_project, + ); + let resolved = resolve_visible_skills(filtered); + + Self::build_mode_skill_infos( + all_skills, + resolved, + mode_id, + &user_overrides, + &disabled_project, + ) + } + pub async fn find_skill_by_key_for_workspace( &self, skill_key: &str, diff --git a/src/crates/core/src/agentic/tools/implementations/skills/resolver.rs b/src/crates/core/src/agentic/tools/implementations/skills/resolver.rs new file mode 100644 index 000000000..e5a8c75e7 --- /dev/null +++ b/src/crates/core/src/agentic/tools/implementations/skills/resolver.rs @@ -0,0 +1,185 @@ +//! Skill resolution helpers. +//! +//! This module combines the built-in policy layer with user/project overrides +//! and produces a single effective availability decision for a skill in a mode. + +use super::mode_overrides::UserModeSkillOverrides; +use super::policy::resolve_builtin_default_enabled; +use super::types::{ModeSkillStateReason, SkillInfo, SkillLocation}; +use std::collections::HashSet; + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ModeSkillState { + pub default_enabled: bool, + pub effective_enabled: bool, + pub reason: ModeSkillStateReason, +} + +pub fn resolve_skill_default_enabled_for_mode(skill: &SkillInfo, mode_id: &str) -> bool { + match skill.level { + SkillLocation::Project => true, + SkillLocation::User => { + if !skill.is_builtin { + true + } else { + resolve_builtin_default_enabled(&skill.dir_name, mode_id).unwrap_or(true) + } + } + } +} + +fn resolve_default_state_for_user_skill(skill: &SkillInfo, mode_id: &str) -> ModeSkillState { + if !skill.is_builtin { + return ModeSkillState { + default_enabled: true, + effective_enabled: true, + reason: ModeSkillStateReason::CustomUserDefaultEnabled, + }; + } + + let default_enabled = resolve_builtin_default_enabled(&skill.dir_name, mode_id).unwrap_or(true); + ModeSkillState { + default_enabled, + effective_enabled: default_enabled, + reason: if default_enabled { + ModeSkillStateReason::BuiltinPolicyEnabled + } else { + ModeSkillStateReason::BuiltinPolicyDisabled + }, + } +} + +pub fn resolve_skill_state_for_mode( + skill: &SkillInfo, + mode_id: &str, + user_overrides: &UserModeSkillOverrides, + disabled_project_skills: &HashSet, +) -> ModeSkillState { + match skill.level { + SkillLocation::Project => { + let disabled = disabled_project_skills.contains(&skill.key); + ModeSkillState { + default_enabled: true, + effective_enabled: !disabled, + reason: if disabled { + ModeSkillStateReason::DisabledByProjectOverride + } else { + ModeSkillStateReason::ProjectDefaultEnabled + }, + } + } + SkillLocation::User => { + let default_state = resolve_default_state_for_user_skill(skill, mode_id); + + if default_state.default_enabled { + if user_overrides.disabled_skills.contains(&skill.key) { + return ModeSkillState { + default_enabled: true, + effective_enabled: false, + reason: ModeSkillStateReason::DisabledByUserOverride, + }; + } + } else if user_overrides.enabled_skills.contains(&skill.key) { + return ModeSkillState { + default_enabled: false, + effective_enabled: true, + reason: ModeSkillStateReason::EnabledByUserOverride, + }; + } + + default_state + } + } +} + +#[cfg(test)] +mod tests { + use super::{resolve_skill_default_enabled_for_mode, resolve_skill_state_for_mode}; + use crate::agentic::tools::implementations::skills::mode_overrides::UserModeSkillOverrides; + use crate::agentic::tools::implementations::skills::types::{ + ModeSkillStateReason, SkillInfo, SkillLocation, + }; + use std::collections::HashSet; + + fn builtin_skill(dir_name: &str) -> SkillInfo { + SkillInfo { + key: format!("user::bitfun-system::{}", dir_name), + name: dir_name.to_string(), + description: String::new(), + path: format!("/tmp/{}", dir_name), + level: SkillLocation::User, + source_slot: "bitfun-system".to_string(), + dir_name: dir_name.to_string(), + is_builtin: true, + group_key: None, + is_shadowed: false, + shadowed_by_key: None, + } + } + + fn custom_user_skill(dir_name: &str) -> SkillInfo { + SkillInfo { + key: format!("user::bitfun::{}", dir_name), + name: dir_name.to_string(), + description: String::new(), + path: format!("/tmp/{}", dir_name), + level: SkillLocation::User, + source_slot: "bitfun".to_string(), + dir_name: dir_name.to_string(), + is_builtin: false, + group_key: None, + is_shadowed: false, + shadowed_by_key: None, + } + } + + #[test] + fn builtin_default_state_follows_policy() { + let pdf = builtin_skill("pdf"); + let browser = builtin_skill("agent-browser"); + + assert!(!resolve_skill_default_enabled_for_mode(&pdf, "agentic")); + assert!(resolve_skill_default_enabled_for_mode(&browser, "agentic")); + assert!(resolve_skill_default_enabled_for_mode(&pdf, "Cowork")); + assert!(!resolve_skill_default_enabled_for_mode(&browser, "Cowork")); + } + + #[test] + fn custom_user_skills_are_enabled_by_default() { + let custom = custom_user_skill("my-custom-skill"); + let state = resolve_skill_state_for_mode( + &custom, + "agentic", + &UserModeSkillOverrides::default(), + &HashSet::new(), + ); + + assert!(state.default_enabled); + assert!(state.effective_enabled); + assert_eq!(state.reason, ModeSkillStateReason::CustomUserDefaultEnabled); + } + + #[test] + fn overrides_apply_on_top_of_defaults() { + let pdf = builtin_skill("pdf"); + let mut overrides = UserModeSkillOverrides::default(); + let disabled_project = HashSet::new(); + + let disabled_state = + resolve_skill_state_for_mode(&pdf, "agentic", &overrides, &disabled_project); + assert!(!disabled_state.effective_enabled); + assert_eq!( + disabled_state.reason, + ModeSkillStateReason::BuiltinPolicyDisabled + ); + + overrides.enabled_skills.push(pdf.key.clone()); + let enabled_state = + resolve_skill_state_for_mode(&pdf, "agentic", &overrides, &disabled_project); + assert!(enabled_state.effective_enabled); + assert_eq!( + enabled_state.reason, + ModeSkillStateReason::EnabledByUserOverride + ); + } +} diff --git a/src/crates/core/src/agentic/tools/implementations/skills/types.rs b/src/crates/core/src/agentic/tools/implementations/skills/types.rs index a6580a7ed..3e0933093 100644 --- a/src/crates/core/src/agentic/tools/implementations/skills/types.rs +++ b/src/crates/core/src/agentic/tools/implementations/skills/types.rs @@ -76,18 +76,36 @@ impl SkillInfo { } } +/// The most specific rule that determined a skill's availability in a mode. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "snake_case")] +pub enum ModeSkillStateReason { + ProjectDefaultEnabled, + DisabledByProjectOverride, + CustomUserDefaultEnabled, + BuiltinPolicyEnabled, + BuiltinPolicyDisabled, + EnabledByUserOverride, + DisabledByUserOverride, +} + /// Skill information annotated for a specific mode. #[derive(Debug, Clone, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ModeSkillInfo { #[serde(flatten)] pub skill: SkillInfo, - /// True when this skill is currently disabled for the mode after applying - /// defaults plus user/project overrides. + /// Whether this skill is enabled by default before user/project overrides. + pub default_enabled: bool, + /// Whether this skill is effectively enabled after applying all overrides. + pub effective_enabled: bool, + /// Backward-compatible inverse of `effective_enabled`. pub disabled_by_mode: bool, /// True when this skill is the one actually selected for runtime after applying /// mode disables and same-name priority resolution. pub selected_for_runtime: bool, + /// The rule that ultimately decided the effective state of this skill. + pub state_reason: ModeSkillStateReason, } /// Skill data (contains content, for execution) diff --git a/src/crates/core/src/infrastructure/app_paths/path_manager.rs b/src/crates/core/src/infrastructure/app_paths/path_manager.rs index d42770ab6..c26fcdb57 100644 --- a/src/crates/core/src/infrastructure/app_paths/path_manager.rs +++ b/src/crates/core/src/infrastructure/app_paths/path_manager.rs @@ -205,6 +205,11 @@ impl PathManager { } } + /// Get BitFun-managed built-in skills directory under the user skills root. + pub fn builtin_skills_dir(&self) -> PathBuf { + self.user_skills_dir().join(".system") + } + /// Get cache root directory: ~/.config/bitfun/cache/ pub fn cache_root(&self) -> PathBuf { self.user_root.join("cache") diff --git a/src/crates/core/src/service/config/mode_config_canonicalizer.rs b/src/crates/core/src/service/config/mode_config_canonicalizer.rs index 4570c86b6..8bdd2f9b8 100644 --- a/src/crates/core/src/service/config/mode_config_canonicalizer.rs +++ b/src/crates/core/src/service/config/mode_config_canonicalizer.rs @@ -461,18 +461,18 @@ mod tests { fn normalize_skill_override_lists_removes_duplicates_and_conflicts() { let (disabled, enabled) = normalize_skill_override_lists( vec![ - "user::bitfun::pdf".to_string(), - "user::bitfun::pdf".to_string(), + "user::bitfun-system::pdf".to_string(), + "user::bitfun-system::pdf".to_string(), ], vec![ - "user::bitfun::pdf".to_string(), - "user::bitfun::docx".to_string(), - "user::bitfun::docx".to_string(), + "user::bitfun-system::pdf".to_string(), + "user::bitfun-system::docx".to_string(), + "user::bitfun-system::docx".to_string(), ], ); - assert_eq!(disabled, vec!["user::bitfun::pdf".to_string()]); - assert_eq!(enabled, vec!["user::bitfun::docx".to_string()]); + assert_eq!(disabled, vec!["user::bitfun-system::pdf".to_string()]); + assert_eq!(enabled, vec!["user::bitfun-system::docx".to_string()]); } #[test] @@ -484,7 +484,7 @@ mod tests { Vec::new(), Vec::new(), Vec::new(), - vec!["user::bitfun::pdf".to_string()], + vec!["user::bitfun-system::pdf".to_string()], &[], &valid_tools, ) @@ -492,7 +492,7 @@ mod tests { assert_eq!( stored.enabled_user_skills, - vec!["user::bitfun::pdf".to_string()] + vec!["user::bitfun-system::pdf".to_string()] ); assert!(stored.disabled_user_skills.is_empty()); } diff --git a/src/web-ui/src/app/scenes/agents/AgentsScene.tsx b/src/web-ui/src/app/scenes/agents/AgentsScene.tsx index ce0da1ed1..ee5f03eec 100644 --- a/src/web-ui/src/app/scenes/agents/AgentsScene.tsx +++ b/src/web-ui/src/app/scenes/agents/AgentsScene.tsx @@ -64,7 +64,7 @@ interface SkillGroup { } function getConfiguredEnabledSkillKeys(skills: ModeSkillInfo[]): string[] { - return skills.filter((skill) => !skill.disabledByMode).map((skill) => skill.key); + return skills.filter((skill) => skill.effectiveEnabled).map((skill) => skill.key); } function modeHasSkillTool(enabledTools: string[]): boolean { @@ -108,8 +108,6 @@ function getSkillGroupLabel(groupKey: string, t: TFunction<'scenes/agents'>): st return t('agentsOverview.skillGroups.meta'); case 'team': return t('agentsOverview.skillGroups.team'); - case 'superpowers': - return t('agentsOverview.skillGroups.superpowers'); default: return t('agentsOverview.skillGroups.other'); } @@ -119,7 +117,7 @@ function getSkillTitle(skill: ModeSkillInfo, t: TFunction<'scenes/agents'>): str return [ skill.description || skill.name, `key: ${skill.key}`, - !skill.disabledByMode && !skill.selectedForRuntime + skill.effectiveEnabled && !skill.selectedForRuntime ? t('agentsOverview.skillShadowed') : null, ].filter(Boolean).join('\n'); @@ -310,7 +308,7 @@ const AgentsHomeView: React.FC = () => { [selectedAgentModeSkills], ); const selectedAgentSkillItems = useMemo( - () => selectedAgentModeSkills.filter((skill) => !skill.disabledByMode), + () => selectedAgentModeSkills.filter((skill) => skill.effectiveEnabled), [selectedAgentModeSkills], ); const selectedAgentSkillGroups = useMemo( @@ -978,7 +976,7 @@ const AgentsHomeView: React.FC = () => {
{group.skills - .filter((skill) => !skill.disabledByMode) + .filter((skill) => skill.effectiveEnabled) .map((skill) => ( { ); const skillsEnabled = useMemo( - () => modeSkills.filter((skill) => !skill.disabledByMode), + () => modeSkills.filter((skill) => skill.effectiveEnabled), [modeSkills], ); const skillsDisabled = useMemo( - () => modeSkills.filter((skill) => skill.disabledByMode), + () => modeSkills.filter((skill) => !skill.effectiveEnabled), [modeSkills], ); const duplicateSkillNames = useMemo( @@ -338,7 +338,7 @@ const TemplateConfigPage: React.FC = () => { const handleSkillToggle = useCallback(async (skill: ModeSkillInfo) => { const loadingKey = skill.key; setSkillsLoading((prev) => ({ ...prev, [loadingKey]: true })); - const nextDisabled = !skill.disabledByMode; + const nextDisabled = skill.effectiveEnabled; try { await configAPI.setModeSkillDisabled({ modeId: 'agentic', @@ -473,7 +473,7 @@ const TemplateConfigPage: React.FC = () => { const renderSkillList = (list: ModeSkillInfo[]) => (
{list.map((skill) => { - const on = !skill.disabledByMode; + const on = skill.effectiveEnabled; const selected = detail?.type === 'skill' && detail.skill.key === skill.key; const displayName = formatSkillDisplayName(skill, duplicateSkillNames); return ( @@ -686,7 +686,7 @@ const TemplateConfigPage: React.FC = () => { } const { skill } = detail; - const on = !skill.disabledByMode; + const on = skill.effectiveEnabled; return (