diff --git a/Cargo.toml b/Cargo.toml index 7ccf186..57e5ded 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,6 +14,8 @@ categories = ["command-line-utilities", "development-tools"] anyhow = "1.0.101" clap = { version = "4.5.57", features = ["derive"] } dirs = "6" +env_logger = "0.11.9" +log = "0.4.29" serde = { version = "1.0.228", features = ["derive"] } serde_json = "1.0.149" diff --git a/src/cli.rs b/src/cli.rs index 0ad1d61..b10f3bd 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -11,6 +11,9 @@ use crate::types::{AgentProvider, FileScope, FileStrategy}; version )] pub struct Cli { + #[arg(short, long, global = true)] + pub verbose: bool, + #[command(subcommand)] pub command: Command, } diff --git a/src/commands.rs b/src/commands.rs index eef6c5a..ec1ad80 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -1,15 +1,12 @@ use std::path::PathBuf; use anyhow::{Context, Result}; +use log::debug; use crate::manifest::{Dependency, FileMapping}; use crate::types::{AgentProvider, FileKind, FileScope, FileStrategy}; use crate::{git, installer, manifest, scanner}; -// --------------------------------------------------------------------------- -// Install command -// --------------------------------------------------------------------------- - /// Options for the install command, collected from CLI arguments. pub struct InstallOptions { pub source: Option, @@ -29,6 +26,10 @@ pub struct InstallOptions { /// - **With source**: resolves the source, scans it for agent files, installs /// them, and (unless `no_save` is set) adds the source to `agentfiles.json`. pub fn cmd_install(opts: InstallOptions) -> Result<()> { + debug!( + "cmd_install: source={:?}, scope={}, dry_run={}", + opts.source, opts.scope, opts.dry_run + ); let providers = opts .providers .unwrap_or_else(|| AgentProvider::ALL.to_vec()); @@ -76,6 +77,12 @@ fn install_from_manifest( } let loaded = manifest::load_manifest(project_root)?; + debug!( + "Loaded manifest '{}' v{} with {} dependencies", + loaded.name, + loaded.version, + loaded.dependencies.len() + ); if loaded.dependencies.is_empty() { println!("No dependencies in agentfiles.json. Add one with 'agentfiles install '."); return Ok(()); @@ -118,6 +125,7 @@ fn install_from_source( no_save: bool, dry_run: bool, ) -> Result<()> { + debug!("Installing from source: {}", source); let (source_dir, mut files) = resolve_source(source, None)?; // Apply pick filter @@ -126,6 +134,7 @@ fn install_from_source( if files.is_empty() { anyhow::bail!("no files matched the pick filter"); } + debug!("After pick filter: {} file(s) remaining", files.len()); } // Apply strategy override (CLI flag takes highest precedence) @@ -155,6 +164,7 @@ fn install_dependency( dry_run: bool, ) -> Result> { let source = dep.source(); + debug!("Installing dependency: {}", source); println!(" -> {source}"); let (source_dir, mut files) = resolve_source(source, dep.paths())?; @@ -182,13 +192,14 @@ fn install_dependency( return Ok(vec![]); } + debug!( + "Dependency '{}': {} file(s) to install", + source, + files.len() + ); installer::install(&files, providers, scope, project_root, &source_dir, dry_run) } -// --------------------------------------------------------------------------- -// Source resolution -// --------------------------------------------------------------------------- - /// Resolve a source (remote or local) to a local directory and scanned files. /// /// When `custom_paths` is provided, the scanner uses those instead of the @@ -197,6 +208,11 @@ fn resolve_source( source: &str, custom_paths: Option<&[manifest::PathMapping]>, ) -> Result<(PathBuf, Vec)> { + debug!( + "Resolving source: {} (is_git={})", + source, + git::is_git_url(source) + ); if git::is_git_url(source) { resolve_remote_source(source, custom_paths) } else { @@ -209,6 +225,7 @@ fn resolve_remote_source( source: &str, custom_paths: Option<&[manifest::PathMapping]>, ) -> Result<(PathBuf, Vec)> { + debug!("Resolving remote source: {}", source); let remote = git::parse_remote(source); let ref_display = remote @@ -237,6 +254,7 @@ fn resolve_local_source( source: &str, custom_paths: Option<&[manifest::PathMapping]>, ) -> Result<(PathBuf, Vec)> { + debug!("Resolving local source: {}", source); let path = PathBuf::from(source); if !path.exists() { anyhow::bail!("source path not found: {}", path.display()); @@ -254,6 +272,11 @@ fn resolve_local_source( if files.is_empty() { anyhow::bail!("no agent files found in {}", dir.display()); } + debug!( + "Local source resolved: {} file(s) in {}", + files.len(), + dir.display() + ); let canonical = dir .canonicalize() @@ -261,10 +284,6 @@ fn resolve_local_source( Ok((canonical, files)) } -// --------------------------------------------------------------------------- -// Manifest auto-save -// --------------------------------------------------------------------------- - /// Add a dependency to agentfiles.json, creating the file if it doesn't exist. /// /// Normalizes the source URL and extracts any inline `@ref` into the @@ -274,6 +293,7 @@ fn save_dependency( pick: Option<&[String]>, project_root: &std::path::Path, ) -> Result<()> { + debug!("Saving dependency: {}", source); let manifest_path = project_root.join("agentfiles.json"); let mut loaded = if manifest_path.is_file() { @@ -315,10 +335,6 @@ fn save_dependency( Ok(()) } -// --------------------------------------------------------------------------- -// Output -// --------------------------------------------------------------------------- - fn print_results(results: &[installer::InstallResult], dry_run: bool) { let prefix = if dry_run { "[dry-run] " } else { "" }; @@ -344,11 +360,8 @@ fn print_results(results: &[installer::InstallResult], dry_run: bool) { } } -// --------------------------------------------------------------------------- -// Init command -// --------------------------------------------------------------------------- - pub fn cmd_init(path: PathBuf, name: Option) -> Result<()> { + debug!("cmd_init: path={}, name={:?}", path.display(), name); let dir = if path.is_dir() { path.clone() } else { @@ -378,11 +391,8 @@ pub fn cmd_init(path: PathBuf, name: Option) -> Result<()> { Ok(()) } -// --------------------------------------------------------------------------- -// Scan command -// --------------------------------------------------------------------------- - pub fn cmd_scan(source: String) -> Result<()> { + debug!("cmd_scan: source={}", source); let files = if git::is_git_url(&source) { let remote = git::parse_remote(&source); @@ -414,10 +424,6 @@ pub fn cmd_scan(source: String) -> Result<()> { Ok(()) } -// --------------------------------------------------------------------------- -// Remove command -// --------------------------------------------------------------------------- - pub fn cmd_remove( source: String, clean: bool, @@ -425,6 +431,7 @@ pub fn cmd_remove( providers: Option>, root: PathBuf, ) -> Result<()> { + debug!("cmd_remove: source={}, clean={}", source, clean); let project_root = root .canonicalize() .context("could not resolve project root")?; @@ -459,6 +466,7 @@ fn clean_installed_files( providers: &[AgentProvider], scope: &FileScope, ) -> Result<()> { + debug!("Cleaning installed files for source: {}", source); // Resolve the source to get the file mappings let scan_result = resolve_source(source, None); @@ -489,6 +497,7 @@ fn clean_installed_files( let target_path = target_dir.join(file_name); if target_path.exists() || target_path.is_symlink() { + debug!("Removing {}", target_path.display()); if target_path.is_dir() && !target_path.is_symlink() { std::fs::remove_dir_all(&target_path) .with_context(|| format!("failed to remove {}", target_path.display()))?; @@ -509,11 +518,8 @@ fn clean_installed_files( Ok(()) } -// --------------------------------------------------------------------------- -// List command -// --------------------------------------------------------------------------- - pub fn cmd_list(root: PathBuf) -> Result<()> { + debug!("cmd_list: root={}", root.display()); let project_root = root .canonicalize() .context("could not resolve project root")?; @@ -567,10 +573,6 @@ pub fn cmd_list(root: PathBuf) -> Result<()> { Ok(()) } -// --------------------------------------------------------------------------- -// Matrix command -// --------------------------------------------------------------------------- - pub fn cmd_matrix() -> Result<()> { let kinds = [FileKind::Skill, FileKind::Command, FileKind::Agent]; let providers = AgentProvider::ALL; @@ -612,10 +614,6 @@ mod tests { use std::fs; use tempfile::TempDir; - // ----------------------------------------------------------------------- - // Scan command tests - // ----------------------------------------------------------------------- - #[test] fn scan_local_with_files() -> Result<()> { let dir = TempDir::new()?; @@ -645,10 +643,6 @@ mod tests { assert!(result.is_err()); } - // ----------------------------------------------------------------------- - // Init command tests - // ----------------------------------------------------------------------- - #[test] fn init_creates_empty_manifest() -> Result<()> { let dir = TempDir::new()?; @@ -673,10 +667,6 @@ mod tests { Ok(()) } - // ----------------------------------------------------------------------- - // Install from manifest tests - // ----------------------------------------------------------------------- - #[test] fn install_no_source_without_manifest_errors() { let dir = TempDir::new().unwrap(); @@ -714,10 +704,6 @@ mod tests { Ok(()) } - // ----------------------------------------------------------------------- - // Auto-save tests - // ----------------------------------------------------------------------- - #[test] fn install_source_auto_saves_to_manifest() -> Result<()> { let src_dir = TempDir::new()?; diff --git a/src/git.rs b/src/git.rs index 57a4430..beb3348 100644 --- a/src/git.rs +++ b/src/git.rs @@ -3,6 +3,7 @@ use std::process::Command; use std::sync::OnceLock; use anyhow::{Context, Result, bail}; +use log::debug; /// Result of resolving a remote git source. /// @@ -111,22 +112,30 @@ pub fn parse_remote(input: &str) -> ParsedRemote { /// /// If a `git_ref` is provided, checks out that ref after clone/fetch. pub fn resolve_remote(remote: &ParsedRemote) -> Result { + debug!( + "Resolving remote: url={}, ref={:?}", + remote.url, remote.git_ref + ); ensure_git_available()?; let cache_dir = get_cache_dir(&remote.url)?; if cache_dir.exists() { + debug!("Cache hit, fetching updates: {}", cache_dir.display()); // Update existing clone fetch_repo(&cache_dir)?; } else { + debug!("Cache miss, cloning to: {}", cache_dir.display()); // Fresh clone clone_repo(&remote.url, &cache_dir)?; } // Check out the requested ref, or reset to the default branch HEAD if let Some(ref git_ref) = remote.git_ref { + debug!("Checking out ref: {}", git_ref); checkout_ref(&cache_dir, git_ref)?; } else { + debug!("Resetting to default branch"); reset_to_default_branch(&cache_dir)?; } @@ -228,6 +237,7 @@ fn ensure_git_available() -> Result<()> { /// Clone a repository into the cache directory. fn clone_repo(url: &str, target: &Path) -> Result<()> { + debug!("Cloning {} into {}", url, target.display()); // Ensure parent directory exists if let Some(parent) = target.parent() { std::fs::create_dir_all(parent) @@ -250,6 +260,7 @@ fn clone_repo(url: &str, target: &Path) -> Result<()> { /// Fetch updates in an existing clone. fn fetch_repo(repo_dir: &Path) -> Result<()> { + debug!("Fetching updates in {}", repo_dir.display()); let output = Command::new("git") .args(["fetch", "--all", "--prune"]) .current_dir(repo_dir) @@ -286,6 +297,7 @@ fn validate_git_ref(git_ref: &str) -> Result<()> { /// Check out a specific ref (branch, tag, or commit hash). fn checkout_ref(repo_dir: &Path, git_ref: &str) -> Result<()> { + debug!("Checking out ref '{}' in {}", git_ref, repo_dir.display()); validate_git_ref(git_ref)?; // First, try a detached checkout (works for tags and commit hashes) @@ -317,6 +329,7 @@ fn checkout_ref(repo_dir: &Path, git_ref: &str) -> Result<()> { /// Reset the working tree to the latest commit on the default branch. fn reset_to_default_branch(repo_dir: &Path) -> Result<()> { + debug!("Resetting to default branch in {}", repo_dir.display()); // Get the default branch name from the remote HEAD let output = Command::new("git") .args(["symbolic-ref", "refs/remotes/origin/HEAD", "--short"]) @@ -330,6 +343,7 @@ fn reset_to_default_branch(repo_dir: &Path) -> Result<()> { } else { "main".to_string() }; + debug!("Default branch: {}", branch); let output = Command::new("git") .args(["checkout", &branch]) diff --git a/src/installer.rs b/src/installer.rs index 2441cad..f01501f 100644 --- a/src/installer.rs +++ b/src/installer.rs @@ -2,6 +2,7 @@ use std::fs; use std::path::Path; use anyhow::{Context, Result}; +use log::debug; use crate::manifest::FileMapping; use crate::types::{AgentProvider, FileKind, FileScope, FileStrategy}; @@ -34,12 +35,25 @@ pub(crate) fn install( source_root: &Path, dry_run: bool, ) -> Result> { + debug!( + "Installing {} file(s) to {} provider(s) (dry_run={})", + files.len(), + providers.len(), + dry_run + ); let mut results = Vec::new(); for file in files { let source_path = source_root.join(&file.path); + debug!( + "Processing: {} ({}, {})", + file.path.display(), + file.kind, + file.strategy + ); if !source_path.exists() { + debug!("Source file not found: {}", source_path.display()); anyhow::bail!( "source file not found: {} (resolved to {})", file.path.display(), @@ -49,11 +63,23 @@ pub(crate) fn install( for provider in providers { if !provider.supports_kind(&file.kind) { + debug!( + "Skipping {} for provider {} (unsupported {})", + file.path.display(), + provider, + file.kind + ); continue; } let target_dir = provider.get_target_dir(scope, &file.kind, project_root)?; let target_path = resolve_target_path(&file.path, &target_dir)?; + debug!( + "Target: {} -> {} (provider={})", + source_path.display(), + target_path.display(), + provider + ); if !dry_run { if let Some(parent) = target_path.parent() { @@ -65,6 +91,11 @@ pub(crate) fn install( // Place the file match file.strategy { FileStrategy::Copy => { + debug!( + "Copying {} -> {}", + source_path.display(), + target_path.display() + ); if source_path.is_dir() { copy_dir_recursive(&source_path, &target_path)?; } else { @@ -78,6 +109,11 @@ pub(crate) fn install( } } FileStrategy::Link => { + debug!( + "Symlinking {} -> {}", + source_path.display(), + target_path.display() + ); if target_path.exists() || target_path.is_symlink() { if target_path.is_dir() && !target_path.is_symlink() { fs::remove_dir_all(&target_path)?; @@ -155,6 +191,7 @@ fn resolve_target_path(relative_path: &Path, target_dir: &Path) -> Result Result<()> { + debug!("Recursively copying {} -> {}", src.display(), dst.display()); fs::create_dir_all(dst)?; for entry in fs::read_dir(src)? { let entry = entry?; @@ -163,6 +200,7 @@ fn copy_dir_recursive(src: &Path, dst: &Path) -> Result<()> { let dst_path = dst.join(entry.file_name()); if file_type.is_symlink() { + debug!("Skipping symlink: {}", src_path.display()); // Skip symlinks to prevent infinite recursion from directory loops continue; } else if file_type.is_dir() { @@ -408,4 +446,72 @@ mod tests { assert!(result.is_err()); } + + #[test] + fn dry_run_does_not_create_files() -> Result<()> { + let src_dir = TempDir::new()?; + let dst_dir = TempDir::new()?; + + let skill_dir = src_dir.path().join("skills").join("review"); + fs::create_dir_all(&skill_dir)?; + fs::write(skill_dir.join("SKILL.md"), "# Review")?; + + let files = vec![FileMapping { + path: PathBuf::from("skills/review"), + kind: FileKind::Skill, + strategy: FileStrategy::Copy, + }]; + + let results = install( + &files, + &[AgentProvider::ClaudeCode], + &FileScope::Project, + dst_dir.path(), + src_dir.path(), + true, // dry_run + )?; + + assert_eq!(results.len(), 1); + // No actual files should be created + assert!(!dst_dir.path().join(".claude").exists()); + Ok(()) + } + + #[cfg(unix)] + #[test] + fn copy_dir_recursive_skips_symlinks() -> Result<()> { + let src_dir = TempDir::new()?; + let dst_dir = TempDir::new()?; + + let skill = src_dir.path().join("my-skill"); + fs::create_dir_all(&skill)?; + fs::write(skill.join("SKILL.md"), "# Skill")?; + // Create a symlink inside the skill dir + std::os::unix::fs::symlink("/dev/null", skill.join("bad-link"))?; + + let target = dst_dir.path().join("my-skill"); + copy_dir_recursive(&skill, &target)?; + + assert!(target.join("SKILL.md").exists()); + assert!(!target.join("bad-link").exists()); // symlink should be skipped + Ok(()) + } + + #[test] + fn install_empty_files_list() -> Result<()> { + let src_dir = TempDir::new()?; + let dst_dir = TempDir::new()?; + + let results = install( + &[], + &[AgentProvider::ClaudeCode], + &FileScope::Project, + dst_dir.path(), + src_dir.path(), + false, + )?; + + assert!(results.is_empty()); + Ok(()) + } } diff --git a/src/main.rs b/src/main.rs index 83ddae6..3e3dc30 100644 --- a/src/main.rs +++ b/src/main.rs @@ -5,6 +5,12 @@ use clap::Parser; fn main() -> Result<()> { let args = cli::Cli::parse(); + if args.verbose { + unsafe { std::env::set_var("RUST_LOG", "debug") }; + } + + env_logger::init(); + match args.command { cli::Command::Install { source, diff --git a/src/manifest.rs b/src/manifest.rs index 9723b1e..4daa430 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1,15 +1,12 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; +use log::debug; use serde::{Deserialize, Serialize}; use crate::git; use crate::types::{FileKind, FileStrategy}; -// --------------------------------------------------------------------------- -// Internal types (scanner output / installer input) -// --------------------------------------------------------------------------- - /// A single discovered agent file used by the scanner and installer. /// /// Not serialized into the manifest — this is an in-memory representation @@ -26,10 +23,6 @@ pub(crate) struct FileMapping { pub strategy: FileStrategy, } -// --------------------------------------------------------------------------- -// Dependency types (serialized in agentfiles.json) -// --------------------------------------------------------------------------- - /// A dependency source -- either a simple URL/path string or a detailed spec. /// /// Simple form: `"github.com/org/repo"` or `"github.com/org/repo@v1.0"` @@ -120,10 +113,6 @@ pub struct PathMapping { pub kind: FileKind, } -// --------------------------------------------------------------------------- -// Manifest -// --------------------------------------------------------------------------- - /// The agentfiles.json project manifest. /// /// Lists dependencies (remote or local sources) that provide agent files. @@ -201,8 +190,10 @@ impl Manifest { pub fn add_dependency(&mut self, dep: Dependency) -> bool { let source = dep.source().to_string(); if self.has_dependency(&source) { + debug!("Dependency already exists: {}", source); return false; } + debug!("Adding dependency: {}", source); self.dependencies.push(dep); true } @@ -222,6 +213,7 @@ impl Manifest { /// /// Uses normalized URL comparison, same as `has_dependency`. pub fn remove_dependency(&mut self, source: &str) -> bool { + debug!("Removing dependency: {}", source); let normalized = git::normalize_source(source); let before = self.dependencies.len(); self.dependencies @@ -234,6 +226,7 @@ impl Manifest { /// /// If `path` is a directory, looks for `agentfiles.json` inside it. pub fn load_manifest(path: &Path) -> Result { + debug!("Loading manifest from {}", path.display()); if path.is_dir() { return load_manifest(&path.join("agentfiles.json")); } @@ -246,6 +239,7 @@ pub fn load_manifest(path: &Path) -> Result { /// Returns the full path of the written file. /// Errors if `path` points to an existing file. pub fn save_manifest(manifest: &Manifest, path: &Path) -> Result { + debug!("Saving manifest to {}", path.display()); if path.is_file() { bail!("cannot save manifest to a file, provide a directory path."); } diff --git a/src/scanner.rs b/src/scanner.rs index 4602e2a..d19d66b 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -2,6 +2,7 @@ use std::fs; use std::path::Path; use anyhow::{Context, Result}; +use log::debug; use crate::manifest::{FileMapping, PathMapping}; use crate::types::{AgentProvider, FileKind, FileStrategy}; @@ -13,10 +14,6 @@ const KIND_DIRS: &[(&str, FileKind)] = &[ ("agents", FileKind::Agent), ]; -// --------------------------------------------------------------------------- -// Public API -// --------------------------------------------------------------------------- - /// Scan a directory for agent files and return discovered file mappings. /// /// When `custom_paths` is `None`, uses the default convention: scans @@ -38,15 +35,22 @@ pub(crate) fn scan_agent_files( .canonicalize() .with_context(|| format!("cannot resolve path: {}", root.display()))?; + debug!("Scanning agent files in {}", root.display()); let mut mappings = Vec::new(); if let Some(paths) = custom_paths { + debug!("Using {} custom path mapping(s)", paths.len()); scan_custom_paths(&root, paths, &mut mappings)?; } else { + debug!("Using default convention scanning"); scan_default_convention(&root, &mut mappings)?; } deduplicate(&mut mappings); + debug!( + "Scan complete: {} mapping(s) after deduplication", + mappings.len() + ); Ok(mappings) } @@ -56,6 +60,11 @@ pub(crate) fn scan_agent_files( /// Pick items can be kind-prefixed (`"skills/review"`, `"commands/deploy"`) /// or plain names (`"review"`). A plain name matches any kind. pub(crate) fn filter_by_pick(mappings: Vec, pick: &[String]) -> Vec { + debug!( + "Filtering {} mapping(s) by {} pick item(s)", + mappings.len(), + pick.len() + ); mappings .into_iter() .filter(|m| { @@ -85,16 +94,13 @@ pub(crate) fn infer_name(path: &Path) -> String { .unwrap_or_else(|| "unnamed".to_string()) } -// --------------------------------------------------------------------------- -// Default convention scanning -// --------------------------------------------------------------------------- - /// Scan using the default convention: provider-prefixed dirs + bare kind dirs. fn scan_default_convention(root: &Path, mappings: &mut Vec) -> Result<()> { // Scan known provider-prefixed directories (derived from provider layouts) for prefix in AgentProvider::PROJECT_BASES { let prefix_dir = root.join(prefix); if prefix_dir.is_dir() { + debug!("Checking provider prefix: {}", prefix_dir.display()); scan_kind_dirs(root, &prefix_dir, mappings)?; } } @@ -103,6 +109,7 @@ fn scan_default_convention(root: &Path, mappings: &mut Vec) -> Resu for &(kind_name, ref kind) in KIND_DIRS { let kind_dir = root.join(kind_name); if kind_dir.is_dir() { + debug!("Checking bare kind directory: {}", kind_dir.display()); scan_kind_dir(root, &kind_dir, kind, mappings)?; } } @@ -132,8 +139,12 @@ fn scan_kind_dir( kind: &FileKind, mappings: &mut Vec, ) -> Result<()> { - let entries = fs::read_dir(kind_dir) - .with_context(|| format!("cannot read directory: {}", kind_dir.display()))?; + debug!("Scanning {} for {}", kind_dir.display(), kind); + let entries: Vec<_> = fs::read_dir(kind_dir) + .with_context(|| format!("cannot read directory: {}", kind_dir.display()))? + .collect(); + + debug!("Found {} entries in {}", entries.len(), kind_dir.display()); for entry in entries { let entry = entry?; @@ -141,8 +152,6 @@ fn scan_kind_dir( match kind { FileKind::Skill => { - // Skills are directories containing SKILL.md. - // We record the directory path so the entire skill dir is installed. if entry_path.is_dir() { let skill_md = entry_path.join("SKILL.md"); if skill_md.is_file() { @@ -150,12 +159,16 @@ fn scan_kind_dir( .strip_prefix(root) .unwrap_or(&entry_path) .to_path_buf(); + debug!("Found skill: {}", rel_path.display()); mappings.push(FileMapping { path: rel_path, kind: FileKind::Skill, strategy: FileStrategy::Copy, }); + continue; } + debug!("No SKILL.md in {}, recursing", entry_path.display()); + scan_kind_dir(root, &entry_path, kind, mappings)?; } } FileKind::Command | FileKind::Agent => { @@ -168,12 +181,18 @@ fn scan_kind_dir( .strip_prefix(root) .unwrap_or(&entry_path) .to_path_buf(); + debug!("Found {}: {}", kind, rel_path.display()); mappings.push(FileMapping { path: rel_path, kind: *kind, strategy: FileStrategy::Copy, }); + continue; } + if entry_path.is_dir() { + debug!("Recursing into {} for {}", entry_path.display(), kind); + } + scan_kind_dir(root, &entry_path, kind, mappings)?; } } } @@ -181,10 +200,6 @@ fn scan_kind_dir( Ok(()) } -// --------------------------------------------------------------------------- -// Custom path scanning -// --------------------------------------------------------------------------- - /// Scan custom path mappings. Each entry maps a relative path to a file kind. /// Directories are scanned using the standard kind convention. Files are /// added directly. @@ -197,16 +212,15 @@ fn scan_custom_paths( let full_path = root.join(&mapping.path); if !full_path.exists() { - // Skip paths that don't exist -- the source may not have all - // configured paths. + debug!("Skipping non-existent path: {}", full_path.display()); continue; } if full_path.is_dir() { - // Scan directory using the standard convention for this kind + debug!("Scanning directory: {}", full_path.display()); scan_kind_dir(root, &full_path, &mapping.kind, mappings)?; } else if full_path.is_file() { - // Single file -- add directly + debug!("Adding file: {}", full_path.display()); let rel_path = full_path .strip_prefix(root) .unwrap_or(&full_path) @@ -222,15 +236,12 @@ fn scan_custom_paths( Ok(()) } -// --------------------------------------------------------------------------- -// Deduplication -// --------------------------------------------------------------------------- - /// Deduplicate file mappings by their name + kind. /// /// If the same skill/command/agent name appears from multiple provider /// directories, keep only the first occurrence. fn deduplicate(mappings: &mut Vec) { + debug!("Deduplicating {} mapping(s)", mappings.len()); let mut seen = std::collections::HashSet::new(); mappings.retain(|m| { let stem = m.path.file_stem().unwrap_or_default().to_string_lossy(); @@ -274,10 +285,6 @@ mod tests { .unwrap(); } - // ----------------------------------------------------------------------- - // Default convention tests - // ----------------------------------------------------------------------- - #[test] fn scans_claude_skills() -> Result<()> { let dir = TempDir::new()?; @@ -365,10 +372,6 @@ mod tests { Ok(()) } - // ----------------------------------------------------------------------- - // Custom path tests - // ----------------------------------------------------------------------- - #[test] fn custom_paths_scan_directory() -> Result<()> { let dir = TempDir::new()?; @@ -463,10 +466,6 @@ mod tests { Ok(()) } - // ----------------------------------------------------------------------- - // Pick filter tests - // ----------------------------------------------------------------------- - #[test] fn filter_by_plain_name() { let mappings = vec![ @@ -531,14 +530,173 @@ mod tests { assert_eq!(filtered.len(), 2); } - // ----------------------------------------------------------------------- - // Infer name - // ----------------------------------------------------------------------- - #[test] fn infer_name_from_path() { assert_eq!(infer_name(Path::new("/home/user/my-project")), "my-project"); assert_eq!(infer_name(Path::new("/")), "unnamed"); assert_eq!(infer_name(Path::new("some-folder")), "some-folder"); } + + #[test] + fn scans_deeply_nested_skills() -> Result<()> { + let dir = TempDir::new()?; + let deep = dir + .path() + .join("skills") + .join("a") + .join("b") + .join("c") + .join("deep-skill"); + fs::create_dir_all(&deep)?; + fs::write(deep.join("SKILL.md"), "# Deep skill")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 1); + assert_eq!(mappings[0].kind, FileKind::Skill); + assert!(mappings[0].path.to_string_lossy().contains("deep-skill")); + Ok(()) + } + + #[test] + fn scans_deeply_nested_commands() -> Result<()> { + let dir = TempDir::new()?; + let nested = dir + .path() + .join("commands") + .join("category") + .join("subcategory"); + fs::create_dir_all(&nested)?; + fs::write(nested.join("deploy.md"), "# Deploy")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 1); + assert_eq!(mappings[0].kind, FileKind::Command); + Ok(()) + } + + #[test] + fn only_md_files_become_commands() -> Result<()> { + let dir = TempDir::new()?; + let cmd_dir = dir.path().join("commands"); + fs::create_dir_all(&cmd_dir)?; + fs::write(cmd_dir.join("deploy.md"), "# Deploy")?; + fs::write(cmd_dir.join("review.md"), "# Review")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 2); + for m in &mappings { + assert_eq!(m.kind, FileKind::Command); + assert!(m.path.to_string_lossy().ends_with(".md")); + } + Ok(()) + } + + #[test] + fn skips_skill_dir_without_skill_md() -> Result<()> { + let dir = TempDir::new()?; + let skill_dir = dir.path().join("skills").join("incomplete"); + fs::create_dir_all(&skill_dir)?; + fs::write(skill_dir.join("README.md"), "# Not a skill")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert!(mappings.is_empty()); + Ok(()) + } + + #[test] + fn empty_kind_dirs_return_empty() -> Result<()> { + let dir = TempDir::new()?; + fs::create_dir_all(dir.path().join("skills"))?; + fs::create_dir_all(dir.path().join("commands"))?; + fs::create_dir_all(dir.path().join("agents"))?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert!(mappings.is_empty()); + Ok(()) + } + + #[test] + fn mixed_kinds_across_providers_and_bare() -> Result<()> { + let dir = TempDir::new()?; + setup_skill(dir.path(), ".claude", "review"); + setup_command(dir.path(), ".opencode", "deploy"); + setup_agent(dir.path(), "", "security"); + setup_skill(dir.path(), "", "lint"); + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 4); + + let skill_count = mappings + .iter() + .filter(|m| m.kind == FileKind::Skill) + .count(); + let cmd_count = mappings + .iter() + .filter(|m| m.kind == FileKind::Command) + .count(); + let agent_count = mappings + .iter() + .filter(|m| m.kind == FileKind::Agent) + .count(); + assert_eq!(skill_count, 2); + assert_eq!(cmd_count, 1); + assert_eq!(agent_count, 1); + Ok(()) + } + + #[test] + fn dedup_prefers_first_occurrence() -> Result<()> { + let dir = TempDir::new()?; + setup_skill(dir.path(), ".claude", "review"); + setup_skill(dir.path(), "", "review"); + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 1); + // Provider-prefixed dirs are scanned first, so .claude/skills/review wins + assert!(mappings[0].path.to_string_lossy().contains(".claude")); + Ok(()) + } + + #[test] + fn scans_sibling_skills_at_different_depths() -> Result<()> { + let dir = TempDir::new()?; + // Shallow skill + let shallow = dir.path().join("skills").join("shallow-skill"); + fs::create_dir_all(&shallow)?; + fs::write(shallow.join("SKILL.md"), "# Shallow")?; + + // Deep skill + let deep = dir + .path() + .join("skills") + .join("category") + .join("deep-skill"); + fs::create_dir_all(&deep)?; + fs::write(deep.join("SKILL.md"), "# Deep")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 2); + let names: Vec = mappings + .iter() + .map(|m| m.path.file_name().unwrap().to_string_lossy().into_owned()) + .collect(); + assert!(names.contains(&"shallow-skill".to_string())); + assert!(names.contains(&"deep-skill".to_string())); + Ok(()) + } + + #[test] + fn scan_defaults_strategy_to_copy() -> Result<()> { + let dir = TempDir::new()?; + setup_skill(dir.path(), ".claude", "s1"); + setup_command(dir.path(), ".claude", "c1"); + setup_agent(dir.path(), ".claude", "a1"); + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 3); + for m in &mappings { + assert_eq!(m.strategy, FileStrategy::Copy); + } + Ok(()) + } }