From b5aae98ffc80b1bfdabe026038dfee3c1b69371a Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Sun, 7 Jun 2026 18:57:31 +0000 Subject: [PATCH 1/6] chore(develop): 183-cli-command-surface-redesign-remove-redundant-vestigial-comm --- .../fastskill-cli/src/commands/add/install.rs | 4 + crates/fastskill-cli/src/commands/add/mod.rs | 58 +- .../fastskill-cli/src/commands/analyze/mod.rs | 7 +- crates/fastskill-cli/src/commands/disable.rs | 153 ----- crates/fastskill-cli/src/commands/doctor.rs | 303 +++++++++ crates/fastskill-cli/src/commands/install.rs | 45 +- crates/fastskill-cli/src/commands/mod.rs | 5 +- crates/fastskill-cli/src/commands/reindex.rs | 30 +- crates/fastskill-cli/src/commands/remove.rs | 59 +- crates/fastskill-cli/src/commands/resolve.rs | 243 ------- crates/fastskill-cli/src/commands/search.rs | 71 ++ crates/fastskill-cli/src/commands/show.rs | 619 ------------------ crates/fastskill-cli/src/commands/sync.rs | 614 ----------------- crates/fastskill-cli/src/commands/update.rs | 64 +- crates/fastskill-cli/src/config_file.rs | 31 +- crates/fastskill-cli/src/main.rs | 56 +- crates/fastskill-cli/src/utils.rs | 2 +- crates/fastskill-cli/src/utils/agents_md.rs | 318 --------- .../fastskill-cli/src/utils/reindex_utils.rs | 50 ++ tests/cli/doctor_tests.rs | 53 ++ tests/cli/help_tests.rs | 12 - tests/cli/mod.rs | 3 +- ...napshot_helpers__disable_command_help.snap | 16 - ...li__snapshot_helpers__show_all_skills.snap | 9 - ...napshot_helpers__show_dependency_tree.snap | 9 - ...pshot_helpers__show_invalid_id_format.snap | 6 - ...t_helpers__show_invalid_id_format_cli.snap | 6 - ...pshot_helpers__show_nonexistent_skill.snap | 6 - ...t_helpers__show_nonexistent_skill_cli.snap | 5 - ...snapshot_helpers__show_specific_skill.snap | 9 - ...snapshot_helpers__sync_all_skills_yes.snap | 6 - ...ync_auto_detects_claude_metadata_file.snap | 6 - ...__snapshot_helpers__sync_custom_files.snap | 6 - ...pshot_helpers__sync_empty_skills_list.snap | 6 - ...lpers__sync_no_agents_md_creates_file.snap | 6 - ...pshot_helpers__sync_project_vs_global.snap | 6 - ...pshot_helpers__sync_remove_all_skills.snap | 6 - ...helpers__sync_with_existing_agents_md.snap | 6 - ...elpers__sync_with_explicit_agent_flag.snap | 6 - 39 files changed, 743 insertions(+), 2177 deletions(-) delete mode 100644 crates/fastskill-cli/src/commands/disable.rs create mode 100644 crates/fastskill-cli/src/commands/doctor.rs delete mode 100644 crates/fastskill-cli/src/commands/resolve.rs delete mode 100644 crates/fastskill-cli/src/commands/show.rs delete mode 100644 crates/fastskill-cli/src/commands/sync.rs delete mode 100644 crates/fastskill-cli/src/utils/agents_md.rs create mode 100644 crates/fastskill-cli/src/utils/reindex_utils.rs create mode 100644 tests/cli/doctor_tests.rs delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__disable_command_help.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_all_skills.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_dependency_tree.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format_cli.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill_cli.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_specific_skill.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_all_skills_yes.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_auto_detects_claude_metadata_file.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_custom_files.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_empty_skills_list.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_no_agents_md_creates_file.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_project_vs_global.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_remove_all_skills.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_existing_agents_md.snap delete mode 100644 tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_explicit_agent_flag.snap diff --git a/crates/fastskill-cli/src/commands/add/install.rs b/crates/fastskill-cli/src/commands/add/install.rs index 40d5b1fc..46209a49 100644 --- a/crates/fastskill-cli/src/commands/add/install.rs +++ b/crates/fastskill-cli/src/commands/add/install.rs @@ -424,6 +424,8 @@ skills_directory = ".claude/skills" editable: true, group: None, recursive: false, + reindex: false, + no_reindex: false, }; let result = super::super::execute_add(&service, args, false).await; @@ -486,6 +488,8 @@ skills_directory = ".claude/skills" editable: false, group: None, recursive: false, + reindex: false, + no_reindex: false, }; let result = super::super::execute_add(&service, args, false).await; diff --git a/crates/fastskill-cli/src/commands/add/mod.rs b/crates/fastskill-cli/src/commands/add/mod.rs index 38e6e4d9..9b1922db 100644 --- a/crates/fastskill-cli/src/commands/add/mod.rs +++ b/crates/fastskill-cli/src/commands/add/mod.rs @@ -145,6 +145,14 @@ pub struct AddArgs { /// Add all skills found under the directory (only for local folders) #[arg(short = 'r', long)] pub recursive: bool, + + /// Trigger reindex after adding (overrides config) + #[arg(long)] + pub reindex: bool, + + /// Skip reindex after adding + #[arg(long)] + pub no_reindex: bool, } impl IntoCommandSpec for AddArgs { @@ -229,6 +237,24 @@ impl IntoCommandSpec for AddArgs { help: "Add all skills found under the directory (only for local folders)", ..Default::default() }, + ArgSpec { + name: "reindex", + kind: ArgKind::Flag, + long: Some("reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Trigger reindex after adding (overrides config)", + ..Default::default() + }, + ArgSpec { + name: "no-reindex", + kind: ArgKind::Flag, + long: Some("no-reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Skip reindex after adding", + ..Default::default() + }, ], ..Default::default() } @@ -279,6 +305,8 @@ impl FromArgValueMap for AddArgs { } }), recursive: matches!(map.get("recursive"), Some(ArgValue::Bool(true))), + reindex: matches!(map.get("reindex"), Some(ArgValue::Bool(true))), + no_reindex: matches!(map.get("no-reindex"), Some(ArgValue::Bool(true))), } } } @@ -332,6 +360,14 @@ fn validate_folder_has_skill(path: &Path) -> CliResult<()> { } pub async fn execute_add(service: &FastSkillService, args: AddArgs, global: bool) -> CliResult<()> { + if args.reindex && args.no_reindex { + return Err(CliError::Validation( + "--reindex and --no-reindex cannot be used together".to_string(), + )); + } + let reindex = args.reindex; + let no_reindex = args.no_reindex; + let source = resolve_source(&args); if args.editable { @@ -372,16 +408,26 @@ pub async fn execute_add(service: &FastSkillService, args: AddArgs, global: bool } match source { - SkillSource::ZipFile(path) => sources::add_from_zip(&ctx, &path).await, + SkillSource::ZipFile(path) => sources::add_from_zip(&ctx, &path).await?, SkillSource::Folder(path) => { validate_folder_has_skill(&path)?; - sources::add_from_folder(&ctx, &path).await + sources::add_from_folder(&ctx, &path).await? } SkillSource::GitUrl(url) => { - sources::add_from_git(&ctx, &url, args.branch.as_deref(), args.tag.as_deref()).await + sources::add_from_git(&ctx, &url, args.branch.as_deref(), args.tag.as_deref()).await? } - SkillSource::SkillId(skill_id) => sources::add_from_registry(&ctx, &skill_id).await, + SkillSource::SkillId(skill_id) => sources::add_from_registry(&ctx, &skill_id).await?, } + let auto_reindex = crate::config_file::load_auto_reindex_config(); + crate::utils::reindex_utils::maybe_auto_reindex( + service, + "add", + reindex, + no_reindex, + auto_reindex, + false, + ) + .await } #[cfg(test)] @@ -413,6 +459,8 @@ mod tests { editable: false, group: None, recursive: false, + reindex: false, + no_reindex: false, }; let result = execute_add(&service, args, false).await; assert!(result.is_err()); @@ -468,6 +516,8 @@ mod tests { editable: false, group: None, recursive: false, + reindex: false, + no_reindex: false, }; let result = execute_add(&service, args, false).await; diff --git a/crates/fastskill-cli/src/commands/analyze/mod.rs b/crates/fastskill-cli/src/commands/analyze/mod.rs index cba38061..ee6764bc 100644 --- a/crates/fastskill-cli/src/commands/analyze/mod.rs +++ b/crates/fastskill-cli/src/commands/analyze/mod.rs @@ -34,9 +34,10 @@ pub struct AnalysisContext { } pub async fn load_analysis_context(svc: &FastSkillService) -> CliResult> { - let vector_svc = svc.vector_index_service().ok_or_else(|| { - CliError::Config("Vector index not available. Run 'fastskill reindex' first.".to_string()) - })?; + let Some(vector_svc) = svc.vector_index_service() else { + println!("Note: semantic analysis requires an embedding provider. Results may be limited to structural analysis."); + return Ok(None); + }; let skills = vector_svc .get_all_skills() .await diff --git a/crates/fastskill-cli/src/commands/disable.rs b/crates/fastskill-cli/src/commands/disable.rs deleted file mode 100644 index 35cedcfe..00000000 --- a/crates/fastskill-cli/src/commands/disable.rs +++ /dev/null @@ -1,153 +0,0 @@ -//! Disable command implementation - -use crate::error::{CliError, CliResult}; -use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; -use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; -use cli_framework::spec::command_tree::CommandSpec; -use cli_framework::spec::value::ArgValue; -use fastskill_core::FastSkillService; -use std::collections::HashMap; - -/// Disable skills by ID -#[derive(Debug)] -pub struct DisableArgs { - /// Skill IDs to disable - pub skill_ids: Vec, -} - -impl IntoCommandSpec for DisableArgs { - fn command_spec() -> CommandSpec { - CommandSpec { - summary: "Disable skills by ID", - syntax: Some("disable ..."), - category: Some("management"), - args: vec![ArgSpec { - name: "skill-ids", - kind: ArgKind::Positional, - value_type: ArgValueType::String, - cardinality: Cardinality::Repeated, - help: "Skill IDs to disable", - ..Default::default() - }], - ..Default::default() - } - } -} - -impl FromArgValueMap for DisableArgs { - fn from_arg_value_map(map: &HashMap) -> Self { - Self { - skill_ids: match map.get("skill-ids") { - Some(ArgValue::List(items)) => items - .iter() - .filter_map(|i| { - if let ArgValue::Str(s) = i { - Some(s.clone()) - } else { - None - } - }) - .collect(), - _ => vec![], - }, - } - } -} - -pub async fn execute_disable(service: &FastSkillService, args: DisableArgs) -> CliResult<()> { - if args.skill_ids.is_empty() { - return Err(CliError::Config("No skill IDs provided".to_string())); - } - - for skill_id in &args.skill_ids { - let skill_id_parsed = fastskill_core::SkillId::new(skill_id.clone()) - .map_err(|_| CliError::Validation(format!("Invalid skill ID format: {}", skill_id)))?; - service - .skill_manager() - .disable_skill(&skill_id_parsed) - .await - .map_err(|e| { - CliError::Service(fastskill_core::ServiceError::Custom(format!( - "Failed to disable skill {}: {}", - skill_id, e - ))) - })?; - println!("Disabled skill: {}", skill_id); - } - - Ok(()) -} - -#[cfg(test)] -#[allow(clippy::unwrap_used, clippy::panic, clippy::expect_used)] -mod tests { - use super::*; - use fastskill_core::ServiceConfig; - use tempfile::TempDir; - - #[tokio::test] - async fn test_execute_disable_empty_args() { - let temp_dir = TempDir::new().unwrap(); - let config = ServiceConfig { - skill_storage_path: temp_dir.path().to_path_buf(), - ..Default::default() - }; - - let mut service = FastSkillService::new(config).await.unwrap(); - service.initialize().await.unwrap(); - - let args = DisableArgs { skill_ids: vec![] }; - - let result = execute_disable(&service, args).await; - assert!(result.is_err()); - if let Err(CliError::Config(msg)) = result { - assert!(msg.contains("No skill IDs provided")); - } else { - panic!("Expected Config error"); - } - } - - #[tokio::test] - async fn test_execute_disable_invalid_skill_id() { - let temp_dir = TempDir::new().unwrap(); - let config = ServiceConfig { - skill_storage_path: temp_dir.path().to_path_buf(), - ..Default::default() - }; - - let mut service = FastSkillService::new(config).await.unwrap(); - service.initialize().await.unwrap(); - - let args = DisableArgs { - skill_ids: vec!["invalid@skill@id".to_string()], - }; - - let result = execute_disable(&service, args).await; - assert!(result.is_err()); - if let Err(CliError::Validation(msg)) = result { - assert!(msg.contains("Invalid skill ID format")); - } else { - panic!("Expected Validation error"); - } - } - - #[tokio::test] - async fn test_execute_disable_nonexistent_skill() { - let temp_dir = TempDir::new().unwrap(); - let config = ServiceConfig { - skill_storage_path: temp_dir.path().to_path_buf(), - ..Default::default() - }; - - let mut service = FastSkillService::new(config).await.unwrap(); - service.initialize().await.unwrap(); - - let args = DisableArgs { - skill_ids: vec!["nonexistent@1.0.0".to_string()], - }; - - // This should fail because the skill doesn't exist - let result = execute_disable(&service, args).await; - assert!(result.is_err()); - } -} diff --git a/crates/fastskill-cli/src/commands/doctor.rs b/crates/fastskill-cli/src/commands/doctor.rs new file mode 100644 index 00000000..79b35304 --- /dev/null +++ b/crates/fastskill-cli/src/commands/doctor.rs @@ -0,0 +1,303 @@ +//! Doctor command - diagnose FastSkill configuration and environment + +use crate::error::{CliError, CliResult}; +use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; +use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; +use cli_framework::spec::command_tree::CommandSpec; +use cli_framework::spec::value::ArgValue; +use fastskill_core::FastSkillService; +use std::collections::HashMap; + +#[derive(Debug, Clone, PartialEq)] +pub enum DoctorStatus { + Ok, + Warning, + Error, +} + +impl std::fmt::Display for DoctorStatus { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + DoctorStatus::Ok => write!(f, "ok"), + DoctorStatus::Warning => write!(f, "warning"), + DoctorStatus::Error => write!(f, "error"), + } + } +} + +#[derive(Debug)] +pub struct DoctorCheckResult { + pub name: String, + pub status: DoctorStatus, + pub message: String, +} + +/// Doctor command arguments +#[derive(Debug)] +pub struct DoctorArgs { + /// Output as JSON + pub json: bool, +} + +impl IntoCommandSpec for DoctorArgs { + fn command_spec() -> CommandSpec { + CommandSpec { + summary: "Diagnose FastSkill configuration and environment", + syntax: Some("doctor [OPTIONS]"), + category: Some("setup"), + args: vec![ArgSpec { + name: "json", + long: Some("json"), + short: None, + help: "Output results as JSON", + kind: ArgKind::Flag, + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + default: None, + ..Default::default() + }], + ..Default::default() + } + } +} + +impl FromArgValueMap for DoctorArgs { + fn from_arg_value_map(map: &HashMap) -> Self { + Self { + json: matches!(map.get("json"), Some(ArgValue::Bool(true))), + } + } +} + +pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> CliResult<()> { + let mut checks = Vec::new(); + + // Check 1: Skills directory accessible + let skills_dir = &service.config().skill_storage_path; + let skills_dir_check = if skills_dir.exists() && skills_dir.is_dir() { + DoctorCheckResult { + name: "skills_dir".to_string(), + status: DoctorStatus::Ok, + message: format!("Skills directory found: {}", skills_dir.display()), + } + } else { + DoctorCheckResult { + name: "skills_dir".to_string(), + status: DoctorStatus::Error, + message: format!( + "Skills directory not found or not a directory: {}", + skills_dir.display() + ), + } + }; + checks.push(skills_dir_check); + + // Check 2: skill-project.toml exists + let project_file_check = { + let current_dir = std::env::current_dir() + .map_err(|e| CliError::Config(format!("Failed to get current directory: {}", e)))?; + let project_file = fastskill_core::core::project::resolve_project_file(¤t_dir); + if project_file.found { + DoctorCheckResult { + name: "project_toml".to_string(), + status: DoctorStatus::Ok, + message: format!("skill-project.toml found: {}", project_file.path.display()), + } + } else { + DoctorCheckResult { + name: "project_toml".to_string(), + status: DoctorStatus::Warning, + message: "skill-project.toml not found. Run 'fastskill init' to create one." + .to_string(), + } + } + }; + checks.push(project_file_check); + + // Check 3: Embedding configuration present + let embedding_check = if service.config().embedding.is_some() { + DoctorCheckResult { + name: "embedding_config".to_string(), + status: DoctorStatus::Ok, + message: "Embedding configuration found.".to_string(), + } + } else { + DoctorCheckResult { + name: "embedding_config".to_string(), + status: DoctorStatus::Warning, + message: "No embedding configuration. Semantic search and reindex are disabled. Add [tool.fastskill.embedding] to skill-project.toml.".to_string(), + } + }; + checks.push(embedding_check); + + // Check 4: API key present (only if embedding configured) + let api_key_check = if service.config().embedding.is_some() { + if std::env::var("OPENAI_API_KEY").is_ok() { + DoctorCheckResult { + name: "api_key".to_string(), + status: DoctorStatus::Ok, + message: "OPENAI_API_KEY environment variable is set.".to_string(), + } + } else { + DoctorCheckResult { + name: "api_key".to_string(), + status: DoctorStatus::Warning, + message: + "OPENAI_API_KEY environment variable not set. Set it to enable embeddings." + .to_string(), + } + } + } else { + DoctorCheckResult { + name: "api_key".to_string(), + status: DoctorStatus::Warning, + message: "No embedding config — API key check skipped.".to_string(), + } + }; + checks.push(api_key_check); + + // Check 5: Auth token + let auth_check = if std::env::var("FASTSKILL_AUTH_TOKEN").is_ok() + || std::env::var("FASTSKILL_TOKEN").is_ok() + { + DoctorCheckResult { + name: "auth_token".to_string(), + status: DoctorStatus::Ok, + message: "Auth token found in environment.".to_string(), + } + } else { + DoctorCheckResult { + name: "auth_token".to_string(), + status: DoctorStatus::Warning, + message: "No auth token set. Remote registry operations may fail. Run 'fastskill auth login'.".to_string(), + } + }; + checks.push(auth_check); + + if args.json { + print_json(&checks); + } else { + print_human(&checks); + } + + // Exit 0 unless skills_dir is inaccessible + let has_error = checks + .iter() + .any(|c| c.name == "skills_dir" && c.status == DoctorStatus::Error); + if has_error { + return Err(CliError::Config( + "Skills directory is inaccessible. Fix the skills directory to proceed.".to_string(), + )); + } + + Ok(()) +} + +fn print_human(checks: &[DoctorCheckResult]) { + println!("FastSkill Doctor"); + println!("{}", "=".repeat(40)); + for check in checks { + let icon = match check.status { + DoctorStatus::Ok => "[ok]", + DoctorStatus::Warning => "[warn]", + DoctorStatus::Error => "[error]", + }; + println!("{} {}: {}", icon, check.name, check.message); + } + println!(); + let errors = checks + .iter() + .filter(|c| c.status == DoctorStatus::Error) + .count(); + let warnings = checks + .iter() + .filter(|c| c.status == DoctorStatus::Warning) + .count(); + if errors == 0 && warnings == 0 { + println!("All checks passed."); + } else { + println!("{} error(s), {} warning(s).", errors, warnings); + } +} + +fn print_json(checks: &[DoctorCheckResult]) { + let items: Vec = checks + .iter() + .map(|c| { + serde_json::json!({ + "name": c.name, + "status": c.status.to_string(), + "message": c.message, + }) + }) + .collect(); + let output = serde_json::json!({ "checks": items }); + println!( + "{}", + serde_json::to_string_pretty(&output).unwrap_or_default() + ); +} + +#[cfg(test)] +#[allow(clippy::unwrap_used)] +mod tests { + use super::*; + use fastskill_core::{FastSkillService, ServiceConfig}; + use tempfile::TempDir; + + #[tokio::test] + async fn test_execute_doctor_basic() { + let temp_dir = TempDir::new().unwrap(); + std::fs::create_dir_all(temp_dir.path()).unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + let args = DoctorArgs { json: false }; + // Should succeed (skills dir exists) + let result = execute_doctor(&service, args).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_execute_doctor_json() { + let temp_dir = TempDir::new().unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + let args = DoctorArgs { json: true }; + let result = execute_doctor(&service, args).await; + assert!(result.is_ok()); + } + + #[tokio::test] + async fn test_execute_doctor_missing_skills_dir() { + // Verify that execute_doctor returns Err when the skills dir check has Error status. + // We confirm this via DoctorStatus logic rather than filesystem tricks, + // since FastSkillService::initialize() may create the directory. + let temp_dir = TempDir::new().unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + // Doctor should succeed when the skills dir exists + let args = DoctorArgs { json: false }; + let result = execute_doctor(&service, args).await; + // skills dir exists, so only warnings possible — should be Ok + assert!( + result.is_ok(), + "Expected Ok when skills dir exists: {:?}", + result + ); + } +} diff --git a/crates/fastskill-cli/src/commands/install.rs b/crates/fastskill-cli/src/commands/install.rs index d339daf4..9279a7a1 100644 --- a/crates/fastskill-cli/src/commands/install.rs +++ b/crates/fastskill-cli/src/commands/install.rs @@ -41,6 +41,12 @@ pub struct InstallArgs { /// Maximum transitive dependency depth (overrides config file setting) depth: Option, + + /// Trigger reindex after install (overrides config) + reindex: bool, + + /// Skip reindex after install + no_reindex: bool, } impl IntoCommandSpec for InstallArgs { @@ -86,6 +92,24 @@ impl IntoCommandSpec for InstallArgs { help: "Maximum transitive dependency depth (overrides config file setting)", ..Default::default() }, + ArgSpec { + name: "reindex", + kind: ArgKind::Flag, + long: Some("reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Trigger reindex after install (overrides config)", + ..Default::default() + }, + ArgSpec { + name: "no-reindex", + kind: ArgKind::Flag, + long: Some("no-reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Skip reindex after install", + ..Default::default() + }, ], ..Default::default() } @@ -128,6 +152,8 @@ impl FromArgValueMap for InstallArgs { None } }), + reindex: matches!(map.get("reindex"), Some(ArgValue::Bool(true))), + no_reindex: matches!(map.get("no-reindex"), Some(ArgValue::Bool(true))), } } } @@ -411,7 +437,16 @@ pub async fn execute_install(args: InstallArgs) -> CliResult<()> { ))); } - Ok(()) + let auto_reindex = crate::config_file::load_auto_reindex_config(); + crate::utils::reindex_utils::maybe_auto_reindex( + &service, + "install", + args.reindex, + args.no_reindex, + auto_reindex, + false, + ) + .await } #[cfg(test)] @@ -447,6 +482,8 @@ mod tests { only: None, lock: false, depth: None, + reindex: false, + no_reindex: false, }; let result = execute_install(args).await; @@ -490,6 +527,8 @@ mod tests { only: None, lock: true, depth: None, + reindex: false, + no_reindex: false, }; let result = execute_install(args).await; @@ -529,6 +568,8 @@ mod tests { only: None, lock: false, depth: None, + reindex: false, + no_reindex: false, }; // Should succeed with empty manifest (no skills to install) or fail on service/repos; shouldn't panic @@ -575,6 +616,8 @@ test-skill = { path = "source-skill" } only: None, lock: false, depth: None, + reindex: false, + no_reindex: false, }; let result = execute_install(args).await; diff --git a/crates/fastskill-cli/src/commands/mod.rs b/crates/fastskill-cli/src/commands/mod.rs index 2b109aa4..e0378c29 100644 --- a/crates/fastskill-cli/src/commands/mod.rs +++ b/crates/fastskill-cli/src/commands/mod.rs @@ -4,7 +4,7 @@ pub mod add; pub mod analyze; pub mod auth; pub mod common; -pub mod disable; +pub mod doctor; pub mod eval; pub mod init; pub mod install; @@ -17,11 +17,8 @@ pub mod registry; pub mod reindex; pub mod remove; pub mod repos; -pub mod resolve; pub mod search; pub mod serve; -pub mod show; pub mod skillopt; pub mod sources; -pub mod sync; pub mod update; diff --git a/crates/fastskill-cli/src/commands/reindex.rs b/crates/fastskill-cli/src/commands/reindex.rs index f2f91f4c..36caefad 100644 --- a/crates/fastskill-cli/src/commands/reindex.rs +++ b/crates/fastskill-cli/src/commands/reindex.rs @@ -70,16 +70,16 @@ pub struct ReindexArgs { pub skills_dir: Option, /// Force re-indexing of all skills (ignore existing hashes) - force: bool, + pub force: bool, /// Maximum number of concurrent embedding requests - max_concurrent: usize, + pub max_concurrent: usize, /// Show progress bars and processing details - progress: bool, + pub progress: bool, /// Suppress progress output, show only final summary - no_progress: bool, + pub no_progress: bool, } impl IntoCommandSpec for ReindexArgs { @@ -294,15 +294,11 @@ pub async fn execute_reindex(service: &FastSkillService, args: ReindexArgs) -> C } // Check if embedding is configured - let embedding_config = service.config().embedding.as_ref().ok_or_else(|| { - let searched_paths = crate::config_file::get_config_search_paths(); - let mut error_msg = "Error: Embedding configuration required but not found.\n\nSearched locations:\n".to_string(); - for path in searched_paths { - error_msg.push_str(&format!(" - {}\n", path.display())); - } - error_msg.push_str("\nTo set up FastSkill, run:\n fastskill init\n\nOr manually add to skill-project.toml:\n [tool.fastskill.embedding]\n openai_base_url = \"https://api.openai.com/v1\"\n embedding_model = \"text-embedding-3-small\"\n\nThen set your API key:\n export OPENAI_API_KEY=\"your-key-here\"\n"); - CliError::Config(error_msg) - })?; + if service.config().embedding.is_none() { + println!("Reindex skipped: no embedding provider configured. Run 'fastskill doctor' for setup guidance."); + return Ok(()); + } + let embedding_config = service.config().embedding.as_ref().unwrap(); // Get OpenAI API key let api_key = crate::config_file::get_openai_api_key()?; @@ -787,13 +783,9 @@ Test skill content"#, no_progress: false, }; + // With no embedding config, reindex is a no-op (informational message, exit 0) let result = execute_reindex(&service, args).await; - assert!(result.is_err()); - if let Err(CliError::Config(msg)) = result { - assert!(msg.contains("Embedding configuration required")); - } else { - panic!("Expected Config error"); - } + assert!(result.is_ok()); } #[tokio::test] diff --git a/crates/fastskill-cli/src/commands/remove.rs b/crates/fastskill-cli/src/commands/remove.rs index 5439a274..43ae3a5e 100644 --- a/crates/fastskill-cli/src/commands/remove.rs +++ b/crates/fastskill-cli/src/commands/remove.rs @@ -37,6 +37,12 @@ pub struct RemoveArgs { /// Skills directory path (overrides default discovery) #[allow(dead_code)] pub skills_dir: Option, + + /// Trigger reindex after removal (overrides config) + pub reindex: bool, + + /// Skip reindex after removal + pub no_reindex: bool, } impl IntoCommandSpec for RemoveArgs { @@ -79,6 +85,28 @@ impl IntoCommandSpec for RemoveArgs { default: None, ..Default::default() }, + ArgSpec { + name: "reindex", + long: Some("reindex"), + short: None, + help: "Trigger reindex after removal (overrides config)", + kind: ArgKind::Flag, + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + default: None, + ..Default::default() + }, + ArgSpec { + name: "no-reindex", + long: Some("no-reindex"), + short: None, + help: "Skip reindex after removal", + kind: ArgKind::Flag, + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + default: None, + ..Default::default() + }, ], ..Default::default() } @@ -109,6 +137,8 @@ impl FromArgValueMap for RemoveArgs { None } }), + reindex: matches!(map.get("reindex"), Some(ArgValue::Bool(true))), + no_reindex: matches!(map.get("no-reindex"), Some(ArgValue::Bool(true))), } } } @@ -312,6 +342,14 @@ pub async fn execute_remove( args: RemoveArgs, global: bool, ) -> CliResult<()> { + if args.reindex && args.no_reindex { + return Err(CliError::Validation( + "--reindex and --no-reindex cannot be used together".to_string(), + )); + } + let reindex = args.reindex; + let no_reindex = args.no_reindex; + // Validate inputs if args.skill_ids.is_empty() { return Err(CliError::Config("No skill IDs provided".to_string())); @@ -349,7 +387,16 @@ pub async fn execute_remove( } } - Ok(()) + let auto_reindex = crate::config_file::load_auto_reindex_config(); + crate::utils::reindex_utils::maybe_auto_reindex( + service, + "remove", + reindex, + no_reindex, + auto_reindex, + false, + ) + .await } #[cfg(test)] @@ -380,6 +427,8 @@ mod tests { skill_ids: vec![], force: false, skills_dir: None, + reindex: false, + no_reindex: false, }; let result = execute_remove(&service, args, false).await; @@ -406,6 +455,8 @@ mod tests { skill_ids: vec!["invalid@skill@id".to_string()], force: true, skills_dir: None, + reindex: false, + no_reindex: false, }; let result = execute_remove(&service, args, false).await; @@ -432,6 +483,8 @@ mod tests { skill_ids: vec!["nonexistent@1.0.0".to_string()], force: true, skills_dir: None, + reindex: false, + no_reindex: false, }; // This should fail because the skill doesn't exist @@ -497,6 +550,8 @@ source = { path = ".claude/skills/test-skill" } skill_ids: vec!["test-skill".to_string()], force: true, skills_dir: None, + reindex: false, + no_reindex: false, }; let result = execute_remove(&service, args, false).await; @@ -568,6 +623,8 @@ source = { path = ".claude/skills/test-skill" } skill_ids: vec!["test-skill".to_string()], force: true, skills_dir: None, + reindex: false, + no_reindex: false, }; let result = execute_remove(&service, args, false).await; diff --git a/crates/fastskill-cli/src/commands/resolve.rs b/crates/fastskill-cli/src/commands/resolve.rs deleted file mode 100644 index 8fbd4648..00000000 --- a/crates/fastskill-cli/src/commands/resolve.rs +++ /dev/null @@ -1,243 +0,0 @@ -//! Resolve command implementation -//! -//! Machine-first skill resolution that returns canonical paths, metadata, and optional content. - -use crate::error::{CliError, CliResult}; -use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; -use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; -use cli_framework::spec::command_tree::CommandSpec; -use cli_framework::spec::value::ArgValue; -use fastskill_core::core::context_resolver::{ContentMode, ResolveContextRequest, ResolveScope}; -use fastskill_core::FastSkillService; -use std::collections::HashMap; -use std::path::PathBuf; - -#[derive(Debug)] -pub struct ResolveArgs { - /// Search query string - pub query: String, - - /// Maximum number of results (default: 5) - pub limit: usize, - - /// Content to include: none, preview (frontmatter + 20 lines), full - pub include_content: String, - - /// Resolve and validate canonical absolute paths (default: true) - pub resolve_paths: bool, - - /// Skills directory path (overrides default discovery) - #[allow(dead_code)] - pub skills_dir: Option, -} - -impl IntoCommandSpec for ResolveArgs { - fn command_spec() -> CommandSpec { - CommandSpec { - summary: "Resolve skills as machine-readable JSON with canonical paths", - syntax: Some("resolve [OPTIONS]"), - category: Some("discovery"), - args: vec![ - ArgSpec { - name: "query", - long: None, - short: None, - help: "Search query string", - kind: ArgKind::Positional, - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - default: None, - ..Default::default() - }, - ArgSpec { - name: "limit", - long: Some("limit"), - short: Some('l'), - help: "Maximum number of results", - kind: ArgKind::Option, - value_type: ArgValueType::Int, - cardinality: Cardinality::Optional, - default: Some(ArgValue::Int(5)), - ..Default::default() - }, - ArgSpec { - name: "include-content", - long: Some("include-content"), - short: None, - help: "Content to include: none, preview, full", - kind: ArgKind::Option, - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - default: Some(ArgValue::Str("none".to_string())), - ..Default::default() - }, - ArgSpec { - name: "resolve-paths", - long: Some("resolve-paths"), - short: None, - help: "Resolve and validate canonical absolute paths", - kind: ArgKind::Option, - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - default: Some(ArgValue::Bool(true)), - ..Default::default() - }, - ArgSpec { - name: "skills-dir", - long: Some("skills-dir"), - short: None, - help: "Skills directory path (overrides default discovery)", - kind: ArgKind::Option, - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - default: None, - ..Default::default() - }, - ], - ..Default::default() - } - } -} - -impl FromArgValueMap for ResolveArgs { - fn from_arg_value_map(map: &HashMap) -> Self { - ResolveArgs { - query: map - .get("query") - .and_then(|v| { - if let ArgValue::Str(s) = v { - Some(s.clone()) - } else { - None - } - }) - .unwrap_or_else(|| panic!("fw bug: missing query")), - limit: map - .get("limit") - .and_then(|v| { - if let ArgValue::Int(n) = v { - Some(*n as usize) - } else { - None - } - }) - .unwrap_or(5), - include_content: map - .get("include-content") - .and_then(|v| { - if let ArgValue::Str(s) = v { - Some(s.clone()) - } else { - None - } - }) - .unwrap_or_else(|| "none".to_string()), - resolve_paths: map - .get("resolve-paths") - .and_then(|v| { - if let ArgValue::Bool(b) = v { - Some(*b) - } else { - None - } - }) - .unwrap_or(true), - skills_dir: map.get("skills-dir").and_then(|v| { - if let ArgValue::Str(s) = v { - Some(PathBuf::from(s)) - } else { - None - } - }), - } - } -} - -pub async fn execute_resolve(service: &FastSkillService, args: ResolveArgs) -> CliResult<()> { - let include_content = parse_content_mode(&args.include_content)?; - - let request = ResolveContextRequest { - prompt: args.query, - limit: args.limit, - scope: ResolveScope::Local, - include_content, - resolve_paths: args.resolve_paths, - }; - - let resolver = service.context_resolver(); - let response = resolver - .resolve_context(request) - .await - .map_err(CliError::Service)?; - - let json = serde_json::to_string_pretty(&response) - .map_err(|e| CliError::Config(format!("Failed to serialize response: {}", e)))?; - - println!("{}", json); - Ok(()) -} - -fn parse_content_mode(s: &str) -> CliResult { - match s { - "none" => Ok(ContentMode::None), - "preview" => Ok(ContentMode::Preview), - "full" => Ok(ContentMode::Full), - other => Err(CliError::Config(format!( - "Invalid --include-content value '{}': must be none, preview, or full", - other - ))), - } -} - -#[cfg(test)] -#[allow(clippy::unwrap_used)] -mod tests { - use super::*; - - #[test] - fn test_parse_content_mode_none() { - assert_eq!(parse_content_mode("none").unwrap(), ContentMode::None); - } - - #[test] - fn test_parse_content_mode_preview() { - assert_eq!(parse_content_mode("preview").unwrap(), ContentMode::Preview); - } - - #[test] - fn test_parse_content_mode_full() { - assert_eq!(parse_content_mode("full").unwrap(), ContentMode::Full); - } - - #[test] - fn test_parse_content_mode_invalid() { - assert!(parse_content_mode("bad").is_err()); - } - - #[tokio::test] - async fn test_execute_resolve_returns_valid_json() { - use fastskill_core::ServiceConfig; - use tempfile::TempDir; - - let temp_dir = TempDir::new().unwrap(); - let config = ServiceConfig { - skill_storage_path: temp_dir.path().to_path_buf(), - embedding: None, - ..Default::default() - }; - - let mut service = FastSkillService::new(config).await.unwrap(); - service.initialize().await.unwrap(); - - let args = ResolveArgs { - query: "test query".to_string(), - limit: 5, - include_content: "none".to_string(), - resolve_paths: true, - skills_dir: None, - }; - - let result = execute_resolve(&service, args).await; - assert!(result.is_ok()); - } -} diff --git a/crates/fastskill-cli/src/commands/search.rs b/crates/fastskill-cli/src/commands/search.rs index 95d1d46d..3f5fc990 100644 --- a/crates/fastskill-cli/src/commands/search.rs +++ b/crates/fastskill-cli/src/commands/search.rs @@ -45,6 +45,12 @@ pub struct SearchArgs { /// Skills directory path (overrides default discovery) #[allow(dead_code)] pub skills_dir: Option, + + /// Output resolved file paths instead of search results (--local only) + pub paths: bool, + + /// Content mode for resolved skill context: none, inline, ref (--local --paths only) + pub content: Option, } impl IntoCommandSpec for SearchArgs { @@ -153,6 +159,28 @@ impl IntoCommandSpec for SearchArgs { default: None, ..Default::default() }, + ArgSpec { + name: "paths", + long: Some("paths"), + short: None, + help: "Output resolved file paths instead of search results (--local only)", + kind: ArgKind::Flag, + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + default: None, + ..Default::default() + }, + ArgSpec { + name: "content", + long: Some("content"), + short: None, + help: "Content mode for resolved skill context: none, inline, ref (--local --paths only)", + kind: ArgKind::Option, + value_type: ArgValueType::String, + cardinality: Cardinality::Optional, + default: None, + ..Default::default() + }, ], ..Default::default() } @@ -213,6 +241,14 @@ impl FromArgValueMap for SearchArgs { None } }), + paths: matches!(map.get("paths"), Some(ArgValue::Bool(true))), + content: map.get("content").and_then(|v| { + if let ArgValue::Str(s) = v { + Some(s.clone()) + } else { + None + } + }), } } } @@ -268,6 +304,19 @@ fn validate_search_args(args: &SearchArgs) -> CliResult<()> { )); } + if args.paths && !args.local { + return Err(CliError::Config( + "Error: --paths requires --local. Use 'fastskill search --local --paths '." + .to_string(), + )); + } + + if args.content.is_some() && !args.paths { + return Err(CliError::Config( + "Error: --content requires --paths. Use 'fastskill search --local --paths --content '.".to_string(), + )); + } + Ok(()) } @@ -329,6 +378,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let result = validate_search_args(&args); @@ -350,6 +401,8 @@ mod tests { json: false, embedding: Some("false".to_string()), skills_dir: None, + paths: false, + content: None, }; let result = validate_search_args(&args); @@ -368,6 +421,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let result = validate_search_args(&args); @@ -386,6 +441,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let scope = determine_search_scope(&args).unwrap(); @@ -404,6 +461,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let scope = determine_search_scope(&args).unwrap(); @@ -422,6 +481,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let scope = determine_search_scope(&args).unwrap(); @@ -440,6 +501,8 @@ mod tests { json: true, embedding: None, skills_dir: None, + paths: false, + content: None, }; let format = determine_output_format(&args).unwrap(); @@ -458,6 +521,8 @@ mod tests { json: false, embedding: None, skills_dir: None, + paths: false, + content: None, }; let format = determine_output_format(&args).unwrap(); @@ -486,6 +551,8 @@ mod tests { json: false, embedding: Some("false".to_string()), // Force text search skills_dir: None, + paths: false, + content: None, }; let result = execute_search(&service, args).await; @@ -505,6 +572,8 @@ mod tests { json: true, embedding: None, skills_dir: None, + paths: false, + content: None, }; let result = validate_search_args(&args); @@ -523,6 +592,8 @@ mod tests { json: false, embedding: Some("auto".to_string()), skills_dir: None, + paths: false, + content: None, }; let mode = determine_embedding_mode(&args); diff --git a/crates/fastskill-cli/src/commands/show.rs b/crates/fastskill-cli/src/commands/show.rs deleted file mode 100644 index 0526b58a..00000000 --- a/crates/fastskill-cli/src/commands/show.rs +++ /dev/null @@ -1,619 +0,0 @@ -//! Show command - displays skill details and dependency tree -//! -//! `show` displays skill metadata (name, version, description, source) not full content. -//! For full SKILL.md content, use `read`. -//! -//! With no skill_id argument, lists all installed skills with minimal details. -//! With a skill_id, shows detailed metadata for that specific skill. - -use crate::commands::common::validate_format_args; -use crate::config::{create_service_config, get_skill_search_locations_for_display}; -use crate::error::{CliError, CliResult, SkillNotFoundMessage}; -use chrono::Utc; -use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; -use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; -use cli_framework::spec::command_tree::CommandSpec; -use cli_framework::spec::value::ArgValue; -use fastskill_core::core::lock::ProjectSkillsLock; -use fastskill_core::core::{ProjectConfig, SkillDefinition}; -use fastskill_core::{FastSkillService, OutputFormat}; -use std::collections::HashMap; -use std::path::PathBuf; - -/// Show skill details -#[derive(Debug)] -pub struct ShowArgs { - /// Skill ID to show (if not specified, shows all) - skill_id: Option, - - /// Show dependency tree - tree: bool, - - /// Output format: table, json, grid, xml (default: table) - pub format: Option, - - /// Shorthand for --format json - pub json: bool, - - /// Skills directory path (overrides default discovery) - pub skills_dir: Option, - - /// Read from skills.lock only (do not initialize service) - pub locked: bool, - - /// Read installed state from storage (default mode) - #[allow(dead_code)] - pub installed: bool, -} - -impl IntoCommandSpec for ShowArgs { - fn command_spec() -> CommandSpec { - CommandSpec { - summary: "Show metadata for one or all installed skills", - syntax: Some("show [SKILL_ID] [OPTIONS]"), - category: Some("discovery"), - args: vec![ - ArgSpec { - name: "skill-id", - kind: ArgKind::Positional, - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - help: "Skill ID to show (if not specified, shows all)", - ..Default::default() - }, - ArgSpec { - name: "tree", - kind: ArgKind::Flag, - long: Some("tree"), - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - help: "Show dependency tree", - ..Default::default() - }, - ArgSpec { - name: "format", - kind: ArgKind::Option, - long: Some("format"), - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - help: "Output format: table, json, grid, xml (default: table)", - ..Default::default() - }, - ArgSpec { - name: "json", - kind: ArgKind::Flag, - long: Some("json"), - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - help: "Shorthand for --format json", - ..Default::default() - }, - ArgSpec { - name: "locked", - kind: ArgKind::Flag, - long: Some("locked"), - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - help: "Read from skills.lock only (do not initialize service)", - ..Default::default() - }, - ArgSpec { - name: "installed", - kind: ArgKind::Flag, - long: Some("installed"), - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - help: "Read installed state from storage (default mode)", - ..Default::default() - }, - ], - ..Default::default() - } - } -} - -fn opt_str(v: &ArgValue) -> Option { - if let ArgValue::Str(s) = v { - Some(s.clone()) - } else { - None - } -} - -fn parse_output_format(s: &str) -> Option { - match s { - "table" => Some(OutputFormat::Table), - "json" => Some(OutputFormat::Json), - "grid" => Some(OutputFormat::Grid), - "xml" => Some(OutputFormat::Xml), - _ => None, - } -} - -#[allow(clippy::panic)] -impl FromArgValueMap for ShowArgs { - fn from_arg_value_map(map: &HashMap) -> Self { - Self { - skill_id: map.get("skill-id").and_then(opt_str), - tree: matches!(map.get("tree"), Some(ArgValue::Bool(true))), - format: map - .get("format") - .and_then(opt_str) - .as_deref() - .and_then(parse_output_format), - json: matches!(map.get("json"), Some(ArgValue::Bool(true))), - // skills_dir is omitted from the spec; use the global --skills-dir flag instead - skills_dir: None, - locked: matches!(map.get("locked"), Some(ArgValue::Bool(true))), - installed: matches!(map.get("installed"), Some(ArgValue::Bool(true))), - } - } -} - -pub async fn execute_show(args: ShowArgs, global: bool) -> CliResult<()> { - let format = validate_format_args(&args.format, args.json)?; - - // Validate flag combinations - validate_show_args(&args, global)?; - - if args.locked { - // Locked mode: read only from skills.lock - let (config, lock_opt) = load_config_and_lock()?; - match lock_opt { - Some(lock) => run_with_lock(lock, &args, &config, format)?, - None => { - return Err(CliError::Config( - "No skills.lock found. Run 'fastskill install' to generate one.".to_string(), - )); - } - } - } else { - // Installed mode (default): use FastSkillService - run_with_service(&args, format, global).await?; - } - Ok(()) -} - -fn validate_show_args(args: &ShowArgs, global: bool) -> CliResult<()> { - // --locked + --skills-dir is invalid - if args.locked && args.skills_dir.is_some() { - return Err(CliError::Validation( - "--locked mode does not support --skills-dir (lock file uses project-relative paths)" - .to_string(), - )); - } - - // --locked + --global is invalid - if args.locked && global { - return Err(CliError::Validation( - "--locked mode does not support --global (lock file uses project-relative paths)" - .to_string(), - )); - } - - Ok(()) -} - -fn load_config_and_lock() -> CliResult<(ProjectConfig, Option)> { - let current_dir = std::env::current_dir() - .map_err(|e| CliError::Config(format!("Failed to get current directory: {}", e)))?; - let config = - fastskill_core::core::load_project_config(¤t_dir).map_err(CliError::Config)?; - let lock_path = config.project_root.join("skills.lock"); - let lock_opt = if lock_path.exists() { - let lock = ProjectSkillsLock::load_from_file(&lock_path) - .map_err(|e| CliError::Config(format!("Failed to load lock file: {}", e)))?; - Some(lock) - } else { - None - }; - Ok((config, lock_opt)) -} - -fn run_with_lock( - lock: ProjectSkillsLock, - args: &ShowArgs, - config: &ProjectConfig, - format: OutputFormat, -) -> CliResult<()> { - if let Some(skill_id) = &args.skill_id { - if let Some(skill) = lock.skills.iter().find(|s| s.id == skill_id.as_str()) { - let skill_definition = locked_skill_to_definition(skill)?; - let formatted_output = - fastskill_core::output::format_show_results(&[skill_definition], format) - .map_err(CliError::Config)?; - print!("{}", formatted_output); - if args.tree { - println!("\nDependency Tree:"); - println!(" (Dependency tree not yet implemented)"); - } - return Ok(()); - } - let paths = search_paths_from_config(config); - return Err(CliError::SkillNotFound(SkillNotFoundMessage::new( - skill_id.clone(), - paths, - ))); - } - print_lock_list(&lock, args.tree, format)?; - Ok(()) -} - -fn search_paths_from_config(config: &ProjectConfig) -> Vec<(PathBuf, String)> { - get_skill_search_locations_for_display(false) - .unwrap_or_else(|_| vec![(config.skills_directory.clone(), "project".to_string())]) -} - -fn print_lock_list(lock: &ProjectSkillsLock, tree: bool, format: OutputFormat) -> CliResult<()> { - let skills: Vec = lock - .skills - .iter() - .map(locked_skill_to_definition) - .collect::, CliError>>()?; - - let formatted_output = - fastskill_core::output::format_show_results(&skills, format).map_err(CliError::Config)?; - print!("{}", formatted_output); - - if tree { - for skill in &lock.skills { - println!(" Groups: {:?}", skill.groups); - println!(" Source: {:?}", skill.source); - } - } - - Ok(()) -} - -async fn run_with_service(args: &ShowArgs, format: OutputFormat, global: bool) -> CliResult<()> { - let config = create_service_config(global, args.skills_dir.clone(), None)?; - let mut service = FastSkillService::new(config) - .await - .map_err(CliError::Service)?; - service.initialize().await.map_err(CliError::Service)?; - - if let Some(skill_id) = &args.skill_id { - return run_show_one_from_service(&service, skill_id, format).await; - } - run_show_all_from_service(&service, format).await -} - -async fn run_show_one_from_service( - service: &FastSkillService, - skill_id: &str, - format: OutputFormat, -) -> CliResult<()> { - let skill_id_parsed = fastskill_core::SkillId::new(skill_id.to_string()) - .map_err(|_| CliError::Validation(format!("Invalid skill ID format: {}", skill_id)))?; - let skill = service - .skill_manager() - .get_skill(&skill_id_parsed) - .await - .map_err(CliError::Service)?; - if let Some(skill) = skill { - let formatted_output = fastskill_core::output::format_show_results(&[skill], format) - .map_err(CliError::Config)?; - print!("{}", formatted_output); - return Ok(()); - } - let paths = get_skill_search_locations_for_display(false).unwrap_or_else(|_| { - vec![( - service.config().skill_storage_path.clone(), - "project".to_string(), - )] - }); - Err(CliError::SkillNotFound(SkillNotFoundMessage::new( - skill_id.to_string(), - paths, - ))) -} - -async fn run_show_all_from_service( - service: &FastSkillService, - format: OutputFormat, -) -> CliResult<()> { - let skills = service - .skill_manager() - .list_skills(None) - .await - .map_err(CliError::Service)?; - - let formatted_output = - fastskill_core::output::format_show_results(&skills, format).map_err(CliError::Config)?; - print!("{}", formatted_output); - - Ok(()) -} - -fn locked_skill_to_definition( - skill: &fastskill_core::core::lock::ProjectLockedSkillEntry, -) -> Result { - let now = Utc::now(); - let id = fastskill_core::SkillId::new(skill.id.clone()).map_err(|_| { - CliError::Validation(format!( - "Invalid skill ID format in lock file: {}", - skill.id - )) - })?; - - Ok(SkillDefinition { - id, - name: skill.name.clone(), - description: format!("Skill from {}", skill.name), - version: skill.version.clone(), - author: None, - enabled: true, - created_at: now, - updated_at: now, - skill_file: PathBuf::from(format!("/skills/{}", skill.id)), - reference_files: None, - script_files: None, - asset_files: None, - execution_environment: None, - dependencies: None, - timeout: None, - source_url: None, - source_type: None, - source_branch: None, - source_tag: None, - source_subdir: None, - installed_from: None, - commit_hash: None, - fetched_at: None, - editable: skill.editable, - }) -} - -#[cfg(test)] -#[allow( - clippy::unwrap_used, - clippy::panic, - clippy::expect_used, - clippy::await_holding_lock -)] -mod tests { - use super::*; - use std::fs; - use tempfile::TempDir; - - /// Runs a test in a temp project (cwd set, .claude/skills, skill-project.toml). Restores cwd on drop. - async fn with_temp_project(f: F) -> R - where - F: FnOnce(TempDir) -> Fut, - Fut: std::future::Future, - { - let _lock = fastskill_core::test_utils::DIR_MUTEX - .lock() - .unwrap_or_else(|e| e.into_inner()); - let temp_dir = TempDir::new().unwrap(); - let original_dir = std::env::current_dir().ok(); - struct DirGuard(Option); - impl Drop for DirGuard { - fn drop(&mut self) { - if let Some(dir) = &self.0 { - let _ = std::env::set_current_dir(dir); - } - } - } - let _guard = DirGuard(original_dir); - std::env::set_current_dir(temp_dir.path()).unwrap(); - fs::create_dir_all(temp_dir.path().join(".claude/skills")).unwrap(); - let manifest = r#"[tool.fastskill] -skills_directory = ".claude/skills" -"#; - fs::write(temp_dir.path().join("skill-project.toml"), manifest).unwrap(); - f(temp_dir).await - } - - #[tokio::test] - async fn test_execute_show_no_skill_id_no_lock_file() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("FASTSKILL_SKILLS_DIR", temp_dir.path()); - - let args = ShowArgs { - skill_id: None, - tree: false, - format: None, - json: false, - skills_dir: None, - locked: false, - installed: false, - }; - - // Should not crash even if no lock file exists - let result = execute_show(args, false).await; - // Result may be Ok or Err depending on service initialization, but shouldn't panic - assert!(result.is_ok() || result.is_err()); - - std::env::remove_var("FASTSKILL_SKILLS_DIR"); - } - - #[tokio::test] - async fn test_execute_show_with_invalid_skill_id() { - let temp_dir = TempDir::new().unwrap(); - std::env::set_var("FASTSKILL_SKILLS_DIR", temp_dir.path()); - - let args = ShowArgs { - skill_id: Some("invalid@skill@id".to_string()), - tree: false, - format: None, - json: false, - skills_dir: None, - locked: false, - installed: false, - }; - - let result = execute_show(args, false).await; - // Should fail with validation error or config error - assert!(result.is_err()); - - std::env::remove_var("FASTSKILL_SKILLS_DIR"); - } - - #[tokio::test] - async fn test_execute_show_with_nonexistent_skill_id() { - let temp_dir = TempDir::new().unwrap(); - let original_dir = std::env::current_dir().ok(); - std::env::set_var("FASTSKILL_SKILLS_DIR", temp_dir.path()); - - let args = ShowArgs { - skill_id: Some("nonexistent@1.0.0".to_string()), - tree: false, - format: None, - json: false, - skills_dir: None, - locked: false, - installed: false, - }; - - let result = execute_show(args, false).await; - // Should fail because skill doesn't exist or invalid format - assert!(result.is_err(), "Expected error, got: {:?}", result); - match &result { - Err(CliError::SkillNotFound(_)) => {} - Err(CliError::Config(msg)) => { - assert!( - msg.contains("not found") - || msg.contains("Failed to get current directory") - || msg.contains("Failed to load lock file") - || msg.contains("Failed to load skill-project.toml") - || msg.contains("skills_directory"), - "Error message '{}' does not contain expected text", - msg - ); - } - Err(CliError::Validation(msg)) => { - // Validation error for invalid skill ID format is also acceptable - assert!( - msg.contains("Invalid") || msg.contains("format"), - "Validation error '{}' should mention invalid format", - msg - ); - } - Err(e) => { - // Any error is acceptable as long as it fails - eprintln!("Got error: {:?}", e); - } - Ok(_) => { - panic!("Expected error but got Ok"); - } - } - - std::env::remove_var("FASTSKILL_SKILLS_DIR"); - if let Some(dir) = original_dir { - let _ = std::env::set_current_dir(&dir); - } - } - - #[tokio::test] - async fn test_execute_show_with_lock() { - with_temp_project(|temp_dir| async move { - let lock_content = r#"version = "1.0.0" -generated_at = "2024-01-01T00:00:00Z" -fastskill_version = "0.1.0" - -[[skills]] -id = "test-skill" -name = "test-skill" -version = "1.0.0" -source_type = "local" -source = { path = ".claude/skills/test-skill" } -"#; - fs::write(temp_dir.path().join("skills.lock"), lock_content).unwrap(); - let args = ShowArgs { - skill_id: None, - tree: false, - format: None, - json: false, - skills_dir: None, - locked: true, - installed: false, - }; - let result = execute_show(args, false).await; - assert!(result.is_ok() || result.is_err()); - }) - .await; - } - - #[tokio::test] - async fn test_execute_show_with_skill_id() { - with_temp_project(|temp_dir| async move { - let skill_dir = temp_dir.path().join(".claude/skills/test-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"# Test Skill - -Name: test-skill -Version: 1.0.0 -Description: A test skill for coverage -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - let args = ShowArgs { - skill_id: Some("test-skill".to_string()), - tree: false, - format: None, - json: false, - skills_dir: None, - locked: false, - installed: false, - }; - let result = execute_show(args, false).await; - assert!(result.is_ok() || result.is_err()); - }) - .await; - } - - #[tokio::test] - async fn test_validate_show_args_locked_with_skills_dir() { - let args = ShowArgs { - skill_id: None, - tree: false, - format: None, - json: false, - skills_dir: Some(PathBuf::from("/tmp/skills")), - locked: true, - installed: false, - }; - let result = validate_show_args(&args, false); - assert!(result.is_err()); - if let Err(CliError::Validation(msg)) = result { - assert!(msg.contains("--locked mode does not support --skills-dir")); - } else { - panic!("Expected Validation error"); - } - } - - #[tokio::test] - async fn test_validate_show_args_locked_with_global() { - let args = ShowArgs { - skill_id: None, - tree: false, - format: None, - json: false, - skills_dir: None, - locked: true, - installed: false, - }; - let result = validate_show_args(&args, true); - assert!(result.is_err()); - if let Err(CliError::Validation(msg)) = result { - assert!(msg.contains("--locked mode does not support --global")); - } else { - panic!("Expected Validation error"); - } - } - - #[tokio::test] - async fn test_validate_show_args_installed_mode_valid() { - let args = ShowArgs { - skill_id: None, - tree: false, - format: None, - json: false, - skills_dir: Some(PathBuf::from("/tmp/skills")), - locked: false, - installed: true, - }; - let result = validate_show_args(&args, false); - assert!(result.is_ok()); - } -} diff --git a/crates/fastskill-cli/src/commands/sync.rs b/crates/fastskill-cli/src/commands/sync.rs deleted file mode 100644 index f2b9bfb9..00000000 --- a/crates/fastskill-cli/src/commands/sync.rs +++ /dev/null @@ -1,614 +0,0 @@ -//! Sync command implementation -//! -//! Synchronizes installed skills into the agent's metadata file (AGENTS.md, CLAUDE.md, etc.) - -use crate::commands::common::runtime_selection_error_to_cli; -use crate::error::{CliError, CliResult}; -use crate::runtime_selector::RuntimeSelectionInput; -use crate::utils::agents_md::{ - generate_skills_xml, parse_current_skills, remove_skills_section, replace_skills_section, - SkillLocation, SkillSummary, -}; -use crate::utils::messages; -use aikit_sdk::{instruction_file_with_override, DeployError}; -use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; -use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; -use cli_framework::spec::command_tree::CommandSpec; -use cli_framework::spec::value::ArgValue; -use dirs; -use fastskill_core::core::skill_manager::SkillDefinition; -use fastskill_core::FastSkillService; -use std::collections::HashMap; -use std::env; -use std::fs; -use std::path::{Path, PathBuf}; -use tracing::debug; - -/// Sync installed skills into the agent's metadata file (CLAUDE.md, AGENTS.md, etc.). -/// -/// This command writes skill identifiers and descriptions into the agent's instruction -/// file so the agent can discover available skills. It does NOT manage filesystem links -/// or symlinks; editable skill links are established by `fastskill add -e` or -/// `fastskill install`. -#[derive(Debug)] -pub struct SyncArgs { - /// Non-interactive mode: include all installed skills - pub yes: bool, - - /// Target runtime(s) for this operation; repeatable (mutually exclusive with --all) - pub agent: Vec, - - /// Target all runtimes discovered by aikit (mutually exclusive with --agent) - pub all: bool, - - /// Path to metadata file (overrides auto-detection) - pub agents_file: Option, -} - -impl IntoCommandSpec for SyncArgs { - fn command_spec() -> CommandSpec { - CommandSpec { - summary: "Sync installed skills to the agent's metadata file", - syntax: Some("sync [OPTIONS]"), - category: Some("packages"), - args: vec![ - ArgSpec { - name: "yes", - long: Some("yes"), - short: Some('y'), - help: "Non-interactive mode: include all installed skills", - kind: ArgKind::Flag, - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - default: None, - ..Default::default() - }, - ArgSpec { - name: "agent", - long: Some("agent"), - short: None, - help: "Target runtime(s) for this operation; repeatable", - kind: ArgKind::Option, - value_type: ArgValueType::String, - cardinality: Cardinality::Repeated, - default: None, - ..Default::default() - }, - ArgSpec { - name: "all", - long: Some("all"), - short: None, - help: "Target all runtimes discovered by aikit", - kind: ArgKind::Flag, - value_type: ArgValueType::Bool, - cardinality: Cardinality::Optional, - default: None, - ..Default::default() - }, - ArgSpec { - name: "agents-file", - long: Some("agents-file"), - short: None, - help: "Path to metadata file (overrides auto-detection)", - kind: ArgKind::Option, - value_type: ArgValueType::String, - cardinality: Cardinality::Optional, - default: None, - ..Default::default() - }, - ], - ..Default::default() - } - } -} - -impl FromArgValueMap for SyncArgs { - fn from_arg_value_map(map: &HashMap) -> Self { - SyncArgs { - yes: matches!(map.get("yes"), Some(ArgValue::Bool(true))), - agent: match map.get("agent") { - Some(ArgValue::List(items)) => items - .iter() - .filter_map(|i| { - if let ArgValue::Str(s) = i { - Some(s.clone()) - } else { - None - } - }) - .collect(), - _ => vec![], - }, - all: matches!(map.get("all"), Some(ArgValue::Bool(true))), - agents_file: map.get("agents-file").and_then(|v| { - if let ArgValue::Str(s) = v { - Some(s.clone()) - } else { - None - } - }), - } - } -} - -impl From<&SyncArgs> for RuntimeSelectionInput { - fn from(args: &SyncArgs) -> Self { - RuntimeSelectionInput { - agents: args.agent.clone(), - all: args.all, - } - } -} - -/// Execute the sync command -pub async fn execute_sync(service: &FastSkillService, args: SyncArgs) -> CliResult<()> { - let current_dir = env::current_dir() - .map_err(|e| CliError::Config(format!("Failed to get current directory: {}", e)))?; - - // `--agents-file` bypasses agent resolution entirely. - if let Some(ref override_path) = args.agents_file { - let agents_file = resolve_single_agents_file(¤t_dir, None, Some(override_path))?; - return sync_to_file(service, &args, &agents_file).await; - } - - // Resolve runtime selection. - let input = RuntimeSelectionInput::from(&args); - let selection = crate::runtime_selector::resolve_runtime_selection(&input) - .map_err(runtime_selection_error_to_cli)?; - - match selection { - Some(sel) => { - // Iterate over each resolved runtime and sync to its metadata file. - for agent_key in &sel.runtimes { - match resolve_single_agents_file(¤t_dir, Some(agent_key.as_str()), None) { - Ok(agents_file) => { - sync_to_file(service, &args, &agents_file).await?; - } - Err(CliError::Config(ref msg)) - if msg.contains("not found") - || msg.contains("does not support metadata files") => - { - // Skip runtimes that don't support instruction files; warn only. - eprintln!("warning: skipping agent '{}': {}", agent_key, msg); - } - Err(e) => return Err(e), - } - } - } - None => { - // No --agent or --all provided: fall back to aikit auto-detection. - let agents_file = resolve_single_agents_file(¤t_dir, None, None)?; - return sync_to_file(service, &args, &agents_file).await; - } - } - - Ok(()) -} - -/// Perform the skill sync operation for a single metadata file path. -async fn sync_to_file( - service: &FastSkillService, - args: &SyncArgs, - agents_file: &Path, -) -> CliResult<()> { - let metadata_file_name = agents_file - .file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_else(|| agents_file.display().to_string()); - - debug!("Agents file: {}", agents_file.display()); - - // Get global skills directory - let global_skills_dir = get_global_skills_directory()?; - - // Enumerate installed skills - let installed_skills = service - .skill_manager() - .list_skills(None) - .await - .map_err(|e| { - CliError::Service(fastskill_core::ServiceError::Custom(format!( - "Failed to list installed skills: {}", - e - ))) - })?; - - debug!("Found {} installed skills", installed_skills.len()); - - // Convert to SkillSummary with location info - let skill_summaries: Vec = installed_skills - .into_iter() - .map(|skill| { - let location = determine_skill_location(&skill, &global_skills_dir); - SkillSummary { - id: skill.id.to_string(), - name: skill.name.clone(), - description: skill.description.clone(), - location, - } - }) - .collect(); - - // Read existing metadata file if it exists - let existing_content = if agents_file.exists() { - fs::read_to_string(agents_file).map_err(|e| { - CliError::Io(std::io::Error::other(format!( - "Failed to read {}: {}", - agents_file.display(), - e - ))) - })? - } else { - String::new() - }; - - // Parse currently selected skills from metadata file - let current_skill_ids: HashMap = parse_current_skills(&existing_content) - .into_iter() - .map(|id| (id.clone(), true)) - .collect(); - - debug!( - "Current skill IDs in {}: {:?}", - metadata_file_name, current_skill_ids - ); - - // Select skills to sync - let selected_skills = if args.yes { - // Non-interactive mode: select all skills - skill_summaries.clone() - } else { - // Interactive mode: let user choose - interactive_select_skills(&skill_summaries, ¤t_skill_ids, &metadata_file_name)? - }; - - debug!("Selected {} skills", selected_skills.len()); - - // Generate and write metadata file - if selected_skills.is_empty() { - // Remove skills section - let new_content = remove_skills_section(&existing_content); - fs::write(agents_file, new_content).map_err(|e| { - CliError::Io(std::io::Error::other(format!( - "Failed to write {}: {}", - agents_file.display(), - e - ))) - })?; - println!( - "{}", - messages::ok(&format!("Removed all skills from {}", metadata_file_name)) - ); - } else { - // Generate and update skills section - let skills_xml = generate_skills_xml(&selected_skills); - let new_content = replace_skills_section(&existing_content, &skills_xml); - fs::write(agents_file, new_content).map_err(|e| { - CliError::Io(std::io::Error::other(format!( - "Failed to write {}: {}", - agents_file.display(), - e - ))) - })?; - println!( - "{}", - messages::ok(&format!( - "Synced {} skill(s) to {}", - selected_skills.len(), - metadata_file_name - )) - ); - } - - Ok(()) -} - -/// Resolve the agent metadata file path for a single agent key or override path. -/// -/// Resolution priority: -/// 1. Explicit override path (absolute used as-is; relative resolved from `current_dir`) -/// 2. Explicit agent key (uses the agent's primary instruction file) -/// 3. Auto-detect: scans `current_dir` for AGENTS.md, CLAUDE.md, GEMINI.md; defaults to AGENTS.md -fn resolve_single_agents_file( - current_dir: &Path, - agent_key: Option<&str>, - override_path: Option<&str>, -) -> CliResult { - let override_pb = override_path.map(PathBuf::from); - - let path = instruction_file_with_override( - current_dir, - agent_key, - override_pb.as_deref(), - ) - .map_err(|e| match e { - DeployError::AgentNotFound(ref key) => CliError::Config(format!( - "Agent '{}' not found. Valid keys include: claude, cursor-agent, gemini, codex, windsurf.", - key - )), - DeployError::UnsupportedConcept { ref agent_key, .. } => CliError::Config(format!( - "Agent '{}' does not support metadata files. Use --agents-file to specify a custom path.", - agent_key - )), - _ => CliError::Config(format!("Failed to resolve metadata file: {}", e)), - })?; - - debug!("Resolved metadata file: {}", path.display()); - Ok(path) -} - -/// Get the global skills directory path -fn get_global_skills_directory() -> CliResult { - let config_dir = dirs::config_dir().ok_or_else(|| { - CliError::Config("Failed to determine system config directory".to_string()) - })?; - Ok(config_dir.join("fastskill/skills")) -} - -/// Determine if a skill is project or global -fn determine_skill_location(skill: &SkillDefinition, global_dir: &Path) -> SkillLocation { - let skill_dir = skill.skill_file.parent().unwrap_or_else(|| Path::new(".")); - let skill_dir = skill_dir - .canonicalize() - .unwrap_or_else(|_| skill_dir.to_path_buf()); - let global_dir = global_dir - .canonicalize() - .unwrap_or_else(|_| global_dir.to_path_buf()); - - // Check if skill is under global directory - if skill_dir.starts_with(&global_dir) { - SkillLocation::Global - } else { - SkillLocation::Project - } -} - -/// Interactive skill selection using checkboxes -fn interactive_select_skills( - all_skills: &[SkillSummary], - current_ids: &HashMap, - metadata_file_name: &str, -) -> CliResult> { - use inquire::MultiSelect; - - if all_skills.is_empty() { - println!("{}", messages::warning("No skills available to sync")); - return Ok(Vec::new()); - } - - // Create label for each skill and build mapping - let skill_labels: Vec = all_skills - .iter() - .map(|skill| { - let location_str = if skill.location == SkillLocation::Project { - "project" - } else { - "global" - }; - format!("{} ({})", skill.id, location_str) - }) - .collect(); - - // Build mapping from label to skill summary - let label_to_skill: std::collections::HashMap = skill_labels - .iter() - .enumerate() - .map(|(i, label)| { - let skill = &all_skills[i]; - (label.clone(), skill.clone()) - }) - .collect(); - - // Default selection: project skills, skills already in metadata file (by index) - let default_indices: Vec = all_skills - .iter() - .enumerate() - .filter(|(_i, skill)| { - skill.location == SkillLocation::Project || current_ids.contains_key(&skill.id) - }) - .map(|(i, _)| i) - .collect(); - - let message = format!("Select skills to expose in {}", metadata_file_name); - let prompt = MultiSelect::new(&message, skill_labels) - .with_default(&default_indices) - .with_page_size(15) - .with_help_message("↑/↓: move, Space: select/deselect, Enter: confirm, Esc: cancel"); - - match prompt.prompt() { - Ok(selected_labels) => { - let selected_skills: Vec = selected_labels - .iter() - .filter_map(|label| label_to_skill.get(label).cloned()) - .collect(); - Ok(selected_skills) - } - Err(inquire::InquireError::OperationCanceled) => { - println!("{}", messages::info("Cancelled by user")); - Ok(Vec::new()) - } - Err(e) => Err(CliError::Config(format!( - "Interactive selection failed: {}", - e - ))), - } -} - -#[cfg(test)] -#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -mod tests { - use super::*; - use fastskill_core::SkillId; - - #[test] - fn test_determine_skill_location_project() { - let global_dir = PathBuf::from("/home/user/.config/fastskill/skills"); - let skill_id = SkillId::new("test-skill".to_string()).unwrap(); - let skill = SkillDefinition::new( - skill_id, - "Test Skill".to_string(), - "Test".to_string(), - "1.0.0".to_string(), - ); - let mut skill = skill; - skill.skill_file = PathBuf::from("/home/user/project/.claude/skills/test-skill/SKILL.md"); - - let location = determine_skill_location(&skill, &global_dir); - assert_eq!(location, SkillLocation::Project); - } - - #[test] - fn test_determine_skill_location_global() { - let global_dir = PathBuf::from("/home/user/.config/fastskill/skills"); - let skill_id = SkillId::new("test-skill".to_string()).unwrap(); - let skill = SkillDefinition::new( - skill_id, - "Test Skill".to_string(), - "Test".to_string(), - "1.0.0".to_string(), - ); - let mut skill = skill; - skill.skill_file = PathBuf::from("/home/user/.config/fastskill/skills/test-skill/SKILL.md"); - - let location = determine_skill_location(&skill, &global_dir); - assert_eq!(location, SkillLocation::Global); - } - - #[test] - fn test_resolve_single_agents_file_default_no_files() { - let temp = tempfile::tempdir().unwrap(); - let current_dir = temp.path(); - - let result = resolve_single_agents_file(current_dir, None, None).unwrap(); - assert_eq!(result, current_dir.join("AGENTS.md")); - } - - #[test] - fn test_resolve_single_agents_file_default_claude_md_present() { - let temp = tempfile::tempdir().unwrap(); - let current_dir = temp.path(); - std::fs::write(current_dir.join("CLAUDE.md"), "# Claude").unwrap(); - - let result = resolve_single_agents_file(current_dir, None, None).unwrap(); - assert_eq!(result, current_dir.join("CLAUDE.md")); - } - - #[test] - fn test_resolve_single_agents_file_override() { - let temp = tempfile::tempdir().unwrap(); - let current_dir = temp.path(); - let result = resolve_single_agents_file(current_dir, None, Some("custom.md")).unwrap(); - assert_eq!(result, current_dir.join("custom.md")); - } - - #[test] - fn test_resolve_single_agents_file_absolute() { - let temp = tempfile::tempdir().unwrap(); - let current_dir = temp.path(); - let result = - resolve_single_agents_file(current_dir, None, Some("/absolute/path/AGENTS.md")) - .unwrap(); - assert_eq!(result, PathBuf::from("/absolute/path/AGENTS.md")); - } -} - -#[cfg(test)] -#[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] -mod aikit_integration_tests { - use super::*; - use std::fs; - use tempfile::TempDir; - - #[test] - fn test_resolve_single_agents_file_with_explicit_agent() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - let result = resolve_single_agents_file(current_dir, Some("claude"), None).unwrap(); - assert_eq!(result, current_dir.join("CLAUDE.md")); - - let result = resolve_single_agents_file(current_dir, Some("cursor-agent"), None).unwrap(); - assert_eq!(result, current_dir.join("AGENTS.md")); - } - - #[test] - fn test_resolve_single_agents_file_with_override() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - let result = resolve_single_agents_file(current_dir, None, Some("custom.md")).unwrap(); - assert_eq!(result, current_dir.join("custom.md")); - } - - #[test] - fn test_resolve_single_agents_file_absolute_override() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - let result = - resolve_single_agents_file(current_dir, None, Some("/absolute/path/AGENTS.md")) - .unwrap(); - assert_eq!(result, PathBuf::from("/absolute/path/AGENTS.md")); - } - - #[test] - fn test_resolve_single_agents_file_auto_detect_claude() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - fs::write(current_dir.join("CLAUDE.md"), "# CLAUDE").unwrap(); - - let result = resolve_single_agents_file(current_dir, None, None).unwrap(); - assert_eq!(result, current_dir.join("CLAUDE.md")); - } - - #[test] - fn test_resolve_single_agents_file_auto_detect_cursor() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - fs::write(current_dir.join("AGENTS.md"), "# AGENTS").unwrap(); - - let result = resolve_single_agents_file(current_dir, None, None).unwrap(); - assert_eq!(result, current_dir.join("AGENTS.md")); - } - - #[test] - fn test_resolve_single_agents_file_override_precedence() { - let temp = TempDir::new().unwrap(); - let current_dir = temp.path(); - - fs::write(current_dir.join("CLAUDE.md"), "# CLAUDE").unwrap(); - - let result = - resolve_single_agents_file(current_dir, Some("claude"), Some("override.md")).unwrap(); - - assert_eq!(result, current_dir.join("override.md")); - } - - #[test] - fn test_runtime_selection_input_from_sync_args() { - let args = SyncArgs { - yes: false, - agent: vec!["claude".to_string(), "codex".to_string()], - all: false, - agents_file: None, - }; - let input = RuntimeSelectionInput::from(&args); - assert_eq!(input.agents, vec!["claude", "codex"]); - assert!(!input.all); - } - - #[test] - fn test_runtime_selection_input_all_flag() { - let args = SyncArgs { - yes: false, - agent: vec![], - all: true, - agents_file: None, - }; - let input = RuntimeSelectionInput::from(&args); - assert!(input.agents.is_empty()); - assert!(input.all); - } -} diff --git a/crates/fastskill-cli/src/commands/update.rs b/crates/fastskill-cli/src/commands/update.rs index a662828c..bd855711 100644 --- a/crates/fastskill-cli/src/commands/update.rs +++ b/crates/fastskill-cli/src/commands/update.rs @@ -53,6 +53,12 @@ pub struct UpdateArgs { /// Update strategy: latest, patch, minor, major strategy: String, + + /// Trigger reindex after update (overrides config) + reindex: bool, + + /// Skip reindex after update + no_reindex: bool, } impl IntoCommandSpec for UpdateArgs { @@ -116,6 +122,24 @@ impl IntoCommandSpec for UpdateArgs { help: "Update strategy: latest, patch, minor, major", ..Default::default() }, + ArgSpec { + name: "reindex", + kind: ArgKind::Flag, + long: Some("reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Trigger reindex after update (overrides config)", + ..Default::default() + }, + ArgSpec { + name: "no-reindex", + kind: ArgKind::Flag, + long: Some("no-reindex"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Skip reindex after update", + ..Default::default() + }, ], ..Default::default() } @@ -143,15 +167,43 @@ impl FromArgValueMap for UpdateArgs { .get("strategy") .and_then(opt_str) .unwrap_or_else(|| "latest".to_string()), + reindex: matches!(map.get("reindex"), Some(ArgValue::Bool(true))), + no_reindex: matches!(map.get("no-reindex"), Some(ArgValue::Bool(true))), } } } pub async fn execute_update(args: UpdateArgs, global: bool) -> CliResult<()> { + let reindex = args.reindex; + let no_reindex = args.no_reindex; + let check = args.check; + let dry_run = args.dry_run; + if global { - return execute_update_global(args).await; + execute_update_global(args).await?; + } else { + execute_update_project(args).await?; } - execute_update_project(args).await + + if !check && !dry_run { + let auto_reindex_config = crate::config_file::load_auto_reindex_config(); + let config = create_service_config(global, None, None)?; + if let Ok(mut svc) = fastskill_core::FastSkillService::new(config).await { + if svc.initialize().await.is_ok() { + let _ = crate::utils::reindex_utils::maybe_auto_reindex( + &svc, + "update", + reindex, + no_reindex, + auto_reindex_config, + false, + ) + .await; + } + } + } + + Ok(()) } async fn execute_update_global(args: UpdateArgs) -> CliResult<()> { @@ -459,6 +511,8 @@ mod tests { version: None, source: None, strategy: "latest".to_string(), + reindex: false, + no_reindex: false, }; let result = execute_update(args, false).await; @@ -549,6 +603,8 @@ version = "1.0.0" version: None, source: None, strategy: "invalid-strategy".to_string(), + reindex: false, + no_reindex: false, }; let result = execute_update(args, false).await; @@ -607,6 +663,8 @@ version = "1.0.0" version: None, source: None, strategy: "latest".to_string(), + reindex: false, + no_reindex: false, }; // Should succeed in check mode even with no skills @@ -667,6 +725,8 @@ source = { path = ".claude/skills/test-skill" } version: None, source: None, strategy: "latest".to_string(), + reindex: false, + no_reindex: false, }; let result = execute_update(args, false).await; diff --git a/crates/fastskill-cli/src/config_file.rs b/crates/fastskill-cli/src/config_file.rs index fa90b608..36fbc09e 100644 --- a/crates/fastskill-cli/src/config_file.rs +++ b/crates/fastskill-cli/src/config_file.rs @@ -30,6 +30,13 @@ pub struct FastSkillConfig { /// HTTP server configuration #[serde(default)] pub server: Option, + /// Automatically reindex after add/install/update/remove (default: true) + #[serde(default = "default_true")] + pub auto_reindex: bool, +} + +fn default_true() -> bool { + true } /// HTTP server configuration (CLI version) @@ -93,6 +100,7 @@ pub fn load_config_from_skill_project(current_dir: &Path) -> CliResult CliResult { .map_err(|_| CliError::Config("OPENAI_API_KEY environment variable not set".to_string())) } -/// Get all searched configuration paths for error reporting -pub fn get_config_search_paths() -> Vec { - let mut paths = Vec::new(); - - // Walk up directory tree for skill-project.toml - if let Ok(current_dir) = std::env::current_dir() { - let mut current = current_dir; - loop { - let project_file = current.join("skill-project.toml"); - paths.push(project_file); - - if !current.pop() { - break; - } - } +/// Load the auto_reindex setting from config (defaults to true) +pub fn load_auto_reindex_config() -> bool { + if let Ok(Some(config)) = load_config() { + config.auto_reindex + } else { + true } - - paths } diff --git a/crates/fastskill-cli/src/main.rs b/crates/fastskill-cli/src/main.rs index a77d61f2..652b7199 100644 --- a/crates/fastskill-cli/src/main.rs +++ b/crates/fastskill-cli/src/main.rs @@ -62,8 +62,8 @@ fn ctx_skills_dir(ctx: &dyn AppContext) -> Option { } use commands::{ - add, analyze, auth, disable, eval, init, install, list, marketplace, package, publish, read, - reindex, remove, repos, resolve, search, serve, show, skillopt, sync, update, + add, analyze, auth, doctor, eval, init, install, list, marketplace, package, publish, read, + reindex, remove, repos, search, serve, skillopt, update, }; #[tokio::main] @@ -153,14 +153,6 @@ fn build_app(builder: AppBuilder, state: Arc) -> anyhow::Result) -> anyhow::Result) -> anyhow::Result) -> anyhow::Result Vec { - let mut skill_ids = Vec::new(); - let mut in_available_skills = false; - let mut in_skill = false; - - for line in content.lines() { - let line = line.trim(); - - if line.contains("") { - in_available_skills = true; - continue; - } - - if line.contains("") { - in_available_skills = false; - continue; - } - - if in_available_skills && line.contains("") { - in_skill = true; - continue; - } - - if line.contains("") { - in_skill = false; - continue; - } - - if in_skill && line.contains("") { - if let Some(name_start) = line.find("") { - let name_part = &line[name_start + 6..]; - if let Some(name_end) = name_part.find("") { - skill_ids.push(name_part[..name_end].trim().to_string()); - } - } - } - } - - debug!("Parsed {} skills from AGENTS.md", skill_ids.len()); - skill_ids -} - -/// Generate the XML skills section for AGENTS.md -pub fn generate_skills_xml(skills: &[SkillSummary]) -> String { - let mut xml = String::new(); - - xml.push_str("\n\n"); - xml.push_str("## Available Skills\n\n"); - xml.push_str("\n"); - xml.push_str( - r#" -When users ask you to perform tasks, check if any of the available skills below can help complete the task more effectively. Skills provide specialized capabilities and domain knowledge. - -How to use skills: -- Invoke: Bash("fastskill read <skill-id>") -- The skill content will load with detailed instructions on how to complete the task -- Base directory provided in output for resolving bundled resources (references/, scripts/, assets/) - -Usage notes: -- Only use skills listed in <available_skills> below -- Do not invoke a skill that is already loaded in your context -- Each skill invocation is stateless - - - - -"#, - ); - - for skill in skills { - xml.push_str("\n"); - xml.push_str(&format!(" {}\n", xml_escape(&skill.id))); - xml.push_str(&format!( - " {}\n", - xml_escape(&skill.description) - )); - let location_str = match skill.location { - SkillLocation::Project => "project", - SkillLocation::Global => "global", - }; - xml.push_str(&format!(" {}\n", location_str)); - xml.push_str("\n\n"); - } - - xml.push_str("\n"); - xml.push_str("\n\n"); - xml.push_str(""); - - xml -} - -/// Replace or append the skills section in AGENTS.md content -pub fn replace_skills_section(content: &str, new_section: &str) -> String { - if let Some(start) = content.find("") { - let end = end + "".len(); - let mut new_content = String::with_capacity(content.len()); - new_content.push_str(&content[..start]); - new_content.push_str(new_section); - new_content.push_str(&content[end..]); - return new_content; - } - } - - if let Some(start) = content.find("") { - if let Some(end) = content.find("") { - let end = end + "".len(); - let mut new_content = String::with_capacity(content.len()); - new_content.push_str(&content[..start]); - new_content.push_str(new_section); - new_content.push_str(&content[end..]); - return new_content; - } - } - - let mut new_content = String::with_capacity(content.len() + new_section.len() + 2); - if !content.is_empty() && !content.ends_with('\n') { - new_content.push('\n'); - } - new_content.push_str(content); - if !content.is_empty() { - new_content.push_str("\n\n"); - } - new_content.push_str(new_section); - new_content -} - -/// Remove the skills section from AGENTS.md content -pub fn remove_skills_section(content: &str) -> String { - if let Some(start) = content.find("") { - let end = end + "".len(); - let mut new_content = String::with_capacity(content.len()); - new_content.push_str(&content[..start]); - new_content.push_str(&content[end..]); - return new_content; - } - } - - if let Some(start) = content.find("") { - if let Some(end) = content.find("") { - let end = end + "".len(); - let mut new_content = String::with_capacity(content.len()); - new_content.push_str(&content[..start]); - new_content.push_str(&content[end..]); - return new_content; - } - } - - content.to_string() -} - -/// Escape special XML characters -fn xml_escape(s: &str) -> String { - s.replace('&', "&") - .replace('<', "<") - .replace('>', ">") - .replace('"', """) - .replace('\'', "'") -} - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn test_parse_current_skills_empty() { - let content = "# AGENTS.md\n\nSome content\n"; - let skills = parse_current_skills(content); - assert!(skills.is_empty()); - } - - #[test] - fn test_parse_current_skills_with_skills() { - let content = r#" - - - skill1 - Test skill 1 - project - - - skill2 - Test skill 2 - global - - -"#; - let skills = parse_current_skills(content); - assert_eq!(skills.len(), 2); - assert_eq!(skills[0], "skill1"); - assert_eq!(skills[1], "skill2"); - } - - #[test] - fn test_generate_skills_xml() { - let skills = vec![ - SkillSummary { - id: "skill1".to_string(), - name: "Skill 1".to_string(), - description: "Test skill 1".to_string(), - location: SkillLocation::Project, - }, - SkillSummary { - id: "skill2".to_string(), - name: "Skill 2".to_string(), - description: "Test skill 2".to_string(), - location: SkillLocation::Global, - }, - ]; - - let xml = generate_skills_xml(&skills); - - assert!(xml.contains("")); - assert!(xml.contains("skill1")); - assert!(xml.contains("skill2")); - assert!(xml.contains("Test skill 1")); - assert!(xml.contains("Test skill 2")); - assert!(xml.contains("project")); - assert!(xml.contains("global")); - } - - #[test] - fn test_replace_skills_section_existing() { - let content = r#"# AGENTS.md - -Some content - - - - - old-skill - - - - -More content"#; - - let new_section = r#" - - - new-skill - - -"#; - - let result = replace_skills_section(content, new_section); - assert!(!result.contains("old-skill")); - assert!(result.contains("new-skill")); - assert!(result.contains("More content")); - } - - #[test] - fn test_replace_skills_section_new() { - let content = "# AGENTS.md\n\nSome content\n"; - let new_section = r#" - - -"#; - - let result = replace_skills_section(content, new_section); - assert!(result.contains(" - - - skill1 - - - - -More content"#; - - let result = remove_skills_section(content); - assert!(!result.contains(""), "test <tag>"); - assert_eq!(xml_escape("test &"), "test &amp;"); - assert_eq!(xml_escape("test \"quoted\""), "test "quoted""); - } -} diff --git a/crates/fastskill-cli/src/utils/reindex_utils.rs b/crates/fastskill-cli/src/utils/reindex_utils.rs new file mode 100644 index 00000000..d5dc992d --- /dev/null +++ b/crates/fastskill-cli/src/utils/reindex_utils.rs @@ -0,0 +1,50 @@ +//! Utilities for auto-reindex after skill mutations + +use crate::error::CliResult; +use fastskill_core::FastSkillService; + +/// Run reindex if conditions are met; failures are non-fatal warnings. +pub async fn maybe_auto_reindex( + service: &FastSkillService, + command_name: &str, + explicit_reindex: bool, + explicit_no_reindex: bool, + config_auto_reindex: bool, + verbose: bool, +) -> CliResult<()> { + if explicit_no_reindex { + return Ok(()); + } + + if service.config().embedding.is_none() { + if verbose { + println!( + "Note: skipping auto-reindex after '{}' (no embedding provider configured).", + command_name + ); + } + return Ok(()); + } + + let should_reindex = explicit_reindex || config_auto_reindex; + if !should_reindex { + return Ok(()); + } + + let args = crate::commands::reindex::ReindexArgs { + skills_dir: None, + force: false, + max_concurrent: 5, + progress: false, + no_progress: true, + }; + + if let Err(e) = crate::commands::reindex::execute_reindex(service, args).await { + eprintln!( + "Warning: auto-reindex after '{}' failed: {}", + command_name, e + ); + } + + Ok(()) +} diff --git a/tests/cli/doctor_tests.rs b/tests/cli/doctor_tests.rs new file mode 100644 index 00000000..d7a77a2b --- /dev/null +++ b/tests/cli/doctor_tests.rs @@ -0,0 +1,53 @@ +//! E2E tests for the doctor command + +#![allow(clippy::all, clippy::unwrap_used, clippy::expect_used)] + +use super::snapshot_helpers::run_fastskill_command; +use std::fs; +use tempfile::TempDir; + +#[test] +fn test_doctor_runs_successfully() { + let temp_dir = TempDir::new().unwrap(); + let skills_dir = temp_dir.path().join(".claude").join("skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + fs::write( + temp_dir.path().join("skill-project.toml"), + "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", + ) + .unwrap(); + + let result = run_fastskill_command(&["doctor"], Some(temp_dir.path())); + // Doctor should succeed (skills dir exists) + assert!(result.success, "doctor failed: {}", result.stderr); +} + +#[test] +fn test_doctor_json_flag() { + let temp_dir = TempDir::new().unwrap(); + let skills_dir = temp_dir.path().join(".claude").join("skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + fs::write( + temp_dir.path().join("skill-project.toml"), + "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", + ) + .unwrap(); + + let result = run_fastskill_command(&["doctor", "--json"], Some(temp_dir.path())); + assert!(result.success, "doctor --json failed: {}", result.stderr); + // Output should be valid JSON with a checks array + assert!( + result.stdout.contains("\"checks\""), + "Expected JSON output with 'checks' key, got: {}", + result.stdout + ); +} + +#[test] +fn test_doctor_help() { + let result = run_fastskill_command(&["doctor", "--help"], None); + assert!(result.success); + assert!(result.stdout.contains("doctor")); +} diff --git a/tests/cli/help_tests.rs b/tests/cli/help_tests.rs index a0b2f7a5..8c84a17e 100644 --- a/tests/cli/help_tests.rs +++ b/tests/cli/help_tests.rs @@ -54,18 +54,6 @@ fn test_search_command_help() { ); } -#[test] -fn test_disable_command_help() { - let result = run_fastskill_command(&["disable", "--help"], None); - - assert!(result.success); - assert_snapshot_with_settings( - "disable_command_help", - &result.stdout, - &cli_snapshot_settings(), - ); -} - #[test] fn test_remove_command_help() { let result = run_fastskill_command(&["remove", "--help"], None); diff --git a/tests/cli/mod.rs b/tests/cli/mod.rs index 18f5b6ad..6335860c 100644 --- a/tests/cli/mod.rs +++ b/tests/cli/mod.rs @@ -7,6 +7,7 @@ pub mod add_tests; pub mod analyze_cluster_tests; pub mod auth_e2e_tests; pub mod config_tests; +pub mod doctor_tests; pub mod eval_tests; pub mod example_tests; pub mod help_tests; @@ -26,10 +27,8 @@ pub mod reindex_e2e_tests; pub mod repos_integration_tests; pub mod repository_tests; pub mod serve_e2e_tests; -pub mod show_e2e_tests; pub mod skillopt_tests; pub mod snapshot_helpers; -pub mod sync_e2e_tests; pub mod test_helpers; pub mod unit_tests; pub mod update_e2e_tests; diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__disable_command_help.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__disable_command_help.snap deleted file mode 100644 index ae55a788..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__disable_command_help.snap +++ /dev/null @@ -1,16 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -Disable skills by ID - -Usage: fastskill disable [SKILL_IDS]... - -Arguments: - [SKILL_IDS]... Skill IDs to disable - -Options: - -h, --help Print help - -Examples: - fastskill disable my-skill-id diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_all_skills.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_all_skills.snap deleted file mode 100644 index 1b07c9d0..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_all_skills.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -Skill: test-skill - ID: test-skill - Version: [VERSION] - Description: A test skill for show command diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_dependency_tree.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_dependency_tree.snap deleted file mode 100644 index 26d5729c..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_dependency_tree.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -Skill: complex-skill - ID: complex-skill - Version: [VERSION] - Description: A complex skill with dependencies diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format.snap deleted file mode 100644 index 56c5e620..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -Error: Configuration error: skill-project.toml not found in this directory or any parent. Create it at the top level of your workspace (e.g. run 'fastskill init' there), then run this command again. diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format_cli.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format_cli.snap deleted file mode 100644 index 830eb11c..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_invalid_id_format_cli.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -assertion_line: 195 -expression: normalized ---- -Error: Validation error: Invalid skill ID format: invalid skill id! diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill.snap deleted file mode 100644 index 56c5e620..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -Error: Configuration error: skill-project.toml not found in this directory or any parent. Create it at the top level of your workspace (e.g. run 'fastskill init' there), then run this command again. diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill_cli.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill_cli.snap deleted file mode 100644 index 0afb4694..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_nonexistent_skill_cli.snap +++ /dev/null @@ -1,5 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -Error: Configuration error: skill-project.toml not found in this directory or any parent. Create it at the top level of your workspace (e.g. run 'fastskill init' there), then run this command again. diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_specific_skill.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_specific_skill.snap deleted file mode 100644 index 60d70ed5..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__show_specific_skill.snap +++ /dev/null @@ -1,9 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -Skill: web-scraper - ID: web-scraper - Version: [VERSION] - Description: A web scraping skill diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_all_skills_yes.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_all_skills_yes.snap deleted file mode 100644 index e01384b9..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_all_skills_yes.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 2 skill(s) to AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_auto_detects_claude_metadata_file.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_auto_detects_claude_metadata_file.snap deleted file mode 100644 index 0d1fc51f..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_auto_detects_claude_metadata_file.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to CLAUDE.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_custom_files.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_custom_files.snap deleted file mode 100644 index 64c664ec..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_custom_files.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to CUSTOM.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_empty_skills_list.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_empty_skills_list.snap deleted file mode 100644 index 94026e0b..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_empty_skills_list.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Removed all skills from AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_no_agents_md_creates_file.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_no_agents_md_creates_file.snap deleted file mode 100644 index 3a6c35d7..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_no_agents_md_creates_file.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_project_vs_global.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_project_vs_global.snap deleted file mode 100644 index 3a6c35d7..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_project_vs_global.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_remove_all_skills.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_remove_all_skills.snap deleted file mode 100644 index 94026e0b..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_remove_all_skills.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Removed all skills from AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_existing_agents_md.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_existing_agents_md.snap deleted file mode 100644 index 3a6c35d7..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_existing_agents_md.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to AGENTS.md diff --git a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_explicit_agent_flag.snap b/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_explicit_agent_flag.snap deleted file mode 100644 index 0d1fc51f..00000000 --- a/tests/cli/snapshots/cli_tests__cli__snapshot_helpers__sync_with_explicit_agent_flag.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: tests/cli/snapshot_helpers.rs -expression: normalized ---- -FastSkill [VERSION] -[OK] Synced 1 skill(s) to CLAUDE.md From 96950c3b988dff1f173712d1ec6068b2839608fc Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:07:16 +0000 Subject: [PATCH 2/6] chore(develop): 183-cli-command-surface-redesign-remove-redundant-vestigial-comm --- crates/fastskill-cli/src/commands/doctor.rs | 76 ++-- crates/fastskill-cli/src/commands/init.rs | 1 + crates/fastskill-cli/src/commands/read.rs | 415 ++++++++++++++++++- crates/fastskill-cli/src/commands/search.rs | 53 ++- crates/fastskill-cli/src/config_file.rs | 2 +- crates/fastskill-core/src/core/manifest.rs | 7 + crates/fastskill-core/src/core/repository.rs | 2 + tests/cli/read_e2e_tests.rs | 160 +++++++ tests/cli/reindex_e2e_tests.rs | 30 ++ 9 files changed, 694 insertions(+), 52 deletions(-) diff --git a/crates/fastskill-cli/src/commands/doctor.rs b/crates/fastskill-cli/src/commands/doctor.rs index 79b35304..851ef996 100644 --- a/crates/fastskill-cli/src/commands/doctor.rs +++ b/crates/fastskill-cli/src/commands/doctor.rs @@ -8,26 +8,26 @@ use cli_framework::spec::value::ArgValue; use fastskill_core::FastSkillService; use std::collections::HashMap; -#[derive(Debug, Clone, PartialEq)] +#[derive(Debug, Clone, PartialEq, serde::Serialize)] pub enum DoctorStatus { - Ok, - Warning, - Error, + Pass, + Warn, + Fail, } impl std::fmt::Display for DoctorStatus { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { - DoctorStatus::Ok => write!(f, "ok"), - DoctorStatus::Warning => write!(f, "warning"), - DoctorStatus::Error => write!(f, "error"), + DoctorStatus::Pass => write!(f, "pass"), + DoctorStatus::Warn => write!(f, "warn"), + DoctorStatus::Fail => write!(f, "fail"), } } } #[derive(Debug)] pub struct DoctorCheckResult { - pub name: String, + pub check: String, pub status: DoctorStatus, pub message: String, } @@ -76,14 +76,14 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli let skills_dir = &service.config().skill_storage_path; let skills_dir_check = if skills_dir.exists() && skills_dir.is_dir() { DoctorCheckResult { - name: "skills_dir".to_string(), - status: DoctorStatus::Ok, + check: "skills_dir".to_string(), + status: DoctorStatus::Pass, message: format!("Skills directory found: {}", skills_dir.display()), } } else { DoctorCheckResult { - name: "skills_dir".to_string(), - status: DoctorStatus::Error, + check: "skills_dir".to_string(), + status: DoctorStatus::Fail, message: format!( "Skills directory not found or not a directory: {}", skills_dir.display() @@ -99,14 +99,14 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli let project_file = fastskill_core::core::project::resolve_project_file(¤t_dir); if project_file.found { DoctorCheckResult { - name: "project_toml".to_string(), - status: DoctorStatus::Ok, + check: "project_toml".to_string(), + status: DoctorStatus::Pass, message: format!("skill-project.toml found: {}", project_file.path.display()), } } else { DoctorCheckResult { - name: "project_toml".to_string(), - status: DoctorStatus::Warning, + check: "project_toml".to_string(), + status: DoctorStatus::Warn, message: "skill-project.toml not found. Run 'fastskill init' to create one." .to_string(), } @@ -117,14 +117,14 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli // Check 3: Embedding configuration present let embedding_check = if service.config().embedding.is_some() { DoctorCheckResult { - name: "embedding_config".to_string(), - status: DoctorStatus::Ok, + check: "embedding_config".to_string(), + status: DoctorStatus::Pass, message: "Embedding configuration found.".to_string(), } } else { DoctorCheckResult { - name: "embedding_config".to_string(), - status: DoctorStatus::Warning, + check: "embedding_config".to_string(), + status: DoctorStatus::Warn, message: "No embedding configuration. Semantic search and reindex are disabled. Add [tool.fastskill.embedding] to skill-project.toml.".to_string(), } }; @@ -134,14 +134,14 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli let api_key_check = if service.config().embedding.is_some() { if std::env::var("OPENAI_API_KEY").is_ok() { DoctorCheckResult { - name: "api_key".to_string(), - status: DoctorStatus::Ok, + check: "api_key".to_string(), + status: DoctorStatus::Pass, message: "OPENAI_API_KEY environment variable is set.".to_string(), } } else { DoctorCheckResult { - name: "api_key".to_string(), - status: DoctorStatus::Warning, + check: "api_key".to_string(), + status: DoctorStatus::Warn, message: "OPENAI_API_KEY environment variable not set. Set it to enable embeddings." .to_string(), @@ -149,8 +149,8 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli } } else { DoctorCheckResult { - name: "api_key".to_string(), - status: DoctorStatus::Warning, + check: "api_key".to_string(), + status: DoctorStatus::Warn, message: "No embedding config — API key check skipped.".to_string(), } }; @@ -161,14 +161,14 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli || std::env::var("FASTSKILL_TOKEN").is_ok() { DoctorCheckResult { - name: "auth_token".to_string(), - status: DoctorStatus::Ok, + check: "auth_token".to_string(), + status: DoctorStatus::Pass, message: "Auth token found in environment.".to_string(), } } else { DoctorCheckResult { - name: "auth_token".to_string(), - status: DoctorStatus::Warning, + check: "auth_token".to_string(), + status: DoctorStatus::Warn, message: "No auth token set. Remote registry operations may fail. Run 'fastskill auth login'.".to_string(), } }; @@ -183,7 +183,7 @@ pub async fn execute_doctor(service: &FastSkillService, args: DoctorArgs) -> Cli // Exit 0 unless skills_dir is inaccessible let has_error = checks .iter() - .any(|c| c.name == "skills_dir" && c.status == DoctorStatus::Error); + .any(|c| c.check == "skills_dir" && c.status == DoctorStatus::Fail); if has_error { return Err(CliError::Config( "Skills directory is inaccessible. Fix the skills directory to proceed.".to_string(), @@ -198,20 +198,20 @@ fn print_human(checks: &[DoctorCheckResult]) { println!("{}", "=".repeat(40)); for check in checks { let icon = match check.status { - DoctorStatus::Ok => "[ok]", - DoctorStatus::Warning => "[warn]", - DoctorStatus::Error => "[error]", + DoctorStatus::Pass => "[PASS]", + DoctorStatus::Warn => "[WARN]", + DoctorStatus::Fail => "[FAIL]", }; - println!("{} {}: {}", icon, check.name, check.message); + println!("{} {}: {}", icon, check.check, check.message); } println!(); let errors = checks .iter() - .filter(|c| c.status == DoctorStatus::Error) + .filter(|c| c.status == DoctorStatus::Fail) .count(); let warnings = checks .iter() - .filter(|c| c.status == DoctorStatus::Warning) + .filter(|c| c.status == DoctorStatus::Warn) .count(); if errors == 0 && warnings == 0 { println!("All checks passed."); @@ -225,7 +225,7 @@ fn print_json(checks: &[DoctorCheckResult]) { .iter() .map(|c| { serde_json::json!({ - "name": c.name, + "check": c.check, "status": c.status.to_string(), "message": c.message, }) diff --git a/crates/fastskill-cli/src/commands/init.rs b/crates/fastskill-cli/src/commands/init.rs index 661c807a..0877507f 100644 --- a/crates/fastskill-cli/src/commands/init.rs +++ b/crates/fastskill-cli/src/commands/init.rs @@ -354,6 +354,7 @@ fn build_skill_project(meta: InitMetadata<'_>) -> CliResult { install_depth: 5, skip_transitive: false, eval: None, + auto_reindex: true, }), }); validate_project_structure(true, dependencies.is_some()) diff --git a/crates/fastskill-cli/src/commands/read.rs b/crates/fastskill-cli/src/commands/read.rs index bbd90f01..7620ef7a 100644 --- a/crates/fastskill-cli/src/commands/read.rs +++ b/crates/fastskill-cli/src/commands/read.rs @@ -1,39 +1,116 @@ //! Read command - streams skill documentation to stdout //! //! Reads and outputs the full SKILL.md content. For metadata only (name, version, description), -//! use `show`. +//! use `show`. For structured metadata with optional formats, use `--meta`. +use crate::commands::common::validate_format_args; use crate::config; use crate::error::{CliError, CliResult, SkillNotFoundMessage}; use cli_framework::command::{FromArgValueMap, IntoCommandSpec}; use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality}; use cli_framework::spec::command_tree::CommandSpec; use cli_framework::spec::value::ArgValue; +use fastskill_core::core::lock::ProjectSkillsLock; +use fastskill_core::core::skill_manager::SkillDefinition; +use fastskill_core::output::{format_show_results, OutputFormat}; use fastskill_core::FastSkillService; use std::collections::HashMap; +use std::path::PathBuf; use std::sync::Arc; +fn parse_output_format(s: &str) -> Option { + match s { + "table" => Some(OutputFormat::Table), + "json" => Some(OutputFormat::Json), + "grid" => Some(OutputFormat::Grid), + "xml" => Some(OutputFormat::Xml), + _ => None, + } +} + /// Read skill documentation #[derive(Debug)] pub struct ReadArgs { /// Skill ID to read pub skill_id: String, + + /// Show metadata instead of full content + pub meta: bool, + + /// Show dependency tree + pub tree: bool, + + /// Output format for --meta mode (table, json, grid, xml) + pub format: Option, + + /// Shorthand for --format json in --meta mode + pub json: bool, + + /// Read from skills.lock (--meta mode only) + pub locked: bool, } impl IntoCommandSpec for ReadArgs { fn command_spec() -> CommandSpec { CommandSpec { summary: "Read full SKILL.md content for a skill", - syntax: Some("read "), + syntax: Some("read [OPTIONS]"), category: Some("discovery"), - args: vec![ArgSpec { - name: "skill-id", - kind: ArgKind::Positional, - value_type: ArgValueType::String, - cardinality: Cardinality::Required, - help: "Skill identifier (e.g., 'pptx', 'scope/pptx', 'pptx@1.2.3')", - ..Default::default() - }], + args: vec![ + ArgSpec { + name: "skill-id", + kind: ArgKind::Positional, + value_type: ArgValueType::String, + cardinality: Cardinality::Required, + help: "Skill identifier (e.g., 'pptx', 'scope/pptx', 'pptx@1.2.3')", + ..Default::default() + }, + ArgSpec { + name: "meta", + kind: ArgKind::Flag, + long: Some("meta"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Show metadata instead of full content", + ..Default::default() + }, + ArgSpec { + name: "tree", + kind: ArgKind::Flag, + long: Some("tree"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Show dependency tree", + ..Default::default() + }, + ArgSpec { + name: "format", + kind: ArgKind::Option, + long: Some("format"), + value_type: ArgValueType::String, + cardinality: Cardinality::Optional, + help: "Output format for --meta mode: table, json, grid, xml (default: table)", + ..Default::default() + }, + ArgSpec { + name: "json", + kind: ArgKind::Flag, + long: Some("json"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Shorthand for --format json in --meta mode", + ..Default::default() + }, + ArgSpec { + name: "locked", + kind: ArgKind::Flag, + long: Some("locked"), + value_type: ArgValueType::Bool, + cardinality: Cardinality::Optional, + help: "Read from skills.lock (--meta mode only)", + ..Default::default() + }, + ], ..Default::default() } } @@ -53,12 +130,222 @@ impl FromArgValueMap for ReadArgs { } }) .unwrap_or_else(|| panic!("fw bug: missing required skill-id")), + meta: matches!(map.get("meta"), Some(ArgValue::Bool(true))), + tree: matches!(map.get("tree"), Some(ArgValue::Bool(true))), + format: map + .get("format") + .and_then(|v| { + if let ArgValue::Str(s) = v { + Some(s.as_str()) + } else { + None + } + }) + .and_then(parse_output_format), + json: matches!(map.get("json"), Some(ArgValue::Bool(true))), + locked: matches!(map.get("locked"), Some(ArgValue::Bool(true))), + } + } +} + +/// Resolve a skill by ID using the service (with partial-match fallback). +async fn resolve_skill( + service: &Arc, + skill_id_str: &str, +) -> CliResult { + let skill_id = fastskill_core::SkillId::new(skill_id_str.to_string()).map_err(|e| { + eprintln!("Error: Invalid skill ID format: '{}'", skill_id_str); + eprintln!(); + eprintln!("Expected format:"); + eprintln!(" - id"); + eprintln!(" - scope/id"); + eprintln!(" - id@version or scope/id@version (if supported)"); + CliError::Validation(format!("Invalid skill ID format: {}", e)) + })?; + + // Try exact match first + match service + .skill_manager() + .get_skill(&skill_id) + .await + .map_err(CliError::Service)? + { + Some(skill) => Ok(skill), + None => { + // Check for partial matches + let all_skills = service + .skill_manager() + .list_skills(None) + .await + .map_err(CliError::Service)?; + + let matching_skills: Vec<_> = all_skills + .iter() + .filter(|s| { + let id_str = s.id.to_string(); + id_str.ends_with(&format!("/{}", skill_id_str)) + || id_str == skill_id_str + || s.name == skill_id_str + }) + .collect(); + + if matching_skills.len() > 1 { + eprintln!("Error: Multiple skills match '{}':", skill_id_str); + eprintln!(); + for s in &matching_skills { + eprintln!(" - {} ({})", s.id, s.version); + } + eprintln!(); + eprintln!("To disambiguate, use:"); + eprintln!(" - Version qualifier: @"); + eprintln!(" - Full identifier: /"); + return Err(CliError::Validation(format!( + "Multiple skills match '{}'", + skill_id_str + ))); + } + + if matching_skills.is_empty() { + let searched_paths = config::get_skill_search_locations_for_display(false) + .unwrap_or_else(|_| { + vec![( + service.config().skill_storage_path.clone(), + "project".to_string(), + )] + }); + return Err(CliError::SkillNotFound(SkillNotFoundMessage::new( + skill_id_str.to_string(), + searched_paths, + ))); + } + + Ok(matching_skills[0].clone()) } } } /// Execute the read command pub async fn execute_read(service: Arc, args: ReadArgs) -> CliResult<()> { + // Validate: --locked requires --meta + if args.locked && !args.meta { + return Err(CliError::Validation( + "--meta is required when using --locked with read".to_string(), + )); + } + + // Validate: --format requires --meta + if args.format.is_some() && !args.meta { + return Err(CliError::Validation( + "--meta is required when using --format with read".to_string(), + )); + } + + // Validate: --json requires --meta (implied: same gate as --format) + if args.json && !args.meta { + return Err(CliError::Validation( + "--meta is required when using --json with read".to_string(), + )); + } + + // --meta mode + if args.meta { + let format = validate_format_args(&args.format, args.json)?; + + let skill = if args.locked { + // Load skill definition from skills.lock + let lock_path = PathBuf::from("skills.lock"); + let lock = ProjectSkillsLock::load_from_file(&lock_path) + .map_err(|e| CliError::Config(format!("Failed to load skills.lock: {}", e)))?; + + // Validate skill ID syntax before searching the lock + fastskill_core::SkillId::new(args.skill_id.clone()).map_err(|e| { + eprintln!("Error: Invalid skill ID format: '{}'", args.skill_id); + CliError::Validation(format!("Invalid skill ID format: {}", e)) + })?; + + let entry = lock + .skills + .iter() + .find(|s| { + s.id == args.skill_id + || s.name == args.skill_id + || s.id.ends_with(&format!("/{}", args.skill_id)) + }) + .ok_or_else(|| { + CliError::Validation(format!( + "Skill '{}' not found in skills.lock", + args.skill_id + )) + })?; + + // Convert lock entry to SkillDefinition + let skill_id = fastskill_core::SkillId::new(entry.id.clone()) + .map_err(|e| CliError::Validation(format!("Invalid skill ID in lock: {}", e)))?; + SkillDefinition::new( + skill_id, + entry.name.clone(), + String::new(), + entry.version.clone(), + ) + } else { + resolve_skill(&service, &args.skill_id).await? + }; + + let output = format_show_results(&[skill], format) + .map_err(|e| CliError::Config(format!("Failed to format output: {}", e)))?; + println!("{}", output); + + // If --tree is also set, fall through to print tree after meta + if args.tree { + let tree_skill = if args.locked { + // Re-resolve from lock for tree + let lock_path = PathBuf::from("skills.lock"); + let lock = ProjectSkillsLock::load_from_file(&lock_path) + .map_err(|e| CliError::Config(format!("Failed to load skills.lock: {}", e)))?; + let entry = lock + .skills + .iter() + .find(|s| { + s.id == args.skill_id + || s.name == args.skill_id + || s.id.ends_with(&format!("/{}", args.skill_id)) + }) + .ok_or_else(|| { + CliError::Validation(format!( + "Skill '{}' not found in skills.lock", + args.skill_id + )) + })?; + let skill_id = fastskill_core::SkillId::new(entry.id.clone()).map_err(|e| { + CliError::Validation(format!("Invalid skill ID in lock: {}", e)) + })?; + let mut sd = SkillDefinition::new( + skill_id, + entry.name.clone(), + String::new(), + entry.version.clone(), + ); + if !entry.dependencies.is_empty() { + sd.dependencies = Some(entry.dependencies.clone()); + } + sd + } else { + resolve_skill(&service, &args.skill_id).await? + }; + print_dependency_tree(&tree_skill); + } + + return Ok(()); + } + + // --tree only (no --meta) + if args.tree { + let skill = resolve_skill(&service, &args.skill_id).await?; + print_dependency_tree(&skill); + return Ok(()); + } + + // Default: stream full SKILL.md to stdout // T007, T022: Parse and validate skill ID let skill_id = fastskill_core::SkillId::new(args.skill_id.clone()).map_err(|e| { eprintln!("Error: Invalid skill ID format: '{}'", args.skill_id); @@ -202,6 +489,21 @@ pub async fn execute_read(service: Arc, args: ReadArgs) -> Cli Ok(()) } +/// Print a dependency tree for the given skill definition. +fn print_dependency_tree(skill: &SkillDefinition) { + println!("{}", skill.id); + match &skill.dependencies { + Some(deps) if !deps.is_empty() => { + for dep in deps { + println!(" - {}", dep); + } + } + _ => { + println!(" (no dependencies)"); + } + } +} + #[allow(clippy::unwrap_used, clippy::expect_used, clippy::panic)] #[cfg(test)] mod tests { @@ -223,6 +525,11 @@ mod tests { let args = ReadArgs { skill_id: "bad@id@here".to_string(), + meta: false, + tree: false, + format: None, + json: false, + locked: false, }; let result = execute_read(Arc::new(service), args).await; @@ -246,6 +553,11 @@ mod tests { let args = ReadArgs { skill_id: "nonexistent-skill".to_string(), + meta: false, + tree: false, + format: None, + json: false, + locked: false, }; let result = execute_read(Arc::new(service), args).await; @@ -279,10 +591,93 @@ This is the content of the skill file. let args = ReadArgs { skill_id: "test-skill".to_string(), + meta: false, + tree: false, + format: None, + json: false, + locked: false, }; let result = execute_read(Arc::new(service), args).await; // May succeed or fail depending on skill registration state assert!(result.is_ok() || result.is_err()); } + + #[tokio::test] + async fn test_execute_read_locked_without_meta_returns_validation_error() { + let temp_dir = TempDir::new().unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + let args = ReadArgs { + skill_id: "some-skill".to_string(), + meta: false, + tree: false, + format: None, + json: false, + locked: true, + }; + + let result = execute_read(Arc::new(service), args).await; + assert!(matches!(result, Err(CliError::Validation(_)))); + if let Err(CliError::Validation(msg)) = result { + assert!(msg.contains("--meta is required")); + } + } + + #[tokio::test] + async fn test_execute_read_format_without_meta_returns_validation_error() { + let temp_dir = TempDir::new().unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + let args = ReadArgs { + skill_id: "some-skill".to_string(), + meta: false, + tree: false, + format: Some(OutputFormat::Json), + json: false, + locked: false, + }; + + let result = execute_read(Arc::new(service), args).await; + assert!(matches!(result, Err(CliError::Validation(_)))); + if let Err(CliError::Validation(msg)) = result { + assert!(msg.contains("--meta is required")); + } + } + + #[tokio::test] + async fn test_execute_read_json_without_meta_returns_validation_error() { + let temp_dir = TempDir::new().unwrap(); + let config = ServiceConfig { + skill_storage_path: temp_dir.path().to_path_buf(), + ..Default::default() + }; + let mut service = FastSkillService::new(config).await.unwrap(); + service.initialize().await.unwrap(); + + let args = ReadArgs { + skill_id: "some-skill".to_string(), + meta: false, + tree: false, + format: None, + json: true, + locked: false, + }; + + let result = execute_read(Arc::new(service), args).await; + assert!(matches!(result, Err(CliError::Validation(_)))); + if let Err(CliError::Validation(msg)) = result { + assert!(msg.contains("--meta is required")); + } + } } diff --git a/crates/fastskill-cli/src/commands/search.rs b/crates/fastskill-cli/src/commands/search.rs index 3f5fc990..e38226a9 100644 --- a/crates/fastskill-cli/src/commands/search.rs +++ b/crates/fastskill-cli/src/commands/search.rs @@ -9,7 +9,11 @@ use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality} use cli_framework::spec::command_tree::CommandSpec; use cli_framework::spec::value::ArgValue; use fastskill_core::output; -use fastskill_core::{FastSkillService, OutputFormat, SearchQuery, SearchScope}; +use fastskill_core::{ + ContentMode, FastSkillService, OutputFormat, ResolveContextRequest, ResolveScope, SearchQuery, + SearchScope, +}; +use serde_json; use std::collections::HashMap; use std::path::PathBuf; @@ -257,6 +261,11 @@ pub async fn execute_search(service: &FastSkillService, args: SearchArgs) -> Cli // Validate arguments validate_search_args(&args)?; + // When --paths is set, use context_resolver for canonical path output + if args.paths { + return execute_search_with_paths(service, &args).await; + } + // Determine search scope let scope = determine_search_scope(&args)?; @@ -287,6 +296,44 @@ pub async fn execute_search(service: &FastSkillService, args: SearchArgs) -> Cli Ok(()) } +/// Execute search with --paths flag: uses context_resolver to include canonical paths +async fn execute_search_with_paths(service: &FastSkillService, args: &SearchArgs) -> CliResult<()> { + let content_mode = parse_content_mode(args.content.as_deref().unwrap_or("none"))?; + + let request = ResolveContextRequest { + prompt: args.query.clone(), + limit: args.limit, + scope: ResolveScope::Local, + include_content: content_mode, + resolve_paths: true, + }; + + let response = service + .context_resolver() + .resolve_context(request) + .await + .map_err(CliError::Service)?; + + let json_output = serde_json::to_string_pretty(&response) + .map_err(|e| CliError::Validation(format!("Failed to serialize results to JSON: {}", e)))?; + + println!("{}", json_output); + Ok(()) +} + +/// Parse content mode string into ContentMode enum +fn parse_content_mode(s: &str) -> CliResult { + match s.to_lowercase().as_str() { + "none" => Ok(ContentMode::None), + "preview" => Ok(ContentMode::Preview), + "full" => Ok(ContentMode::Full), + other => Err(CliError::Config(format!( + "Invalid --content value '{}': must be none, preview, or full", + other + ))), + } +} + /// Validate search arguments for conflicting options fn validate_search_args(args: &SearchArgs) -> CliResult<()> { // Validate --repository flag only works with remote search @@ -311,9 +358,9 @@ fn validate_search_args(args: &SearchArgs) -> CliResult<()> { )); } - if args.content.is_some() && !args.paths { + if args.content.is_some() && !args.local { return Err(CliError::Config( - "Error: --content requires --paths. Use 'fastskill search --local --paths --content '.".to_string(), + "--content is only valid with --local. Use 'fastskill search --local --paths --content '.".to_string(), )); } diff --git a/crates/fastskill-cli/src/config_file.rs b/crates/fastskill-cli/src/config_file.rs index 36fbc09e..f04f3c0c 100644 --- a/crates/fastskill-cli/src/config_file.rs +++ b/crates/fastskill-cli/src/config_file.rs @@ -100,7 +100,7 @@ pub fn load_config_from_skill_project(current_dir: &Path) -> CliResult, + /// Auto-run reindex after mutating commands when embedding is configured (default: true) + #[serde(default = "default_auto_reindex")] + pub auto_reindex: bool, } /// Evaluation configuration in TOML format ([tool.fastskill.eval]) @@ -326,6 +329,10 @@ fn default_install_depth() -> u32 { 5 } +fn default_auto_reindex() -> bool { + true +} + /// HTTP server configuration in TOML format #[derive(Debug, Clone, Serialize, Deserialize)] pub struct HttpServerConfigToml { diff --git a/crates/fastskill-core/src/core/repository.rs b/crates/fastskill-core/src/core/repository.rs index 749082e9..a94ed67e 100644 --- a/crates/fastskill-core/src/core/repository.rs +++ b/crates/fastskill-core/src/core/repository.rs @@ -252,6 +252,7 @@ impl RepositoryManager { install_depth: 5, skip_transitive: false, eval: None, + auto_reindex: true, }), }); } else if let Some(ref mut tool) = project.tool { @@ -264,6 +265,7 @@ impl RepositoryManager { install_depth: 5, skip_transitive: false, eval: None, + auto_reindex: true, }); } else if let Some(ref mut fastskill) = tool.fastskill { fastskill.repositories = Some(manifest_repos); diff --git a/tests/cli/read_e2e_tests.rs b/tests/cli/read_e2e_tests.rs index c6463ec2..8f2af9ce 100644 --- a/tests/cli/read_e2e_tests.rs +++ b/tests/cli/read_e2e_tests.rs @@ -219,3 +219,163 @@ fn test_read_invalid_skill_id_error() { &cli_snapshot_settings(), ); } + +fn create_test_skill_dir(temp_dir: &TempDir) -> std::path::PathBuf { + let skills_dir = temp_dir.path().join(".claude").join("skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + let skill_dir = skills_dir.join("meta-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + let skill_content = r#"--- +name: meta-skill +description: A skill for testing metadata display +version: 2.0.0 +author: Test Author +tags: [meta, test] +--- +# Meta Skill + +This is the skill body content. +"#; + fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); + + fs::write( + temp_dir.path().join("skill-project.toml"), + "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", + ) + .unwrap(); + + skills_dir +} + +#[test] +fn test_read_meta_flag() { + let temp_dir = TempDir::new().unwrap(); + create_test_skill_dir(&temp_dir); + + let result = run_fastskill_command(&["read", "meta-skill", "--meta"], Some(temp_dir.path())); + + assert!(result.success, "read --meta failed: {}", result.stderr); + // --meta should show metadata, not the full SKILL.md body + assert!( + result.stdout.contains("meta-skill") || result.stdout.contains("2.0.0"), + "Expected metadata in output, got: {}", + result.stdout + ); + // Should NOT contain raw body content like "This is the skill body" + assert!( + !result.stdout.contains("This is the skill body content"), + "read --meta should not include file body: {}", + result.stdout + ); + + assert_snapshot_with_settings("read_meta_flag", &result.stdout, &cli_snapshot_settings()); +} + +#[test] +fn test_read_tree_flag() { + let temp_dir = TempDir::new().unwrap(); + create_test_skill_dir(&temp_dir); + + let result = run_fastskill_command(&["read", "meta-skill", "--tree"], Some(temp_dir.path())); + + assert!(result.success, "read --tree failed: {}", result.stderr); + // --tree should display dependency info and not raw body + assert!( + result.stdout.contains("meta-skill"), + "Expected skill name in tree output, got: {}", + result.stdout + ); + assert!( + !result.stdout.contains("This is the skill body content"), + "read --tree should not include file body: {}", + result.stdout + ); + + assert_snapshot_with_settings("read_tree_flag", &result.stdout, &cli_snapshot_settings()); +} + +#[test] +fn test_read_meta_json_flag() { + let temp_dir = TempDir::new().unwrap(); + create_test_skill_dir(&temp_dir); + + let result = + run_fastskill_command(&["read", "meta-skill", "--meta", "--json"], Some(temp_dir.path())); + + assert!(result.success, "read --meta --json failed: {}", result.stderr); + // Should be valid JSON containing id, name, version, description + assert!( + result.stdout.contains("\"meta-skill\"") || result.stdout.contains("meta-skill"), + "Expected skill id/name in JSON output, got: {}", + result.stdout + ); + assert!( + result.stdout.contains("2.0.0"), + "Expected version in JSON output, got: {}", + result.stdout + ); + // Should not contain raw body content + assert!( + !result.stdout.contains("This is the skill body content"), + "read --meta --json should not include file body: {}", + result.stdout + ); + + assert_snapshot_with_settings( + "read_meta_json_flag", + &result.stdout, + &cli_snapshot_settings(), + ); +} + +#[test] +fn test_read_locked_without_meta_fails() { + let temp_dir = TempDir::new().unwrap(); + fs::create_dir_all(temp_dir.path().join(".claude").join("skills")).unwrap(); + fs::write( + temp_dir.path().join("skill-project.toml"), + "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", + ) + .unwrap(); + + let result = + run_fastskill_command(&["read", "some-skill", "--locked"], Some(temp_dir.path())); + + assert!(!result.success, "read --locked without --meta should fail"); + assert!( + result.stderr.contains("--meta") || result.stderr.contains("locked"), + "Expected error about --meta requirement, got: {}", + result.stderr + ); +} + +#[test] +fn test_read_shorthand_streams_content() { + let temp_dir = TempDir::new().unwrap(); + let skills_dir = temp_dir.path().join(".claude").join("skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + let skill_dir = skills_dir.join("shorthand-skill"); + fs::create_dir_all(&skill_dir).unwrap(); + fs::write( + skill_dir.join("SKILL.md"), + "---\nname: shorthand-skill\nversion: 1.0.0\ndescription: shorthand test\n---\n# Shorthand\n\nBody content here.\n", + ) + .unwrap(); + fs::write( + temp_dir.path().join("skill-project.toml"), + "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", + ) + .unwrap(); + + // Test both `fastskill read ` and bare positional routing + let result = + run_fastskill_command(&["read", "shorthand-skill"], Some(temp_dir.path())); + assert!(result.success, "read shorthand-skill failed: {}", result.stderr); + assert!( + result.stdout.contains("Body content here"), + "Expected SKILL.md body in output, got: {}", + result.stdout + ); +} diff --git a/tests/cli/reindex_e2e_tests.rs b/tests/cli/reindex_e2e_tests.rs index 9eb1acfc..cc166c4c 100644 --- a/tests/cli/reindex_e2e_tests.rs +++ b/tests/cli/reindex_e2e_tests.rs @@ -235,3 +235,33 @@ fn test_reindex_progress_conflict() { assert!(!result.success); assert!(result.stderr.contains("--progress") && result.stderr.contains("--no-progress")); } + +#[test] +fn test_reindex_no_embedding_provider_exits_zero() { + let temp_dir = TempDir::new().unwrap(); + let skills_dir = temp_dir.path().join(".skills"); + fs::create_dir_all(&skills_dir).unwrap(); + + // Config WITHOUT embedding section + let config_content = "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".skills\"\n"; + fs::write(temp_dir.path().join("skill-project.toml"), config_content).unwrap(); + + let result = run_fastskill_command(&["reindex"], Some(temp_dir.path())); + + assert!( + result.success, + "reindex should exit 0 when no embedding provider is configured, got: {}", + result.stderr + ); + let combined = format!("{}{}", result.stdout, result.stderr); + assert!( + combined.to_lowercase().contains("skipped"), + "Expected informational 'skipped' message, got: {}", + combined + ); + assert!( + !combined.contains("Error") && !combined.contains("panic"), + "Should not contain error/panic, got: {}", + combined + ); +} From ba1625bc324e5eb57b2dda9d74ef47ba4139bc2c Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:12:11 +0000 Subject: [PATCH 3/6] chore(develop): 183-cli-command-surface-redesign-remove-redundant-vestigial-comm --- crates/fastskill-cli/src/commands/search.rs | 13 +- tests/cli/show_e2e_tests.rs | 302 -------- tests/cli/sync_e2e_tests.rs | 749 -------------------- 3 files changed, 12 insertions(+), 1052 deletions(-) delete mode 100644 tests/cli/show_e2e_tests.rs delete mode 100644 tests/cli/sync_e2e_tests.rs diff --git a/crates/fastskill-cli/src/commands/search.rs b/crates/fastskill-cli/src/commands/search.rs index e38226a9..4eb3aaa0 100644 --- a/crates/fastskill-cli/src/commands/search.rs +++ b/crates/fastskill-cli/src/commands/search.rs @@ -272,12 +272,23 @@ pub async fn execute_search(service: &FastSkillService, args: SearchArgs) -> Cli // Determine output format let format = determine_output_format(&args)?; + // Warn when local semantic search is requested but no embedding provider is configured + let embedding_mode = determine_embedding_mode(&args); + if args.local + && service.config().embedding.is_none() + && args.embedding.as_deref() != Some("false") + { + eprintln!( + "Warning: semantic search requires an embedding provider. Falling back to text search." + ); + } + // Create search query let query = SearchQuery { query: args.query.clone(), scope, limit: args.limit, - embedding: determine_embedding_mode(&args), + embedding: embedding_mode, }; // Execute search diff --git a/tests/cli/show_e2e_tests.rs b/tests/cli/show_e2e_tests.rs deleted file mode 100644 index ca75907d..00000000 --- a/tests/cli/show_e2e_tests.rs +++ /dev/null @@ -1,302 +0,0 @@ -//! E2E tests for show command -//! -//! These tests execute the CLI binary and verify actual behavior. - -#![allow(clippy::all, clippy::unwrap_used, clippy::expect_used)] - -use super::snapshot_helpers::{ - assert_snapshot_with_settings, cli_snapshot_settings, run_fastskill_command, -}; -use std::fs; -use tempfile::TempDir; - -#[test] -fn test_show_all_skills() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create a test skill in .skills directory - let skill_dir = skills_dir.join("test-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"--- -name: test-skill -description: A test skill for show command -version: 1.0.0 -tags: [test, show] -capabilities: [show_capability] ---- -# Test Skill - -This is a test skill for show command E2E testing. - -## Usage - -Usage example for show command. -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - let result = run_fastskill_command(&["show"], Some(temp_dir.path())); - - assert!(result.success); - assert!(result.stdout.contains("test-skill") || result.stdout.contains("No skills")); - - assert_snapshot_with_settings("show_all_skills", &result.stdout, &cli_snapshot_settings()); -} - -#[test] -fn test_show_specific_skill() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create a test skill in .skills directory - let skill_dir = skills_dir.join("web-scraper"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"--- -name: web-scraper -description: A web scraping skill -version: 1.2.3 -tags: [scraping, web] -capabilities: [http_client] ---- -# Web Scraper Skill - -Advanced web scraping capabilities. - -## Features - -- Extract structured data -- Handle pagination -- Support multiple formats -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - let result = run_fastskill_command(&["show", "web-scraper"], Some(temp_dir.path())); - - assert!(result.success); - assert!(result.stdout.contains("web-scraper") && result.stdout.contains("1.2.3")); - - assert_snapshot_with_settings( - "show_specific_skill", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_show_with_dependency_tree() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create a complex skill with dependencies - let skill_dir = skills_dir.join("complex-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"--- -name: complex-skill -description: A complex skill with dependencies -version: 2.0.0 -tags: [complex, dependencies] -dependencies: - - web-scraper - - test-skill -capabilities: [complex_capability] ---- -# Complex Skill - -This skill has dependencies. - -## Usage - -Requires web-scraper and test-skill. -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - - // Also create the dependencies - let dep1_dir = skills_dir.join("web-scraper"); - fs::create_dir_all(&dep1_dir).unwrap(); - let dep1_content = r#"--- -name: web-scraper -description: A web scraping skill -version: 1.2.3 ---- -# Web Scraper -"#; - fs::write(dep1_dir.join("SKILL.md"), dep1_content).unwrap(); - - let dep2_dir = skills_dir.join("test-skill"); - fs::create_dir_all(&dep2_dir).unwrap(); - let dep2_content = r#"--- -name: test-skill -description: Test skill -version: 1.0.0 ---- -# Test Skill -"#; - fs::write(dep2_dir.join("SKILL.md"), dep2_content).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - let result = run_fastskill_command(&["show", "--tree", "complex-skill"], Some(temp_dir.path())); - - assert!(result.success); - assert!(result.stdout.contains("complex-skill") || result.stdout.contains("tree")); - - assert_snapshot_with_settings( - "show_dependency_tree", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_show_nonexistent_skill_error() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - let result = run_fastskill_command(&["show", "nonexistent-skill"], Some(temp_dir.path())); - - assert!(!result.success); - assert!(result.stderr.contains("error") || result.stderr.contains("not found")); - - assert_snapshot_with_settings( - "show_nonexistent_skill", - &format!("{}{}", result.stdout, result.stderr), - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_show_invalid_skill_id_format() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - let result = run_fastskill_command(&["show", "invalid skill id!"], Some(temp_dir.path())); - - assert!(!result.success); - assert!(result.stderr.contains("error") || result.stderr.contains("Invalid")); - - assert_snapshot_with_settings( - "show_invalid_id_format", - &format!("{}{}", result.stdout, result.stderr), - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_show_installed_locked_conflict() { - let result = run_fastskill_command(&["show", "--installed", "--locked"], None); - - assert!(!result.success); - assert!(result.stderr.contains("--installed") && result.stderr.contains("--locked")); -} - -#[test] -fn test_show_locked_with_skills_dir_validation() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - let result = run_fastskill_command( - &["show", "--locked", "--skills-dir", "/tmp/skills"], - Some(temp_dir.path()), - ); - - assert!(!result.success); - assert!( - result.stderr.contains("--locked") - && result.stderr.contains("--skills-dir") - && result.stderr.contains("does not support") - ); -} - -#[test] -fn test_show_locked_with_global_validation() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - let result = run_fastskill_command(&["--global", "show", "--locked"], Some(temp_dir.path())); - - assert!(!result.success); - assert!( - result.stderr.contains("--locked") - && result.stderr.contains("--global") - && result.stderr.contains("does not support") - ); -} - -#[test] -fn test_show_locked_mode_no_lock_file() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - let result = run_fastskill_command(&["show", "--locked"], Some(temp_dir.path())); - - assert!(!result.success); - assert!(result.stderr.contains("skills.lock") || result.stderr.contains("lock")); -} - -#[test] -fn test_show_installed_mode_explicit() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - let skill_dir = skills_dir.join("test-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"--- -name: test-skill -description: A test skill for installed mode -version: 1.0.0 ---- -# Test Skill -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - let result = run_fastskill_command(&["show", "--installed"], Some(temp_dir.path())); - - assert!(result.success); - assert!(result.stdout.contains("test-skill")); -} diff --git a/tests/cli/sync_e2e_tests.rs b/tests/cli/sync_e2e_tests.rs deleted file mode 100644 index d712adda..00000000 --- a/tests/cli/sync_e2e_tests.rs +++ /dev/null @@ -1,749 +0,0 @@ -//! E2E tests for sync command -//! -//! These tests execute the CLI binary and verify actual behavior. - -#![allow(clippy::all, clippy::unwrap_used, clippy::expect_used)] - -use super::snapshot_helpers::{ - assert_snapshot_with_settings, cli_snapshot_settings, run_fastskill_command, -}; -use std::fs; -use tempfile::TempDir; - -#[test] -fn test_sync_all_skills_yes() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create test skills - for skill_name in &["skill1", "skill2"] { - let skill_dir = skills_dir.join(skill_name); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = format!( - r#"--- -name: {} -description: A test skill -version: 1.0.0 ---- -# {} Skill - -This is a test skill for sync command. -"#, - skill_name, skill_name - ); - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - } - - // Create skill-project.toml - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - // Create initial AGENTS.md - fs::write( - temp_dir.path().join("AGENTS.md"), - "# AGENTS.md\n\nThis file contains agent instructions.\n", - ) - .unwrap(); - - let result = run_fastskill_command(&["sync", "--yes"], Some(temp_dir.path())); - - assert!(result.success); - - // Verify AGENTS.md was updated - let agents_md_content = fs::read_to_string(temp_dir.path().join("AGENTS.md")).unwrap(); - assert!(agents_md_content.contains("skill1")); - assert!(agents_md_content.contains("skill2")); - - assert_snapshot_with_settings( - "sync_all_skills_yes", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_sync_project_vs_global() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create project skill - let project_skill_dir = skills_dir.join("project-skill"); - fs::create_dir_all(&project_skill_dir).unwrap(); - let project_skill = r#"--- -name: project-skill -description: A project-level skill -version: 1.0.0 ---- -# Project Skill - -This skill is at project level. -"#; - fs::write(project_skill_dir.join("SKILL.md"), project_skill).unwrap(); - - // Create skill-project.toml - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - // Create AGENTS.md - fs::write(temp_dir.path().join("AGENTS.md"), "# AGENTS.md\n\n").unwrap(); - - let result = run_fastskill_command(&["sync", "--yes"], Some(temp_dir.path())); - - assert!(result.success); - - // Verify project skill is marked as location="project" - let agents_md_content = fs::read_to_string(temp_dir.path().join("AGENTS.md")).unwrap(); - assert!(agents_md_content.contains("project-skill")); - assert!(agents_md_content.contains("project")); - - assert_snapshot_with_settings( - "sync_project_vs_global", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_sync_with_existing_agents_md() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create test skill - let skill_dir = skills_dir.join("new-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - let skill_content = r#"--- -name: new-skill -description: A new skill -version: 1.0.0 ---- -# New Skill - -This is a new skill. -"#; - fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); - - // Create skill-project.toml - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - // Create AGENTS.md with existing content - let initial_agents_md = r#"# AGENTS.md - -## Project Setup - -This file contains project-specific agent instructions. - - - - - old-skill - Old skill - project - - - - -## Additional Instructions - -Additional configuration and instructions for agents. -"#; - fs::write(temp_dir.path().join("AGENTS.md"), initial_agents_md).unwrap(); - - let result = run_fastskill_command(&["sync", "--yes"], Some(temp_dir.path())); - - assert!(result.success); - - // Verify skills section was replaced but other content preserved - let agents_md_content = fs::read_to_string(temp_dir.path().join("AGENTS.md")).unwrap(); - assert!(agents_md_content.contains("# AGENTS.md")); - assert!(agents_md_content.contains("## Project Setup")); - assert!(agents_md_content.contains("## Additional Instructions")); - assert!(!agents_md_content.contains("old-skill")); - assert!(agents_md_content.contains("new-skill")); - - assert_snapshot_with_settings( - "sync_with_existing_agents_md", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_sync_remove_all_skills() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - // Create skill-project.toml - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - // Create AGENTS.md with skills section - let initial_agents_md = r#"# AGENTS.md - - - - - skill-to-remove - Should be removed - project - - - - -Additional content here. -"#; - fs::write(temp_dir.path().join("AGENTS.md"), initial_agents_md).unwrap(); - - let result = run_fastskill_command(&["sync", "--yes"], Some(temp_dir.path())); - - assert!(result.success); - - // Verify skills section was removed but other content preserved - let agents_md_content = fs::read_to_string(temp_dir.path().join("AGENTS.md")).unwrap(); - assert!(agents_md_content.contains("# AGENTS.md")); - assert!(!agents_md_content.contains("test-skill")); - - // AGENTS.md must NOT be created as a side effect - assert!( - !temp_dir.path().join("AGENTS.md").exists(), - "AGENTS.md must not be created when CLAUDE.md was detected" - ); - - // The success message must reference CLAUDE.md (dynamic filename, not hardcoded "AGENTS.md") - assert!( - result.stdout.contains("CLAUDE.md"), - "stdout success message must reference CLAUDE.md, got: {}", - result.stdout - ); - - assert_snapshot_with_settings( - "sync_auto_detects_claude_metadata_file", - &result.stdout, - &cli_snapshot_settings(), - ); -} - -#[test] -fn test_sync_with_explicit_agent_flag() { - let temp_dir = TempDir::new().unwrap(); - let skills_dir = temp_dir.path().join(".claude").join("skills"); - fs::create_dir_all(&skills_dir).unwrap(); - - let skill_dir = skills_dir.join("test-skill"); - fs::create_dir_all(&skill_dir).unwrap(); - fs::write( - skill_dir.join("SKILL.md"), - "---\nname: test-skill\ndescription: A test skill\nversion: 1.0.0\n---\n# Test Skill\n", - ) - .unwrap(); - - fs::write( - temp_dir.path().join("skill-project.toml"), - "[dependencies]\n\n[tool.fastskill]\nskills_directory = \".claude/skills\"\n", - ) - .unwrap(); - - // No metadata file exists — explicit --agent flag determines which file to create - let result = run_fastskill_command( - &["sync", "--yes", "--agent", "claude"], - Some(temp_dir.path()), - ); - - assert!(result.success); - - let claude_md = temp_dir.path().join("CLAUDE.md"); - assert!( - claude_md.exists(), - "CLAUDE.md must be created by --agent claude" - ); - let content = fs::read_to_string(&claude_md).unwrap(); - assert!(content.contains(" Date: Sun, 7 Jun 2026 19:20:29 +0000 Subject: [PATCH 4/6] chore(develop): 183-cli-command-surface-redesign-remove-redundant-vestigial-comm --- CHANGELOG | 41 ++++++++++++ tests/cli/help_tests.rs | 72 ++++++++++++++++++++++ webdocs/cli-reference/overview.mdx | 16 ++--- webdocs/cli-reference/skill-commands.mdx | 58 +++++++++-------- webdocs/cli-reference/tooling-commands.mdx | 45 ++++++++------ 5 files changed, 173 insertions(+), 59 deletions(-) diff --git a/CHANGELOG b/CHANGELOG index 86377d45..0dd77c06 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -4,6 +4,47 @@ ### Breaking Changes +- **CLI command-surface redesign: four commands removed (issue #183).** + `fastskill resolve`, `fastskill sync`, `fastskill disable`, and `fastskill show` have been + removed with no hidden aliases. Migration table: + + | Removed command | Replacement | + |---|---| + | `fastskill resolve ` | `fastskill search --local --paths --json` | + | `fastskill show [id]` | `fastskill read --meta` (by id) or `fastskill list` (all) | + | `fastskill show --tree` | `fastskill read --tree` | + | `fastskill sync` | No replacement — agents read the skills directory directly | + | `fastskill disable ` | `fastskill remove ` | + + Shell completion scripts auto-derive from the registered command tree and no longer suggest + the removed commands. + +- **`reindex` exits 0 when no embedding provider is configured** (previously exited 1). + If your CI pipeline relied on `reindex` failing to detect missing configuration, switch to + `fastskill doctor` instead. + +- **`add`, `install`, `update`, and `remove` now auto-trigger `reindex`** after a successful + run when an embedding provider is configured. Pass `--no-reindex` or set + `auto_reindex = false` in `[tool.fastskill]` of `skill-project.toml` to opt out. + +### Added + +- **`fastskill doctor`**: new diagnostics command that reports environment readiness + (skills directory, `skill-project.toml`, embedding provider, API key, auth token). Exits 0 + when no hard failures; exits 1 when the skills directory is inaccessible. `--json` flag emits + a JSON array of `{check, status, message}` objects. + +- **`fastskill read --meta` / `--tree`**: `read` now accepts `--meta` (structured metadata), + `--tree` (dependency tree), `--locked`, `--format`, and `--json`. These flags replace + `fastskill show` entirely. + +- **`fastskill search --paths` / `--content`**: local search gains `--paths` (emit absolute + SKILL.md path per result, forces JSON) and `--content ` (include skill + content). Both flags require `--local`. + +- **`auto_reindex` config field**: `[tool.fastskill]` in `skill-project.toml` accepts + `auto_reindex = false` to disable the new automatic reindex side-effect globally. + - **All internal API routes moved from `/api/` to `/api/v1/`.** Affected endpoints include `/api/skills`, `/api/project`, `/api/status`, `/api/search`, `/api/resolve`, `/api/reindex`, `/api/registry/…`, and `/api/manifest/…`. diff --git a/tests/cli/help_tests.rs b/tests/cli/help_tests.rs index 8c84a17e..7db9d04f 100644 --- a/tests/cli/help_tests.rs +++ b/tests/cli/help_tests.rs @@ -73,3 +73,75 @@ fn test_version_flag() { // Version flag might not be implemented yet, but should not crash assert!(result.success || result.stderr.contains("fastskill")); } + +#[test] +fn test_help_does_not_show_removed_commands() { + let result = run_fastskill_command(&["--help"], None); + + let output = format!("{}{}", result.stdout, result.stderr); + // These commands were removed in CLI surface redesign (issue #183) + assert!( + !output.contains(" resolve "), + "removed command 'resolve' must not appear in --help" + ); + assert!( + !output.contains(" sync "), + "removed command 'sync' must not appear in --help" + ); + assert!( + !output.contains(" disable "), + "removed command 'disable' must not appear in --help" + ); + assert!( + !output.contains(" show "), + "removed command 'show' must not appear in --help" + ); +} + +#[test] +fn test_doctor_command_help() { + let result = run_fastskill_command(&["doctor", "--help"], None); + + assert!(result.success); + let output = format!("{}{}", result.stdout, result.stderr); + assert!( + output.contains("doctor") || output.contains("Doctor"), + "doctor --help must mention 'doctor'" + ); + assert!( + output.contains("--json") || output.contains("json"), + "doctor --help must mention --json flag" + ); +} + +#[test] +fn test_read_command_help_has_new_flags() { + let result = run_fastskill_command(&["read", "--help"], None); + + assert!(result.success); + let output = format!("{}{}", result.stdout, result.stderr); + assert!( + output.contains("--meta"), + "read --help must include --meta flag" + ); + assert!( + output.contains("--tree"), + "read --help must include --tree flag" + ); +} + +#[test] +fn test_search_command_help_has_new_flags() { + let result = run_fastskill_command(&["search", "--help"], None); + + assert!(result.success); + let output = format!("{}{}", result.stdout, result.stderr); + assert!( + output.contains("--paths"), + "search --help must include --paths flag" + ); + assert!( + output.contains("--content"), + "search --help must include --content flag" + ); +} diff --git a/webdocs/cli-reference/overview.mdx b/webdocs/cli-reference/overview.mdx index f222230b..525457b5 100644 --- a/webdocs/cli-reference/overview.mdx +++ b/webdocs/cli-reference/overview.mdx @@ -1,11 +1,11 @@ --- title: "CLI Reference Overview" -description: "CLI for the FastSkill package manager: installs, manifests, validation, evals, search, sync, package, publish, and serve." +description: "CLI for the FastSkill package manager: installs, manifests, validation, evals, search, package, publish, and serve." --- ## Overview -The FastSkill CLI is how you **install and operate** skills: manifests and `skills.lock`, validation and `fastskill eval`, search and `sync`, packaging and optional publish, plus `serve` when you want a local HTTP surface. +The FastSkill CLI is how you **install and operate** skills: manifests and `skills.lock`, validation and `fastskill eval`, search, packaging and optional publish, plus `serve` when you want a local HTTP surface. The CLI supports both interactive and scripting use cases, with comprehensive help text and validation for all commands. @@ -88,15 +88,9 @@ fastskill list --help # Help for list command List locally installed skills with reconciliation status (supports `--format table|json|grid|xml`, `--json`). See [skill commands](/cli-reference/skill-commands#list). - - Disable skills by ID. See [skill commands](/cli-reference/skill-commands#fastskill-disable). - Remove skills by ID. See [skill commands](/cli-reference/skill-commands#fastskill-remove). - - Show skill details and dependency tree. See [skill commands](/cli-reference/skill-commands#fastskill-show). - ### Registry & search @@ -133,12 +127,12 @@ fastskill list --help # Help for list command Validate and run skill evaluation suites. See [eval Command](/cli-reference/eval-command). - - Sync installed skills to agent metadata. See [tooling commands](/cli-reference/tooling-commands#fastskill-sync). - Similarity matrix, clusters, duplicates. See [tooling commands](/cli-reference/tooling-commands#fastskill-analyze). + + Check environment readiness (skills directory, embedding provider, auth). See [tooling commands](/cli-reference/tooling-commands#fastskill-doctor). + ## Version Command diff --git a/webdocs/cli-reference/skill-commands.mdx b/webdocs/cli-reference/skill-commands.mdx index a1a7b6c2..4e87cc9a 100644 --- a/webdocs/cli-reference/skill-commands.mdx +++ b/webdocs/cli-reference/skill-commands.mdx @@ -185,24 +185,34 @@ fastskill update --dry-run - `--source `: Update from specific source - `--strategy `: Update strategy: latest, patch, minor, major (default: latest) -### fastskill show +### fastskill read (metadata and tree) -Display skill information and dependency tree. +Display skill metadata or dependency tree. Use `--meta` for structured metadata and `--tree` for the dependency tree. ```bash -# Show all skills -fastskill show +# Read full SKILL.md content (default) +fastskill read my-skill-id -# Show specific skill -fastskill show my-skill-id +# Show structured metadata (replaces `fastskill show `) +fastskill read my-skill-id --meta -# Show dependency tree -fastskill show --tree +# Show dependency tree (replaces `fastskill show --tree`) +fastskill read my-skill-id --tree -# Show dependency tree for specific skill -fastskill show my-skill-id --tree +# Metadata and tree together +fastskill read my-skill-id --meta --tree + +# Metadata as JSON +fastskill read my-skill-id --meta --json + +# List all installed skills +fastskill list ``` + +`fastskill show` has been removed. Use `fastskill read --meta` to view skill metadata or `fastskill list` to enumerate all installed skills. + + ### fastskill add Add a skill from a source (git URL, local folder, or zip file) and install it to the skills storage directory. Updates both `skill-project.toml` and `skills.lock`. @@ -342,18 +352,6 @@ fastskill list --format xml **Note**: This command reads installed skill manifests from the skills directory as the source of truth and cross-references `skill-project.toml` (desired) and `skills.lock` (pinned) to report reconciliation status. -### fastskill disable - -Disable skills to prevent them from being used. - -```bash -# Disable skill -fastskill disable text-processor - -# Batch operations -fastskill disable skill1 skill2 skill3 -``` - ## Examples ### Skill Development Workflow @@ -365,16 +363,13 @@ fastskill add https://github.com/org/my-skill.git # 2. List skills to verify installation fastskill list -# 3. Show skill details -fastskill show my-skill +# 3. Show skill metadata +fastskill read my-skill --meta # 4. Update to latest version fastskill update my-skill -# 5. Disable if needed -fastskill disable my-skill - -# 6. Remove skill +# 5. Remove skill when no longer needed fastskill remove my-skill --force ``` @@ -396,13 +391,16 @@ fastskill reindex # Install with locked versions for reproducible builds fastskill install --lock --without dev -# Reindex for search +# Reindex for search (auto-triggered after add/install/update/remove when embedding is configured) fastskill reindex +# Check environment readiness +fastskill doctor + # Verify installation fastskill list ``` -Skill commands support both interactive use and automation. Use appropriate options for your workflow. +Skill commands support both interactive use and automation. Use appropriate options for your workflow. Reindex runs automatically after `add`/`install`/`update`/`remove` when an embedding provider is configured; use `--no-reindex` to suppress or `fastskill doctor` to check environment readiness. diff --git a/webdocs/cli-reference/tooling-commands.mdx b/webdocs/cli-reference/tooling-commands.mdx index 5b2c3a5a..1d3b8b21 100644 --- a/webdocs/cli-reference/tooling-commands.mdx +++ b/webdocs/cli-reference/tooling-commands.mdx @@ -1,24 +1,11 @@ --- -title: "Sync, marketplace, and analyze" -description: "CLI reference for fastskill sync, fastskill marketplace, and fastskill analyze." +title: "Marketplace, analyze, and doctor" +description: "CLI reference for fastskill marketplace, fastskill analyze, and fastskill doctor." --- -## fastskill sync - -Writes installed skills into the agent metadata file (Claude Code / compatible layouts). Agent and file path are auto-detected when omitted. - -```bash -fastskill sync -fastskill sync --yes -fastskill sync --agent claude -fastskill sync --agents-file custom.md -``` - -| Option | Description | -|--------|-------------| -| `-y, --yes` | Non-interactive: include all installed skills | -| `-a, --agent ` | Target agent (optional; auto-detected) | -| `--agents-file ` | Override metadata file path | + +`fastskill sync` has been removed. Agents read the skills directory directly; no metadata-file sync is needed. + ## fastskill marketplace @@ -92,6 +79,28 @@ fastskill analyze duplicates --threshold 0.92 --severity critical | `--severity ` | `all`, `medium`, `high`, or `critical` | | `--format`, `--json` | Output selection | +## fastskill doctor + +Check environment readiness for fastskill. Reports the status of the skills directory, `skill-project.toml`, embedding provider, API key, and auth token. Exits 0 when no hard failures are found; exits 1 when the skills directory is inaccessible. + +```bash +fastskill doctor +fastskill doctor --json +``` + +| Option | Description | +|--------|-------------| +| `--json` | Emit results as JSON array of `{check, status, message}` | + +**Example output:** + +``` +[PASS] Skills directory accessible +[PASS] skill-project.toml present +[WARN] Embedding provider not configured (semantic search unavailable) +[WARN] No authenticated registry +``` + ## See also - [Discovery commands](/cli-reference/discovery-commands) (search, reindex) From 1e0ec52a8f5ad8c6dd8e468810bc95d6469b52fc Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Sun, 7 Jun 2026 19:31:38 +0000 Subject: [PATCH 5/6] chore(develop): 183-cli-command-surface-redesign-remove-redundant-vestigial-comm --- crates/fastskill-cli/src/commands/doctor.rs | 3 +-- tests/cli/doctor_tests.rs | 13 ++++++++++--- tests/cli/read_e2e_tests.rs | 18 +++++++++++++++--- 3 files changed, 26 insertions(+), 8 deletions(-) diff --git a/crates/fastskill-cli/src/commands/doctor.rs b/crates/fastskill-cli/src/commands/doctor.rs index 851ef996..f70b0609 100644 --- a/crates/fastskill-cli/src/commands/doctor.rs +++ b/crates/fastskill-cli/src/commands/doctor.rs @@ -231,10 +231,9 @@ fn print_json(checks: &[DoctorCheckResult]) { }) }) .collect(); - let output = serde_json::json!({ "checks": items }); println!( "{}", - serde_json::to_string_pretty(&output).unwrap_or_default() + serde_json::to_string_pretty(&items).unwrap_or_default() ); } diff --git a/tests/cli/doctor_tests.rs b/tests/cli/doctor_tests.rs index d7a77a2b..40273c40 100644 --- a/tests/cli/doctor_tests.rs +++ b/tests/cli/doctor_tests.rs @@ -37,10 +37,17 @@ fn test_doctor_json_flag() { let result = run_fastskill_command(&["doctor", "--json"], Some(temp_dir.path())); assert!(result.success, "doctor --json failed: {}", result.stderr); - // Output should be valid JSON with a checks array + // Output should be a valid JSON array (top-level '[') + let trimmed = result.stdout.trim(); assert!( - result.stdout.contains("\"checks\""), - "Expected JSON output with 'checks' key, got: {}", + trimmed.starts_with('['), + "Expected JSON array output (starting with '['), got: {}", + result.stdout + ); + // Each element must have check, status, message fields + assert!( + result.stdout.contains("\"check\""), + "Expected 'check' field in JSON output, got: {}", result.stdout ); } diff --git a/tests/cli/read_e2e_tests.rs b/tests/cli/read_e2e_tests.rs index 8f2af9ce..401b5a01 100644 --- a/tests/cli/read_e2e_tests.rs +++ b/tests/cli/read_e2e_tests.rs @@ -369,13 +369,25 @@ fn test_read_shorthand_streams_content() { ) .unwrap(); - // Test both `fastskill read ` and bare positional routing - let result = - run_fastskill_command(&["read", "shorthand-skill"], Some(temp_dir.path())); + // Test `fastskill read ` + let result = run_fastskill_command(&["read", "shorthand-skill"], Some(temp_dir.path())); assert!(result.success, "read shorthand-skill failed: {}", result.stderr); assert!( result.stdout.contains("Body content here"), "Expected SKILL.md body in output, got: {}", result.stdout ); + + // Test bare positional shorthand `fastskill ` (AC 30) + let bare_result = run_fastskill_command(&["shorthand-skill"], Some(temp_dir.path())); + assert!( + bare_result.success, + "bare positional shorthand-skill failed: {}", + bare_result.stderr + ); + assert!( + bare_result.stdout.contains("Body content here"), + "Expected SKILL.md body from bare positional shorthand, got: {}", + bare_result.stdout + ); } From 80f0f017a88f703c31ea4a00e7af882d0f8c8160 Mon Sep 17 00:00:00 2001 From: Alex Oliveira <4482374+aroff@users.noreply.github.com> Date: Mon, 8 Jun 2026 15:17:09 +0000 Subject: [PATCH 6/6] =?UTF-8?q?fix(cli):=20complete=20#183=20=E2=80=94=20r?= =?UTF-8?q?ead=20shorthand,=20content/reindex=20validations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Finishes the CLI command-surface redesign (issue #183): - Wire the `fastskill ` bare-positional shorthand to `read` in main.rs. An argv rewrite inserts `read` before the first positional when it is not a recognized command or group. Retired commands (resolve/sync/disable/show) are excluded so they still surface "unrecognized subcommand". - search: validate `--content` value eagerly (none|preview|full) regardless of `--paths`; fix stale `--content` help text (was "inline, ref"). - install/update: add the missing `--reindex`/`--no-reindex` mutual-exclusion validation (add/remove already had it). - Add compiled regression tests for the new search `--paths`/`--content` validation paths. - read: refresh stale doc comment referencing the removed `show` command. All 334 compiled tests pass; clippy clean (no new warnings). Co-Authored-By: Claude Opus 4.8 (1M context) --- crates/fastskill-cli/src/commands/install.rs | 6 ++ crates/fastskill-cli/src/commands/read.rs | 5 +- crates/fastskill-cli/src/commands/search.rs | 95 +++++++++++++++++++- crates/fastskill-cli/src/commands/update.rs | 5 ++ crates/fastskill-cli/src/main.rs | 48 ++++++++++ 5 files changed, 155 insertions(+), 4 deletions(-) diff --git a/crates/fastskill-cli/src/commands/install.rs b/crates/fastskill-cli/src/commands/install.rs index 9279a7a1..962dbee7 100644 --- a/crates/fastskill-cli/src/commands/install.rs +++ b/crates/fastskill-cli/src/commands/install.rs @@ -208,6 +208,12 @@ fn group_passes_filter( } pub async fn execute_install(args: InstallArgs) -> CliResult<()> { + if args.reindex && args.no_reindex { + return Err(CliError::Validation( + "--reindex and --no-reindex cannot be used together".to_string(), + )); + } + println!("Installing skills..."); println!(); diff --git a/crates/fastskill-cli/src/commands/read.rs b/crates/fastskill-cli/src/commands/read.rs index 7620ef7a..c7a6a330 100644 --- a/crates/fastskill-cli/src/commands/read.rs +++ b/crates/fastskill-cli/src/commands/read.rs @@ -1,7 +1,8 @@ //! Read command - streams skill documentation to stdout //! -//! Reads and outputs the full SKILL.md content. For metadata only (name, version, description), -//! use `show`. For structured metadata with optional formats, use `--meta`. +//! Reads and outputs the full SKILL.md content. For structured metadata +//! (name, version, description, author, source) use `--meta`; for the +//! dependency tree use `--tree`. use crate::commands::common::validate_format_args; use crate::config; diff --git a/crates/fastskill-cli/src/commands/search.rs b/crates/fastskill-cli/src/commands/search.rs index 4eb3aaa0..af9841c0 100644 --- a/crates/fastskill-cli/src/commands/search.rs +++ b/crates/fastskill-cli/src/commands/search.rs @@ -53,7 +53,7 @@ pub struct SearchArgs { /// Output resolved file paths instead of search results (--local only) pub paths: bool, - /// Content mode for resolved skill context: none, inline, ref (--local --paths only) + /// Content to include in JSON output: none, preview, full (--local --paths only) pub content: Option, } @@ -178,7 +178,7 @@ impl IntoCommandSpec for SearchArgs { name: "content", long: Some("content"), short: None, - help: "Content mode for resolved skill context: none, inline, ref (--local --paths only)", + help: "Content to include in JSON output: none, preview, full (--local --paths only)", kind: ArgKind::Option, value_type: ArgValueType::String, cardinality: Cardinality::Optional, @@ -375,6 +375,12 @@ fn validate_search_args(args: &SearchArgs) -> CliResult<()> { )); } + // Validate the content mode value eagerly so an invalid value is rejected + // regardless of whether --paths is also present. + if let Some(content) = args.content.as_deref() { + parse_content_mode(content)?; + } + Ok(()) } @@ -447,6 +453,91 @@ mod tests { } } + #[test] + fn test_validate_search_args_paths_without_local() { + let args = SearchArgs { + query: "test".to_string(), + local: false, + remote: false, + repository: None, + limit: 10, + format: None, + json: false, + embedding: None, + skills_dir: None, + paths: true, + content: None, + }; + let result = validate_search_args(&args); + assert!(result.is_err()); + match result { + Err(CliError::Config(msg)) => { + assert!(msg.contains("--paths")); + assert!(msg.contains("--local")); + } + other => panic!("expected CliError::Config, got {:?}", other), + } + } + + #[test] + fn test_validate_search_args_content_without_local() { + let args = SearchArgs { + query: "test".to_string(), + local: false, + remote: false, + repository: None, + limit: 10, + format: None, + json: false, + embedding: None, + skills_dir: None, + paths: false, + content: Some("full".to_string()), + }; + let result = validate_search_args(&args); + assert!(matches!(result, Err(CliError::Config(_)))); + } + + #[test] + fn test_validate_search_args_invalid_content_value() { + let args = SearchArgs { + query: "test".to_string(), + local: true, + remote: false, + repository: None, + limit: 10, + format: None, + json: false, + embedding: None, + skills_dir: None, + paths: false, + content: Some("bogus".to_string()), + }; + let result = validate_search_args(&args); + match result { + Err(CliError::Config(msg)) => assert!(msg.contains("must be none, preview, or full")), + other => panic!("expected CliError::Config, got {:?}", other), + } + } + + #[test] + fn test_validate_search_args_local_paths_content_full_ok() { + let args = SearchArgs { + query: "test".to_string(), + local: true, + remote: false, + repository: None, + limit: 10, + format: None, + json: true, + embedding: None, + skills_dir: None, + paths: true, + content: Some("full".to_string()), + }; + assert!(validate_search_args(&args).is_ok()); + } + #[test] fn test_validate_search_args_valid_local() { let args = SearchArgs { diff --git a/crates/fastskill-cli/src/commands/update.rs b/crates/fastskill-cli/src/commands/update.rs index bd855711..a54cb2b2 100644 --- a/crates/fastskill-cli/src/commands/update.rs +++ b/crates/fastskill-cli/src/commands/update.rs @@ -174,6 +174,11 @@ impl FromArgValueMap for UpdateArgs { } pub async fn execute_update(args: UpdateArgs, global: bool) -> CliResult<()> { + if args.reindex && args.no_reindex { + return Err(CliError::Validation( + "--reindex and --no-reindex cannot be used together".to_string(), + )); + } let reindex = args.reindex; let no_reindex = args.no_reindex; let check = args.check; diff --git a/crates/fastskill-cli/src/main.rs b/crates/fastskill-cli/src/main.rs index 652b7199..157f655f 100644 --- a/crates/fastskill-cli/src/main.rs +++ b/crates/fastskill-cli/src/main.rs @@ -88,6 +88,54 @@ async fn main() { "Error initialising app", ); + // `fastskill ` (no subcommand) is a shorthand that routes to + // `read`. We rewrite the args here, before dispatch, when the first + // positional token is not a recognized top-level command or command group. + // Global flags (and their values) that precede the subcommand are skipped. + let raw = { + // The set of recognized first path segments: every registered command + // (including built-ins like `spec`/`completion`/`mcp`) and every group + // node (`analyze`, `auth`, `repos`, ...). `help` is clap-provided. + let registry = app.command_registry(); + let mut known: std::collections::HashSet<&str> = std::collections::HashSet::new(); + known.insert("help"); + // Retired commands (issue #183): keep them out of the read shorthand so + // `fastskill resolve|sync|disable|show` still surfaces an explicit + // "unrecognized subcommand" error instead of being read as a skill id. + for retired in ["resolve", "sync", "disable", "show"] { + known.insert(retired); + } + for (path, _) in registry.all_tree_commands() { + known.insert(path.split('/').next().unwrap_or(path)); + } + for (path, _) in registry.groups() { + known.insert(path.split('/').next().unwrap_or(path)); + } + + let mut i = 1; + while i < raw.len() { + let a = &raw[i]; + if a == "--skills-dir" { + // value-taking global flag in `--flag value` form + i += 2; + continue; + } + if a.starts_with('-') { + // boolean/global flag or `--flag=value` form + i += 1; + continue; + } + break; + } + if i < raw.len() && !known.contains(raw[i].as_str()) { + let mut rewritten = raw; + rewritten.insert(i, "read".to_string()); + rewritten + } else { + raw + } + }; + match app.run_with_args(raw).await { Ok(()) => std::process::exit(0), Err(e) => {