From bc0dfbd19c7d6e612daeebc344c80582a2407d11 Mon Sep 17 00:00:00 2001 From: leodiegues Date: Sun, 15 Feb 2026 18:57:36 -0300 Subject: [PATCH 1/2] refactor: install from manifest and add dependency options Make install accept no source to install all dependencies from agentfiles.json. Add --pick and --no-save CLI options. Move FileMapping out of Manifest and introduce Dependency/DependencySpec/PathMapping to model entries in agentfiles.json. Refactor cmd_install into install_from_manifest, install_from_source, and install_dependency, and add source resolution helpers plus per-dependency pick/strategy/path handling. --- src/cli.rs | 19 +- src/commands.rs | 504 +++++++++++++++++++++++++++++++++++++---------- src/installer.rs | 139 +++++++------ src/main.rs | 4 +- src/manifest.rs | 449 +++++++++++++++++++++++++++++------------ src/scanner.rs | 371 ++++++++++++++++++++++++++++++---- 6 files changed, 1142 insertions(+), 344 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index fd8643b..f673eef 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -17,11 +17,11 @@ pub struct Cli { #[derive(Subcommand)] pub enum Command { - /// Install agent files from a manifest or remote git repository + /// Install agent files from dependencies in agentfiles.json, or add a new source Install { - /// Source: local path, directory, or git URL (e.g., github.com/org/repo@v1.0) - #[arg(default_value = ".")] - source: String, + /// Source: local path or git URL (e.g., github.com/org/repo@v1.0). + /// If omitted, installs all dependencies from agentfiles.json. + source: Option, /// Installation scope: project or global #[arg(short, long, default_value = "project")] @@ -32,10 +32,19 @@ pub enum Command { #[arg(short, long, value_delimiter = ',')] providers: Option>, - /// File placement strategy: copy or link (symlink). Can be overridden per-file in the manifest. + /// File placement strategy: copy or link (symlink). Can be overridden per-dependency in the manifest. #[arg(long)] strategy: Option, + /// Cherry-pick specific items by name (comma-separated). + /// Supports kind prefix: skills/review, commands/deploy, or plain: review + #[arg(long, value_delimiter = ',')] + pick: Option>, + + /// Do not save the source to agentfiles.json after installing + #[arg(long)] + no_save: bool, + /// Project root directory (for project scope installations) #[arg(long, default_value = ".")] root: PathBuf, diff --git a/src/commands.rs b/src/commands.rs index 54ed3a7..17cd160 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -2,66 +2,190 @@ use std::path::PathBuf; use anyhow::{Context, Result}; +use crate::manifest::{Dependency, FileMapping}; use crate::types::{AgentProvider, FileKind, FileScope, FileStrategy}; use crate::{git, installer, manifest, scanner}; +// --------------------------------------------------------------------------- +// Install command +// --------------------------------------------------------------------------- + +/// Install agent files. Two flows: +/// +/// - **No source**: reads `agentfiles.json` from the project root and installs +/// all dependencies listed there. +/// - **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( - source: String, + source: Option, scope: FileScope, providers: Option>, strategy_override: Option, + pick: Option>, + no_save: bool, root: PathBuf, ) -> Result<()> { let providers = providers.unwrap_or_else(|| AgentProvider::ALL.to_vec()); - let (manifest_dir, mut loaded) = if git::is_git_url(&source) { - resolve_remote_source(&source)? - } else { - resolve_local_source(&source)? - }; + let project_root = root + .canonicalize() + .context("could not resolve project root")?; + + match source { + None => install_from_manifest(&project_root, &providers, &scope, strategy_override), + Some(src) => install_from_source( + &src, + &project_root, + &providers, + &scope, + strategy_override, + pick.as_deref(), + no_save, + ), + } +} + +/// Install all dependencies listed in the project's `agentfiles.json`. +fn install_from_manifest( + project_root: &std::path::Path, + providers: &[AgentProvider], + scope: &FileScope, + strategy_override: Option, +) -> Result<()> { + let manifest_path = project_root.join("agentfiles.json"); + if !manifest_path.is_file() { + anyhow::bail!( + "no agentfiles.json found in {}. Run 'agentfiles init' first or specify a source.", + project_root.display() + ); + } + + let loaded = manifest::load_manifest(project_root)?; + if loaded.dependencies.is_empty() { + println!("No dependencies in agentfiles.json. Add one with 'agentfiles install '."); + return Ok(()); + } + + println!( + "Installing {} dependency(ies) from '{}' (v{})...\n", + loaded.dependencies.len(), + loaded.name, + loaded.version, + ); + + let mut total_results = Vec::new(); + for dep in &loaded.dependencies { + let dep_results = + install_dependency(dep, project_root, providers, scope, strategy_override)?; + total_results.extend(dep_results); + } + + print_results(&total_results); + Ok(()) +} + +/// Install from a specific source, optionally saving it to agentfiles.json. +fn install_from_source( + source: &str, + project_root: &std::path::Path, + providers: &[AgentProvider], + scope: &FileScope, + strategy_override: Option, + pick: Option<&[String]>, + no_save: bool, +) -> Result<()> { + let (source_dir, mut files) = resolve_source(source)?; + + // Apply pick filter + if let Some(pick_list) = pick { + files = scanner::filter_by_pick(files, pick_list); + if files.is_empty() { + anyhow::bail!("no files matched the pick filter"); + } + } + + // Apply strategy override if let Some(strategy) = strategy_override { - for file in &mut loaded.files { + for file in &mut files { if file.strategy == FileStrategy::Copy { file.strategy = strategy; } } } - let project_root = root - .canonicalize() - .context("could not resolve project root")?; + let results = installer::install(&files, providers, scope, project_root, &source_dir)?; - let results = installer::install(&loaded, &providers, &scope, &project_root, &manifest_dir)?; + if !no_save { + save_dependency(source, pick, project_root)?; + } - if results.is_empty() { - println!("No files installed (no compatible provider/kind combinations found)."); - } else { - println!( - "Installed {} file(s) from '{}' (v{}):\n", - results.len(), - loaded.name, - loaded.version - ); - for r in &results { - println!( - " [{:>11}] {} -> {} ({})", - r.provider.to_string(), - r.source, - r.target, - r.strategy - ); + print_results(&results); + Ok(()) +} + +/// Install a single dependency from the manifest. +fn install_dependency( + dep: &Dependency, + project_root: &std::path::Path, + providers: &[AgentProvider], + scope: &FileScope, + strategy_override: Option, +) -> Result> { + let source = dep.source(); + println!(" -> {source}"); + + let (source_dir, mut files) = resolve_source(source)?; + + // Apply custom path mappings if specified + if let Some(custom_paths) = dep.paths() { + // Re-scan with custom paths (the initial resolve_source used defaults) + files = scanner::scan_agent_files(&source_dir, Some(custom_paths))?; + } + + // Apply pick filter + if let Some(pick_list) = dep.pick() { + files = scanner::filter_by_pick(files, pick_list); + } + + // Apply per-dependency strategy, then global override + let dep_strategy = dep.strategy(); + for file in &mut files { + if let Some(strategy) = dep_strategy + && file.strategy == FileStrategy::Copy + { + file.strategy = strategy; + } + if let Some(strategy) = strategy_override + && file.strategy == FileStrategy::Copy + { + file.strategy = strategy; } } - Ok(()) + if files.is_empty() { + println!(" (no matching files found)"); + return Ok(vec![]); + } + + installer::install(&files, providers, scope, project_root, &source_dir) } -/// Resolve a remote git URL to a local directory and manifest. -/// -/// Clones (or updates the cache of) the remote repository, then either -/// loads `agentfiles.json` from it or auto-discovers agent files via scanning. -fn resolve_remote_source(source: &str) -> Result<(PathBuf, manifest::Manifest)> { +// --------------------------------------------------------------------------- +// Source resolution +// --------------------------------------------------------------------------- + +/// Resolve a source (remote or local) to a local directory and scanned files. +fn resolve_source(source: &str) -> Result<(PathBuf, Vec)> { + if git::is_git_url(source) { + resolve_remote_source(source) + } else { + resolve_local_source(source) + } +} + +/// Clone/fetch a remote git repo and scan for agent files. +fn resolve_remote_source(source: &str) -> Result<(PathBuf, Vec)> { let remote = git::parse_remote(source); let ref_display = remote @@ -76,54 +200,107 @@ fn resolve_remote_source(source: &str) -> Result<(PathBuf, manifest::Manifest)> println!("Cached at: {}\n", local_path.display()); - let manifest_path = local_path.join("agentfiles.json"); - let loaded = if manifest_path.is_file() { - println!("Found agentfiles.json in remote repository."); - manifest::load_manifest(&local_path)? - } else { - println!("No agentfiles.json found — scanning for agent files..."); - let files = scanner::scan_agent_files(&local_path)?; - if files.is_empty() { - anyhow::bail!( - "no agentfiles.json and no agent files found in {}", - git_source.url - ); - } - println!("Discovered {} agent file(s) via scan.\n", files.len()); - - let name = remote - .url - .rsplit('/') - .next() - .unwrap_or("remote") - .trim_end_matches(".git") - .to_string(); - - manifest::Manifest::default() - .with_name(name) - .with_repository(remote.url.clone()) - .with_files(files) - }; + let files = scanner::scan_agent_files(&local_path, None)?; + if files.is_empty() { + anyhow::bail!("no agent files found in {}", git_source.url); + } + println!("Discovered {} agent file(s).\n", files.len()); - Ok((local_path, loaded)) + Ok((local_path, files)) } -/// Resolve a local path to a directory and manifest. -fn resolve_local_source(source: &str) -> Result<(PathBuf, manifest::Manifest)> { +/// Resolve a local path and scan for agent files. +fn resolve_local_source(source: &str) -> Result<(PathBuf, Vec)> { let path = PathBuf::from(source); + if !path.exists() { + anyhow::bail!("source path not found: {}", path.display()); + } - let manifest_dir = if path.is_dir() { - path.clone() + let dir = if path.is_dir() { + path } else { path.parent() .map(|p| p.to_path_buf()) .unwrap_or_else(|| PathBuf::from(".")) }; - let loaded = manifest::load_manifest(&path)?; - Ok((manifest_dir, loaded)) + let files = scanner::scan_agent_files(&dir, None)?; + if files.is_empty() { + anyhow::bail!("no agent files found in {}", dir.display()); + } + + let canonical = dir + .canonicalize() + .context("could not resolve source path")?; + Ok((canonical, files)) } +// --------------------------------------------------------------------------- +// Manifest auto-save +// --------------------------------------------------------------------------- + +/// Add a dependency to agentfiles.json, creating the file if it doesn't exist. +fn save_dependency( + source: &str, + pick: Option<&[String]>, + project_root: &std::path::Path, +) -> Result<()> { + let manifest_path = project_root.join("agentfiles.json"); + + let mut loaded = if manifest_path.is_file() { + manifest::load_manifest(project_root)? + } else { + let name = scanner::infer_name(project_root); + manifest::Manifest::default().with_name(name) + }; + + let dep = if let Some(pick_list) = pick { + Dependency::Detailed(manifest::DependencySpec { + source: source.to_string(), + git_ref: None, + pick: Some(pick_list.to_vec()), + strategy: None, + paths: None, + }) + } else { + Dependency::Simple(source.to_string()) + }; + + if loaded.add_dependency(dep) { + manifest::save_manifest(&loaded, project_root)?; + println!("Saved to agentfiles.json"); + } else { + println!("Dependency already in agentfiles.json"); + } + + Ok(()) +} + +// --------------------------------------------------------------------------- +// Output +// --------------------------------------------------------------------------- + +fn print_results(results: &[installer::InstallResult]) { + if results.is_empty() { + println!("No files installed (no compatible provider/kind combinations found)."); + } else { + println!("\nInstalled {} file(s):\n", results.len(),); + for r in results { + println!( + " [{:>11}] {} -> {} ({})", + r.provider.to_string(), + r.source, + r.target, + r.strategy + ); + } + } +} + +// --------------------------------------------------------------------------- +// Init command +// --------------------------------------------------------------------------- + pub fn cmd_init(path: PathBuf, name: Option) -> Result<()> { let dir = if path.is_dir() { path.clone() @@ -141,31 +318,23 @@ pub fn cmd_init(path: PathBuf, name: Option) -> Result<()> { ); } - let files = scanner::scan_agent_files(&dir).unwrap_or_default(); - let pkg_name = name.unwrap_or_else(|| scanner::infer_name(&dir)); - let m = manifest::Manifest::default() - .with_name(pkg_name) - .with_files(files); + let m = manifest::Manifest::default().with_name(pkg_name); let output_path = manifest::save_manifest(&m, &dir)?; println!("Created {}", output_path.display()); - - if !m.files.is_empty() { - println!("Discovered {} agent file(s):", m.files.len()); - for f in &m.files { - println!(" {} ({})", f.path.display(), f.kind); - } - } else { - println!( - "No agent files found. Add files to skills/, commands/, or agents/ and run 'agentfiles scan'." - ); - } + println!( + "Add dependencies with 'agentfiles install ' or edit agentfiles.json directly." + ); Ok(()) } +// --------------------------------------------------------------------------- +// Scan command +// --------------------------------------------------------------------------- + pub fn cmd_scan(source: String) -> Result<()> { let files = if git::is_git_url(&source) { let remote = git::parse_remote(&source); @@ -180,9 +349,9 @@ pub fn cmd_scan(source: String) -> Result<()> { let git_source = git::resolve_remote(&remote)?; println!("Cached at: {}\n", git_source.local_path.display()); - scanner::scan_agent_files(&git_source.local_path)? + scanner::scan_agent_files(&git_source.local_path, None)? } else { - scanner::scan_agent_files(&PathBuf::from(&source))? + scanner::scan_agent_files(&PathBuf::from(&source), None)? }; if files.is_empty() { @@ -198,6 +367,10 @@ pub fn cmd_scan(source: String) -> Result<()> { Ok(()) } +// --------------------------------------------------------------------------- +// Matrix command +// --------------------------------------------------------------------------- + pub fn cmd_matrix() -> Result<()> { let kinds = [FileKind::Skill, FileKind::Command, FileKind::Agent]; let providers = AgentProvider::ALL; @@ -239,30 +412,17 @@ mod tests { use std::fs; use tempfile::TempDir; - fn setup_skill(dir: &std::path::Path, name: &str) { - let skill_dir = dir.join(".claude").join("skills").join(name); - fs::create_dir_all(&skill_dir).unwrap(); - fs::write(skill_dir.join("SKILL.md"), "test skill").unwrap(); - } - - fn setup_command(dir: &std::path::Path, name: &str) { - let cmd_dir = dir.join(".claude").join("commands"); - fs::create_dir_all(&cmd_dir).unwrap(); - fs::write(cmd_dir.join(format!("{name}.md")), "test command").unwrap(); - } - - fn setup_agent(dir: &std::path::Path, name: &str) { - let agent_dir = dir.join(".claude").join("agents"); - fs::create_dir_all(&agent_dir).unwrap(); - fs::write(agent_dir.join(format!("{name}.md")), "test agent").unwrap(); - } + // ----------------------------------------------------------------------- + // Scan command tests + // ----------------------------------------------------------------------- #[test] fn scan_local_with_files() -> Result<()> { let dir = TempDir::new()?; - setup_skill(dir.path(), "review"); - setup_command(dir.path(), "deploy"); - setup_agent(dir.path(), "security"); + // Create bare skills directory + let skill_dir = dir.path().join("skills").join("review"); + fs::create_dir_all(&skill_dir)?; + fs::write(skill_dir.join("SKILL.md"), "test skill")?; let source = dir.path().to_string_lossy().into_owned(); let result = cmd_scan(source); @@ -273,7 +433,6 @@ mod tests { #[test] fn scan_local_empty() -> Result<()> { let dir = TempDir::new()?; - let source = dir.path().to_string_lossy().into_owned(); let result = cmd_scan(source); assert!(result.is_ok()); @@ -285,4 +444,133 @@ mod tests { let result = cmd_scan("/tmp/this-path-definitely-does-not-exist-agentfiles".into()); assert!(result.is_err()); } + + // ----------------------------------------------------------------------- + // Init command tests + // ----------------------------------------------------------------------- + + #[test] + fn init_creates_empty_manifest() -> Result<()> { + let dir = TempDir::new()?; + cmd_init(dir.path().to_path_buf(), Some("my-project".to_string()))?; + + let manifest_path = dir.path().join("agentfiles.json"); + assert!(manifest_path.exists()); + + let loaded = manifest::load_manifest(dir.path())?; + assert_eq!(loaded.name, "my-project"); + assert!(loaded.dependencies.is_empty()); + Ok(()) + } + + #[test] + fn init_errors_if_manifest_exists() -> Result<()> { + let dir = TempDir::new()?; + fs::write(dir.path().join("agentfiles.json"), "{}")?; + + let result = cmd_init(dir.path().to_path_buf(), None); + assert!(result.is_err()); + Ok(()) + } + + // ----------------------------------------------------------------------- + // Install from manifest tests + // ----------------------------------------------------------------------- + + #[test] + fn install_no_source_without_manifest_errors() { + let dir = TempDir::new().unwrap(); + let result = cmd_install( + None, + FileScope::Project, + None, + None, + None, + false, + dir.path().to_path_buf(), + ); + assert!(result.is_err()); + } + + #[test] + fn install_no_source_with_empty_manifest() -> Result<()> { + let dir = TempDir::new()?; + let manifest = manifest::Manifest::default().with_name("test".to_string()); + manifest::save_manifest(&manifest, dir.path())?; + + // Should succeed but print "no dependencies" message + let result = cmd_install( + None, + FileScope::Project, + None, + None, + None, + false, + dir.path().to_path_buf(), + ); + assert!(result.is_ok()); + Ok(()) + } + + // ----------------------------------------------------------------------- + // Auto-save tests + // ----------------------------------------------------------------------- + + #[test] + fn install_source_auto_saves_to_manifest() -> Result<()> { + let src_dir = TempDir::new()?; + let dst_dir = TempDir::new()?; + + // Create agent files in source + 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 source = src_dir.path().to_string_lossy().into_owned(); + + cmd_install( + Some(source.clone()), + FileScope::Project, + None, + None, + None, + false, // auto-save on + dst_dir.path().to_path_buf(), + )?; + + // agentfiles.json should be created in the project root + let manifest_path = dst_dir.path().join("agentfiles.json"); + assert!(manifest_path.exists()); + + let loaded = manifest::load_manifest(dst_dir.path())?; + assert_eq!(loaded.dependencies.len(), 1); + assert_eq!(loaded.dependencies[0].source(), source); + Ok(()) + } + + #[test] + fn install_source_no_save_skips_manifest() -> 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 source = src_dir.path().to_string_lossy().into_owned(); + + cmd_install( + Some(source), + FileScope::Project, + None, + None, + None, + true, // no-save + dst_dir.path().to_path_buf(), + )?; + + // agentfiles.json should NOT be created + assert!(!dst_dir.path().join("agentfiles.json").exists()); + Ok(()) + } } diff --git a/src/installer.rs b/src/installer.rs index 9ab3505..b8c0cef 100644 --- a/src/installer.rs +++ b/src/installer.rs @@ -3,7 +3,7 @@ use std::path::Path; use anyhow::{Context, Result}; -use crate::manifest::Manifest; +use crate::manifest::FileMapping; use crate::types::{AgentProvider, FileScope, FileStrategy}; /// Result of installing a single file to a single provider. @@ -16,24 +16,24 @@ pub struct InstallResult { pub kind: String, } -/// Install all files from a manifest to the specified providers. +/// Install all files from a list of file mappings to the specified providers. /// -/// For each file in the manifest, iterates over `providers` and installs the file -/// to every provider that supports the file's kind. The `manifest_dir` is the -/// directory containing the manifest (used to resolve relative source paths). +/// For each file, iterates over `providers` and installs the file to every +/// provider that supports the file's kind. The `source_root` is the directory +/// containing the source files (used to resolve relative source paths). /// /// Returns a list of `InstallResult` entries describing what was installed. pub fn install( - manifest: &Manifest, + files: &[FileMapping], providers: &[AgentProvider], scope: &FileScope, project_root: &Path, - manifest_dir: &Path, + source_root: &Path, ) -> Result> { let mut results = Vec::new(); - for file in &manifest.files { - let source_path = manifest_dir.join(&file.path); + for file in files { + let source_path = source_root.join(&file.path); if !source_path.exists() { anyhow::bail!( @@ -133,25 +133,15 @@ pub fn install( /// Resolve where the file should land inside the target directory. /// -/// For skills (paths containing SKILL.md), we place the entire skill directory -/// (e.g., `review/SKILL.md` -> `/review/SKILL.md`). -/// -/// For commands/agents (single .md files), we place the file directly -/// (e.g., `deploy.md` -> `/deploy.md`). +/// Uses the last component of the relative path as the target name: +/// - Skills (directories): `skills/review` -> `/review` +/// - Commands/agents (files): `commands/deploy.md` -> `/deploy.md` fn resolve_target_path(relative_path: &Path, target_dir: &Path) -> Result { let file_name = relative_path .file_name() .context("file path has no filename")?; - if file_name == "SKILL.md" { - let parent_name = relative_path - .parent() - .and_then(|p| p.file_name()) - .context("SKILL.md must be inside a named directory")?; - Ok(target_dir.join(parent_name).join("SKILL.md")) - } else { - Ok(target_dir.join(file_name)) - } + Ok(target_dir.join(file_name)) } /// Recursively copy a directory, skipping symlinks to avoid infinite loops. @@ -178,38 +168,31 @@ fn copy_dir_recursive(src: &Path, dst: &Path) -> Result<()> { #[cfg(test)] mod tests { use super::*; - use crate::manifest::FileMapping; - use crate::types::{FileKind, FileStrategy}; + use crate::types::FileKind; use std::path::PathBuf; use tempfile::TempDir; - fn make_manifest(files: Vec) -> Manifest { - Manifest { - name: "test".to_string(), - version: "0.1.0".to_string(), - files, - ..Default::default() - } - } - #[test] - fn install_skill_to_claude_code() -> Result<()> { + fn install_skill_directory_to_claude_code() -> Result<()> { let src_dir = TempDir::new()?; let dst_dir = TempDir::new()?; - // Create source skill + // Create source skill with supporting files let skill_dir = src_dir.path().join("skills").join("review"); fs::create_dir_all(&skill_dir)?; fs::write(skill_dir.join("SKILL.md"), "# Review skill")?; + fs::write(skill_dir.join("helper.py"), "# Helper script")?; + fs::create_dir_all(skill_dir.join("templates"))?; + fs::write(skill_dir.join("templates/base.html"), "")?; - let manifest = make_manifest(vec![FileMapping { - path: PathBuf::from("skills/review/SKILL.md"), + let files = vec![FileMapping { + path: PathBuf::from("skills/review"), kind: FileKind::Skill, strategy: FileStrategy::Copy, - }]); + }]; let results = install( - &manifest, + &files, &[AgentProvider::ClaudeCode], &FileScope::Project, dst_dir.path(), @@ -219,10 +202,15 @@ mod tests { assert_eq!(results.len(), 1); assert_eq!(results[0].provider, AgentProvider::ClaudeCode); - // Verify the file was copied - let target = dst_dir.path().join(".claude/skills/review/SKILL.md"); - assert!(target.exists()); - assert_eq!(fs::read_to_string(&target)?, "# Review skill"); + // Verify the entire directory was copied + let target = dst_dir.path().join(".claude/skills/review"); + assert!(target.join("SKILL.md").exists()); + assert!(target.join("helper.py").exists()); + assert!(target.join("templates/base.html").exists()); + assert_eq!( + fs::read_to_string(target.join("SKILL.md"))?, + "# Review skill" + ); Ok(()) } @@ -237,14 +225,14 @@ mod tests { fs::create_dir_all(&cmd_dir)?; fs::write(cmd_dir.join("deploy.md"), "# Deploy")?; - let manifest = make_manifest(vec![FileMapping { + let files = vec![FileMapping { path: PathBuf::from("commands/deploy.md"), kind: FileKind::Command, strategy: FileStrategy::Copy, - }]); + }]; let results = install( - &manifest, + &files, &[AgentProvider::Codex, AgentProvider::ClaudeCode], &FileScope::Project, dst_dir.path(), @@ -268,14 +256,14 @@ mod tests { fs::create_dir_all(&skill_dir)?; fs::write(skill_dir.join("SKILL.md"), "# Review")?; - let manifest = make_manifest(vec![FileMapping { - path: PathBuf::from("skills/review/SKILL.md"), + let files = vec![FileMapping { + path: PathBuf::from("skills/review"), kind: FileKind::Skill, strategy: FileStrategy::Copy, - }]); + }]; let results = install( - &manifest, + &files, AgentProvider::ALL, &FileScope::Project, dst_dir.path(), @@ -325,14 +313,14 @@ mod tests { fs::create_dir_all(&cmd_dir)?; fs::write(cmd_dir.join("deploy.md"), "# Deploy")?; - let manifest = make_manifest(vec![FileMapping { + let files = vec![FileMapping { path: PathBuf::from("commands/deploy.md"), kind: FileKind::Command, strategy: FileStrategy::Link, - }]); + }]; let results = install( - &manifest, + &files, &[AgentProvider::ClaudeCode], &FileScope::Project, dst_dir.path(), @@ -349,19 +337,54 @@ mod tests { Ok(()) } + #[cfg(unix)] + #[test] + fn install_skill_directory_with_symlink() -> 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")?; + fs::write(skill_dir.join("helper.sh"), "#!/bin/bash")?; + + let files = vec![FileMapping { + path: PathBuf::from("skills/review"), + kind: FileKind::Skill, + strategy: FileStrategy::Link, + }]; + + let results = install( + &files, + &[AgentProvider::ClaudeCode], + &FileScope::Project, + dst_dir.path(), + src_dir.path(), + )?; + + assert_eq!(results.len(), 1); + let target = dst_dir.path().join(".claude/skills/review"); + assert!(target.is_symlink()); + // Contents should be accessible through the symlink + assert!(target.join("SKILL.md").exists()); + assert!(target.join("helper.sh").exists()); + + Ok(()) + } + #[test] fn missing_source_file_errors() { let src_dir = TempDir::new().unwrap(); let dst_dir = TempDir::new().unwrap(); - let manifest = make_manifest(vec![FileMapping { - path: PathBuf::from("nonexistent/SKILL.md"), + let files = vec![FileMapping { + path: PathBuf::from("nonexistent"), kind: FileKind::Skill, strategy: FileStrategy::Copy, - }]); + }]; let result = install( - &manifest, + &files, &[AgentProvider::ClaudeCode], &FileScope::Project, dst_dir.path(), diff --git a/src/main.rs b/src/main.rs index f459e36..44c02d7 100644 --- a/src/main.rs +++ b/src/main.rs @@ -11,8 +11,10 @@ fn main() -> Result<()> { scope, providers, strategy, + pick, + no_save, root, - } => commands::cmd_install(source, scope, providers, strategy, root), + } => commands::cmd_install(source, scope, providers, strategy, pick, no_save, root), cli::Command::Init { path, name } => commands::cmd_init(path, name), cli::Command::Scan { source } => commands::cmd_scan(source), cli::Command::Matrix => commands::cmd_matrix(), diff --git a/src/manifest.rs b/src/manifest.rs index 9d44502..471ebc3 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -1,37 +1,139 @@ -use serde::{Deserialize, Serialize}; use std::path::{Path, PathBuf}; -use crate::types::{FileKind, FileStrategy}; use anyhow::{Context, Result, bail}; +use serde::{Deserialize, Serialize}; -/// A single file mapping in the manifest. -/// -/// Declares a source file path, its kind (Skill/Agent/Command), and an optional -/// installation strategy (Copy or Link). The CLI routes this file to the correct -/// provider directories based on the compatibility matrix. -#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +use crate::types::{FileKind, FileStrategy}; + +// --------------------------------------------------------------------------- +// Internal types (scanner output / installer input) +// --------------------------------------------------------------------------- + +/// A single discovered agent file. Internal type used by the scanner and +/// installer -- not serialized into the manifest. +#[derive(Clone, Debug, PartialEq)] pub struct FileMapping { - /// Path to the source file, relative to the manifest location. + /// Path to the source file or directory, relative to the source root. pub path: PathBuf, /// What kind of agent file this is. pub kind: FileKind, - /// How to place the file at the target. Defaults to Copy if omitted. - #[serde(default, skip_serializing_if = "is_default_strategy")] + /// How to place the file at the target. Defaults to Copy. pub strategy: FileStrategy, } -fn is_default_strategy(s: &FileStrategy) -> bool { - *s == FileStrategy::Copy +// --------------------------------------------------------------------------- +// 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"` +/// Detailed form: `{ "source": "...", "ref": "v1.0", "pick": [...], ... }` +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +#[serde(untagged)] +pub enum Dependency { + Simple(String), + Detailed(DependencySpec), +} + +impl Dependency { + /// The source URL or local path for this dependency. + pub fn source(&self) -> &str { + match self { + Dependency::Simple(s) => s, + Dependency::Detailed(d) => &d.source, + } + } + + /// Explicit git ref override, if any. Note that `@ref` in Simple strings + /// is handled by git::parse_remote, not here. + pub fn git_ref(&self) -> Option<&str> { + match self { + Dependency::Simple(_) => None, + Dependency::Detailed(d) => d.git_ref.as_deref(), + } + } + + /// Cherry-pick list, if any. + pub fn pick(&self) -> Option<&[String]> { + match self { + Dependency::Simple(_) => None, + Dependency::Detailed(d) => d.pick.as_deref(), + } + } + + /// Per-dependency strategy override, if any. + pub fn strategy(&self) -> Option { + match self { + Dependency::Simple(_) => None, + Dependency::Detailed(d) => d.strategy, + } + } + + /// Custom path-to-kind mappings. When set, replaces the default + /// `skills/`, `commands/`, `agents/` scanning convention. + pub fn paths(&self) -> Option<&[PathMapping]> { + match self { + Dependency::Simple(_) => None, + Dependency::Detailed(d) => d.paths.as_deref(), + } + } +} + +/// Detailed dependency specification with optional configuration. +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +pub struct DependencySpec { + /// Source URL or local path. + pub source: String, + + /// Git ref (branch, tag, or commit) to check out. + #[serde(rename = "ref", skip_serializing_if = "Option::is_none")] + pub git_ref: Option, + + /// Cherry-pick specific items by name. Supports kind prefix: + /// `"skills/review"`, `"commands/deploy"`, or plain `"review"`. + #[serde(skip_serializing_if = "Option::is_none")] + pub pick: Option>, + + /// Override the installation strategy for all files from this dependency. + #[serde(skip_serializing_if = "Option::is_none")] + pub strategy: Option, + + /// Custom directory/file-to-kind mappings. Replaces the default convention + /// when specified. + #[serde(skip_serializing_if = "Option::is_none")] + pub paths: Option>, +} + +/// Maps a custom path in a source repository to a file kind. +/// +/// If the path resolves to a directory, it is scanned using the standard +/// convention for that kind (skills = subdirs with SKILL.md, commands/agents +/// = .md files). If it resolves to a file, that file is installed directly. +#[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] +pub struct PathMapping { + /// Relative path within the source repository. + pub path: String, + + /// What kind of agent file this path contains. + pub kind: FileKind, } -/// The agentfiles.json manifest. +// --------------------------------------------------------------------------- +// Manifest +// --------------------------------------------------------------------------- + +/// The agentfiles.json project manifest. /// -/// Declares a package of agent files with metadata and a list of file mappings. +/// Lists dependencies (remote or local sources) that provide agent files. +/// Similar to package.json -- lives in the consumer's project. #[derive(Serialize, Deserialize, Clone, Debug, PartialEq)] pub struct Manifest { pub name: String, + + #[serde(default = "default_version")] pub version: String, #[serde(default, skip_serializing_if = "Option::is_none")] @@ -43,18 +145,23 @@ pub struct Manifest { #[serde(default, skip_serializing_if = "Option::is_none")] pub repository: Option, - pub files: Vec, + #[serde(default, skip_serializing_if = "Vec::is_empty")] + pub dependencies: Vec, +} + +fn default_version() -> String { + "0.0.1".to_string() } impl Default for Manifest { fn default() -> Self { Manifest { name: "unnamed".to_string(), - version: "0.0.1".to_string(), + version: default_version(), description: None, author: None, repository: None, - files: vec![], + dependencies: vec![], } } } @@ -85,14 +192,25 @@ impl Manifest { self } - pub fn with_files(mut self, files: Vec) -> Self { - self.files = files; + pub fn with_dependencies(mut self, dependencies: Vec) -> Self { + self.dependencies = dependencies; self } - pub fn add_file(mut self, file: FileMapping) -> Self { - self.files.push(file); - self + /// Add a dependency if one with the same source doesn't already exist. + /// Returns true if the dependency was added, false if it was already present. + pub fn add_dependency(&mut self, dep: Dependency) -> bool { + let source = dep.source().to_string(); + if self.has_dependency(&source) { + return false; + } + self.dependencies.push(dep); + true + } + + /// Check whether a dependency with the given source already exists. + pub fn has_dependency(&self, source: &str) -> bool { + self.dependencies.iter().any(|d| d.source() == source) } } @@ -123,160 +241,231 @@ pub fn save_manifest(manifest: &Manifest, path: &Path) -> Result { #[cfg(test)] mod tests { - mod load_manifest { + mod dependency_serialization { use super::super::*; - use tempfile::TempDir; #[test] - fn load_from_directory() -> Result<()> { - let dir = TempDir::new().unwrap(); - let test_file = dir.path().join("agentfiles.json"); - - std::fs::write( - &test_file, - r#"{ - "name": "Test Agent", - "author": "Test Author", - "version": "0.1.0", - "files": [ - { - "path": "skills/review/SKILL.md", - "kind": "Skill" - }, - { - "path": "commands/deploy.md", - "kind": "Command", - "strategy": "Link" - } - ] - }"#, - )?; - - let manifest = load_manifest(&test_file)?; - - assert_eq!(manifest.author, Some("Test Author".to_string())); - assert_eq!(manifest.name, "Test Agent"); - assert_eq!(manifest.version, "0.1.0"); - assert_eq!(manifest.files.len(), 2); - - assert_eq!( - manifest.files[0].path, - PathBuf::from("skills/review/SKILL.md") - ); - assert_eq!(manifest.files[0].kind, FileKind::Skill); - assert_eq!(manifest.files[0].strategy, FileStrategy::Copy); // default - - assert_eq!(manifest.files[1].path, PathBuf::from("commands/deploy.md")); - assert_eq!(manifest.files[1].kind, FileKind::Command); - assert_eq!(manifest.files[1].strategy, FileStrategy::Link); - + fn simple_dependency_roundtrip() -> Result<()> { + let dep = Dependency::Simple("github.com/org/repo".to_string()); + let json = serde_json::to_string(&dep)?; + assert_eq!(json, r#""github.com/org/repo""#); + + let parsed: Dependency = serde_json::from_str(&json)?; + assert_eq!(parsed, dep); + assert_eq!(parsed.source(), "github.com/org/repo"); + assert_eq!(parsed.git_ref(), None); + assert_eq!(parsed.pick(), None); + assert_eq!(parsed.strategy(), None); + assert_eq!(parsed.paths(), None); Ok(()) } #[test] - fn load_from_directory_path() -> Result<()> { - let dir = TempDir::new().unwrap(); - let test_file = dir.path().join("agentfiles.json"); - - std::fs::write( - &test_file, - r#"{ - "name": "Test", - "version": "0.1.0", - "files": [] - }"#, - )?; + fn detailed_dependency_roundtrip() -> Result<()> { + let dep = Dependency::Detailed(DependencySpec { + source: "github.com/org/repo".to_string(), + git_ref: Some("v2.0".to_string()), + pick: Some(vec![ + "skills/review".to_string(), + "commands/deploy".to_string(), + ]), + strategy: Some(FileStrategy::Link), + paths: Some(vec![PathMapping { + path: "prompts".to_string(), + kind: FileKind::Skill, + }]), + }); + + let json = serde_json::to_string_pretty(&dep)?; + let parsed: Dependency = serde_json::from_str(&json)?; + assert_eq!(parsed, dep); + + assert_eq!(parsed.source(), "github.com/org/repo"); + assert_eq!(parsed.git_ref(), Some("v2.0")); + assert_eq!(parsed.pick().unwrap().len(), 2); + assert_eq!(parsed.strategy(), Some(FileStrategy::Link)); + assert_eq!(parsed.paths().unwrap().len(), 1); + Ok(()) + } - // Pass the directory, not the file - let manifest = load_manifest(dir.path())?; - assert_eq!(manifest.name, "Test"); + #[test] + fn detailed_dependency_minimal() -> Result<()> { + let json = r#"{"source": "github.com/org/repo"}"#; + let parsed: Dependency = serde_json::from_str(json)?; + assert_eq!(parsed.source(), "github.com/org/repo"); + assert_eq!(parsed.git_ref(), None); + assert_eq!(parsed.pick(), None); + assert_eq!(parsed.strategy(), None); + assert_eq!(parsed.paths(), None); + Ok(()) + } + #[test] + fn ref_field_uses_serde_rename() -> Result<()> { + let json = r#"{"source": "github.com/org/repo", "ref": "main"}"#; + let parsed: Dependency = serde_json::from_str(json)?; + assert_eq!(parsed.git_ref(), Some("main")); + + // Serializes back as "ref", not "git_ref" + let serialized = serde_json::to_string(&parsed)?; + assert!(serialized.contains(r#""ref":"main""#)); + assert!(!serialized.contains("git_ref")); Ok(()) } } - mod save_manifest { + mod manifest_serialization { use super::super::*; use tempfile::TempDir; #[test] fn save_and_roundtrip() -> Result<()> { - let dir = TempDir::new().unwrap(); + let dir = TempDir::new()?; let manifest = Manifest { - name: "Test Agent".to_string(), + name: "my-project".to_string(), version: "0.1.0".to_string(), author: Some("Test Author".to_string()), repository: Some("https://github.com/org/repo".to_string()), - files: vec![ - FileMapping { - path: PathBuf::from("skills/review/SKILL.md"), - kind: FileKind::Skill, - strategy: FileStrategy::Copy, - }, - FileMapping { - path: PathBuf::from("commands/deploy.md"), - kind: FileKind::Command, - strategy: FileStrategy::Link, - }, + dependencies: vec![ + Dependency::Simple("github.com/anthropics/skills".to_string()), + Dependency::Detailed(DependencySpec { + source: "github.com/mitsuhiko/agent-stuff".to_string(), + git_ref: Some("main".to_string()), + pick: Some(vec!["skills/commit".to_string()]), + strategy: None, + paths: None, + }), ], ..Default::default() }; save_manifest(&manifest, dir.path())?; - let loaded = load_manifest(dir.path())?; - assert_eq!(loaded.author, Some("Test Author".to_string())); - assert_eq!(loaded.name, "Test Agent"); + assert_eq!(loaded.name, "my-project"); assert_eq!(loaded.version, "0.1.0"); + assert_eq!(loaded.author, Some("Test Author".to_string())); + assert_eq!(loaded.dependencies.len(), 2); + assert_eq!( + loaded.dependencies[0].source(), + "github.com/anthropics/skills" + ); assert_eq!( - loaded.repository, - Some("https://github.com/org/repo".to_string()) + loaded.dependencies[1].source(), + "github.com/mitsuhiko/agent-stuff" ); - assert_eq!(loaded.files.len(), 2); - assert_eq!(loaded.files[0].kind, FileKind::Skill); - assert_eq!(loaded.files[0].strategy, FileStrategy::Copy); - assert_eq!(loaded.files[1].kind, FileKind::Command); - assert_eq!(loaded.files[1].strategy, FileStrategy::Link); + assert_eq!(loaded.dependencies[1].git_ref(), Some("main")); + Ok(()) + } + + #[test] + fn empty_dependencies_not_serialized() -> Result<()> { + let dir = TempDir::new()?; + let manifest = Manifest::default(); + save_manifest(&manifest, dir.path())?; + + let content = std::fs::read_to_string(dir.path().join("agentfiles.json"))?; + assert!(!content.contains("dependencies")); + Ok(()) + } + #[test] + fn load_from_directory_path() -> Result<()> { + let dir = TempDir::new()?; + std::fs::write( + dir.path().join("agentfiles.json"), + r#"{"name": "test", "version": "0.1.0"}"#, + )?; + let manifest = load_manifest(dir.path())?; + assert_eq!(manifest.name, "test"); + assert!(manifest.dependencies.is_empty()); Ok(()) } #[test] fn save_to_file_error() -> Result<()> { - let dir = TempDir::new().unwrap(); - let test_file = dir.path().join("agentfiles.json"); + let dir = TempDir::new()?; + let file = dir.path().join("agentfiles.json"); + std::fs::write(&file, "{}")?; + let result = save_manifest(&Manifest::default(), &file); + assert!(result.is_err()); + Ok(()) + } + } - // Create the file first so is_file() returns true - std::fs::write(&test_file, "{}")?; + mod manifest_helpers { + use super::super::*; - let manifest = Manifest::default(); - let result = save_manifest(&manifest, &test_file); - assert!(result.is_err()); + #[test] + fn add_dependency_deduplicates() { + let mut manifest = Manifest::default(); + let dep = Dependency::Simple("github.com/org/repo".to_string()); - Ok(()) + assert!(manifest.add_dependency(dep.clone())); + assert!(!manifest.add_dependency(dep)); + assert_eq!(manifest.dependencies.len(), 1); } #[test] - fn default_strategy_not_serialized() -> Result<()> { - let dir = TempDir::new().unwrap(); - let manifest = Manifest { - name: "Test".to_string(), - version: "0.1.0".to_string(), - files: vec![FileMapping { - path: PathBuf::from("skills/test/SKILL.md"), - kind: FileKind::Skill, - strategy: FileStrategy::Copy, // default, should be omitted from JSON - }], - ..Default::default() - }; + fn has_dependency_checks_source() { + let mut manifest = Manifest::default(); + manifest + .dependencies + .push(Dependency::Simple("github.com/org/repo".to_string())); + + assert!(manifest.has_dependency("github.com/org/repo")); + assert!(!manifest.has_dependency("github.com/other/repo")); + } - save_manifest(&manifest, dir.path())?; + #[test] + fn builder_methods() { + let manifest = Manifest::default() + .with_name("test".to_string()) + .with_version("1.0.0".to_string()) + .with_description("A test project".to_string()) + .with_author("Author".to_string()) + .with_repository("https://github.com/test".to_string()) + .with_dependencies(vec![Dependency::Simple("dep1".to_string())]); + + assert_eq!(manifest.name, "test"); + assert_eq!(manifest.version, "1.0.0"); + assert_eq!(manifest.description, Some("A test project".to_string())); + assert_eq!(manifest.author, Some("Author".to_string())); + assert_eq!(manifest.dependencies.len(), 1); + } + } - let content = std::fs::read_to_string(dir.path().join("agentfiles.json"))?; - // The "strategy" key should NOT appear for Copy (the default) - assert!(!content.contains("strategy")); + mod path_mapping { + use super::super::*; + #[test] + fn path_mapping_roundtrip() -> Result<()> { + let mapping = PathMapping { + path: "prompts".to_string(), + kind: FileKind::Skill, + }; + let json = serde_json::to_string(&mapping)?; + let parsed: PathMapping = serde_json::from_str(&json)?; + assert_eq!(parsed, mapping); + Ok(()) + } + + #[test] + fn dependency_with_custom_paths() -> Result<()> { + let json = r#"{ + "source": "github.com/some/repo", + "paths": [ + {"path": "prompts", "kind": "Skill"}, + {"path": "macros", "kind": "Command"} + ] + }"#; + let dep: Dependency = serde_json::from_str(json)?; + let paths = dep.paths().unwrap(); + assert_eq!(paths.len(), 2); + assert_eq!(paths[0].path, "prompts"); + assert_eq!(paths[0].kind, FileKind::Skill); + assert_eq!(paths[1].path, "macros"); + assert_eq!(paths[1].kind, FileKind::Command); Ok(()) } } diff --git a/src/scanner.rs b/src/scanner.rs index 9161b0c..0d24eda 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -3,7 +3,7 @@ use std::path::Path; use anyhow::{Context, Result}; -use crate::manifest::FileMapping; +use crate::manifest::{FileMapping, PathMapping}; use crate::types::{AgentProvider, FileKind, FileStrategy}; /// Subdirectory names and their corresponding file kind. @@ -13,28 +13,89 @@ const KIND_DIRS: &[(&str, FileKind)] = &[ ("agents", FileKind::Agent), ]; +// --------------------------------------------------------------------------- +// Public API +// --------------------------------------------------------------------------- + /// Scan a directory for agent files and return discovered file mappings. /// -/// Looks for files under known provider directories (e.g., `.claude/skills/`, -/// `.opencode/commands/`, `.agents/skills/`) as well as bare `skills/`, -/// `commands/`, and `agents/` directories at the root. +/// When `custom_paths` is `None`, uses the default convention: scans +/// provider-prefixed directories (`.claude/skills/`, `.opencode/commands/`, etc.) +/// and bare `skills/`, `commands/`, `agents/` directories at the root. +/// +/// When `custom_paths` is `Some`, only scans the specified paths using their +/// declared kind. The default convention is entirely replaced. /// -/// Skills are expected to be directories containing a `SKILL.md` file. -/// Commands and agents are expected to be `.md` files directly. +/// Skills are directories containing a `SKILL.md` file (the whole directory +/// is recorded, not just the SKILL.md). Commands and agents are `.md` files. /// /// The returned `FileMapping` paths are relative to `root`. -pub fn scan_agent_files(root: &Path) -> Result> { +pub fn scan_agent_files( + root: &Path, + custom_paths: Option<&[PathMapping]>, +) -> Result> { let root = root .canonicalize() .with_context(|| format!("cannot resolve path: {}", root.display()))?; let mut mappings = Vec::new(); + if let Some(paths) = custom_paths { + scan_custom_paths(&root, paths, &mut mappings)?; + } else { + scan_default_convention(&root, &mut mappings)?; + } + + deduplicate(&mut mappings); + + Ok(mappings) +} + +/// Filter a list of file mappings by a pick list. +/// +/// Pick items can be kind-prefixed (`"skills/review"`, `"commands/deploy"`) +/// or plain names (`"review"`). A plain name matches any kind. +pub fn filter_by_pick(mappings: Vec, pick: &[String]) -> Vec { + mappings + .into_iter() + .filter(|m| { + let name = m.path.file_stem().unwrap_or_default().to_string_lossy(); + + pick.iter().any(|p| { + if let Some((kind_prefix, pick_name)) = p.split_once('/') { + let kind_matches = match kind_prefix { + "skills" => m.kind == FileKind::Skill, + "commands" => m.kind == FileKind::Command, + "agents" => m.kind == FileKind::Agent, + _ => false, + }; + kind_matches && name == pick_name + } else { + name == p.as_str() + } + }) + }) + .collect() +} + +/// Infer the folder name from a path to use as a manifest name. +pub fn infer_name(path: &Path) -> String { + path.file_name() + .map(|n| n.to_string_lossy().into_owned()) + .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() { - scan_kind_dirs(&root, &prefix_dir, &mut mappings)?; + scan_kind_dirs(root, &prefix_dir, mappings)?; } } @@ -42,14 +103,11 @@ pub fn scan_agent_files(root: &Path) -> Result> { for &(kind_name, ref kind) in KIND_DIRS { let kind_dir = root.join(kind_name); if kind_dir.is_dir() { - scan_kind_dir(&root, &kind_dir, kind, &mut mappings)?; + scan_kind_dir(root, &kind_dir, kind, mappings)?; } } - // Deduplicate by target filename (keep the first occurrence) - deduplicate(&mut mappings); - - Ok(mappings) + Ok(()) } /// Scan subdirectories inside a provider prefix directory for skills/commands/agents. @@ -65,8 +123,9 @@ fn scan_kind_dirs(root: &Path, prefix_dir: &Path, mappings: &mut Vec/SKILL.md` subdirectories -/// - Commands/Agents: looks for `.md` files +/// - Skills: looks for `/SKILL.md` subdirectories. Records the directory +/// path (not the SKILL.md), so the full skill directory is installed. +/// - Commands/Agents: looks for `.md` files. fn scan_kind_dir( root: &Path, kind_dir: &Path, @@ -82,13 +141,14 @@ fn scan_kind_dir( match kind { FileKind::Skill => { - // Skills are directories containing SKILL.md + // 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() { - let rel_path = skill_md + let rel_path = entry_path .strip_prefix(root) - .unwrap_or(&skill_md) + .unwrap_or(&entry_path) .to_path_buf(); mappings.push(FileMapping { path: rel_path, @@ -121,33 +181,64 @@ fn scan_kind_dir( Ok(()) } -/// Deduplicate file mappings by their filename stem + kind. +// --------------------------------------------------------------------------- +// 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. +fn scan_custom_paths( + root: &Path, + paths: &[PathMapping], + mappings: &mut Vec, +) -> Result<()> { + for mapping in 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. + continue; + } + + if full_path.is_dir() { + // Scan directory using the standard convention for this kind + scan_kind_dir(root, &full_path, &mapping.kind, mappings)?; + } else if full_path.is_file() { + // Single file -- add directly + let rel_path = full_path + .strip_prefix(root) + .unwrap_or(&full_path) + .to_path_buf(); + mappings.push(FileMapping { + path: rel_path, + kind: mapping.kind, + strategy: FileStrategy::Copy, + }); + } + } + + 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) { let mut seen = std::collections::HashSet::new(); mappings.retain(|m| { - let key = format!( - "{}:{}", - m.kind, - m.path - .file_stem() - .or_else(|| m.path.parent().and_then(|p| p.file_name())) - .unwrap_or_default() - .to_string_lossy() - ); + let stem = m.path.file_stem().unwrap_or_default().to_string_lossy(); + let key = format!("{}:{}", m.kind, stem); seen.insert(key) }); } -/// Infer the folder name from a path to use as a manifest name. -pub fn infer_name(path: &Path) -> String { - path.file_name() - .map(|n| n.to_string_lossy().into_owned()) - .unwrap_or_else(|| "unnamed".to_string()) -} - #[cfg(test)] mod tests { use super::*; @@ -183,15 +274,21 @@ mod tests { .unwrap(); } + // ----------------------------------------------------------------------- + // Default convention tests + // ----------------------------------------------------------------------- + #[test] fn scans_claude_skills() -> Result<()> { let dir = TempDir::new()?; setup_skill(dir.path(), ".claude", "review"); - let mappings = scan_agent_files(dir.path())?; + let mappings = scan_agent_files(dir.path(), None)?; assert_eq!(mappings.len(), 1); assert_eq!(mappings[0].kind, FileKind::Skill); - assert!(mappings[0].path.ends_with("SKILL.md")); + // Should store the directory path, not SKILL.md + assert!(mappings[0].path.ends_with("review")); + assert!(!mappings[0].path.to_string_lossy().contains("SKILL.md")); Ok(()) } @@ -202,7 +299,7 @@ mod tests { setup_command(dir.path(), ".opencode", "deploy"); setup_agent(dir.path(), ".cursor", "security"); - let mappings = scan_agent_files(dir.path())?; + let mappings = scan_agent_files(dir.path(), None)?; assert_eq!(mappings.len(), 3); let kinds: Vec<&FileKind> = mappings.iter().map(|m| &m.kind).collect(); @@ -218,8 +315,7 @@ mod tests { setup_skill(dir.path(), ".claude", "review"); setup_skill(dir.path(), ".opencode", "review"); - let mappings = scan_agent_files(dir.path())?; - // Should deduplicate to 1 + let mappings = scan_agent_files(dir.path(), None)?; let skills: Vec<_> = mappings .iter() .filter(|m| m.kind == FileKind::Skill) @@ -234,20 +330,211 @@ mod tests { // Create bare skills/ directory (not under a provider prefix) setup_skill(dir.path(), "", "my-skill"); - let mappings = scan_agent_files(dir.path())?; + let mappings = scan_agent_files(dir.path(), None)?; assert_eq!(mappings.len(), 1); assert_eq!(mappings[0].kind, FileKind::Skill); Ok(()) } + #[test] + fn skill_stores_directory_path() -> Result<()> { + let dir = TempDir::new()?; + // Create a skill with extra files alongside SKILL.md + let skill_dir = dir.path().join("skills").join("my-skill"); + fs::create_dir_all(&skill_dir)?; + fs::write(skill_dir.join("SKILL.md"), "# My Skill")?; + fs::write(skill_dir.join("helper.py"), "# helper script")?; + fs::create_dir_all(skill_dir.join("templates"))?; + fs::write(skill_dir.join("templates/base.html"), "")?; + + let mappings = scan_agent_files(dir.path(), None)?; + assert_eq!(mappings.len(), 1); + // The path should be the directory, not SKILL.md + assert_eq!( + mappings[0].path, + std::path::PathBuf::from("skills/my-skill") + ); + Ok(()) + } + #[test] fn empty_dir_returns_empty() -> Result<()> { let dir = TempDir::new()?; - let mappings = scan_agent_files(dir.path())?; + let mappings = scan_agent_files(dir.path(), None)?; + assert!(mappings.is_empty()); + Ok(()) + } + + // ----------------------------------------------------------------------- + // Custom path tests + // ----------------------------------------------------------------------- + + #[test] + fn custom_paths_scan_directory() -> Result<()> { + let dir = TempDir::new()?; + // Create skills in a non-standard directory + let prompts_dir = dir.path().join("prompts").join("my-skill"); + fs::create_dir_all(&prompts_dir)?; + fs::write(prompts_dir.join("SKILL.md"), "# My Skill")?; + + let custom = vec![PathMapping { + path: "prompts".to_string(), + kind: FileKind::Skill, + }]; + + let mappings = scan_agent_files(dir.path(), Some(&custom))?; + assert_eq!(mappings.len(), 1); + assert_eq!(mappings[0].kind, FileKind::Skill); + assert!(mappings[0].path.to_string_lossy().contains("my-skill")); + Ok(()) + } + + #[test] + fn custom_paths_scan_single_file() -> Result<()> { + let dir = TempDir::new()?; + fs::write(dir.path().join("GUIDELINES.md"), "# Coding Guidelines")?; + + let custom = vec![PathMapping { + path: "GUIDELINES.md".to_string(), + kind: FileKind::Skill, + }]; + + let mappings = scan_agent_files(dir.path(), Some(&custom))?; + assert_eq!(mappings.len(), 1); + assert_eq!(mappings[0].kind, FileKind::Skill); + assert_eq!(mappings[0].path, std::path::PathBuf::from("GUIDELINES.md")); + Ok(()) + } + + #[test] + fn custom_paths_replaces_defaults() -> Result<()> { + let dir = TempDir::new()?; + // Create standard skills/ dir AND a custom prompts/ dir + setup_skill(dir.path(), "", "standard-skill"); + let prompts_dir = dir.path().join("prompts").join("custom-skill"); + fs::create_dir_all(&prompts_dir)?; + fs::write(prompts_dir.join("SKILL.md"), "# Custom")?; + + // With custom paths, only prompts/ should be scanned + let custom = vec![PathMapping { + path: "prompts".to_string(), + kind: FileKind::Skill, + }]; + + let mappings = scan_agent_files(dir.path(), Some(&custom))?; + assert_eq!(mappings.len(), 1); + assert!(mappings[0].path.to_string_lossy().contains("custom-skill")); + Ok(()) + } + + #[test] + fn custom_paths_skips_missing() -> Result<()> { + let dir = TempDir::new()?; + let custom = vec![PathMapping { + path: "nonexistent".to_string(), + kind: FileKind::Skill, + }]; + + let mappings = scan_agent_files(dir.path(), Some(&custom))?; assert!(mappings.is_empty()); Ok(()) } + #[test] + fn custom_paths_nested_directory() -> Result<()> { + let dir = TempDir::new()?; + let nested = dir + .path() + .join("src") + .join("ai") + .join("prompts") + .join("review"); + fs::create_dir_all(&nested)?; + fs::write(nested.join("SKILL.md"), "# Review")?; + + let custom = vec![PathMapping { + path: "src/ai/prompts".to_string(), + kind: FileKind::Skill, + }]; + + let mappings = scan_agent_files(dir.path(), Some(&custom))?; + assert_eq!(mappings.len(), 1); + assert_eq!(mappings[0].kind, FileKind::Skill); + Ok(()) + } + + // ----------------------------------------------------------------------- + // Pick filter tests + // ----------------------------------------------------------------------- + + #[test] + fn filter_by_plain_name() { + let mappings = vec![ + FileMapping { + path: "skills/review".into(), + kind: FileKind::Skill, + strategy: FileStrategy::Copy, + }, + FileMapping { + path: "skills/deploy".into(), + kind: FileKind::Skill, + strategy: FileStrategy::Copy, + }, + FileMapping { + path: "commands/deploy.md".into(), + kind: FileKind::Command, + strategy: FileStrategy::Copy, + }, + ]; + + let filtered = filter_by_pick(mappings, &["review".to_string()]); + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].kind, FileKind::Skill); + } + + #[test] + fn filter_by_kind_prefix() { + let mappings = vec![ + FileMapping { + path: "skills/deploy".into(), + kind: FileKind::Skill, + strategy: FileStrategy::Copy, + }, + FileMapping { + path: "commands/deploy.md".into(), + kind: FileKind::Command, + strategy: FileStrategy::Copy, + }, + ]; + + let filtered = filter_by_pick(mappings, &["commands/deploy".to_string()]); + assert_eq!(filtered.len(), 1); + assert_eq!(filtered[0].kind, FileKind::Command); + } + + #[test] + fn filter_plain_name_matches_all_kinds() { + let mappings = vec![ + FileMapping { + path: "skills/deploy".into(), + kind: FileKind::Skill, + strategy: FileStrategy::Copy, + }, + FileMapping { + path: "commands/deploy.md".into(), + kind: FileKind::Command, + strategy: FileStrategy::Copy, + }, + ]; + + let filtered = filter_by_pick(mappings, &["deploy".to_string()]); + 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"); From f301c0327faaede1b486206f164eea9fef4ac80f Mon Sep 17 00:00:00 2001 From: leodiegues Date: Mon, 16 Feb 2026 12:59:39 -0300 Subject: [PATCH 2/2] refactor: add dry-run and remove/list CLI commands Introduce InstallOptions and thread a dry_run flag through cmd_install to installer so installs can be previewed without making changes. Add Remove and List subcommands and wire cmd_remove/cmd_list in main. Refactor internals: make FileMapping and InstallResult crate-private, make scan/filter helpers crate-private, and expose provider project bases as a const. Add git.normalize_source, cache the git availability check, and switch URL hashing to deterministic FNV-1a. Add serde rename for FileKind --- src/cli.rs | 33 ++++ src/commands.rs | 388 ++++++++++++++++++++++++++++++++++++----------- src/git.rs | 68 ++++++--- src/installer.rs | 137 +++++++++-------- src/main.rs | 20 ++- src/manifest.rs | 60 +++++--- src/provider.rs | 13 +- src/scanner.rs | 8 +- src/types.rs | 1 + 9 files changed, 520 insertions(+), 208 deletions(-) diff --git a/src/cli.rs b/src/cli.rs index f673eef..0ad1d61 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -45,6 +45,10 @@ pub enum Command { #[arg(long)] no_save: bool, + /// Preview what would be installed without making changes + #[arg(long)] + dry_run: bool, + /// Project root directory (for project scope installations) #[arg(long, default_value = ".")] root: PathBuf, @@ -68,6 +72,35 @@ pub enum Command { source: String, }, + /// Remove a dependency from agentfiles.json + Remove { + /// Source to remove (matches by normalized URL) + source: String, + + /// Also delete installed files from provider directories + #[arg(long)] + clean: bool, + + /// Installation scope used when installing (for --clean) + #[arg(short, long, default_value = "project")] + scope: FileScope, + + /// Target providers to clean (for --clean). Defaults to all. + #[arg(short, long, value_delimiter = ',')] + providers: Option>, + + /// Project root directory + #[arg(long, default_value = ".")] + root: PathBuf, + }, + + /// List dependencies from agentfiles.json + List { + /// Project root directory + #[arg(default_value = ".")] + root: PathBuf, + }, + /// Show the provider compatibility matrix Matrix, } diff --git a/src/commands.rs b/src/commands.rs index 17cd160..eef6c5a 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -10,37 +10,51 @@ use crate::{git, installer, manifest, scanner}; // Install command // --------------------------------------------------------------------------- +/// Options for the install command, collected from CLI arguments. +pub struct InstallOptions { + pub source: Option, + pub scope: FileScope, + pub providers: Option>, + pub strategy: Option, + pub pick: Option>, + pub no_save: bool, + pub dry_run: bool, + pub root: PathBuf, +} + /// Install agent files. Two flows: /// /// - **No source**: reads `agentfiles.json` from the project root and installs /// all dependencies listed there. /// - **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( - source: Option, - scope: FileScope, - providers: Option>, - strategy_override: Option, - pick: Option>, - no_save: bool, - root: PathBuf, -) -> Result<()> { - let providers = providers.unwrap_or_else(|| AgentProvider::ALL.to_vec()); +pub fn cmd_install(opts: InstallOptions) -> Result<()> { + let providers = opts + .providers + .unwrap_or_else(|| AgentProvider::ALL.to_vec()); - let project_root = root + let project_root = opts + .root .canonicalize() .context("could not resolve project root")?; - match source { - None => install_from_manifest(&project_root, &providers, &scope, strategy_override), + match opts.source { + None => install_from_manifest( + &project_root, + &providers, + &opts.scope, + opts.strategy, + opts.dry_run, + ), Some(src) => install_from_source( &src, &project_root, &providers, - &scope, - strategy_override, - pick.as_deref(), - no_save, + &opts.scope, + opts.strategy, + opts.pick.as_deref(), + opts.no_save, + opts.dry_run, ), } } @@ -51,6 +65,7 @@ fn install_from_manifest( providers: &[AgentProvider], scope: &FileScope, strategy_override: Option, + dry_run: bool, ) -> Result<()> { let manifest_path = project_root.join("agentfiles.json"); if !manifest_path.is_file() { @@ -76,16 +91,23 @@ fn install_from_manifest( let mut total_results = Vec::new(); for dep in &loaded.dependencies { - let dep_results = - install_dependency(dep, project_root, providers, scope, strategy_override)?; + let dep_results = install_dependency( + dep, + project_root, + providers, + scope, + strategy_override, + dry_run, + )?; total_results.extend(dep_results); } - print_results(&total_results); + print_results(&total_results, dry_run); Ok(()) } /// Install from a specific source, optionally saving it to agentfiles.json. +#[allow(clippy::too_many_arguments)] fn install_from_source( source: &str, project_root: &std::path::Path, @@ -94,8 +116,9 @@ fn install_from_source( strategy_override: Option, pick: Option<&[String]>, no_save: bool, + dry_run: bool, ) -> Result<()> { - let (source_dir, mut files) = resolve_source(source)?; + let (source_dir, mut files) = resolve_source(source, None)?; // Apply pick filter if let Some(pick_list) = pick { @@ -105,22 +128,20 @@ fn install_from_source( } } - // Apply strategy override + // Apply strategy override (CLI flag takes highest precedence) if let Some(strategy) = strategy_override { for file in &mut files { - if file.strategy == FileStrategy::Copy { - file.strategy = strategy; - } + file.strategy = strategy; } } - let results = installer::install(&files, providers, scope, project_root, &source_dir)?; + let results = installer::install(&files, providers, scope, project_root, &source_dir, dry_run)?; - if !no_save { + if !no_save && !dry_run { save_dependency(source, pick, project_root)?; } - print_results(&results); + print_results(&results, dry_run); Ok(()) } @@ -131,24 +152,19 @@ fn install_dependency( providers: &[AgentProvider], scope: &FileScope, strategy_override: Option, + dry_run: bool, ) -> Result> { let source = dep.source(); println!(" -> {source}"); - let (source_dir, mut files) = resolve_source(source)?; - - // Apply custom path mappings if specified - if let Some(custom_paths) = dep.paths() { - // Re-scan with custom paths (the initial resolve_source used defaults) - files = scanner::scan_agent_files(&source_dir, Some(custom_paths))?; - } + let (source_dir, mut files) = resolve_source(source, dep.paths())?; // Apply pick filter if let Some(pick_list) = dep.pick() { files = scanner::filter_by_pick(files, pick_list); } - // Apply per-dependency strategy, then global override + // Apply strategy: dep-level overrides default, CLI overrides everything let dep_strategy = dep.strategy(); for file in &mut files { if let Some(strategy) = dep_strategy @@ -156,9 +172,7 @@ fn install_dependency( { file.strategy = strategy; } - if let Some(strategy) = strategy_override - && file.strategy == FileStrategy::Copy - { + if let Some(strategy) = strategy_override { file.strategy = strategy; } } @@ -168,7 +182,7 @@ fn install_dependency( return Ok(vec![]); } - installer::install(&files, providers, scope, project_root, &source_dir) + installer::install(&files, providers, scope, project_root, &source_dir, dry_run) } // --------------------------------------------------------------------------- @@ -176,16 +190,25 @@ fn install_dependency( // --------------------------------------------------------------------------- /// Resolve a source (remote or local) to a local directory and scanned files. -fn resolve_source(source: &str) -> Result<(PathBuf, Vec)> { +/// +/// When `custom_paths` is provided, the scanner uses those instead of the +/// default directory convention. +fn resolve_source( + source: &str, + custom_paths: Option<&[manifest::PathMapping]>, +) -> Result<(PathBuf, Vec)> { if git::is_git_url(source) { - resolve_remote_source(source) + resolve_remote_source(source, custom_paths) } else { - resolve_local_source(source) + resolve_local_source(source, custom_paths) } } /// Clone/fetch a remote git repo and scan for agent files. -fn resolve_remote_source(source: &str) -> Result<(PathBuf, Vec)> { +fn resolve_remote_source( + source: &str, + custom_paths: Option<&[manifest::PathMapping]>, +) -> Result<(PathBuf, Vec)> { let remote = git::parse_remote(source); let ref_display = remote @@ -200,7 +223,7 @@ fn resolve_remote_source(source: &str) -> Result<(PathBuf, Vec)> { println!("Cached at: {}\n", local_path.display()); - let files = scanner::scan_agent_files(&local_path, None)?; + let files = scanner::scan_agent_files(&local_path, custom_paths)?; if files.is_empty() { anyhow::bail!("no agent files found in {}", git_source.url); } @@ -210,7 +233,10 @@ fn resolve_remote_source(source: &str) -> Result<(PathBuf, Vec)> { } /// Resolve a local path and scan for agent files. -fn resolve_local_source(source: &str) -> Result<(PathBuf, Vec)> { +fn resolve_local_source( + source: &str, + custom_paths: Option<&[manifest::PathMapping]>, +) -> Result<(PathBuf, Vec)> { let path = PathBuf::from(source); if !path.exists() { anyhow::bail!("source path not found: {}", path.display()); @@ -224,7 +250,7 @@ fn resolve_local_source(source: &str) -> Result<(PathBuf, Vec)> { .unwrap_or_else(|| PathBuf::from(".")) }; - let files = scanner::scan_agent_files(&dir, None)?; + let files = scanner::scan_agent_files(&dir, custom_paths)?; if files.is_empty() { anyhow::bail!("no agent files found in {}", dir.display()); } @@ -240,6 +266,9 @@ fn resolve_local_source(source: &str) -> Result<(PathBuf, Vec)> { // --------------------------------------------------------------------------- /// 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 +/// structured `DependencySpec.git_ref` field. fn save_dependency( source: &str, pick: Option<&[String]>, @@ -254,16 +283,26 @@ fn save_dependency( manifest::Manifest::default().with_name(name) }; - let dep = if let Some(pick_list) = pick { + // Parse the source to extract any inline @ref and normalize the URL + let parsed = git::parse_remote(source); + let normalized_source = if git::is_git_url(source) { + parsed.url.clone() + } else { + source.to_string() + }; + + let has_details = parsed.git_ref.is_some() || pick.is_some(); + + let dep = if has_details { Dependency::Detailed(manifest::DependencySpec { - source: source.to_string(), - git_ref: None, - pick: Some(pick_list.to_vec()), + source: normalized_source, + git_ref: parsed.git_ref, + pick: pick.map(|p| p.to_vec()), strategy: None, paths: None, }) } else { - Dependency::Simple(source.to_string()) + Dependency::Simple(normalized_source) }; if loaded.add_dependency(dep) { @@ -280,15 +319,23 @@ fn save_dependency( // Output // --------------------------------------------------------------------------- -fn print_results(results: &[installer::InstallResult]) { +fn print_results(results: &[installer::InstallResult], dry_run: bool) { + let prefix = if dry_run { "[dry-run] " } else { "" }; + if results.is_empty() { - println!("No files installed (no compatible provider/kind combinations found)."); + println!("{prefix}No files installed (no compatible provider/kind combinations found)."); } else { - println!("\nInstalled {} file(s):\n", results.len(),); + let verb = if dry_run { + "Would install" + } else { + "Installed" + }; + println!("\n{prefix}{verb} {} file(s):\n", results.len()); for r in results { println!( - " [{:>11}] {} -> {} ({})", + " [{:>11}] [{}] {} -> {} ({})", r.provider.to_string(), + r.kind, r.source, r.target, r.strategy @@ -367,6 +414,159 @@ pub fn cmd_scan(source: String) -> Result<()> { Ok(()) } +// --------------------------------------------------------------------------- +// Remove command +// --------------------------------------------------------------------------- + +pub fn cmd_remove( + source: String, + clean: bool, + scope: FileScope, + providers: Option>, + root: PathBuf, +) -> Result<()> { + let project_root = root + .canonicalize() + .context("could not resolve project root")?; + + let manifest_path = project_root.join("agentfiles.json"); + if !manifest_path.is_file() { + anyhow::bail!("no agentfiles.json found in {}", project_root.display()); + } + + let mut loaded = manifest::load_manifest(&project_root)?; + + if !loaded.remove_dependency(&source) { + anyhow::bail!("dependency '{}' not found in agentfiles.json", source); + } + + // Optionally clean installed files + if clean { + let providers = providers.unwrap_or_else(|| AgentProvider::ALL.to_vec()); + clean_installed_files(&source, &project_root, &providers, &scope)?; + } + + manifest::save_manifest(&loaded, &project_root)?; + println!("Removed '{}' from agentfiles.json", source); + + Ok(()) +} + +/// Delete installed files for a source from all provider directories. +fn clean_installed_files( + source: &str, + project_root: &std::path::Path, + providers: &[AgentProvider], + scope: &FileScope, +) -> Result<()> { + // Resolve the source to get the file mappings + let scan_result = resolve_source(source, None); + + let files = match scan_result { + Ok((_, files)) => files, + Err(_) => { + println!(" (could not resolve source for cleanup — skipping file deletion)"); + return Ok(()); + } + }; + + let mut cleaned = 0; + for file in &files { + for provider in providers { + if !provider.supports_kind(&file.kind) { + continue; + } + + let target_dir = match provider.get_target_dir(scope, &file.kind, project_root) { + Ok(dir) => dir, + Err(_) => continue, + }; + + let file_name = match file.path.file_name() { + Some(name) => name, + None => continue, + }; + + let target_path = target_dir.join(file_name); + if target_path.exists() || target_path.is_symlink() { + 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()))?; + } else { + std::fs::remove_file(&target_path) + .with_context(|| format!("failed to remove {}", target_path.display()))?; + } + println!(" Removed {}", target_path.display()); + cleaned += 1; + } + } + } + + if cleaned == 0 { + println!(" (no installed files found to clean)"); + } + + Ok(()) +} + +// --------------------------------------------------------------------------- +// List command +// --------------------------------------------------------------------------- + +pub fn cmd_list(root: PathBuf) -> Result<()> { + let project_root = root + .canonicalize() + .context("could not resolve project root")?; + + let manifest_path = project_root.join("agentfiles.json"); + if !manifest_path.is_file() { + println!("No agentfiles.json found. Run 'agentfiles init' to create one."); + return Ok(()); + } + + let loaded = manifest::load_manifest(&project_root)?; + + println!("{} v{}", loaded.name, loaded.version); + + if let Some(desc) = &loaded.description { + println!("{desc}"); + } + println!(); + + if loaded.dependencies.is_empty() { + println!("No dependencies. Add one with 'agentfiles install '."); + return Ok(()); + } + + println!("{} dependency(ies):\n", loaded.dependencies.len()); + for dep in &loaded.dependencies { + let source = dep.source(); + let mut details = Vec::new(); + + if let Some(r) = dep.git_ref() { + details.push(format!("ref={r}")); + } + if let Some(picks) = dep.pick() { + details.push(format!("pick=[{}]", picks.join(", "))); + } + if let Some(strategy) = dep.strategy() { + details.push(format!("strategy={strategy}")); + } + if let Some(paths) = dep.paths() { + let path_strs: Vec<&str> = paths.iter().map(|p| p.path.as_str()).collect(); + details.push(format!("paths=[{}]", path_strs.join(", "))); + } + + if details.is_empty() { + println!(" {source}"); + } else { + println!(" {source} ({})", details.join(", ")); + } + } + + Ok(()) +} + // --------------------------------------------------------------------------- // Matrix command // --------------------------------------------------------------------------- @@ -480,15 +680,16 @@ mod tests { #[test] fn install_no_source_without_manifest_errors() { let dir = TempDir::new().unwrap(); - let result = cmd_install( - None, - FileScope::Project, - None, - None, - None, - false, - dir.path().to_path_buf(), - ); + let result = cmd_install(InstallOptions { + source: None, + scope: FileScope::Project, + providers: None, + strategy: None, + pick: None, + no_save: false, + dry_run: false, + root: dir.path().to_path_buf(), + }); assert!(result.is_err()); } @@ -499,15 +700,16 @@ mod tests { manifest::save_manifest(&manifest, dir.path())?; // Should succeed but print "no dependencies" message - let result = cmd_install( - None, - FileScope::Project, - None, - None, - None, - false, - dir.path().to_path_buf(), - ); + let result = cmd_install(InstallOptions { + source: None, + scope: FileScope::Project, + providers: None, + strategy: None, + pick: None, + no_save: false, + dry_run: false, + root: dir.path().to_path_buf(), + }); assert!(result.is_ok()); Ok(()) } @@ -528,15 +730,16 @@ mod tests { let source = src_dir.path().to_string_lossy().into_owned(); - cmd_install( - Some(source.clone()), - FileScope::Project, - None, - None, - None, - false, // auto-save on - dst_dir.path().to_path_buf(), - )?; + cmd_install(InstallOptions { + source: Some(source.clone()), + scope: FileScope::Project, + providers: None, + strategy: None, + pick: None, + no_save: false, + dry_run: false, + root: dst_dir.path().to_path_buf(), + })?; // agentfiles.json should be created in the project root let manifest_path = dst_dir.path().join("agentfiles.json"); @@ -559,15 +762,16 @@ mod tests { let source = src_dir.path().to_string_lossy().into_owned(); - cmd_install( - Some(source), - FileScope::Project, - None, - None, - None, - true, // no-save - dst_dir.path().to_path_buf(), - )?; + cmd_install(InstallOptions { + source: Some(source), + scope: FileScope::Project, + providers: None, + strategy: None, + pick: None, + no_save: true, + dry_run: false, + root: dst_dir.path().to_path_buf(), + })?; // agentfiles.json should NOT be created assert!(!dst_dir.path().join("agentfiles.json").exists()); diff --git a/src/git.rs b/src/git.rs index 2d0104c..57a4430 100644 --- a/src/git.rs +++ b/src/git.rs @@ -1,7 +1,6 @@ -use std::collections::hash_map::DefaultHasher; -use std::hash::{Hash, Hasher}; use std::path::{Path, PathBuf}; use std::process::Command; +use std::sync::OnceLock; use anyhow::{Context, Result, bail}; @@ -27,6 +26,17 @@ pub struct ParsedRemote { pub git_ref: Option, } +/// Normalize a source string for deduplication. +/// +/// Strips `@ref` suffixes, normalizes URL schemes, and removes trailing `.git` +/// so that `github.com/org/repo`, `https://github.com/org/repo`, and +/// `https://github.com/org/repo.git` all compare as equal. +pub fn normalize_source(input: &str) -> String { + let (base, _ref) = strip_ref(input); + let url = normalize_url(base); + url.trim_end_matches(".git").to_string() +} + /// Check whether a string looks like a git remote URL rather than a local path. /// /// Recognizes: @@ -175,24 +185,45 @@ fn normalize_url(url: &str) -> String { } } -/// Hash a URL string to a 16-character hex string. +/// Hash a URL string to a 16-character hex string using FNV-1a. +/// +/// Uses a deterministic hash algorithm (FNV-1a 64-bit) so that cache +/// directory names remain stable across Rust toolchain upgrades. fn hash_url(url: &str) -> String { - let mut hasher = DefaultHasher::new(); - url.hash(&mut hasher); - format!("{:016x}", hasher.finish()) + const FNV_OFFSET: u64 = 0xcbf29ce484222325; + const FNV_PRIME: u64 = 0x00000100000001B3; + + let mut hash = FNV_OFFSET; + for byte in url.bytes() { + hash ^= byte as u64; + hash = hash.wrapping_mul(FNV_PRIME); + } + format!("{hash:016x}") } /// Verify that `git` is available on the system. +/// +/// Only runs the actual check once per process; subsequent calls return +/// the cached result immediately. fn ensure_git_available() -> Result<()> { - let output = Command::new("git") - .arg("--version") - .output() - .context("'git' is not installed or not in PATH")?; + static GIT_CHECKED: OnceLock> = OnceLock::new(); - if !output.status.success() { - bail!("'git --version' failed — is git installed?"); - } - Ok(()) + let result = GIT_CHECKED.get_or_init(|| { + let output = Command::new("git") + .arg("--version") + .output() + .map_err(|_| "'git' is not installed or not in PATH".to_string())?; + + if !output.status.success() { + return Err("'git --version' failed — is git installed?".to_string()); + } + Ok(()) + }); + + result + .as_ref() + .map(|_| ()) + .map_err(|e| anyhow::anyhow!("{e}")) } /// Clone a repository into the cache directory. @@ -293,7 +324,7 @@ fn reset_to_default_branch(repo_dir: &Path) -> Result<()> { .output() .context("failed to determine default branch")?; - let default_branch = if output.status.success() { + let mut branch = if output.status.success() { let full = String::from_utf8_lossy(&output.stdout).trim().to_string(); full.strip_prefix("origin/").unwrap_or(&full).to_string() } else { @@ -301,10 +332,10 @@ fn reset_to_default_branch(repo_dir: &Path) -> Result<()> { }; let output = Command::new("git") - .args(["checkout", &default_branch]) + .args(["checkout", &branch]) .current_dir(repo_dir) .output() - .with_context(|| format!("failed to checkout '{default_branch}'"))?; + .with_context(|| format!("failed to checkout '{branch}'"))?; if !output.status.success() { // Try "master" as a fallback @@ -317,11 +348,12 @@ fn reset_to_default_branch(repo_dir: &Path) -> Result<()> { if !output2.status.success() { bail!("could not determine or checkout the default branch"); } + branch = "master".to_string(); } // Reset to match remote and pull latest let reset_output = Command::new("git") - .args(["reset", "--hard", &format!("origin/{default_branch}")]) + .args(["reset", "--hard", &format!("origin/{branch}")]) .current_dir(repo_dir) .output() .context("failed to run 'git reset'")?; diff --git a/src/installer.rs b/src/installer.rs index b8c0cef..2441cad 100644 --- a/src/installer.rs +++ b/src/installer.rs @@ -4,16 +4,16 @@ use std::path::Path; use anyhow::{Context, Result}; use crate::manifest::FileMapping; -use crate::types::{AgentProvider, FileScope, FileStrategy}; +use crate::types::{AgentProvider, FileKind, FileScope, FileStrategy}; /// Result of installing a single file to a single provider. #[derive(Debug)] -pub struct InstallResult { +pub(crate) struct InstallResult { pub provider: AgentProvider, pub source: String, pub target: String, pub strategy: FileStrategy, - pub kind: String, + pub kind: FileKind, } /// Install all files from a list of file mappings to the specified providers. @@ -22,13 +22,17 @@ pub struct InstallResult { /// provider that supports the file's kind. The `source_root` is the directory /// containing the source files (used to resolve relative source paths). /// +/// When `dry_run` is true, resolves target paths and builds `InstallResult` +/// entries without creating directories or copying/linking files. +/// /// Returns a list of `InstallResult` entries describing what was installed. -pub fn install( +pub(crate) fn install( files: &[FileMapping], providers: &[AgentProvider], scope: &FileScope, project_root: &Path, source_root: &Path, + dry_run: bool, ) -> Result> { let mut results = Vec::new(); @@ -51,68 +55,73 @@ pub fn install( let target_dir = provider.get_target_dir(scope, &file.kind, project_root)?; let target_path = resolve_target_path(&file.path, &target_dir)?; - if let Some(parent) = target_path.parent() { - fs::create_dir_all(parent) - .with_context(|| format!("failed to create directory: {}", parent.display()))?; - } - - // Place the file - match file.strategy { - FileStrategy::Copy => { - if source_path.is_dir() { - copy_dir_recursive(&source_path, &target_path)?; - } else { - fs::copy(&source_path, &target_path).with_context(|| { - format!( - "failed to copy {} -> {}", - source_path.display(), - target_path.display() - ) - })?; - } + if !dry_run { + if let Some(parent) = target_path.parent() { + fs::create_dir_all(parent).with_context(|| { + format!("failed to create directory: {}", parent.display()) + })?; } - FileStrategy::Link => { - if target_path.exists() || target_path.is_symlink() { - if target_path.is_dir() && !target_path.is_symlink() { - fs::remove_dir_all(&target_path)?; + + // Place the file + match file.strategy { + FileStrategy::Copy => { + if source_path.is_dir() { + copy_dir_recursive(&source_path, &target_path)?; } else { - fs::remove_file(&target_path)?; + fs::copy(&source_path, &target_path).with_context(|| { + format!( + "failed to copy {} -> {}", + source_path.display(), + target_path.display() + ) + })?; } } + FileStrategy::Link => { + if target_path.exists() || target_path.is_symlink() { + if target_path.is_dir() && !target_path.is_symlink() { + fs::remove_dir_all(&target_path)?; + } else { + fs::remove_file(&target_path)?; + } + } - let abs_source = source_path.canonicalize().with_context(|| { - format!("failed to resolve absolute path: {}", source_path.display()) - })?; - - #[cfg(unix)] - std::os::unix::fs::symlink(&abs_source, &target_path).with_context(|| { - format!( - "failed to symlink {} -> {}", - abs_source.display(), - target_path.display() - ) - })?; + let abs_source = source_path.canonicalize().with_context(|| { + format!("failed to resolve absolute path: {}", source_path.display()) + })?; - #[cfg(windows)] - { - if abs_source.is_dir() { - std::os::windows::fs::symlink_dir(&abs_source, &target_path) - .with_context(|| { - format!( - "failed to symlink {} -> {}", - abs_source.display(), - target_path.display() - ) - })?; - } else { - std::os::windows::fs::symlink_file(&abs_source, &target_path) - .with_context(|| { - format!( - "failed to symlink {} -> {}", - abs_source.display(), - target_path.display() - ) - })?; + #[cfg(unix)] + std::os::unix::fs::symlink(&abs_source, &target_path).with_context( + || { + format!( + "failed to symlink {} -> {}", + abs_source.display(), + target_path.display() + ) + }, + )?; + + #[cfg(windows)] + { + if abs_source.is_dir() { + std::os::windows::fs::symlink_dir(&abs_source, &target_path) + .with_context(|| { + format!( + "failed to symlink {} -> {}", + abs_source.display(), + target_path.display() + ) + })?; + } else { + std::os::windows::fs::symlink_file(&abs_source, &target_path) + .with_context(|| { + format!( + "failed to symlink {} -> {}", + abs_source.display(), + target_path.display() + ) + })?; + } } } } @@ -123,7 +132,7 @@ pub fn install( source: file.path.display().to_string(), target: target_path.display().to_string(), strategy: file.strategy, - kind: file.kind.to_string(), + kind: file.kind, }); } } @@ -197,6 +206,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, )?; assert_eq!(results.len(), 1); @@ -237,6 +247,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, )?; // Should only install to ClaudeCode, not Codex @@ -268,6 +279,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, )?; // All 4 providers support skills @@ -325,6 +337,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, )?; assert_eq!(results.len(), 1); @@ -360,6 +373,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, )?; assert_eq!(results.len(), 1); @@ -389,6 +403,7 @@ mod tests { &FileScope::Project, dst_dir.path(), src_dir.path(), + false, ); assert!(result.is_err()); diff --git a/src/main.rs b/src/main.rs index 44c02d7..83ddae6 100644 --- a/src/main.rs +++ b/src/main.rs @@ -13,10 +13,28 @@ fn main() -> Result<()> { strategy, pick, no_save, + dry_run, root, - } => commands::cmd_install(source, scope, providers, strategy, pick, no_save, root), + } => commands::cmd_install(commands::InstallOptions { + source, + scope, + providers, + strategy, + pick, + no_save, + dry_run, + root, + }), cli::Command::Init { path, name } => commands::cmd_init(path, name), cli::Command::Scan { source } => commands::cmd_scan(source), + cli::Command::Remove { + source, + clean, + scope, + providers, + root, + } => commands::cmd_remove(source, clean, scope, providers, root), + cli::Command::List { root } => commands::cmd_list(root), cli::Command::Matrix => commands::cmd_matrix(), } } diff --git a/src/manifest.rs b/src/manifest.rs index 471ebc3..9723b1e 100644 --- a/src/manifest.rs +++ b/src/manifest.rs @@ -3,16 +3,19 @@ use std::path::{Path, PathBuf}; use anyhow::{Context, Result, bail}; use serde::{Deserialize, Serialize}; +use crate::git; use crate::types::{FileKind, FileStrategy}; // --------------------------------------------------------------------------- // Internal types (scanner output / installer input) // --------------------------------------------------------------------------- -/// A single discovered agent file. Internal type used by the scanner and -/// installer -- not serialized into the manifest. +/// A single discovered agent file used by the scanner and installer. +/// +/// Not serialized into the manifest — this is an in-memory representation +/// of a file to be installed. #[derive(Clone, Debug, PartialEq)] -pub struct FileMapping { +pub(crate) struct FileMapping { /// Path to the source file or directory, relative to the source root. pub path: PathBuf, @@ -47,38 +50,34 @@ impl Dependency { } } - /// Explicit git ref override, if any. Note that `@ref` in Simple strings - /// is handled by git::parse_remote, not here. - pub fn git_ref(&self) -> Option<&str> { + /// Returns the detailed spec, if this is a Detailed dependency. + pub fn spec(&self) -> Option<&DependencySpec> { match self { Dependency::Simple(_) => None, - Dependency::Detailed(d) => d.git_ref.as_deref(), + Dependency::Detailed(d) => Some(d), } } + /// Explicit git ref override, if any. Note that `@ref` in Simple strings + /// is handled by git::parse_remote, not here. + pub fn git_ref(&self) -> Option<&str> { + self.spec().and_then(|d| d.git_ref.as_deref()) + } + /// Cherry-pick list, if any. pub fn pick(&self) -> Option<&[String]> { - match self { - Dependency::Simple(_) => None, - Dependency::Detailed(d) => d.pick.as_deref(), - } + self.spec().and_then(|d| d.pick.as_deref()) } /// Per-dependency strategy override, if any. pub fn strategy(&self) -> Option { - match self { - Dependency::Simple(_) => None, - Dependency::Detailed(d) => d.strategy, - } + self.spec().and_then(|d| d.strategy) } /// Custom path-to-kind mappings. When set, replaces the default /// `skills/`, `commands/`, `agents/` scanning convention. pub fn paths(&self) -> Option<&[PathMapping]> { - match self { - Dependency::Simple(_) => None, - Dependency::Detailed(d) => d.paths.as_deref(), - } + self.spec().and_then(|d| d.paths.as_deref()) } } @@ -209,8 +208,25 @@ impl Manifest { } /// Check whether a dependency with the given source already exists. + /// + /// Compares using normalized URLs so that `github.com/org/repo` and + /// `https://github.com/org/repo.git` are treated as the same source. pub fn has_dependency(&self, source: &str) -> bool { - self.dependencies.iter().any(|d| d.source() == source) + let normalized = git::normalize_source(source); + self.dependencies + .iter() + .any(|d| git::normalize_source(d.source()) == normalized) + } + + /// Remove a dependency by source. Returns true if a dependency was removed. + /// + /// Uses normalized URL comparison, same as `has_dependency`. + pub fn remove_dependency(&mut self, source: &str) -> bool { + let normalized = git::normalize_source(source); + let before = self.dependencies.len(); + self.dependencies + .retain(|d| git::normalize_source(d.source()) != normalized); + self.dependencies.len() < before } } @@ -455,8 +471,8 @@ mod tests { let json = r#"{ "source": "github.com/some/repo", "paths": [ - {"path": "prompts", "kind": "Skill"}, - {"path": "macros", "kind": "Command"} + {"path": "prompts", "kind": "skill"}, + {"path": "macros", "kind": "command"} ] }"#; let dep: Dependency = serde_json::from_str(json)?; diff --git a/src/provider.rs b/src/provider.rs index c52a8af..0455ce8 100644 --- a/src/provider.rs +++ b/src/provider.rs @@ -99,18 +99,11 @@ impl AgentProvider { .collect() } - /// Returns the project-scope base directories for all providers. + /// Project-scope base directories for all providers (deduplicated). /// /// Used by the scanner to know which directory prefixes to look for /// when auto-discovering agent files. - pub fn project_bases() -> Vec<&'static str> { - let mut seen = std::collections::HashSet::new(); - AgentProvider::ALL - .iter() - .map(|p| p.layout().project_base) - .filter(|b| seen.insert(*b)) - .collect() - } + pub const PROJECT_BASES: &[&str] = &[".claude", ".opencode", ".agents", ".cursor"]; /// Resolves the full target directory for a given scope and file kind. /// @@ -293,7 +286,7 @@ mod tests { #[test] fn project_bases_contains_all_providers() { - let bases = AgentProvider::project_bases(); + let bases = AgentProvider::PROJECT_BASES; assert!(bases.contains(&".claude")); assert!(bases.contains(&".opencode")); assert!(bases.contains(&".agents")); diff --git a/src/scanner.rs b/src/scanner.rs index 0d24eda..4602e2a 100644 --- a/src/scanner.rs +++ b/src/scanner.rs @@ -30,7 +30,7 @@ const KIND_DIRS: &[(&str, FileKind)] = &[ /// is recorded, not just the SKILL.md). Commands and agents are `.md` files. /// /// The returned `FileMapping` paths are relative to `root`. -pub fn scan_agent_files( +pub(crate) fn scan_agent_files( root: &Path, custom_paths: Option<&[PathMapping]>, ) -> Result> { @@ -55,7 +55,7 @@ pub 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 fn filter_by_pick(mappings: Vec, pick: &[String]) -> Vec { +pub(crate) fn filter_by_pick(mappings: Vec, pick: &[String]) -> Vec { mappings .into_iter() .filter(|m| { @@ -79,7 +79,7 @@ pub fn filter_by_pick(mappings: Vec, pick: &[String]) -> Vec String { +pub(crate) fn infer_name(path: &Path) -> String { path.file_name() .map(|n| n.to_string_lossy().into_owned()) .unwrap_or_else(|| "unnamed".to_string()) @@ -92,7 +92,7 @@ pub fn infer_name(path: &Path) -> String { /// 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() { + for prefix in AgentProvider::PROJECT_BASES { let prefix_dir = root.join(prefix); if prefix_dir.is_dir() { scan_kind_dirs(root, &prefix_dir, mappings)?; diff --git a/src/types.rs b/src/types.rs index f3753a9..07d9de6 100644 --- a/src/types.rs +++ b/src/types.rs @@ -33,6 +33,7 @@ impl FromStr for FileScope { /// The kind of agent file. Determines the target subdirectory. #[derive(Serialize, Deserialize, Clone, Copy, Debug, PartialEq, Eq, Hash)] +#[serde(rename_all = "lowercase")] pub enum FileKind { Skill, Agent,