diff --git a/README.md b/README.md index b9b00df..330b01e 100644 --- a/README.md +++ b/README.md @@ -23,7 +23,7 @@ Coming from [AIChat](https://github.com/sigoden/aichat)? Follow the [migration g * [AIChat Migration Guide](https://github.com/Dark-Alex-17/coyote/wiki/AIChat-Migration): Coming from AIChat? Follow the migration guide to get started. * [Installation](#install): Install Coyote * [Getting Started](#getting-started): Get started with Coyote by doing first-run setup steps. -* [Sharing Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Sharing-Configurations): Install bundles of agents, roles, macros, tools, and MCP servers from any git repo, and share your own. +* [Sharing Configurations](https://github.com/Dark-Alex-17/coyote/wiki/Sharing-Configurations): Install bundles of agents, roles, skills, macros, tools, and MCP servers from any git repo, and share your own. Bundles are Coyote's equivalent of plugins in other CLI agents. * [REPL](https://github.com/Dark-Alex-17/coyote/wiki/REPL): Interactive Read-Eval-Print Loop for conversational interactions with LLMs and Coyote. * [Custom REPL Prompt](https://github.com/Dark-Alex-17/coyote/wiki/REPL-Prompt): Customize the REPL prompt to provide useful contextual information. * [Vault](https://github.com/Dark-Alex-17/coyote/wiki/Vault): Securely store and manage sensitive information such as API keys and credentials. diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 9c4394d..1114217 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -50,9 +50,10 @@ pub enum McpScopeArg { "model", "prompt", "role", "session", "agent", "rag", "rebuild_rag", "macro_name", "execute", "code", "file", "no_stream", "no_memory", "init_memory", "dry_run", "info", "build_tools", "install", - "install_from", "sync_models", "list_models", "list_roles", + "install_builtins", "sync_models", "list_models", "list_roles", "list_sessions", "list_agents", "list_rags", "list_macros", - "list_skills", "skill", "tail_logs", "completions", "update", + "list_skills", "list_bundles", "skill", "tail_logs", "completions", + "update", "update_bundle", "uninstall", ]) ), group( @@ -175,34 +176,79 @@ pub struct Cli { /// List all installed skills #[arg(long, help_heading = "List & Discovery")] pub list_skills: bool, + /// List installed bundles and their drift status + #[arg(long, help_heading = "List & Discovery")] + pub list_bundles: bool, - /// Reinstall bundled assets, overwriting any local changes + /// Install assets from a remote git repository (a URL or / shorthand, optionally suffixed with #), or update an already-installed bundle by name #[arg( long, - value_name = "CATEGORY", - value_enum, + value_name = "GIT_URL|OWNER/REPO|NAME", + conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"], help_heading = "Installation & Updates" )] - pub install: Option, - /// Install assets from a remote git repository (URL may be suffixed with #) - #[arg(long, value_name = "GIT_URL", help_heading = "Installation & Updates")] - pub install_from: Option, - /// Restrict --install-from to a single asset category + pub install: Option, + /// Git host used to expand / shorthand values passed to --install (also forces the value to be treated as a source when it matches an installed bundle name) + #[arg( + long, + value_name = "HOST", + requires = "install", + conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"], + help_heading = "Installation & Updates" + )] + pub git_host: Option, + /// Reinstall bundled assets for a category (asks before overwriting your local changes) #[arg( long, value_name = "CATEGORY", value_enum, - requires = "install_from", + conflicts_with_all = ["update_bundle", "uninstall"], + help_heading = "Installation & Updates" + )] + pub install_builtins: Option, + /// Restrict a remote install to a single asset category + #[arg( + long, + value_name = "CATEGORY", + value_enum, + requires = "install", + conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"], help_heading = "Installation & Updates" )] pub filter: Option, - /// Overwrite all conflicts without prompting (used with --install-from) + /// Overwrite all conflicts without prompting (remote installs only) #[arg( long, - requires = "install_from", + requires = "install", + conflicts_with_all = ["install_builtins", "update_bundle", "uninstall"], help_heading = "Installation & Updates" )] pub install_force: bool, + /// Update an installed bundle from its recorded source (NAME may be suffixed with # to move a pin) + #[arg( + long, + value_name = "NAME", + group = "yes_scope", + conflicts_with_all = ["uninstall"], + help_heading = "Installation & Updates" + )] + pub update_bundle: Option, + /// Uninstall a bundle: delete its owned files and remove its mcp.json entries + #[arg( + long, + value_name = "NAME", + group = "yes_scope", + help_heading = "Installation & Updates" + )] + pub uninstall: Option, + /// Proceed without prompts for --uninstall and --update-bundle (locally modified items are always kept) + #[arg( + long, + requires = "yes_scope", + conflicts_with_all = ["install", "install_builtins"], + help_heading = "Installation & Updates" + )] + pub yes: bool, /// Sync models updates #[arg(long, help_heading = "Installation & Updates")] pub sync_models: bool, @@ -495,6 +541,7 @@ mod tests { assert!(parse(&["--list-rags"]).list_rags); assert!(parse(&["--list-macros"]).list_macros); assert!(parse(&["--list-skills"]).list_skills); + assert!(parse(&["--list-bundles"]).list_bundles); } #[test] @@ -503,6 +550,119 @@ mod tests { assert!(parse(&[]).skill.is_empty()); } + #[test] + fn parse_update_bundle_flag_takes_name() { + assert_eq!( + parse(&["--update-bundle", "foo"]).update_bundle.as_deref(), + Some("foo") + ); + } + + #[test] + fn parse_uninstall_flag_takes_name() { + assert_eq!( + parse(&["--uninstall", "foo"]).uninstall.as_deref(), + Some("foo") + ); + assert!(!parse(&["--uninstall", "foo"]).yes); + } + + #[test] + fn parse_yes_flag_requires_uninstall_or_update_bundle() { + assert!(parse(&["--uninstall", "foo", "--yes"]).yes); + assert!(parse(&["--update-bundle", "foo", "--yes"]).yes); + assert!(Cli::try_parse_from(["coyote", "--yes"]).is_err()); + } + + #[test] + fn parse_install_flag_takes_url_or_name() { + assert_eq!( + parse(&["--install", "https://github.com/x/y"]) + .install + .as_deref(), + Some("https://github.com/x/y") + ); + } + + #[test] + fn parse_install_builtins_flag_takes_category() { + assert_eq!( + parse(&["--install-builtins", "agents"]).install_builtins, + Some(AssetCategory::Agents) + ); + assert_eq!( + parse(&["--install-builtins", "mcp_config"]).install_builtins, + Some(AssetCategory::McpConfig) + ); + } + + #[test] + fn parse_install_builtins_conflicts_with_install() { + assert!( + Cli::try_parse_from(["coyote", "--install-builtins", "agents", "--install", "x"]) + .is_err() + ); + } + + #[test] + fn parse_lifecycle_flags_are_mutually_exclusive() { + assert!(Cli::try_parse_from(["coyote", "--install", "x", "--uninstall", "y"]).is_err()); + assert!( + Cli::try_parse_from(["coyote", "--update-bundle", "x", "--uninstall", "y"]).is_err() + ); + assert!( + Cli::try_parse_from(["coyote", "--install-builtins", "agents", "--uninstall", "y"]) + .is_err() + ); + assert!(Cli::try_parse_from(["coyote", "--install", "x", "--update-bundle", "y"]).is_err()); + } + + #[test] + fn parse_companion_flags_conflict_with_other_lifecycle_actions() { + assert!( + Cli::try_parse_from(["coyote", "--update-bundle", "x", "--filter", "agents"]).is_err() + ); + assert!(Cli::try_parse_from(["coyote", "--uninstall", "x", "--install-force"]).is_err()); + assert!(Cli::try_parse_from(["coyote", "--install", "x", "--yes"]).is_err()); + } + + #[test] + fn parse_filter_requires_install() { + assert!(Cli::try_parse_from(["coyote", "--filter", "agents"]).is_err()); + assert_eq!( + parse(&["--install", "https://github.com/x/y", "--filter", "agents"]).filter, + Some(InstallFilter::Agents) + ); + } + + #[test] + fn parse_install_force_requires_install() { + assert!(Cli::try_parse_from(["coyote", "--install-force"]).is_err()); + assert!(parse(&["--install", "https://github.com/x/y", "--install-force"]).install_force); + } + + #[test] + fn parse_git_host_requires_install() { + assert!(Cli::try_parse_from(["coyote", "--git-host", "git.x.com"]).is_err()); + assert!( + Cli::try_parse_from(["coyote", "--git-host", "gitlab.com", "--update-bundle", "x"]) + .is_err() + ); + assert_eq!( + parse(&["--install", "someuser/omc", "--git-host", "git.x.com"]) + .git_host + .as_deref(), + Some("git.x.com") + ); + } + + #[test] + fn help_shows_install_builtins() { + use clap::CommandFactory; + let help = Cli::command().render_long_help().to_string(); + assert!(help.contains("--install-builtins")); + } + #[test] fn parse_multiple_skill_flags_preserves_order() { assert_eq!( diff --git a/src/config/bundles.rs b/src/config/bundles.rs new file mode 100644 index 0000000..3499227 --- /dev/null +++ b/src/config/bundles.rs @@ -0,0 +1,1489 @@ +use super::install_remote::{ + canonical_source_url, owner_qualifier, repo_name_slug, validate_bundle_name, +}; +use super::{paths, request_context}; +use crate::config::AssetCategory; +use crate::function::write_file_atomic; +use crate::utils::IS_STDOUT_TERMINAL; + +use anyhow::{Context, Result, bail}; +use chrono::{SecondsFormat, Utc}; +use inquire::Confirm; +use serde::{Deserialize, Serialize}; +use sha2::{Digest, Sha256}; +use std::collections::BTreeMap; +use std::fs; +use std::path::{Path, PathBuf}; + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub(crate) enum FileAction { + New, + Replaced, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] +#[serde(rename_all = "lowercase")] +pub(crate) enum McpAction { + Added, + Replaced, + Renamed, + Transferred, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub(crate) struct FileRecord { + /// Relative to the config dir. + pub(crate) path: String, + pub(crate) category: String, + /// Content hash at install time; a later mismatch means the user modified the file. + pub(crate) sha256: String, + pub(crate) action: FileAction, +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub(crate) struct McpServerRecord { + pub(crate) name: String, + pub(crate) action: McpAction, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) renamed_to: Option, + /// Hash of the mcp.json entry as written; a later mismatch means the user + /// modified it. Absent on records made before entry hashing existed. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) sha256: Option, +} + +impl McpServerRecord { + /// The key this entry actually occupies in mcp.json (the rename target, if any). + pub(crate) fn effective_key(&self) -> &str { + self.renamed_to.as_deref().unwrap_or(&self.name) + } +} + +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub(crate) struct BundleRecord { + pub(crate) source: String, + #[serde(rename = "ref", default, skip_serializing_if = "Option::is_none")] + pub(crate) git_ref: Option, + pub(crate) commit: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) version: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) description: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) homepage: Option, + pub(crate) installed_at: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub(crate) updated_at: Option, + #[serde(default)] + pub(crate) files: Vec, + #[serde(default)] + pub(crate) mcp_servers: Vec, +} + +#[derive(Debug, Clone)] +pub(crate) struct InstallMetadata { + pub(crate) source: String, + pub(crate) git_ref: Option, + pub(crate) commit: String, + pub(crate) version: Option, + pub(crate) description: Option, + pub(crate) homepage: Option, +} + +#[derive(Debug, Clone, PartialEq)] +pub(crate) struct ResolvedBundleName { + pub(crate) name: String, + /// The unqualified name this install asked for, when it had to be owner-qualified. + pub(crate) qualified_from: Option, + /// Source URL of the different-source bundle that already holds the unqualified name. + pub(crate) same_name_other_source: Option, +} + +const STORE_VERSION: u32 = 1; + +#[derive(Debug, Default, Deserialize)] +struct StoreContents { + #[serde(default)] + version: u32, + #[serde(default)] + bundles: BTreeMap, +} + +#[derive(Serialize)] +struct StoreContentsRef<'a> { + version: u32, + bundles: &'a BTreeMap, +} + +#[derive(Debug)] +pub(crate) struct BundleStore { + path: PathBuf, + bundles: BTreeMap, +} + +impl BundleStore { + pub(crate) fn load() -> Result { + Self::load_from(paths::installed_bundles_file()) + } + + /// A corrupt store is an error, never an empty store: treating it as empty + /// would let a reinstall re-acquire ownership over files the user may have + /// modified since. + pub(crate) fn load_from(path: PathBuf) -> Result { + if !path.exists() { + return Ok(Self { + path, + bundles: BTreeMap::new(), + }); + } + + let content = fs::read_to_string(&path) + .with_context(|| format!("failed to read {}", path.display()))?; + let contents: StoreContents = serde_yaml::from_str(&content).with_context(|| { + format!( + "failed to parse {}; refusing to treat it as empty. \ + Restore or repair the file to continue; removing it forfeits \ + uninstall tracking for every installed bundle", + path.display() + ) + })?; + + if contents.version > STORE_VERSION { + bail!( + "{} has store version {}, but this coyote build supports up to \ + {STORE_VERSION}; update coyote or restore a matching store", + path.display(), + contents.version + ); + } + + Ok(Self { + path, + bundles: contents.bundles, + }) + } + + pub(crate) fn save(&self) -> Result<()> { + let content = serde_yaml::to_string(&StoreContentsRef { + version: STORE_VERSION, + bundles: &self.bundles, + }) + .context("failed to serialize the installed-bundles store")?; + + if let Some(parent) = self.path.parent() { + fs::create_dir_all(parent) + .with_context(|| format!("failed to create directory {}", parent.display()))?; + } + + write_file_atomic(&self.path, &content, None) + .with_context(|| format!("failed to write {}", self.path.display())) + } + + pub(crate) fn get(&self, name: &str) -> Option<&BundleRecord> { + self.bundles.get(name) + } + + pub(crate) fn iter(&self) -> impl Iterator { + self.bundles + .iter() + .map(|(name, record)| (name.as_str(), record)) + } + + pub(crate) fn bundle_names(&self) -> Vec<&str> { + self.bundles.keys().map(String::as_str).collect() + } + + pub(crate) fn find_by_source(&self, url: &str) -> Option<(&str, &BundleRecord)> { + let canonical = canonical_source_url(url); + self.bundles + .iter() + .find(|(_, record)| canonical_source_url(&record.source) == canonical) + .map(|(name, record)| (name.as_str(), record)) + } + + /// A URL already tracked under a different key migrates to the new key, so + /// the same URL never gets a second record. A name held by a + /// different-source bundle is owner-qualified instead. + pub(crate) fn resolve_bundle_name( + &mut self, + url: &str, + manifest_name: Option<&str>, + ) -> Result { + let canonical = canonical_source_url(url); + let existing_key = self + .bundles + .iter() + .find(|(_, record)| canonical_source_url(&record.source) == canonical) + .map(|(name, _)| name.clone()); + + let base = match manifest_name { + Some(name) => { + validate_bundle_name(name)?; + name.to_string() + } + None => { + let slug = sanitize_name_segment(&repo_name_slug(url)); + if slug.is_empty() { + bail!( + "cannot derive a bundle name from '{url}'; \ + add a coyote-bundle.yaml manifest with a name" + ); + } + slug + } + }; + + let mut resolved = ResolvedBundleName { + name: base.clone(), + qualified_from: None, + same_name_other_source: None, + }; + + let reserved = AssetCategory::parse(&base).is_some(); + let collision = self.source_of_other_bundle(&base, &canonical); + if reserved || collision.is_some() { + let reason = match &collision { + Some(other_source) => { + format!("is already used by an install from '{other_source}'") + } + None => "is reserved for an asset category".to_string(), + }; + + if base.contains('/') { + bail!( + "bundle name '{base}' {reason} and cannot be qualified further; \ + uninstall it or pick a different manifest name" + ); + } + + let owner = owner_qualifier(url) + .map(|owner| sanitize_name_segment(&owner)) + .filter(|owner| !owner.is_empty()); + let Some(owner) = owner else { + bail!( + "bundle name '{base}' {reason}, and no owner qualifier \ + can be derived from '{url}'" + ); + }; + let qualified = format!("{owner}/{base}"); + + if let Some(source) = self.source_of_other_bundle(&qualified, &canonical) { + bail!( + "bundle name '{base}' {reason}, and '{qualified}' is already \ + used by an install from '{source}'; \ + uninstall one or pick a different manifest name" + ); + } + + if let Some(other_source) = &collision + && manifest_name.is_some() + && *IS_STDOUT_TERMINAL + { + let proceed = Confirm::new(&format!( + "Bundle name '{base}' is already used by an install from \ + '{other_source}' (a fork or typo-squat?). Track this install \ + as '{qualified}'?" + )) + .with_default(false) + .prompt() + .with_context(|| "failed to read bundle name confirmation")?; + if !proceed { + bail!("install aborted: bundle name '{base}' {reason}"); + } + } + resolved.name = qualified; + resolved.qualified_from = Some(base); + resolved.same_name_other_source = collision; + } + + let already_recorded = existing_key.as_deref() == Some(resolved.name.as_str()); + if let Some(old_key) = existing_key + && !already_recorded + { + if self.bundles.contains_key(&resolved.name) { + bail!( + "records '{old_key}' and '{}' both track source '{url}'; \ + uninstall one or repair installed-bundles.yaml before continuing", + resolved.name + ); + } + let record = self + .bundles + .remove(&old_key) + .expect("existing_key was found in the map"); + self.bundles.insert(resolved.name.clone(), record); + println!( + "Bundle '{old_key}' from {url} is now tracked as '{}'.", + resolved.name + ); + self.save()?; + } + + if let (Some(from), false) = (&resolved.qualified_from, already_recorded) { + let detail = match resolved.same_name_other_source.as_deref() { + Some(other) => format!("is already used by an install from '{other}'"), + None => "is reserved for an asset category".to_string(), + }; + println!( + "Bundle name '{from}' {detail}; tracking this install as '{}'.", + resolved.name + ); + } + + debug_assert!( + validate_bundle_name(&resolved.name).is_ok(), + "derived bundle names must satisfy validate_bundle_name" + ); + Ok(resolved) + } + + pub(crate) fn upsert_bundle(&mut self, name: &str, metadata: InstallMetadata) -> Result<()> { + match self.bundles.get_mut(name) { + Some(record) => { + record.source = metadata.source; + record.git_ref = metadata.git_ref; + record.commit = metadata.commit; + record.version = metadata.version; + record.description = metadata.description; + record.homepage = metadata.homepage; + } + None => { + self.bundles.insert( + name.to_string(), + BundleRecord { + source: metadata.source, + git_ref: metadata.git_ref, + commit: metadata.commit, + version: metadata.version, + description: metadata.description, + homepage: metadata.homepage, + installed_at: Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true), + updated_at: None, + files: Vec::new(), + mcp_servers: Vec::new(), + }, + ); + } + } + + self.save() + } + + pub(crate) fn mark_updated(&mut self, name: &str) -> Result<()> { + self.ensure_bundle_exists(name)?; + let record = self + .bundles + .get_mut(name) + .expect("bundle existence checked above"); + + record.updated_at = Some(Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true)); + + self.save() + } + + /// Written only after an update's files and mcp entries land, so an + /// aborted update cannot leave the record claiming a commit whose content + /// never finished applying. + pub(crate) fn set_bundle_versions( + &mut self, + name: &str, + commit: &str, + version: Option, + ) -> Result<()> { + self.ensure_bundle_exists(name)?; + let record = self + .bundles + .get_mut(name) + .expect("bundle existence checked above"); + + record.commit = commit.to_string(); + record.version = version; + + self.save() + } + + pub(crate) fn remove_file_record(&mut self, bundle: &str, path: &str) -> Result<()> { + self.ensure_bundle_exists(bundle)?; + let record = self + .bundles + .get_mut(bundle) + .expect("bundle existence checked above"); + + record.files.retain(|owned| owned.path != path); + + self.save() + } + + pub(crate) fn remove_mcp_record(&mut self, bundle: &str, effective_key: &str) -> Result<()> { + self.ensure_bundle_exists(bundle)?; + let record = self + .bundles + .get_mut(bundle) + .expect("bundle existence checked above"); + + record + .mcp_servers + .retain(|owned| owned.effective_key() != effective_key); + + self.save() + } + + pub(crate) fn remove_bundle(&mut self, name: &str) -> Result<()> { + self.ensure_bundle_exists(name)?; + self.bundles.remove(name); + self.save() + } + + /// Persists per call, so an install aborted partway through still has + /// provenance for everything already on disk. + /// A path owned by another bundle transfers to `bundle`. + pub(crate) fn record_file(&mut self, bundle: &str, file: FileRecord) -> Result<()> { + self.ensure_bundle_exists(bundle)?; + for (name, record) in self.bundles.iter_mut() { + if name != bundle { + record.files.retain(|owned| owned.path != file.path); + } + } + let record = self + .bundles + .get_mut(bundle) + .expect("bundle existence checked above"); + + record.files.retain(|owned| owned.path != file.path); + record.files.push(file); + + self.save() + } + + /// An entry whose key any bundle already owns transfers to `bundle`, and + /// its `replaced` action upgrades to `transferred` only when the prior + /// record proves bundle origin. A prior `replaced` record marks a + /// pre-existing user entry that uninstall must never delete, and that + /// marker survives re-records and cross-bundle transfers. + pub(crate) fn record_mcp_servers( + &mut self, + bundle: &str, + entries: Vec, + ) -> Result<()> { + self.ensure_bundle_exists(bundle)?; + for mut entry in entries { + let key = entry.effective_key().to_string(); + let mut previously_owned = false; + let mut prior_user_origin = false; + for record in self.bundles.values_mut() { + let before = record.mcp_servers.len(); + record.mcp_servers.retain(|owned| { + if owned.effective_key() != key { + return true; + } + + if owned.action == McpAction::Replaced { + prior_user_origin = true; + } + + false + }); + previously_owned |= record.mcp_servers.len() != before; + } + + if previously_owned && !prior_user_origin && entry.action == McpAction::Replaced { + entry.action = McpAction::Transferred; + } + + self.bundles + .get_mut(bundle) + .expect("bundle existence checked above") + .mcp_servers + .push(entry); + } + + self.save() + } + + fn ensure_bundle_exists(&self, bundle: &str) -> Result<()> { + if !self.bundles.contains_key(bundle) { + bail!( + "no installed bundle named '{bundle}' (installed: {})", + if self.bundles.is_empty() { + "none".to_string() + } else { + self.bundle_names().join(", ") + } + ); + } + + Ok(()) + } + + fn source_of_other_bundle(&self, name: &str, canonical: &str) -> Option { + self.bundles + .get(name) + .filter(|record| canonical_source_url(&record.source) != canonical) + .map(|record| record.source.clone()) + } +} + +pub(crate) fn hash_bytes(bytes: &[u8]) -> String { + let mut hasher = Sha256::new(); + hasher.update(bytes); + format!("{:x}", hasher.finalize()) +} + +pub(crate) fn hash_file(path: &Path) -> Result { + let bytes = + fs::read(path).with_context(|| format!("failed to read {} for hashing", path.display()))?; + Ok(hash_bytes(&bytes)) +} + +#[derive(Debug, Default, PartialEq, Eq)] +pub(crate) struct DriftSummary { + pub(crate) intact: usize, + pub(crate) modified: usize, + pub(crate) missing: usize, +} + +impl DriftSummary { + pub(crate) fn display(&self) -> String { + if self.intact + self.modified + self.missing == 0 { + return "-".to_string(); + } + let mut parts = Vec::new(); + + if self.intact > 0 { + parts.push(format!("{} intact", self.intact)); + } + + if self.modified > 0 { + parts.push(format!("{} modified locally", self.modified)); + } + + if self.missing > 0 { + parts.push(format!("{} missing", self.missing)); + } + + parts.join(", ") + } +} + +#[derive(Debug)] +pub(crate) struct BundleListRow { + pub(crate) name: String, + pub(crate) version: String, + pub(crate) source: String, + pub(crate) git_ref: String, + pub(crate) installed_at: String, + pub(crate) file_counts: String, + pub(crate) drift: DriftSummary, +} + +/// An unreadable file counts as locally modified: it exists but its integrity +/// cannot be verified. +pub(crate) fn bundle_list_rows(store: &BundleStore, config_dir: &Path) -> Vec { + store + .iter() + .map(|(name, record)| { + let mut counts: BTreeMap<&str, usize> = BTreeMap::new(); + let mut drift = DriftSummary::default(); + for file in &record.files { + *counts.entry(file.category.as_str()).or_default() += 1; + let path = config_dir.join(&file.path); + if !path.exists() { + drift.missing += 1; + } else { + match hash_file(&path) { + Ok(hash) if hash == file.sha256 => drift.intact += 1, + _ => drift.modified += 1, + } + } + } + let file_counts = if counts.is_empty() { + "-".to_string() + } else { + counts + .iter() + .map(|(category, count)| format!("{category}: {count}")) + .collect::>() + .join(", ") + }; + BundleListRow { + name: name.to_string(), + version: record + .version + .clone() + .unwrap_or_else(|| record.commit.chars().take(7).collect()), + source: record.source.clone(), + git_ref: record.git_ref.clone().unwrap_or_else(|| "-".to_string()), + installed_at: record.installed_at.clone(), + file_counts, + drift, + } + }) + .collect() +} + +pub fn list_installed_bundles() -> Result<()> { + let store = BundleStore::load()?; + let rows = bundle_list_rows(&store, &paths::config_dir()); + if rows.is_empty() { + println!("No bundles installed. Install one with `coyote --install `."); + return Ok(()); + } + + let mut table = request_context::asset_table(&[ + "name", + "version", + "source", + "ref", + "installed", + "files", + "drift", + ]); + for row in rows { + table.add_row(vec![ + row.name.as_str(), + &row.version, + &row.source, + &row.git_ref, + &row.installed_at, + &row.file_counts, + &row.drift.display(), + ]); + } + + println!("Bundles:"); + println!("{table}"); + Ok(()) +} + +fn sanitize_name_segment(segment: &str) -> String { + segment + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '-' + } + }) + .collect::() + .trim_matches('-') + .to_string() +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::utils::{get_env_name, temp_file}; + use serial_test::serial; + use std::env; + use std::ffi::OsString; + + struct TempStoreDir(PathBuf); + + impl TempStoreDir { + fn new(label: &str) -> Self { + let dir = temp_file(label, ""); + fs::create_dir_all(&dir).unwrap(); + Self(dir) + } + + fn store_path(&self) -> PathBuf { + self.0.join("installed-bundles.yaml") + } + + fn store(&self) -> BundleStore { + BundleStore::load_from(self.store_path()).unwrap() + } + } + + impl Drop for TempStoreDir { + fn drop(&mut self) { + let _ = fs::remove_dir_all(&self.0); + } + } + + fn metadata(source: &str, commit: &str) -> InstallMetadata { + InstallMetadata { + source: source.to_string(), + git_ref: None, + commit: commit.to_string(), + version: None, + description: None, + homepage: None, + } + } + + fn file_record(path: &str, contents: &str) -> FileRecord { + FileRecord { + path: path.to_string(), + category: "macros".to_string(), + sha256: hash_bytes(contents.as_bytes()), + action: FileAction::New, + } + } + + fn mcp_record(name: &str, action: McpAction, renamed_to: Option<&str>) -> McpServerRecord { + McpServerRecord { + name: name.to_string(), + action, + renamed_to: renamed_to.map(str::to_string), + sha256: None, + } + } + + #[test] + fn load_missing_file_yields_empty_store() { + let dir = TempStoreDir::new("bundles-empty"); + + let store = dir.store(); + + assert!(store.bundle_names().is_empty()); + } + + #[test] + fn corrupt_store_fails_closed() { + let dir = TempStoreDir::new("bundles-corrupt"); + fs::write(dir.store_path(), "bundles:\n - this is not a map\n").unwrap(); + + let result = BundleStore::load_from(dir.store_path()); + + let message = format!("{:#}", result.unwrap_err()); + assert!( + message.contains("refusing to treat it as empty"), + "{message}" + ); + } + + #[test] + fn save_and_reload_roundtrip() { + let dir = TempStoreDir::new("bundles-roundtrip"); + let mut store = dir.store(); + store + .upsert_bundle( + "omc", + InstallMetadata { + source: "https://github.com/x/omc".to_string(), + git_ref: Some("main".to_string()), + commit: "abc123".to_string(), + version: Some("1.4.0".to_string()), + description: Some("Opinionated roles and macros".to_string()), + homepage: Some("https://github.com/x/omc".to_string()), + }, + ) + .unwrap(); + store + .record_file("omc", file_record("macros/a.yaml", "a")) + .unwrap(); + store + .record_mcp_servers("omc", vec![mcp_record("srv", McpAction::Added, None)]) + .unwrap(); + + let raw = fs::read_to_string(dir.store_path()).unwrap(); + let reloaded = dir.store(); + + assert!(raw.contains("ref: main"), "{raw}"); + assert!(!raw.contains("git_ref"), "{raw}"); + assert!(raw.contains("action: added"), "{raw}"); + let record = reloaded.get("omc").unwrap(); + assert_eq!(record.git_ref.as_deref(), Some("main")); + assert_eq!(record.version.as_deref(), Some("1.4.0")); + assert_eq!( + record.description.as_deref(), + Some("Opinionated roles and macros") + ); + assert_eq!(record.homepage.as_deref(), Some("https://github.com/x/omc")); + assert_eq!(record.files.len(), 1); + assert_eq!(record.mcp_servers.len(), 1); + } + + #[test] + fn files_recorded_before_an_abort_survive_it() { + let dir = TempStoreDir::new("bundles-abort"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + store + .record_file("omc", file_record("macros/a.yaml", "a")) + .unwrap(); + store + .record_file("omc", file_record("skills/b.md", "b")) + .unwrap(); + drop(store); + + let reloaded = dir.store(); + let paths: Vec<&str> = reloaded + .get("omc") + .unwrap() + .files + .iter() + .map(|f| f.path.as_str()) + .collect(); + assert_eq!(paths, vec!["macros/a.yaml", "skills/b.md"]); + } + + #[test] + fn recording_the_same_path_twice_updates_in_place() { + let dir = TempStoreDir::new("bundles-dedupe"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + store + .record_file("omc", file_record("macros/a.yaml", "v1")) + .unwrap(); + let mut updated = file_record("macros/a.yaml", "v2"); + updated.action = FileAction::Replaced; + store.record_file("omc", updated).unwrap(); + + let record = store.get("omc").unwrap(); + assert_eq!(record.files.len(), 1); + assert_eq!(record.files[0].sha256, hash_bytes(b"v2")); + assert_eq!(record.files[0].action, FileAction::Replaced); + } + + #[test] + fn overwritten_file_transfers_ownership() { + let dir = TempStoreDir::new("bundles-transfer"); + let mut store = dir.store(); + store + .upsert_bundle("alpha", metadata("https://github.com/a/alpha", "abc123")) + .unwrap(); + store + .upsert_bundle("beta", metadata("https://github.com/b/beta", "def456")) + .unwrap(); + store + .record_file("alpha", file_record("macros/shared.yaml", "a")) + .unwrap(); + + let mut taken = file_record("macros/shared.yaml", "b"); + taken.action = FileAction::Replaced; + store.record_file("beta", taken).unwrap(); + + let reloaded = dir.store(); + assert!(reloaded.get("alpha").unwrap().files.is_empty()); + let beta_files = &reloaded.get("beta").unwrap().files; + assert_eq!(beta_files.len(), 1); + assert_eq!(beta_files[0].path, "macros/shared.yaml"); + } + + #[test] + fn overwritten_mcp_entry_transfers_ownership_as_transferred() { + let dir = TempStoreDir::new("bundles-mcp-transfer"); + let mut store = dir.store(); + store + .upsert_bundle("alpha", metadata("https://github.com/a/alpha", "abc123")) + .unwrap(); + store + .upsert_bundle("beta", metadata("https://github.com/b/beta", "def456")) + .unwrap(); + store + .record_mcp_servers("alpha", vec![mcp_record("srv", McpAction::Added, None)]) + .unwrap(); + + store + .record_mcp_servers("beta", vec![mcp_record("srv", McpAction::Replaced, None)]) + .unwrap(); + + let reloaded = dir.store(); + assert!(reloaded.get("alpha").unwrap().mcp_servers.is_empty()); + let beta_servers = &reloaded.get("beta").unwrap().mcp_servers; + assert_eq!(beta_servers.len(), 1); + assert_eq!(beta_servers[0].action, McpAction::Transferred); + } + + #[test] + fn mcp_replacement_of_unowned_entry_stays_replaced() { + let dir = TempStoreDir::new("bundles-mcp-replaced"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + store + .record_mcp_servers( + "omc", + vec![mcp_record("user-srv", McpAction::Replaced, None)], + ) + .unwrap(); + + assert_eq!( + store.get("omc").unwrap().mcp_servers[0].action, + McpAction::Replaced + ); + } + + #[test] + fn mcp_rerecord_of_own_entry_upgrades_replaced_to_transferred() { + let dir = TempStoreDir::new("bundles-mcp-self-update"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_mcp_servers("omc", vec![mcp_record("srv", McpAction::Added, None)]) + .unwrap(); + + let mut updated = mcp_record("srv", McpAction::Replaced, None); + updated.sha256 = Some("deadbeef".to_string()); + store.record_mcp_servers("omc", vec![updated]).unwrap(); + + let servers = &store.get("omc").unwrap().mcp_servers; + assert_eq!(servers.len(), 1); + assert_eq!(servers[0].action, McpAction::Transferred); + assert_eq!(servers[0].sha256.as_deref(), Some("deadbeef")); + } + + #[test] + fn mcp_rerecord_of_own_replaced_entry_stays_replaced() { + let dir = TempStoreDir::new("bundles-mcp-sticky-self"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_mcp_servers( + "omc", + vec![mcp_record("user-srv", McpAction::Replaced, None)], + ) + .unwrap(); + + store + .record_mcp_servers( + "omc", + vec![mcp_record("user-srv", McpAction::Replaced, None)], + ) + .unwrap(); + + assert_eq!( + store.get("omc").unwrap().mcp_servers[0].action, + McpAction::Replaced + ); + } + + #[test] + fn mcp_transfer_of_replaced_entry_keeps_user_origin_marker() { + let dir = TempStoreDir::new("bundles-mcp-sticky-transfer"); + let mut store = dir.store(); + store + .upsert_bundle("alpha", metadata("https://github.com/a/alpha", "abc123")) + .unwrap(); + store + .upsert_bundle("beta", metadata("https://github.com/b/beta", "def456")) + .unwrap(); + store + .record_mcp_servers( + "alpha", + vec![mcp_record("user-srv", McpAction::Replaced, None)], + ) + .unwrap(); + + store + .record_mcp_servers( + "beta", + vec![mcp_record("user-srv", McpAction::Replaced, None)], + ) + .unwrap(); + + assert!(store.get("alpha").unwrap().mcp_servers.is_empty()); + assert_eq!( + store.get("beta").unwrap().mcp_servers[0].action, + McpAction::Replaced + ); + } + + #[test] + fn mcp_transfer_matches_renamed_entries_by_effective_key() { + let dir = TempStoreDir::new("bundles-mcp-renamed"); + let mut store = dir.store(); + store + .upsert_bundle("alpha", metadata("https://github.com/a/alpha", "abc123")) + .unwrap(); + store + .upsert_bundle("beta", metadata("https://github.com/b/beta", "def456")) + .unwrap(); + store + .record_mcp_servers( + "alpha", + vec![mcp_record("srv", McpAction::Renamed, Some("srv-remote"))], + ) + .unwrap(); + + store + .record_mcp_servers( + "beta", + vec![mcp_record("srv-remote", McpAction::Replaced, None)], + ) + .unwrap(); + + let reloaded = dir.store(); + assert!(reloaded.get("alpha").unwrap().mcp_servers.is_empty()); + assert_eq!( + reloaded.get("beta").unwrap().mcp_servers[0].action, + McpAction::Transferred + ); + } + + #[test] + fn resolve_same_url_same_name_is_an_update() { + let dir = TempStoreDir::new("bundles-resolve-update"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + let resolved = store + .resolve_bundle_name("git@github.com:x/omc.git", None) + .unwrap(); + + assert_eq!(resolved.name, "omc"); + assert_eq!(resolved.qualified_from, None); + } + + #[test] + fn resolve_migrates_record_when_identity_changes() { + let dir = TempStoreDir::new("bundles-migrate"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_file("omc", file_record("macros/a.yaml", "a")) + .unwrap(); + + let resolved = store + .resolve_bundle_name("git@github.com:x/omc.git", Some("oh-my-coyote")) + .unwrap(); + + assert_eq!(resolved.name, "oh-my-coyote"); + let reloaded = dir.store(); + assert!(reloaded.get("omc").is_none()); + assert_eq!(reloaded.get("oh-my-coyote").unwrap().files.len(), 1); + } + + #[test] + fn resolve_qualifies_colliding_name_from_https_source() { + let dir = TempStoreDir::new("bundles-qualify-https"); + let mut store = dir.store(); + store + .upsert_bundle("repo", metadata("https://github.com/a/repo", "abc123")) + .unwrap(); + + let resolved = store + .resolve_bundle_name("https://gitlab.com/b/repo.git", None) + .unwrap(); + + assert_eq!(resolved.name, "b/repo"); + assert_eq!(resolved.qualified_from.as_deref(), Some("repo")); + assert_eq!( + resolved.same_name_other_source.as_deref(), + Some("https://github.com/a/repo") + ); + } + + #[test] + fn resolve_qualifies_colliding_name_from_scp_source() { + let dir = TempStoreDir::new("bundles-qualify-scp"); + let mut store = dir.store(); + store + .upsert_bundle("repo", metadata("https://github.com/a/repo", "abc123")) + .unwrap(); + + let resolved = store + .resolve_bundle_name("git@bitbucket.org:c/repo.git", None) + .unwrap(); + + assert_eq!(resolved.name, "c/repo"); + assert_eq!(resolved.qualified_from.as_deref(), Some("repo")); + } + + #[test] + fn resolve_of_already_qualified_record_is_stable() { + let dir = TempStoreDir::new("bundles-qualify-stable"); + let mut store = dir.store(); + store + .upsert_bundle("repo", metadata("https://github.com/a/repo", "abc123")) + .unwrap(); + store + .upsert_bundle("b/repo", metadata("https://gitlab.com/b/repo", "def456")) + .unwrap(); + + let resolved = store + .resolve_bundle_name("https://gitlab.com/b/repo.git", None) + .unwrap(); + + assert_eq!(resolved.name, "b/repo"); + assert_eq!(resolved.qualified_from.as_deref(), Some("repo")); + } + + #[test] + fn resolve_sanitizes_derived_names() { + let dir = TempStoreDir::new("bundles-sanitize"); + let mut store = dir.store(); + + let resolved = store + .resolve_bundle_name("https://github.com/vercel/next.js.git", None) + .unwrap(); + + assert_eq!(resolved.name, "next-js"); + } + + #[test] + fn resolve_reserves_asset_category_names() { + let dir = TempStoreDir::new("bundles-reserved"); + let mut store = dir.store(); + + let resolved = store + .resolve_bundle_name("https://github.com/x/agents", None) + .unwrap(); + + assert_eq!(resolved.name, "x/agents"); + assert_eq!(resolved.qualified_from.as_deref(), Some("agents")); + assert!(resolved.same_name_other_source.is_none()); + + let resolved = store + .resolve_bundle_name("https://github.com/y/repo", Some("mcp_config")) + .unwrap(); + + assert_eq!(resolved.name, "y/mcp_config"); + assert_eq!(resolved.qualified_from.as_deref(), Some("mcp_config")); + } + + #[test] + fn resolve_rejects_invalid_manifest_names() { + let dir = TempStoreDir::new("bundles-invalid-name"); + let mut store = dir.store(); + + let result = store.resolve_bundle_name("https://github.com/x/repo", Some("a/b/c")); + + assert!(result.is_err()); + } + + #[test] + fn upsert_merges_metadata_and_preserves_files_and_install_time() { + let dir = TempStoreDir::new("bundles-merge"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_file("omc", file_record("macros/a.yaml", "a")) + .unwrap(); + let installed_at = store.get("omc").unwrap().installed_at.clone(); + + store + .upsert_bundle("omc", metadata("https://github.com/x/omc.git", "def456")) + .unwrap(); + + let record = store.get("omc").unwrap(); + assert_eq!(record.commit, "def456"); + assert_eq!(record.source, "https://github.com/x/omc.git"); + assert_eq!(record.installed_at, installed_at); + assert_eq!(record.files.len(), 1); + } + + #[test] + fn recording_against_unknown_bundle_fails() { + let dir = TempStoreDir::new("bundles-unknown"); + let mut store = dir.store(); + + let result = store.record_file("ghost", file_record("macros/a.yaml", "a")); + + assert!(result.unwrap_err().to_string().contains("ghost")); + } + + #[test] + fn mark_updated_stamps_and_round_trips() { + let dir = TempStoreDir::new("bundles-mark-updated"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + let raw_before = fs::read_to_string(dir.store_path()).unwrap(); + assert!(!raw_before.contains("updated_at"), "{raw_before}"); + assert_eq!(store.get("omc").unwrap().updated_at, None); + + store.mark_updated("omc").unwrap(); + + let raw_after = fs::read_to_string(dir.store_path()).unwrap(); + assert!(raw_after.contains("updated_at"), "{raw_after}"); + let reloaded = dir.store(); + assert!(reloaded.get("omc").unwrap().updated_at.is_some()); + } + + #[test] + fn mark_updated_unknown_bundle_fails() { + let dir = TempStoreDir::new("bundles-mark-unknown"); + let mut store = dir.store(); + + let result = store.mark_updated("ghost"); + + assert!(result.unwrap_err().to_string().contains("ghost")); + } + + #[test] + fn remove_file_record_drops_only_the_named_path() { + let dir = TempStoreDir::new("bundles-remove-file"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_file("omc", file_record("macros/a.yaml", "a")) + .unwrap(); + store + .record_file("omc", file_record("macros/b.yaml", "b")) + .unwrap(); + + store.remove_file_record("omc", "macros/a.yaml").unwrap(); + + let reloaded = dir.store(); + let paths: Vec<&str> = reloaded + .get("omc") + .unwrap() + .files + .iter() + .map(|f| f.path.as_str()) + .collect(); + assert_eq!(paths, vec!["macros/b.yaml"]); + } + + #[test] + fn remove_file_record_unknown_bundle_fails() { + let dir = TempStoreDir::new("bundles-remove-unknown"); + let mut store = dir.store(); + + let result = store.remove_file_record("ghost", "macros/a.yaml"); + + assert!(result.unwrap_err().to_string().contains("ghost")); + } + + #[test] + fn remove_mcp_record_drops_only_the_named_effective_key() { + let dir = TempStoreDir::new("bundles-remove-mcp"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + store + .record_mcp_servers( + "omc", + vec![ + mcp_record("srv", McpAction::Renamed, Some("srv-remote")), + mcp_record("other", McpAction::Added, None), + ], + ) + .unwrap(); + + store.remove_mcp_record("omc", "srv-remote").unwrap(); + + let reloaded = dir.store(); + let keys: Vec<&str> = reloaded + .get("omc") + .unwrap() + .mcp_servers + .iter() + .map(|s| s.effective_key()) + .collect(); + assert_eq!(keys, vec!["other"]); + } + + #[test] + fn remove_bundle_deletes_the_record_and_persists() { + let dir = TempStoreDir::new("bundles-remove-bundle"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + store.remove_bundle("omc").unwrap(); + + let reloaded = dir.store(); + assert!(reloaded.get("omc").is_none()); + assert!(reloaded.bundle_names().is_empty()); + } + + #[test] + fn remove_bundle_unknown_name_fails() { + let dir = TempStoreDir::new("bundles-remove-bundle-unknown"); + let mut store = dir.store(); + + let result = store.remove_bundle("ghost"); + + assert!(result.unwrap_err().to_string().contains("ghost")); + } + + #[cfg(unix)] + #[test] + fn failed_save_preserves_the_existing_store() { + use std::os::unix::fs::PermissionsExt; + + let dir = TempStoreDir::new("bundles-atomic"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + let before = fs::read_to_string(dir.store_path()).unwrap(); + + fs::set_permissions(&dir.0, fs::Permissions::from_mode(0o555)).unwrap(); + let probe = dir.0.join(".write-probe"); + if fs::write(&probe, "x").is_ok() { + // A privileged user bypasses permission bits; the failure path + // cannot be provoked this way. + let _ = fs::remove_file(&probe); + fs::set_permissions(&dir.0, fs::Permissions::from_mode(0o755)).unwrap(); + return; + } + + let result = store.record_file("omc", file_record("macros/a.yaml", "a")); + fs::set_permissions(&dir.0, fs::Permissions::from_mode(0o755)).unwrap(); + + assert!(result.is_err()); + assert_eq!(fs::read_to_string(dir.store_path()).unwrap(), before); + let reloaded = dir.store(); + assert!(reloaded.get("omc").unwrap().files.is_empty()); + } + + #[test] + fn hash_helpers_are_stable() { + let dir = TempStoreDir::new("bundles-hash"); + let path = dir.0.join("artifact.yaml"); + fs::write(&path, "hello").unwrap(); + + assert_eq!( + hash_bytes(b"hello"), + "2cf24dba5fb0a30e26e83b2ac5b9e29e1b161e5c1fa7425e73043362938b9824" + ); + assert_eq!(hash_file(&path).unwrap(), hash_bytes(b"hello")); + assert_ne!(hash_bytes(b"hello"), hash_bytes(b"hello!")); + } + + #[test] + #[serial] + fn default_store_path_follows_the_config_dir() { + let dir = TempStoreDir::new("bundles-env"); + let key = get_env_name("config_dir"); + let previous: Option = env::var_os(&key); + unsafe { + env::set_var(&key, &dir.0); + } + + let result = (|| -> Result<()> { + let mut store = BundleStore::load()?; + assert!(store.bundle_names().is_empty()); + store.upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123"))?; + let reloaded = BundleStore::load()?; + assert!(reloaded.get("omc").is_some()); + assert!(dir.store_path().is_file()); + Ok(()) + })(); + + unsafe { + match &previous { + Some(value) => env::set_var(&key, value), + None => env::remove_var(&key), + } + } + result.unwrap(); + } + + #[test] + fn bundle_rows_classify_drift_per_file() { + let dir = TempStoreDir::new("bundles-list-drift"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123")) + .unwrap(); + + fs::create_dir_all(dir.0.join("macros")).unwrap(); + fs::write(dir.0.join("macros/intact.yaml"), "a").unwrap(); + store + .record_file("omc", file_record("macros/intact.yaml", "a")) + .unwrap(); + + fs::create_dir_all(dir.0.join("skills")).unwrap(); + fs::write(dir.0.join("skills/modified.md"), "changed").unwrap(); + let mut modified = file_record("skills/modified.md", "original"); + modified.category = "skills".to_string(); + store.record_file("omc", modified).unwrap(); + + let mut missing = file_record("roles/missing.md", "gone"); + missing.category = "roles".to_string(); + store.record_file("omc", missing).unwrap(); + + let rows = bundle_list_rows(&store, &dir.0); + + assert_eq!(rows.len(), 1); + let row = &rows[0]; + assert_eq!(row.name, "omc"); + assert_eq!(row.source, "https://github.com/x/omc"); + assert_eq!( + row.drift, + DriftSummary { + intact: 1, + modified: 1, + missing: 1, + } + ); + assert_eq!(row.file_counts, "macros: 1, roles: 1, skills: 1"); + assert_eq!( + row.drift.display(), + "1 intact, 1 modified locally, 1 missing" + ); + } + + #[test] + fn bundle_rows_fall_back_to_the_short_commit_when_unversioned() { + let dir = TempStoreDir::new("bundles-list-fallback"); + let mut store = dir.store(); + store + .upsert_bundle("omc", metadata("https://github.com/x/omc", "abc123def456")) + .unwrap(); + + let rows = bundle_list_rows(&store, &dir.0); + + assert_eq!(rows[0].version, "abc123d"); + assert_eq!(rows[0].git_ref, "-"); + assert_eq!(rows[0].file_counts, "-"); + assert_eq!(rows[0].drift, DriftSummary::default()); + assert_eq!(rows[0].drift.display(), "-"); + } + + #[test] + fn bundle_rows_show_manifest_version_and_pinned_ref() { + let dir = TempStoreDir::new("bundles-list-versioned"); + let mut store = dir.store(); + store + .upsert_bundle( + "omc", + InstallMetadata { + source: "git@github.com:x/omc.git".to_string(), + git_ref: Some("v1.4.0".to_string()), + commit: "abc123def456".to_string(), + version: Some("1.4.0".to_string()), + description: None, + homepage: None, + }, + ) + .unwrap(); + + let rows = bundle_list_rows(&store, &dir.0); + + assert_eq!(rows[0].version, "1.4.0"); + assert_eq!(rows[0].git_ref, "v1.4.0"); + assert_eq!(rows[0].source, "git@github.com:x/omc.git"); + assert!(!rows[0].installed_at.is_empty()); + } + + #[test] + fn bundle_rows_are_empty_for_an_empty_store() { + let dir = TempStoreDir::new("bundles-list-empty"); + + let rows = bundle_list_rows(&dir.store(), &dir.0); + + assert!(rows.is_empty()); + } +} diff --git a/src/config/install_remote.rs b/src/config/install_remote.rs index f5a8eaf..c590e9e 100644 --- a/src/config/install_remote.rs +++ b/src/config/install_remote.rs @@ -1,24 +1,32 @@ -use crate::config::{InstallFilter, paths}; +use super::bundles::{ + BundleStore, FileAction, FileRecord, InstallMetadata, McpAction, McpServerRecord, hash_bytes, + hash_file, +}; +use crate::config::{AssetCategory, BUNDLE_MANIFEST_FILE, InstallFilter, paths}; #[cfg(not(windows))] use crate::function::Language; use crate::mcp::{McpServer, McpServersConfig}; use crate::utils; use crate::utils::IS_STDOUT_TERMINAL; -use crate::vault::{Vault, create_vault_password_file, interpolate_secrets}; +use crate::vault::{SECRET_RE, Vault, create_vault_password_file, interpolate_secrets}; use anyhow::{Context, Result, anyhow, bail}; +use clap::ValueEnum; use indexmap::IndexMap; use indoc::formatdoc; use inquire::{Confirm, Select}; +use serde::Deserialize; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::ffi::{OsStr, OsString}; -use std::fs; -use std::path::{Path, PathBuf}; +use std::path::{Component, Path, PathBuf}; +use std::{fs, iter}; pub fn install_remote(git_url: &str, filter: Option, force: bool) -> Result<()> { let (url, reference) = parse_url_with_ref(git_url)?; let temp = clone_to_temp(&url, reference.as_deref())?; println!("Cloned {git_url} to {}", temp.path().display()); - let layout = scan_remote_layout(temp.path())?; + let mut layout = scan_remote_layout(temp.path())?; + layout.head_sha = Some(temp.head_sha().to_string()); let layout = apply_filter(layout, filter); if layout.is_empty() { @@ -29,16 +37,33 @@ pub fn install_remote(git_url: &str, filter: Option, force: bool) return Ok(()); } + let mut store = BundleStore::load()?; + let bundle = register_bundle( + &mut store, + &url, + reference.as_deref(), + layout.manifest.as_ref(), + temp.head_sha(), + false, + )?; + let plan = plan_changes(&layout)?; + let plan = reclassify_owned_unmodified(plan, &store, &bundle)?; if !plan.files.is_empty() { print_plan_summary(&plan); - apply_plan(&plan, force)?; + let sticky = if force { + StickyMode::ReplaceAll + } else { + StickyMode::None + }; + apply_plan(&plan, sticky, &mut store, &bundle)?; } if let Some((remote_mcp, local_mcp)) = &plan.mcp_json { let local = local_mcp.exists().then_some(local_mcp.as_path()); - let report = merge_mcp_json(local, remote_mcp, local_mcp, force)?; + let report = merge_mcp_json(local, remote_mcp, local_mcp, force, &HashSet::new(), false)?; + record_mcp_merge(&mut store, &bundle, &report)?; print_mcp_merge_report(&report); handle_missing_secrets(&report.missing_secrets)?; } @@ -46,20 +71,13 @@ pub fn install_remote(git_url: &str, filter: Option, force: bool) Ok(()) } -pub fn install_remote_from_repl_args(args: &str) -> Result<()> { - let tokens = shell_words::split(args) - .with_context(|| format!("failed to parse '.install remote' args: {args}"))?; - - let mut iter = tokens.into_iter(); - let url = iter.next().with_context(|| { - format!( - "Usage: .install remote [--filter <{}>] [--force]", - InstallFilter::NAMES.join("|") - ) - })?; - +fn parse_repl_install_flags( + command: &str, + mut iter: impl Iterator, +) -> Result<(Option, bool, Option)> { let mut filter: Option = None; let mut force = false; + let mut git_host: Option = None; while let Some(tok) = iter.next() { match tok.as_str() { @@ -76,11 +94,897 @@ pub fn install_remote_from_repl_args(args: &str) -> Result<()> { s if s.starts_with("--filter=") => { filter = Some(parse_filter(&s["--filter=".len()..])?); } - other => bail!("Unexpected argument to '.install remote': {other}"), + "--git-host" => { + let val = iter + .next() + .with_context(|| "--git-host requires a value (e.g. git.somedomain.com)")?; + git_host = Some(val); + } + s if s.starts_with("--git-host=") => { + git_host = Some(s["--git-host=".len()..].to_string()); + } + other => bail!("Unexpected argument to '{command}': {other}"), } } - install_remote(&url, filter, force) + Ok((filter, force, git_host)) +} + +#[derive(Debug, Clone, PartialEq)] +enum InstallTarget { + Category(AssetCategory), + InstalledBundle, + RemoteSource, + Shorthand, + Unknown, +} + +fn classify_install_target(value: &str, installed_names: &[String]) -> InstallTarget { + if let Some(category) = AssetCategory::parse(value) { + return InstallTarget::Category(category); + } + + let name = strip_ref_suffix(value); + + if installed_names.iter().any(|installed| installed == name) { + return InstallTarget::InstalledBundle; + } + + if looks_like_remote_source(value) { + return InstallTarget::RemoteSource; + } + + if is_repo_shorthand(value) { + return InstallTarget::Shorthand; + } + + InstallTarget::Unknown +} + +fn looks_like_remote_source(value: &str) -> bool { + if value.contains("://") + || value.starts_with("./") + || value.starts_with("../") + || value.starts_with('/') + || value.starts_with('~') + { + return true; + } + + match value.split_once(':') { + Some((host, _)) => { + !host.is_empty() && !host.contains('/') && !host.chars().any(char::is_whitespace) + } + None => false, + } +} + +pub(crate) const DEFAULT_GIT_HOST: &str = "github.com"; + +fn is_repo_shorthand(value: &str) -> bool { + let path = strip_ref_suffix(value); + if path.contains("://") + || path.contains(':') + || path.contains('\\') + || path.contains(char::is_whitespace) + || path.starts_with(['/', '~', '.', '-']) + { + return false; + } + + let mut segments = path.split('/'); + segments.clone().count() >= 2 && segments.all(|segment| !segment.is_empty()) +} + +fn expand_repo_shorthand(value: &str, git_host: Option<&str>) -> Result { + let raw = git_host.unwrap_or(DEFAULT_GIT_HOST); + let host = raw + .strip_prefix("https://") + .or_else(|| raw.strip_prefix("http://")) + .unwrap_or(raw) + .trim_matches('/'); + + if host.is_empty() || host.contains(['/', '#', '?']) || host.chars().any(char::is_whitespace) { + bail!("invalid --git-host '{raw}': expected a bare host like git.somedomain.com"); + } + + Ok(format!("https://{host}/{value}")) +} + +pub fn install_or_update( + value: &str, + git_host: Option<&str>, + filter: Option, + force: bool, +) -> Result<()> { + if let Some(host) = git_host { + if !is_repo_shorthand(value) { + bail!( + "--git-host only applies to / shorthand values; \ + '{value}' is not one" + ); + } + + let url = expand_repo_shorthand(value, Some(host))?; + println!("Resolved '{value}' to '{url}'"); + return install_remote(&url, filter, force); + } + + let store = BundleStore::load()?; + let installed: Vec = store + .bundle_names() + .into_iter() + .map(str::to_string) + .collect(); + + match classify_install_target(value, &installed) { + InstallTarget::Category(category) => { + let name = category + .to_possible_value() + .map_or_else(|| value.to_string(), |v| v.get_name().to_string()); + let update_hint = if installed.iter().any(|installed| installed == value) { + format!(" To update the installed bundle '{value}', use `--update-bundle {value}`.") + } else { + String::new() + }; + bail!( + "'{value}' is an asset category, not a bundle; did you mean \ + `--install-builtins {name}`? (categories are reinstalled from \ + the assets built into coyote, not from a remote){update_hint}" + ); + } + InstallTarget::InstalledBundle => { + if filter.is_some() || force { + bail!("--filter/--install-force only apply to remote installs, not bundle updates"); + } + update_bundle(value, false) + } + InstallTarget::RemoteSource => install_remote(value, filter, force), + InstallTarget::Shorthand => { + let url = expand_repo_shorthand(value, None)?; + println!("Resolved '{value}' to '{url}'"); + install_remote(&url, filter, force) + } + InstallTarget::Unknown => { + let hint = "a remote source must be a git URL, an / shorthand \ + (expanded against --git-host, default github.com), an scp-style \ + host:path, or an explicit local path (./dir, /abs, ~)"; + + if installed.is_empty() { + bail!("no bundle named '{value}' is installed; none are installed ({hint})"); + } + + bail!( + "no bundle named '{value}' is installed; installed bundles: {} ({hint})", + installed.join(", ") + ) + } + } +} + +pub fn install_or_update_from_repl_args(args: &str) -> Result<()> { + let tokens = shell_words::split(args) + .with_context(|| format!("failed to parse '.install' args: {args}"))?; + + let mut iter = tokens.into_iter(); + let value = iter.next().with_context(|| { + format!( + "Usage: .install \ + [--git-host ] [--filter <{}>] [--force]", + InstallFilter::NAMES.join("|") + ) + })?; + + let (filter, force, git_host) = parse_repl_install_flags(".install", iter)?; + install_or_update(&value, git_host.as_deref(), filter, force) +} + +/// The whole remote is always processed, including categories a filtered +/// install excluded, because filtered installs merge into a single record. +pub fn update_bundle(spec: &str, assume_yes: bool) -> Result<()> { + let (name, ref_override) = parse_url_with_ref(spec)?; + + let mut store = BundleStore::load()?; + let Some(record) = store.get(&name) else { + let installed = store.bundle_names(); + if installed.is_empty() { + bail!("no bundle named '{name}' is installed; none are installed"); + } + bail!( + "no bundle named '{name}' is installed; installed bundles: {}", + installed.join(", ") + ); + }; + let source = record.source.clone(); + let recorded_ref = record.git_ref.clone(); + + let has_override = ref_override.is_some(); + let effective_ref = ref_override.or(recorded_ref); + if !has_override + && let Some(pinned) = effective_ref.as_deref() + && is_commit_sha(pinned) + { + println!("Bundle '{name}' is pinned to commit {pinned}; pass # to move the pin."); + } + + let temp = clone_to_temp(&source, effective_ref.as_deref())?; + println!("Cloned {source} to {}", temp.path().display()); + + let mut layout = scan_remote_layout(temp.path())?; + layout.head_sha = Some(temp.head_sha().to_string()); + if layout.is_empty() { + println!( + "The source for '{name}' no longer contains recognized assets; \ + leaving installed files and the bundle record untouched." + ); + return Ok(()); + } + + let bundle = register_bundle( + &mut store, + &source, + effective_ref.as_deref(), + layout.manifest.as_ref(), + temp.head_sha(), + true, + )?; + + let plan = plan_changes(&layout)?; + let plan = reclassify_owned_unmodified(plan, &store, &bundle)?; + + if !plan.files.is_empty() { + print_plan_summary(&plan); + let sticky = if assume_yes { + StickyMode::KeepAll + } else { + StickyMode::None + }; + apply_plan(&plan, sticky, &mut store, &bundle)?; + } + + handle_obsolete_files(&mut store, &bundle, &plan, assume_yes)?; + + if let Some((remote_mcp, local_mcp)) = &plan.mcp_json { + let local = local_mcp.exists().then_some(local_mcp.as_path()); + let auto_take = owned_unmodified_mcp_keys(&store, &bundle, local)?; + let report = merge_mcp_json(local, remote_mcp, local_mcp, false, &auto_take, assume_yes)?; + record_mcp_merge(&mut store, &bundle, &report)?; + print_mcp_merge_report(&report); + handle_missing_secrets(&report.missing_secrets)?; + } + + let version = layout + .manifest + .as_ref() + .and_then(|m| m.version.clone()) + .unwrap_or_else(|| temp.head_sha().chars().take(7).collect()); + store.set_bundle_versions(&bundle, temp.head_sha(), Some(version))?; + store.mark_updated(&bundle)?; + + Ok(()) +} + +fn reclassify_owned_unmodified( + mut plan: InstallPlan, + store: &BundleStore, + bundle: &str, +) -> Result { + let owned: HashMap<&str, &str> = store + .get(bundle) + .map(|record| { + record + .files + .iter() + .map(|file| (file.path.as_str(), file.sha256.as_str())) + .collect() + }) + .unwrap_or_default(); + + for planned in &mut plan.files { + if planned.kind != PlannedKind::Conflict { + continue; + } + + let Some(recorded) = owned.get(provenance_path(&planned.dst).as_str()) else { + continue; + }; + + if hash_file(&planned.dst)? == *recorded { + planned.kind = PlannedKind::Refresh; + } + } + + Ok(plan) +} + +/// Keys of this bundle's mcp entries whose recorded hash still matches the +/// local entry: the bundle wrote them and the user never touched them, so an +/// upstream change takes the remote side without prompting. +fn owned_unmodified_mcp_keys( + store: &BundleStore, + bundle: &str, + local: Option<&Path>, +) -> Result> { + let mut keys = HashSet::new(); + let (Some(local_path), Some(record)) = (local, store.get(bundle)) else { + return Ok(keys); + }; + let content = fs::read_to_string(local_path) + .with_context(|| format!("failed to read local mcp.json at {}", local_path.display()))?; + let config: McpServersConfig = serde_json::from_str(&content) + .with_context(|| format!("failed to parse local mcp.json at {}", local_path.display()))?; + + for server in &record.mcp_servers { + let Some(recorded_hash) = server.sha256.as_deref() else { + continue; + }; + let key = server.effective_key(); + let Some(entry) = config.mcp_servers.get(key) else { + continue; + }; + let serialized = serde_json::to_string(entry) + .with_context(|| format!("failed to serialize MCP server '{key}'"))?; + + if hash_bytes(serialized.as_bytes()) == recorded_hash { + keys.insert(key.to_string()); + } + } + + Ok(keys) +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum ObsoleteAction { + Keep, + Delete, +} + +/// Kept files stay in the record, so a later uninstall still offers to +/// remove them. +fn handle_obsolete_files( + store: &mut BundleStore, + bundle: &str, + plan: &InstallPlan, + assume_yes: bool, +) -> Result<()> { + let planned: HashSet = plan + .files + .iter() + .map(|planned| provenance_path(&planned.dst)) + .collect(); + let obsolete: Vec = store + .get(bundle) + .map(|record| { + record + .files + .iter() + .map(|file| file.path.clone()) + .filter(|path| !planned.contains(path)) + .collect() + }) + .unwrap_or_default(); + + let config_dir = paths::config_dir(); + let mut sticky: Option = assume_yes.then_some(ObsoleteAction::Keep); + for path in obsolete { + if !is_safe_relative_path(&path) { + eprintln!("skipping suspicious recorded path {path}; keeping its record"); + continue; + } + + let full = config_dir.join(&path); + if !full.exists() { + println!("dropped record for obsolete file {path} (already absent locally)"); + store.remove_file_record(bundle, &path)?; + continue; + } + + let action = resolve_obsolete(&path, &mut sticky)?; + apply_obsolete_action(store, bundle, &path, &full, &config_dir, action)?; + } + + Ok(()) +} + +fn resolve_obsolete(path: &str, sticky: &mut Option) -> Result { + if let Some(action) = *sticky { + return Ok(action); + } + + if !*IS_STDOUT_TERMINAL { + return Ok(ObsoleteAction::Keep); + } + + let prompt = format!("Obsolete file {path} is no longer shipped by the bundle"); + let choice = Select::new(&prompt, vec!["keep", "delete", "keep-all", "delete-all"]) + .prompt() + .with_context(|| "failed to read obsolete-file choice")?; + + match choice { + "keep" => Ok(ObsoleteAction::Keep), + "delete" => Ok(ObsoleteAction::Delete), + "keep-all" => { + *sticky = Some(ObsoleteAction::Keep); + Ok(ObsoleteAction::Keep) + } + "delete-all" => { + *sticky = Some(ObsoleteAction::Delete); + Ok(ObsoleteAction::Delete) + } + _ => unreachable!("inquire::Select returned an unexpected option"), + } +} + +fn apply_obsolete_action( + store: &mut BundleStore, + bundle: &str, + path: &str, + full: &Path, + config_dir: &Path, + action: ObsoleteAction, +) -> Result<()> { + match action { + ObsoleteAction::Keep => { + println!("kept obsolete file {path} (no longer shipped by the bundle)"); + } + ObsoleteAction::Delete => { + fs::remove_file(full) + .with_context(|| format!("failed to delete obsolete file {}", full.display()))?; + store.remove_file_record(bundle, path)?; + prune_empty_dirs(full, config_dir); + println!("deleted obsolete file {path}"); + } + } + + Ok(()) +} + +#[derive(Debug, Default, PartialEq, Eq)] +struct UninstallFileSummary { + deleted: usize, + kept: usize, + missing: usize, + failed: usize, + /// Set when a functions/tools file was found on disk; its compiled binary + /// lingers in the functions bin dir until the next --build-tools prune. + tools_seen: bool, +} + +#[derive(Debug, Default)] +struct UninstallMcpSummary { + removed: Vec, + kept: Vec, + secrets: Vec, +} + +/// Kept and failed items stay in the record, so a re-run offers them again. +pub fn uninstall_bundle(spec: &str, assume_yes: bool) -> Result<()> { + let mut store = BundleStore::load()?; + let name = match store.get(spec) { + Some(_) => spec.to_string(), + None => match store.find_by_source(spec) { + Some((name, _)) => name.to_string(), + None => match select_uninstall_candidate(&store, spec)? { + Some(name) => name, + None => { + let installed = store.bundle_names(); + if installed.is_empty() { + bail!("no bundle named '{spec}' is installed; none are installed"); + } + + bail!( + "no bundle named '{spec}' is installed; installed bundles: {}", + installed.join(", ") + ); + } + }, + }, + }; + let record = store.get(&name).expect("resolved above").clone(); + + if !assume_yes { + if !*IS_STDOUT_TERMINAL { + bail!( + "refusing to uninstall bundle '{name}' non-interactively; \ + re-run with --yes to confirm" + ); + } + println!( + "Bundle '{name}' owns {} file(s) and {} mcp.json server(s).", + record.files.len(), + record.mcp_servers.len() + ); + let proceed = Confirm::new(&format!("Uninstall bundle '{name}'?")) + .with_default(false) + .prompt() + .with_context(|| "failed to read uninstall confirmation")?; + if !proceed { + println!("Uninstall of '{name}' aborted; nothing was changed."); + return Ok(()); + } + } + + let files = uninstall_owned_files( + &mut store, + &name, + &record.files, + &paths::config_dir(), + assume_yes, + )?; + + if files.tools_seen { + println!( + "Note: compiled tool binaries remain in {} until the next --build-tools prune.", + paths::functions_bin_dir().display() + ); + } + + let mcp = uninstall_mcp_entries( + &mut store, + &name, + &record.mcp_servers, + &paths::mcp_config_file(), + assume_yes, + )?; + + let empty = store + .get(&name) + .map(|record| record.files.is_empty() && record.mcp_servers.is_empty()) + .unwrap_or(true); + + if empty { + store.remove_bundle(&name)?; + println!("\nUninstalled bundle '{name}' and removed its record."); + } else { + println!( + "\nBundle '{name}' partially uninstalled; its record keeps the remaining \ + items, so re-running --uninstall offers them again." + ); + } + + println!( + " files: deleted={} kept={} missing={} failed={}", + files.deleted, files.kept, files.missing, files.failed + ); + println!( + " mcp servers: removed={} kept={}", + mcp.removed.len(), + mcp.kept.len() + ); + + if !mcp.removed.is_empty() { + println!(" - removed servers: {}", mcp.removed.join(", ")); + } + + if !mcp.kept.is_empty() { + println!(" = kept servers: {}", mcp.kept.join(", ")); + } + + if !mcp.secrets.is_empty() { + println!( + " ~ vault secrets referenced by this bundle's servers \ + (installed by this bundle, not removed): {}", + mcp.secrets.join(", ") + ); + } + + Ok(()) +} + +/// Never auto-picks between multiple matches: ambiguity is resolved by an +/// interactive prompt, and non-interactive runs bail even under --yes. +fn select_uninstall_candidate(store: &BundleStore, spec: &str) -> Result> { + if !is_repo_shorthand(spec) { + return Ok(None); + } + + let needle = format!("/{}", canonical_source_url(spec)); + let candidates: Vec<(String, String)> = store + .iter() + .filter(|(_, record)| canonical_source_url(&record.source).ends_with(&needle)) + .map(|(name, record)| (name.to_string(), record.source.clone())) + .collect(); + + match candidates.as_slice() { + [] => Ok(None), + [(name, _)] => Ok(Some(name.clone())), + _ => { + let described: Vec = candidates + .iter() + .map(|(name, source)| format!("{name} ({source})")) + .collect(); + + if !*IS_STDOUT_TERMINAL { + bail!( + "'{spec}' matches multiple installed bundles: {}; re-run with the \ + exact bundle name or source URL", + described.join(", ") + ); + } + + let picked = Select::new( + &format!("Multiple bundles match '{spec}'; which one should be uninstalled?"), + described.clone(), + ) + .prompt() + .with_context(|| "failed to read uninstall selection")?; + + let index = described + .iter() + .position(|option| option == &picked) + .expect("selection came from the presented options"); + + Ok(Some(candidates[index].0.clone())) + } + } +} + +/// True only when joining the path onto the config dir cannot escape it: +/// relative and made of plain components. Anything else in a recorded path +/// means a tampered store, and it must never become a file deletion. +fn is_safe_relative_path(path: &str) -> bool { + let recorded = Path::new(path); + !recorded.is_absolute() + && recorded + .components() + .all(|c| matches!(c, Component::Normal(_))) +} + +fn uninstall_owned_files( + store: &mut BundleStore, + bundle: &str, + files: &[FileRecord], + config_dir: &Path, + assume_yes: bool, +) -> Result { + let mut summary = UninstallFileSummary::default(); + let mut sticky: Option = None; + for file in files { + if !is_safe_relative_path(&file.path) { + eprintln!( + "skipping suspicious recorded path {}; keeping its record", + file.path + ); + summary.failed += 1; + continue; + } + + let full = config_dir.join(Path::new(&file.path)); + if !full.exists() { + println!( + "dropped record for missing file {} (already absent locally)", + file.path + ); + store.remove_file_record(bundle, &file.path)?; + summary.missing += 1; + continue; + } + + summary.tools_seen |= file.category == "functions/tools"; + + if matches!(hash_file(&full), Ok(hash) if hash == file.sha256) { + delete_owned_file(store, bundle, &file.path, &full, config_dir, &mut summary)?; + continue; + } + + let prompt = format!("File {} was modified locally after install", file.path); + let action = resolve_uninstall_action(&prompt, assume_yes, &mut sticky)?; + apply_uninstall_file_action( + store, + bundle, + &file.path, + &full, + config_dir, + action, + &mut summary, + )?; + } + + Ok(summary) +} + +fn apply_uninstall_file_action( + store: &mut BundleStore, + bundle: &str, + path: &str, + full: &Path, + config_dir: &Path, + action: ObsoleteAction, + summary: &mut UninstallFileSummary, +) -> Result<()> { + match action { + ObsoleteAction::Keep => { + println!("kept modified file {path}"); + summary.kept += 1; + } + ObsoleteAction::Delete => { + delete_owned_file(store, bundle, path, full, config_dir, summary)?; + } + } + + Ok(()) +} + +fn delete_owned_file( + store: &mut BundleStore, + bundle: &str, + path: &str, + full: &Path, + config_dir: &Path, + summary: &mut UninstallFileSummary, +) -> Result<()> { + match fs::remove_file(full) { + Ok(()) => { + store.remove_file_record(bundle, path)?; + println!("deleted {path}"); + summary.deleted += 1; + prune_empty_dirs(full, config_dir); + } + Err(err) => { + eprintln!( + "failed to delete {}: {err}; keeping its record", + full.display() + ); + summary.failed += 1; + } + } + Ok(()) +} + +/// Remove now-empty ancestors of a deleted file, climbing strictly below +/// `config_dir`. `fs::remove_dir` fails on a non-empty directory, which is +/// the stop condition. +fn prune_empty_dirs(full: &Path, config_dir: &Path) { + let mut current = full.parent(); + while let Some(dir) = current { + if dir == config_dir || !dir.starts_with(config_dir) { + break; + } + + if fs::remove_dir(dir).is_err() { + break; + } + + current = dir.parent(); + } +} + +fn resolve_uninstall_action( + prompt: &str, + assume_yes: bool, + sticky: &mut Option, +) -> Result { + if assume_yes || !*IS_STDOUT_TERMINAL { + return Ok(ObsoleteAction::Keep); + } + + if let Some(action) = *sticky { + return Ok(action); + } + + let choice = Select::new(prompt, vec!["keep", "delete", "keep-all", "delete-all"]) + .prompt() + .with_context(|| "failed to read uninstall choice")?; + match choice { + "keep" => Ok(ObsoleteAction::Keep), + "delete" => Ok(ObsoleteAction::Delete), + "keep-all" => { + *sticky = Some(ObsoleteAction::Keep); + Ok(ObsoleteAction::Keep) + } + "delete-all" => { + *sticky = Some(ObsoleteAction::Delete); + Ok(ObsoleteAction::Delete) + } + _ => unreachable!("inquire::Select returned an unexpected option"), + } +} + +/// mcp.json is written before any ownership record is dropped: a failed write +/// leaves every record intact for a retry, while a crash after it leaves +/// stale records the absent-server branch cleans up on re-run. +fn uninstall_mcp_entries( + store: &mut BundleStore, + bundle: &str, + servers: &[McpServerRecord], + mcp_path: &Path, + assume_yes: bool, +) -> Result { + let mut summary = UninstallMcpSummary::default(); + if servers.is_empty() { + return Ok(summary); + } + + let mut config = if mcp_path.exists() { + let content = fs::read_to_string(mcp_path) + .with_context(|| format!("failed to read {}", mcp_path.display()))?; + Some( + serde_json::from_str::(&content) + .with_context(|| format!("failed to parse {}", mcp_path.display()))?, + ) + } else { + None + }; + + let mut secret_names: BTreeSet = BTreeSet::new(); + for server in servers { + if let Some(entry) = config + .as_ref() + .and_then(|cfg| cfg.mcp_servers.get(server.effective_key())) + && let Ok(serialized) = serde_json::to_string(entry) + { + for capture in SECRET_RE.captures_iter(&serialized) { + if let Ok(capture) = capture + && let Some(name) = capture.get(1) + { + secret_names.insert(name.as_str().to_string()); + } + } + } + } + summary.secrets = secret_names.into_iter().collect(); + + let mut changed = false; + let mut released = Vec::new(); + let mut sticky: Option = None; + for server in servers { + let key = server.effective_key().to_string(); + if server.action == McpAction::Replaced { + println!("kept pre-existing server '{key}'"); + released.push(key.clone()); + summary.kept.push(key); + continue; + } + + let Some(entry) = config.as_ref().and_then(|cfg| cfg.mcp_servers.get(&key)) else { + println!("dropped record for absent server '{key}'"); + released.push(key); + continue; + }; + let serialized = serde_json::to_string(entry) + .with_context(|| format!("failed to serialize MCP server '{key}'"))?; + let intact = server.sha256.as_deref() == Some(hash_bytes(serialized.as_bytes()).as_str()); + let action = if intact { + ObsoleteAction::Delete + } else { + let reason = if server.sha256.is_none() { + "predates entry hashing" + } else { + "was modified locally after install" + }; + let prompt = format!("MCP server '{key}' {reason}"); + resolve_uninstall_action(&prompt, assume_yes, &mut sticky)? + }; + match action { + ObsoleteAction::Keep => { + println!("kept server '{key}'"); + summary.kept.push(key); + } + ObsoleteAction::Delete => { + config + .as_mut() + .expect("entry was found in the config above") + .mcp_servers + .shift_remove(&key); + changed = true; + println!("removed server '{key}'"); + released.push(key.clone()); + summary.removed.push(key); + } + } + } + + if changed { + let config = config.expect("changed implies a parsed config"); + let serialized = serde_json::to_string_pretty(&config) + .context("failed to serialize mcp.json after uninstall")?; + write_atomically(mcp_path, &serialized)?; + } + for key in &released { + store.remove_mcp_record(bundle, key)?; + } + + Ok(summary) } fn parse_filter(name: &str) -> Result { @@ -121,12 +1025,17 @@ fn parse_url_with_ref(input: &str) -> Result<(String, Option)> { struct TempRepoDir { path: PathBuf, + head_sha: String, } impl TempRepoDir { fn path(&self) -> &Path { &self.path } + + fn head_sha(&self) -> &str { + &self.head_sha + } } impl Drop for TempRepoDir { @@ -135,13 +1044,17 @@ impl Drop for TempRepoDir { } } +fn is_commit_sha(reference: &str) -> bool { + reference.len() >= 4 + && reference.len() <= 40 + && reference.chars().all(|c| c.is_ascii_hexdigit()) +} + fn clone_to_temp(url: &str, reference: Option<&str>) -> Result { let dest = utils::temp_file("coyote-remote-install-", ""); let dest_arg: OsString = dest.as_os_str().into(); - let is_sha = reference - .map(|r| r.len() >= 4 && r.len() <= 40 && r.chars().all(|c| c.is_ascii_hexdigit())) - .unwrap_or(false); + let is_sha = reference.is_some_and(is_commit_sha); match reference { Some(r) if !is_sha => { @@ -151,12 +1064,18 @@ fn clone_to_temp(url: &str, reference: Option<&str>) -> Result { "1".into(), "--branch".into(), r.into(), + "--".into(), url.into(), dest_arg, ])?; } Some(r) => { - run_git(vec!["clone".into(), url.into(), dest_arg.clone()])?; + run_git(vec![ + "clone".into(), + "--".into(), + url.into(), + dest_arg.clone(), + ])?; run_git(vec!["-C".into(), dest_arg, "checkout".into(), r.into()])?; } None => { @@ -164,17 +1083,30 @@ fn clone_to_temp(url: &str, reference: Option<&str>) -> Result { "clone".into(), "--depth".into(), "1".into(), + "--".into(), url.into(), dest_arg, ])?; } } - Ok(TempRepoDir { path: dest }) + let head_sha = run_git_capture(vec![ + "-C".into(), + dest.as_os_str().into(), + "rev-parse".into(), + "HEAD".into(), + ])?; + + Ok(TempRepoDir { + path: dest, + head_sha, + }) } fn run_git(args: Vec) -> Result<()> { let output = duct::cmd("git", &args) + .env("GIT_TERMINAL_PROMPT", "0") + .stdin_null() .stderr_to_stdout() .stdout_capture() .unchecked() @@ -189,7 +1121,26 @@ fn run_git(args: Vec) -> Result<()> { Ok(()) } -#[derive(Default)] +fn run_git_capture(args: Vec) -> Result { + let output = duct::cmd("git", &args) + .env("GIT_TERMINAL_PROMPT", "0") + .stdin_null() + .stdout_capture() + .stderr_capture() + .unchecked() + .run() + .context("failed to spawn git (is it installed and on PATH?)")?; + + if !output.status.success() { + let stdout = String::from_utf8_lossy(&output.stdout); + let stderr = String::from_utf8_lossy(&output.stderr); + bail!("git failed: {}", format!("{stdout} {stderr}").trim()); + } + + Ok(String::from_utf8_lossy(&output.stdout).trim().to_string()) +} + +#[derive(Debug, Default)] struct RemoteLayout { agents: Option, roles: Option, @@ -197,6 +1148,8 @@ struct RemoteLayout { macros: Option, functions_tools: Option, mcp_json: Option, + manifest: Option, + head_sha: Option, } impl RemoteLayout { @@ -211,7 +1164,10 @@ impl RemoteLayout { } fn scan_remote_layout(root: &Path) -> Result { - let mut layout = RemoteLayout::default(); + let mut layout = RemoteLayout { + manifest: parse_bundle_manifest(root)?, + ..RemoteLayout::default() + }; let agents = root.join("agents"); if agents.is_dir() { @@ -249,34 +1205,214 @@ fn scan_remote_layout(root: &Path) -> Result { Ok(layout) } +#[derive(Debug, Clone, PartialEq, Deserialize)] +pub(crate) struct BundleManifest { + pub(crate) name: String, + pub(crate) version: Option, + pub(crate) description: Option, + pub(crate) homepage: Option, +} + +/// Returns the record key, possibly migrated or owner-qualified, that all +/// subsequent recording must use in place of the requested name. +/// An update passes `preserve_versions` so the record keeps claiming the old +/// commit and version until the new content actually lands on disk. +fn register_bundle( + store: &mut BundleStore, + url: &str, + git_ref: Option<&str>, + manifest: Option<&BundleManifest>, + commit: &str, + preserve_versions: bool, +) -> Result { + let resolved = store.resolve_bundle_name(url, manifest.map(|m| m.name.as_str()))?; + let version = manifest + .and_then(|m| m.version.clone()) + .unwrap_or_else(|| commit.chars().take(7).collect()); + let (commit, version) = match (preserve_versions, store.get(&resolved.name)) { + (true, Some(existing)) => (existing.commit.clone(), existing.version.clone()), + _ => (commit.to_string(), Some(version)), + }; + store.upsert_bundle( + &resolved.name, + InstallMetadata { + source: url.to_string(), + git_ref: git_ref.map(str::to_string), + commit, + version, + description: manifest.and_then(|m| m.description.clone()), + homepage: manifest.and_then(|m| m.homepage.clone()), + }, + )?; + + Ok(resolved.name) +} + +fn parse_bundle_manifest(root: &Path) -> Result> { + let path = root.join(BUNDLE_MANIFEST_FILE); + if !path.is_file() { + return Ok(None); + } + + let content = fs::read_to_string(&path) + .with_context(|| format!("failed to read bundle manifest at {}", path.display()))?; + let manifest: BundleManifest = serde_yaml::from_str(&content) + .with_context(|| format!("invalid bundle manifest at {}", path.display()))?; + validate_bundle_name(&manifest.name) + .with_context(|| format!("invalid bundle name in manifest at {}", path.display()))?; + + Ok(Some(manifest)) +} + +pub(crate) fn validate_bundle_name(name: &str) -> Result<()> { + let (owner, base) = match name.split_once('/') { + Some((owner, base)) => (Some(owner), base), + None => (None, name), + }; + if base.contains('/') { + bail!( + "Invalid bundle name '{name}': at most one '/' is allowed \ + (as the owner qualifier separator)" + ); + } + + for part in owner.into_iter().chain(iter::once(base)) { + if part.is_empty() { + bail!("Invalid bundle name '{name}': name segments cannot be empty"); + } + if !part + .chars() + .all(|c| c.is_ascii_alphanumeric() || c == '-' || c == '_') + { + bail!( + "Invalid bundle name '{name}': only letters, digits, '-', and '_' are allowed \ + (plus a single '/' separating an owner qualifier)" + ); + } + } + + Ok(()) +} + +fn strip_ref_suffix(url: &str) -> &str { + match url.rsplit_once('#') { + Some((base, _)) if !base.is_empty() => base, + _ => url, + } +} + +fn split_host_and_path(url: &str) -> (String, String) { + let url = strip_ref_suffix(url); + let url = &url.replace('\\', "/"); + if let Some((_, rest)) = url.split_once("://") { + let (authority, path) = rest.split_once('/').unwrap_or((rest, "")); + let host = authority.rsplit_once('@').map_or(authority, |(_, h)| h); + (host.to_string(), path.trim_matches('/').to_string()) + } else if let Some((prefix, path)) = url.split_once(':') + && !prefix.contains('/') + { + let host = prefix.rsplit_once('@').map_or(prefix, |(_, h)| h); + (host.to_string(), path.trim_matches('/').to_string()) + } else { + (String::new(), url.trim_end_matches('/').to_string()) + } +} + +fn strip_git_suffix(segment: &str) -> &str { + if segment.len() > 4 && segment.to_ascii_lowercase().ends_with(".git") { + &segment[..segment.len() - 4] + } else { + segment + } +} + +pub(crate) fn repo_name_slug(url: &str) -> String { + let (_, path) = split_host_and_path(url); + let last = path.rsplit('/').next().unwrap_or(""); + strip_git_suffix(last).to_string() +} + +pub(crate) fn owner_qualifier(url: &str) -> Option { + let (host, path) = split_host_and_path(url); + let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); + if segments.len() >= 2 { + return Some(segments[segments.len() - 2].to_string()); + } + + let sanitized = sanitize_host(&host); + (!sanitized.is_empty()).then_some(sanitized) +} + +fn sanitize_host(host: &str) -> String { + host.to_ascii_lowercase() + .chars() + .map(|c| { + if c.is_ascii_alphanumeric() || c == '-' || c == '_' { + c + } else { + '-' + } + }) + .collect::() + .trim_matches('-') + .to_string() +} + +/// The host compares case-insensitively but the path keeps its case: many +/// self-hosted forges treat repository paths as case-sensitive, and collapsing +/// distinct repos into one record misdirects updates and uninstalls. +pub(crate) fn canonical_source_url(url: &str) -> String { + let (host, path) = split_host_and_path(url); + let mut path = path; + if path.to_ascii_lowercase().ends_with(".git") { + let stripped = &path[..path.len() - 4]; + if !stripped.is_empty() && !stripped.ends_with('/') { + path.truncate(path.len() - 4); + } + } + let host = host.to_ascii_lowercase(); + if host.is_empty() { + path + } else if path.is_empty() { + host + } else { + format!("{host}/{path}") + } +} + fn apply_filter(mut layout: RemoteLayout, filter: Option) -> RemoteLayout { let Some(filter) = filter else { return layout; }; + let base = RemoteLayout { + manifest: layout.manifest.take(), + head_sha: layout.head_sha.take(), + ..RemoteLayout::default() + }; match filter { InstallFilter::Agents => RemoteLayout { agents: layout.agents.take(), - ..RemoteLayout::default() + ..base }, InstallFilter::Roles => RemoteLayout { roles: layout.roles.take(), - ..RemoteLayout::default() + ..base }, InstallFilter::Skills => RemoteLayout { skills: layout.skills.take(), - ..RemoteLayout::default() + ..base }, InstallFilter::Macros => RemoteLayout { macros: layout.macros.take(), - ..RemoteLayout::default() + ..base }, InstallFilter::Functions => RemoteLayout { functions_tools: layout.functions_tools.take(), - ..RemoteLayout::default() + ..base }, InstallFilter::McpConfig => RemoteLayout { mcp_json: layout.mcp_json.take(), - ..RemoteLayout::default() + ..base }, } } @@ -345,6 +1481,9 @@ enum PlannedKind { New, Identical, Conflict, + /// A conflict downgraded because this bundle owns the file and the local + /// content still matches the recorded hash; applied without prompting. + Refresh, } struct PlannedFile { @@ -479,14 +1618,13 @@ fn files_equal(a: &Path, b: &Path) -> Result { } fn files_equal_streaming(a: &Path, b: &Path) -> Result { - use std::io::Read; let mut fa = fs::File::open(a).with_context(|| format!("open {}", a.display()))?; let mut fb = fs::File::open(b).with_context(|| format!("open {}", b.display()))?; let mut buf_a = [0u8; 8192]; let mut buf_b = [0u8; 8192]; loop { - let na = fa.read(&mut buf_a)?; - let nb = fb.read(&mut buf_b)?; + let na = read_full(&mut fa, &mut buf_a)?; + let nb = read_full(&mut fb, &mut buf_b)?; if na != nb { return Ok(false); } @@ -499,6 +1637,22 @@ fn files_equal_streaming(a: &Path, b: &Path) -> Result { } } +/// `read` may return short counts without EOF; comparing partially filled +/// buffers positionally would misreport identical files as different. +fn read_full(file: &mut fs::File, buf: &mut [u8]) -> std::io::Result { + use std::io::Read; + let mut filled = 0; + while filled < buf.len() { + let n = file.read(&mut buf[filled..])?; + if n == 0 { + break; + } + filled += n; + } + + Ok(filled) +} + fn print_plan_summary(plan: &InstallPlan) { println!("Plan:"); for cat in [ @@ -511,11 +1665,17 @@ fn print_plan_summary(plan: &InstallPlan) { let new_ = count_kind(plan, cat, PlannedKind::New); let identical = count_kind(plan, cat, PlannedKind::Identical); let conflict = count_kind(plan, cat, PlannedKind::Conflict); - if new_ + identical + conflict > 0 { - println!( + let refresh = count_kind(plan, cat, PlannedKind::Refresh); + if new_ + identical + conflict + refresh > 0 { + let mut line = format!( " {:<16} new={new_} identical={identical} conflict={conflict}", cat.label() ); + if refresh > 0 { + line.push_str(&format!(" refresh={refresh}")); + } + + println!("{line}"); } } } @@ -539,53 +1699,135 @@ enum ConflictAction { Replace, } +#[derive(Debug)] struct ApplyReport { new_count: usize, identical_count: usize, replaced_count: usize, + refreshed_count: usize, kept_count: usize, } -fn apply_plan(plan: &InstallPlan, force: bool) -> Result { +fn apply_plan( + plan: &InstallPlan, + initial_mode: StickyMode, + store: &mut BundleStore, + bundle: &str, +) -> Result { let mut report = ApplyReport { new_count: 0, identical_count: 0, replaced_count: 0, + refreshed_count: 0, kept_count: 0, }; - let mut sticky = if force { - StickyMode::ReplaceAll - } else { - StickyMode::None - }; + let mut sticky = initial_mode; for planned in &plan.files { match planned.kind { PlannedKind::New => { write_file(&planned.src, &planned.dst)?; + record_written_file(store, bundle, planned, FileAction::New)?; report.new_count += 1; } PlannedKind::Identical => { report.identical_count += 1; } + PlannedKind::Refresh => { + write_file(&planned.src, &planned.dst)?; + record_written_file(store, bundle, planned, FileAction::Replaced)?; + report.refreshed_count += 1; + } PlannedKind::Conflict => match resolve_conflict(planned, &mut sticky)? { ConflictAction::Keep => report.kept_count += 1, ConflictAction::Replace => { write_file(&planned.src, &planned.dst)?; + record_written_file(store, bundle, planned, FileAction::Replaced)?; report.replaced_count += 1; } }, } } - println!( - "\nInstalled: {} new, {} replaced, {} kept, {} identical.", - report.new_count, report.replaced_count, report.kept_count, report.identical_count - ); + if report.refreshed_count > 0 { + println!( + "\nInstalled: {} new, {} refreshed, {} replaced, {} kept, {} identical.", + report.new_count, + report.refreshed_count, + report.replaced_count, + report.kept_count, + report.identical_count + ); + } else { + println!( + "\nInstalled: {} new, {} replaced, {} kept, {} identical.", + report.new_count, report.replaced_count, report.kept_count, report.identical_count + ); + } Ok(report) } +/// Kept and identical files are never recorded, because ownership means +/// "this content exists because of this bundle": a file another bundle's +/// record already owns stays with that owner. +fn record_written_file( + store: &mut BundleStore, + bundle: &str, + planned: &PlannedFile, + action: FileAction, +) -> Result<()> { + store.record_file( + bundle, + FileRecord { + path: provenance_path(&planned.dst), + category: planned.top_category.label().to_string(), + sha256: hash_file(&planned.dst)?, + action, + }, + ) +} + +fn provenance_path(dst: &Path) -> String { + let rel = dst.strip_prefix(paths::config_dir()).unwrap_or(dst); + rel.to_string_lossy().replace('\\', "/") +} + +/// Entries the merge kept local are deliberately absent: an entry another +/// bundle already owns stays with that owner. +fn record_mcp_merge(store: &mut BundleStore, bundle: &str, report: &McpMergeReport) -> Result<()> { + let mut entries: Vec = Vec::new(); + entries.extend(report.added.iter().map(|name| McpServerRecord { + name: name.clone(), + action: McpAction::Added, + renamed_to: None, + sha256: report.entry_hashes.get(name).cloned(), + })); + entries.extend(report.replaced.iter().map(|name| McpServerRecord { + name: name.clone(), + action: McpAction::Replaced, + renamed_to: None, + sha256: report.entry_hashes.get(name).cloned(), + })); + entries.extend( + report + .renamed + .iter() + .map(|(name, renamed_to)| McpServerRecord { + name: name.clone(), + action: McpAction::Renamed, + renamed_to: Some(renamed_to.clone()), + sha256: report.entry_hashes.get(renamed_to).cloned(), + }), + ); + + if entries.is_empty() { + return Ok(()); + } + + store.record_mcp_servers(bundle, entries) +} + fn resolve_conflict(planned: &PlannedFile, sticky: &mut StickyMode) -> Result { match *sticky { StickyMode::KeepAll => return Ok(ConflictAction::Keep), @@ -596,7 +1838,8 @@ fn resolve_conflict(planned: &PlannedFile, sticky: &mut StickyMode) -> Result, replaced: Vec, renamed: Vec<(String, String)>, + /// Hash of each entry this merge wrote, by its final key in mcp.json, so + /// uninstall can tell the entry apart from a later local edit. + entry_hashes: HashMap, final_path: PathBuf, missing_secrets: Vec, } @@ -681,6 +1927,8 @@ fn merge_mcp_json( remote: &Path, target: &Path, force: bool, + auto_take: &HashSet, + assume_yes: bool, ) -> Result { let remote_content = fs::read_to_string(remote) .with_context(|| format!("failed to read remote mcp.json at {}", remote.display()))?; @@ -706,6 +1954,7 @@ fn merge_mcp_json( kept_local: Vec::new(), replaced: Vec::new(), renamed: Vec::new(), + entry_hashes: HashMap::new(), final_path: final_path.clone(), missing_secrets: Vec::new(), }; @@ -716,7 +1965,12 @@ fn merge_mcp_json( if local_server == &remote_server { continue; } - match resolve_mcp_conflict(&name, force)? { + let action = if auto_take.contains(&name) { + McpConflictAction::TakeRemote + } else { + resolve_mcp_conflict(&name, force, assume_yes)? + }; + match action { McpConflictAction::KeepLocal => report.kept_local.push(name), McpConflictAction::TakeRemote => { merged.mcp_servers.insert(name.clone(), remote_server); @@ -745,6 +1999,11 @@ fn merge_mcp_json( spec.validate(key).with_context(|| { format!("MCP server '{key}' failed validation; refusing to write merged mcp.json") })?; + let serialized = serde_json::to_string(spec) + .with_context(|| format!("failed to serialize MCP server '{key}'"))?; + report + .entry_hashes + .insert(key.clone(), hash_bytes(serialized.as_bytes())); } let serialized = @@ -777,14 +2036,17 @@ fn merge_mcp_json( Ok(report) } -fn resolve_mcp_conflict(name: &str, force: bool) -> Result { +fn resolve_mcp_conflict(name: &str, force: bool, assume_yes: bool) -> Result { if force { return Ok(McpConflictAction::TakeRemote); } + if assume_yes { + return Ok(McpConflictAction::KeepLocal); + } if !*IS_STDOUT_TERMINAL { bail!( "MCP server '{name}' already exists locally. Refusing to merge non-interactively. \ - Re-run with --install-force or in a terminal." + Re-run in a terminal, with --install-force (installs), or with --yes (updates)." ); } let rename_label = format!("rename remote as \"{name}-remote\""); @@ -1104,6 +2366,7 @@ mod tests { macros: Some(PathBuf::from("m")), functions_tools: Some(PathBuf::from("f")), mcp_json: Some(PathBuf::from("j")), + ..RemoteLayout::default() }; let out = apply_filter(l, None); @@ -1121,6 +2384,7 @@ mod tests { macros: None, functions_tools: Some(PathBuf::from("f")), mcp_json: Some(PathBuf::from("j")), + ..RemoteLayout::default() }; let out = apply_filter(l, Some(InstallFilter::Functions)); @@ -1140,6 +2404,7 @@ mod tests { macros: None, functions_tools: Some(PathBuf::from("f")), mcp_json: Some(PathBuf::from("j")), + ..RemoteLayout::default() }; let out = apply_filter(l, Some(InstallFilter::McpConfig)); @@ -1157,6 +2422,7 @@ mod tests { macros: Some(PathBuf::from("m")), functions_tools: Some(PathBuf::from("f")), mcp_json: Some(PathBuf::from("j")), + ..RemoteLayout::default() }; let out = apply_filter(l, Some(InstallFilter::Roles)); @@ -1175,6 +2441,7 @@ mod tests { macros: Some(PathBuf::from("m")), functions_tools: Some(PathBuf::from("f")), mcp_json: Some(PathBuf::from("j")), + ..RemoteLayout::default() }; let out = apply_filter(l, Some(InstallFilter::Skills)); @@ -1351,7 +2618,7 @@ mod tests { let target = dir.join("target.json"); write_mcp(&remote, FIXTURE_REMOTE); - let report = merge_mcp_json(None, &remote, &target, false).unwrap(); + let report = merge_mcp_json(None, &remote, &target, false, &HashSet::new(), false).unwrap(); assert_eq!(report.added, vec!["alpha", "beta"]); assert!(report.kept_local.is_empty()); @@ -1374,7 +2641,15 @@ mod tests { ); write_mcp(&remote, FIXTURE_REMOTE); - let report = merge_mcp_json(Some(&target), &remote, &target, true).unwrap(); + let report = merge_mcp_json( + Some(&target), + &remote, + &target, + true, + &HashSet::new(), + false, + ) + .unwrap(); assert_eq!(report.added, vec!["beta"]); assert_eq!(report.replaced, vec!["alpha"]); @@ -1403,7 +2678,15 @@ mod tests { ); write_mcp(&remote, FIXTURE_REMOTE); - let err = merge_mcp_json(Some(&target), &remote, &target, false).unwrap_err(); + let err = merge_mcp_json( + Some(&target), + &remote, + &target, + false, + &HashSet::new(), + false, + ) + .unwrap_err(); assert!( err.to_string() @@ -1420,7 +2703,8 @@ mod tests { let target = dir.join("target.json"); write_mcp(&remote, r#"{"mcpServers": {"broken": {"type": "stdio"}}}"#); - let err = merge_mcp_json(None, &remote, &target, false).unwrap_err(); + let err = + merge_mcp_json(None, &remote, &target, false, &HashSet::new(), false).unwrap_err(); assert!( format!("{err:#}").contains("missing a \"command\" field"), @@ -1447,7 +2731,7 @@ mod tests { r#"{"mcpServers": {"x": {"type":"stdio","command":"echo","env":{"K":"{{COYOTE_TEST_MERGE_SECRET}}"}}}}"#, ); - let report = merge_mcp_json(None, &remote, &target, false).unwrap(); + let report = merge_mcp_json(None, &remote, &target, false, &HashSet::new(), false).unwrap(); assert_eq!(report.missing_secrets, vec!["COYOTE_TEST_MERGE_SECRET"]); let _ = fs::remove_dir_all(&dir); @@ -1462,10 +2746,18 @@ mod tests { let target = dir.join("target.json"); write_mcp(&remote, FIXTURE_REMOTE); - merge_mcp_json(None, &remote, &target, false).unwrap(); + merge_mcp_json(None, &remote, &target, false, &HashSet::new(), false).unwrap(); let after_first = fs::read(&target).unwrap(); - let report = merge_mcp_json(Some(&target), &remote, &target, false).unwrap(); + let report = merge_mcp_json( + Some(&target), + &remote, + &target, + false, + &HashSet::new(), + false, + ) + .unwrap(); assert!(report.added.is_empty(), "got: {:?}", report.added); let after_second = fs::read(&target).unwrap(); @@ -1493,4 +2785,1994 @@ mod tests { assert!(handle_missing_secrets(&missing).is_ok()); } + + fn write_manifest(root: &Path, yaml: &str) { + fs::write(root.join(BUNDLE_MANIFEST_FILE), yaml).unwrap(); + } + + #[test] + fn scan_remote_layout_without_manifest_has_none() { + let root = fresh_temp_dir("scan-no-manifest-"); + fs::create_dir_all(root.join("macros")).unwrap(); + + let layout = scan_remote_layout(&root).unwrap(); + + assert_eq!(layout.manifest, None); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_remote_layout_parses_full_manifest_and_ignores_unknown_fields() { + let root = fresh_temp_dir("scan-manifest-full-"); + fs::create_dir_all(root.join("macros")).unwrap(); + write_manifest( + &root, + "name: oh-my-coyote\n\ + version: \"1.4.0\"\n\ + description: Opinionated roles and macros\n\ + homepage: https://github.com/x/oh-my-coyote\n\ + future_field: ignored\n", + ); + + let manifest = scan_remote_layout(&root).unwrap().manifest.unwrap(); + + assert_eq!(manifest.name, "oh-my-coyote"); + assert_eq!(manifest.version.as_deref(), Some("1.4.0")); + assert_eq!( + manifest.description.as_deref(), + Some("Opinionated roles and macros") + ); + assert_eq!( + manifest.homepage.as_deref(), + Some("https://github.com/x/oh-my-coyote") + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_remote_layout_parses_name_only_manifest() { + let root = fresh_temp_dir("scan-manifest-minimal-"); + write_manifest(&root, "name: minimal\n"); + + let manifest = scan_remote_layout(&root).unwrap().manifest.unwrap(); + + assert_eq!(manifest.name, "minimal"); + assert_eq!(manifest.version, None); + assert_eq!(manifest.description, None); + assert_eq!(manifest.homepage, None); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_remote_layout_fails_on_malformed_manifest() { + let root = fresh_temp_dir("scan-manifest-malformed-"); + write_manifest(&root, "name: [unclosed\n"); + + let err = scan_remote_layout(&root).unwrap_err(); + + assert!( + format!("{err:#}").contains("invalid bundle manifest"), + "got: {err:#}" + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_remote_layout_fails_on_manifest_missing_name() { + let root = fresh_temp_dir("scan-manifest-no-name-"); + write_manifest(&root, "version: \"1.0\"\n"); + + let err = scan_remote_layout(&root).unwrap_err(); + + assert!( + format!("{err:#}").contains("invalid bundle manifest"), + "got: {err:#}" + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn scan_remote_layout_fails_on_invalid_manifest_name() { + let root = fresh_temp_dir("scan-manifest-bad-name-"); + write_manifest(&root, "name: not a valid name\n"); + + let err = scan_remote_layout(&root).unwrap_err(); + + assert!( + format!("{err:#}").contains("Invalid bundle name"), + "got: {err:#}" + ); + let _ = fs::remove_dir_all(&root); + } + + #[test] + fn apply_filter_carries_manifest_and_head_sha_through() { + let l = RemoteLayout { + macros: Some(PathBuf::from("m")), + manifest: Some(BundleManifest { + name: "bundle".to_string(), + version: None, + description: None, + homepage: None, + }), + head_sha: Some("abc123".to_string()), + ..RemoteLayout::default() + }; + + let out = apply_filter(l, Some(InstallFilter::Macros)); + + assert_eq!(out.macros, Some(PathBuf::from("m"))); + assert_eq!(out.manifest.unwrap().name, "bundle"); + assert_eq!(out.head_sha.as_deref(), Some("abc123")); + } + + #[test] + fn validate_bundle_name_accepts_simple_names() { + assert!(validate_bundle_name("oh-my-coyote").is_ok()); + assert!(validate_bundle_name("under_score").is_ok()); + assert!(validate_bundle_name("Abc123").is_ok()); + } + + #[test] + fn validate_bundle_name_accepts_owner_qualified_form() { + assert!(validate_bundle_name("x/oh-my-coyote").is_ok()); + } + + #[test] + fn validate_bundle_name_rejects_empty() { + assert!(validate_bundle_name("").is_err()); + } + + #[test] + fn validate_bundle_name_rejects_multiple_slashes() { + assert!(validate_bundle_name("a/b/c").is_err()); + } + + #[test] + fn validate_bundle_name_rejects_empty_segments() { + assert!(validate_bundle_name("/repo").is_err()); + assert!(validate_bundle_name("owner/").is_err()); + assert!(validate_bundle_name("/").is_err()); + } + + #[test] + fn validate_bundle_name_rejects_disallowed_characters() { + assert!(validate_bundle_name("bad name").is_err()); + assert!(validate_bundle_name("dot.name").is_err()); + assert!(validate_bundle_name("owner/bad!base").is_err()); + } + + #[test] + fn repo_name_slug_from_https_url() { + assert_eq!( + repo_name_slug("https://github.com/x/oh-my-coyote"), + "oh-my-coyote" + ); + } + + #[test] + fn repo_name_slug_strips_git_suffix() { + assert_eq!( + repo_name_slug("https://github.com/x/oh-my-coyote.git"), + "oh-my-coyote" + ); + } + + #[test] + fn repo_name_slug_from_scp_style_url() { + assert_eq!( + repo_name_slug("git@github.com:x/oh-my-coyote.git"), + "oh-my-coyote" + ); + } + + #[test] + fn repo_name_slug_ignores_ref_suffix() { + assert_eq!( + repo_name_slug("https://github.com/x/repo.git#release/v2"), + "repo" + ); + } + + #[test] + fn repo_name_slug_ignores_trailing_slash() { + assert_eq!(repo_name_slug("https://github.com/x/repo/"), "repo"); + } + + #[test] + fn owner_qualifier_from_https_url() { + assert_eq!( + owner_qualifier("https://github.com/x/repo.git").as_deref(), + Some("x") + ); + } + + #[test] + fn owner_qualifier_from_scp_style_url() { + assert_eq!( + owner_qualifier("git@github.com:x/repo.git").as_deref(), + Some("x") + ); + } + + #[test] + fn owner_qualifier_falls_back_to_sanitized_host() { + assert_eq!( + owner_qualifier("https://example.com/repo.git").as_deref(), + Some("example-com") + ); + } + + #[test] + fn owner_qualifier_scp_without_owner_falls_back_to_host() { + assert_eq!( + owner_qualifier("git@host.example.com:repo.git").as_deref(), + Some("host-example-com") + ); + } + + #[test] + fn owner_qualifier_none_without_owner_or_host() { + assert_eq!(owner_qualifier("repo"), None); + } + + #[test] + fn canonical_source_url_treats_equivalent_forms_identically() { + let canonical = canonical_source_url("https://github.com/x/r"); + + assert_eq!(canonical, "github.com/x/r"); + assert_eq!( + canonical_source_url("https://github.com/x/r.git"), + canonical + ); + assert_eq!(canonical_source_url("git@github.com:x/r.git"), canonical); + } + + #[test] + fn canonical_source_url_lowercases_host_but_not_path() { + assert_eq!( + canonical_source_url("https://GitHub.COM/X/R.git"), + "github.com/X/R" + ); + assert_ne!( + canonical_source_url("https://gitlab.example.com/team/Repo"), + canonical_source_url("https://gitlab.example.com/team/repo") + ); + } + + #[test] + fn canonical_source_url_ignores_ref_suffix() { + assert_eq!( + canonical_source_url("https://github.com/x/r.git#main"), + "github.com/x/r" + ); + } + + #[test] + fn canonical_source_url_strips_userinfo() { + assert_eq!( + canonical_source_url("https://user@github.com/x/r.git"), + "github.com/x/r" + ); + } + + fn git_in(dir: &Path, args: &[&str]) { + let mut full: Vec = vec!["-C".into(), dir.as_os_str().into()]; + full.extend(args.iter().map(OsString::from)); + run_git(full).unwrap(); + } + + fn commit_file(dir: &Path, name: &str, content: &str) -> String { + fs::write(dir.join(name), content).unwrap(); + git_in(dir, &["add", "."]); + git_in( + dir, + &[ + "-c", + "user.email=coyote-test@localhost", + "-c", + "user.name=coyote-test", + "-c", + "commit.gpgsign=false", + "commit", + "-q", + "-m", + name, + ], + ); + run_git_capture(vec![ + "-C".into(), + dir.as_os_str().into(), + "rev-parse".into(), + "HEAD".into(), + ]) + .unwrap() + } + + fn init_git_repo(dir: &Path) -> String { + run_git(vec!["init".into(), "-q".into(), dir.as_os_str().into()]).unwrap(); + fs::write(dir.join(".gitattributes"), "* -text\n").unwrap(); + commit_file(dir, "seed.txt", "one") + } + + #[test] + fn clone_to_temp_captures_resolved_head_sha() { + let repo = fresh_temp_dir("clone-sha-"); + let sha = init_git_repo(&repo); + + let temp = clone_to_temp(repo.to_str().unwrap(), None).unwrap(); + + assert_eq!(temp.head_sha(), sha); + assert_eq!(temp.head_sha().len(), 40); + assert!(temp.head_sha().chars().all(|c| c.is_ascii_hexdigit())); + let _ = fs::remove_dir_all(&repo); + } + + #[test] + fn clone_to_temp_respects_sha_ref_pinning() { + let repo = fresh_temp_dir("clone-pin-sha-"); + let first = init_git_repo(&repo); + let second = commit_file(&repo, "next.txt", "two"); + assert_ne!(first, second); + + let temp = clone_to_temp(repo.to_str().unwrap(), Some(&first)).unwrap(); + + assert_eq!(temp.head_sha(), first); + let _ = fs::remove_dir_all(&repo); + } + + #[test] + fn clone_to_temp_respects_branch_ref_pinning() { + let repo = fresh_temp_dir("clone-pin-branch-"); + let first = init_git_repo(&repo); + git_in(&repo, &["branch", "pinned"]); + let second = commit_file(&repo, "next.txt", "two"); + assert_ne!(first, second); + + let temp = clone_to_temp(repo.to_str().unwrap(), Some("pinned")).unwrap(); + + assert_eq!(temp.head_sha(), first); + let _ = fs::remove_dir_all(&repo); + } + + fn write_src(root: &Path, rel: &str, content: &str) { + let path = root.join(rel); + if let Some(parent) = path.parent() { + fs::create_dir_all(parent).unwrap(); + } + fs::write(path, content).unwrap(); + } + + fn init_bundle_repo(dir: &Path) -> String { + run_git(vec!["init".into(), "-q".into(), dir.as_os_str().into()]).unwrap(); + fs::write(dir.join(".gitattributes"), "* -text\n").unwrap(); + commit_file(dir, ".seed", "seed") + } + + fn test_metadata(source: &str) -> InstallMetadata { + InstallMetadata { + source: source.to_string(), + git_ref: None, + commit: "abc123".to_string(), + version: None, + description: None, + homepage: None, + } + } + + #[test] + #[serial] + fn install_remote_records_files_mcp_entries_and_metadata() { + use crate::config::bundles::hash_bytes; + + let _guard = TestVaultConfigGuard::new("prov-full"); + let src_root = fresh_temp_dir("prov-full-src-"); + let repo = src_root.join("my-bundle"); + write_src( + &repo, + BUNDLE_MANIFEST_FILE, + "name: my-bundle\n\ + version: \"2.0\"\n\ + description: Test bundle\n\ + homepage: https://example.com/my-bundle\n", + ); + write_src(&repo, "macros/hello.yaml", "name: hello\n"); + write_src( + &repo, + "functions/mcp.json", + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "echo"}}}"#, + ); + let sha = init_bundle_repo(&repo); + + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + + let store = BundleStore::load().unwrap(); + let record = store.get("my-bundle").unwrap(); + assert_eq!(record.source, repo.to_str().unwrap()); + assert_eq!(record.git_ref, None); + assert_eq!(record.commit, sha); + assert_eq!(record.version.as_deref(), Some("2.0")); + assert_eq!(record.description.as_deref(), Some("Test bundle")); + assert_eq!( + record.homepage.as_deref(), + Some("https://example.com/my-bundle") + ); + assert_eq!(record.files.len(), 1); + assert_eq!(record.files[0].path, "macros/hello.yaml"); + assert_eq!(record.files[0].category, "macros"); + assert_eq!(record.files[0].action, FileAction::New); + assert_eq!(record.files[0].sha256, hash_bytes(b"name: hello\n")); + assert_eq!(record.mcp_servers.len(), 1); + assert_eq!(record.mcp_servers[0].name, "srv"); + assert_eq!(record.mcp_servers[0].action, McpAction::Added); + assert_eq!(record.mcp_servers[0].renamed_to, None); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn install_remote_without_manifest_uses_repo_slug_and_short_sha_version() { + let _guard = TestVaultConfigGuard::new("prov-slug"); + let src_root = fresh_temp_dir("prov-slug-src-"); + let repo = src_root.join("plain.bundle"); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + let sha = init_bundle_repo(&repo); + + install_remote(&format!("{}#{sha}", repo.display()), None, false).unwrap(); + + let store = BundleStore::load().unwrap(); + let record = store.get("plain-bundle").unwrap(); + assert_eq!(record.git_ref.as_deref(), Some(sha.as_str())); + assert_eq!(record.commit, sha); + assert_eq!(record.version.as_deref(), Some(&sha[..7])); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn apply_plan_records_files_written_before_a_mid_run_abort() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping apply_plan_records_files_written_before_a_mid_run_abort: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("prov-abort"); + let src_dir = fresh_temp_dir("prov-abort-src-"); + let src_new = src_dir.join("new.yaml"); + let src_conflict = src_dir.join("conflict.yaml"); + fs::write(&src_new, "new content").unwrap(); + fs::write(&src_conflict, "remote content").unwrap(); + let dst_new = paths::macros_dir().join("new.yaml"); + let dst_conflict = paths::macros_dir().join("conflict.yaml"); + fs::create_dir_all(paths::macros_dir()).unwrap(); + fs::write(&dst_conflict, "local content").unwrap(); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("aborty", test_metadata("https://github.com/x/aborty")) + .unwrap(); + let plan = InstallPlan { + files: vec![ + PlannedFile { + src: src_new, + dst: dst_new.clone(), + kind: PlannedKind::New, + top_category: TopCategory::Macros, + }, + PlannedFile { + src: src_conflict, + dst: dst_conflict.clone(), + kind: PlannedKind::Conflict, + top_category: TopCategory::Macros, + }, + ], + mcp_json: None, + }; + + let err = apply_plan(&plan, StickyMode::None, &mut store, "aborty").unwrap_err(); + + assert!( + err.to_string().contains("Refusing to overwrite"), + "got: {err}" + ); + assert_eq!(fs::read_to_string(&dst_new).unwrap(), "new content"); + assert_eq!(fs::read_to_string(&dst_conflict).unwrap(), "local content"); + let reloaded = BundleStore::load().unwrap(); + let files = &reloaded.get("aborty").unwrap().files; + assert_eq!(files.len(), 1); + assert_eq!(files[0].path, "macros/new.yaml"); + assert_eq!(files[0].action, FileAction::New); + let _ = fs::remove_dir_all(&src_dir); + } + + #[test] + fn apply_plan_keep_all_leaves_prior_owner_intact() { + let dir = fresh_temp_dir("prov-keep-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("alpha", test_metadata("https://github.com/a/alpha")) + .unwrap(); + store + .upsert_bundle("beta", test_metadata("https://github.com/b/beta")) + .unwrap(); + let dst = dir.join("macros/owned.yaml"); + write_src(&dir, "macros/owned.yaml", "alpha content"); + store + .record_file( + "alpha", + FileRecord { + path: "macros/owned.yaml".to_string(), + category: "macros".to_string(), + sha256: hash_file(&dst).unwrap(), + action: FileAction::New, + }, + ) + .unwrap(); + let src = dir.join("beta-src/owned.yaml"); + write_src(&dir, "beta-src/owned.yaml", "beta content"); + let plan = InstallPlan { + files: vec![PlannedFile { + src, + dst: dst.clone(), + kind: PlannedKind::Conflict, + top_category: TopCategory::Macros, + }], + mcp_json: None, + }; + + apply_plan(&plan, StickyMode::KeepAll, &mut store, "beta").unwrap(); + + assert_eq!(fs::read_to_string(&dst).unwrap(), "alpha content"); + let reloaded = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + assert_eq!(reloaded.get("alpha").unwrap().files.len(), 1); + assert!(reloaded.get("beta").unwrap().files.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn second_install_transfers_file_and_mcp_ownership() { + use crate::config::bundles::hash_bytes; + + let _guard = TestVaultConfigGuard::new("prov-transfer"); + let src_root = fresh_temp_dir("prov-transfer-src-"); + let alpha = src_root.join("alpha"); + write_src(&alpha, "macros/shared.yaml", "from: alpha\n"); + write_src( + &alpha, + "functions/mcp.json", + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "alpha"}}}"#, + ); + init_bundle_repo(&alpha); + let beta = src_root.join("beta"); + write_src(&beta, "macros/shared.yaml", "from: beta\n"); + write_src( + &beta, + "functions/mcp.json", + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "beta"}}}"#, + ); + init_bundle_repo(&beta); + + install_remote(alpha.to_str().unwrap(), None, false).unwrap(); + install_remote(beta.to_str().unwrap(), None, true).unwrap(); + + let store = BundleStore::load().unwrap(); + let alpha_record = store.get("alpha").unwrap(); + assert!(alpha_record.files.is_empty()); + assert!(alpha_record.mcp_servers.is_empty()); + let beta_record = store.get("beta").unwrap(); + assert_eq!(beta_record.files.len(), 1); + assert_eq!(beta_record.files[0].path, "macros/shared.yaml"); + assert_eq!(beta_record.files[0].action, FileAction::Replaced); + assert_eq!(beta_record.files[0].sha256, hash_bytes(b"from: beta\n")); + assert_eq!(beta_record.mcp_servers.len(), 1); + assert_eq!(beta_record.mcp_servers[0].name, "srv"); + assert_eq!(beta_record.mcp_servers[0].action, McpAction::Transferred); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn filtered_installs_merge_into_one_record() { + let _guard = TestVaultConfigGuard::new("prov-filter"); + let src_root = fresh_temp_dir("prov-filter-src-"); + let repo = src_root.join("combo"); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + write_src(&repo, "skills/myskill/SKILL.md", "# skill\n"); + let first_sha = init_bundle_repo(&repo); + + install_remote(repo.to_str().unwrap(), Some(InstallFilter::Macros), false).unwrap(); + let second_sha = commit_file(&repo, "extra.txt", "two"); + install_remote(repo.to_str().unwrap(), Some(InstallFilter::Skills), false).unwrap(); + + let store = BundleStore::load().unwrap(); + assert_eq!(store.bundle_names(), vec!["combo"]); + let record = store.get("combo").unwrap(); + assert_ne!(first_sha, second_sha); + assert_eq!(record.commit, second_sha); + let mut owned: Vec<&str> = record.files.iter().map(|f| f.path.as_str()).collect(); + owned.sort(); + assert_eq!(owned, vec!["macros/m.yaml", "skills/myskill/SKILL.md"]); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + fn record_mcp_merge_ignores_kept_local_entries() { + let dir = fresh_temp_dir("prov-mcp-keep-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("alpha", test_metadata("https://github.com/a/alpha")) + .unwrap(); + store + .upsert_bundle("beta", test_metadata("https://github.com/b/beta")) + .unwrap(); + store + .record_mcp_servers( + "alpha", + vec![McpServerRecord { + name: "srv".to_string(), + action: McpAction::Added, + renamed_to: None, + sha256: None, + }], + ) + .unwrap(); + let report = McpMergeReport { + added: Vec::new(), + kept_local: vec!["srv".to_string()], + replaced: Vec::new(), + renamed: Vec::new(), + entry_hashes: HashMap::new(), + final_path: dir.join("mcp.json"), + missing_secrets: Vec::new(), + }; + + record_mcp_merge(&mut store, "beta", &report).unwrap(); + + assert_eq!(store.get("alpha").unwrap().mcp_servers.len(), 1); + assert!(store.get("beta").unwrap().mcp_servers.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn builtin_asset_install_writes_no_provenance() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping builtin_asset_install_writes_no_provenance: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("prov-builtin"); + + crate::config::install_assets(crate::config::AssetCategory::Macros).unwrap(); + + assert!(fs::read_dir(paths::macros_dir()).unwrap().next().is_some()); + assert!(!paths::installed_bundles_file().exists()); + } + + #[test] + #[serial] + fn update_silently_refreshes_owned_unmodified_files() { + use crate::config::bundles::hash_bytes; + + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping update_silently_refreshes_owned_unmodified_files: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("upd-refresh"); + let src_root = fresh_temp_dir("upd-refresh-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: refresh-bundle\n"); + write_src(&repo, "macros/hello.yaml", "v1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + commit_file(&repo, "macros/hello.yaml", "v2\n"); + + update_bundle("refresh-bundle", false).unwrap(); + + let installed = paths::macros_dir().join("hello.yaml"); + assert_eq!(fs::read_to_string(&installed).unwrap(), "v2\n"); + let store = BundleStore::load().unwrap(); + let files = &store.get("refresh-bundle").unwrap().files; + assert_eq!(files.len(), 1); + assert_eq!(files[0].sha256, hash_bytes(b"v2\n")); + assert_eq!(files[0].action, FileAction::Replaced); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn update_with_yes_keeps_modified_files_and_updates_the_rest() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping update_with_yes_keeps_modified_files_and_updates_the_rest: \ + requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("upd-yes"); + let src_root = fresh_temp_dir("upd-yes-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: yes-bundle\n"); + write_src(&repo, "macros/edited.yaml", "v1\n"); + write_src(&repo, "macros/pristine.yaml", "v1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + fs::write(paths::macros_dir().join("edited.yaml"), "local\n").unwrap(); + commit_file(&repo, "macros/edited.yaml", "v2\n"); + commit_file(&repo, "macros/pristine.yaml", "v2\n"); + + let err = update_bundle("yes-bundle", false).unwrap_err(); + assert!( + format!("{err:#}").contains("Refusing to overwrite"), + "err: {err:#}" + ); + + update_bundle("yes-bundle", true).unwrap(); + + assert_eq!( + fs::read_to_string(paths::macros_dir().join("edited.yaml")).unwrap(), + "local\n" + ); + assert_eq!( + fs::read_to_string(paths::macros_dir().join("pristine.yaml")).unwrap(), + "v2\n" + ); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn update_takes_remote_for_owned_unmodified_mcp_entries() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping update_takes_remote_for_owned_unmodified_mcp_entries: \ + requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("upd-mcp-auto"); + let src_root = fresh_temp_dir("upd-mcp-auto-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: mcp-auto-bundle\n"); + write_src( + &repo, + "functions/mcp.json", + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "echo"}}}"#, + ); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + commit_file( + &repo, + "functions/mcp.json", + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "printf"}}}"#, + ); + + update_bundle("mcp-auto-bundle", false).unwrap(); + + let merged = fs::read_to_string(paths::mcp_config_file()).unwrap(); + assert!(merged.contains("printf"), "merged mcp.json: {merged}"); + assert!(!merged.contains("echo"), "merged mcp.json: {merged}"); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + fn apply_plan_keep_all_preserves_local_edit_and_stale_record() { + let dir = fresh_temp_dir("upd-keep-modified-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let dst = dir.join("macros/owned.yaml"); + write_src(&dir, "macros/owned.yaml", "installed content"); + let recorded_sha = hash_file(&dst).unwrap(); + store + .record_file( + "omc", + FileRecord { + path: provenance_path(&dst), + category: "macros".to_string(), + sha256: recorded_sha.clone(), + action: FileAction::New, + }, + ) + .unwrap(); + fs::write(&dst, "local edit").unwrap(); + let src = dir.join("upstream/owned.yaml"); + write_src(&dir, "upstream/owned.yaml", "upstream content"); + let plan = InstallPlan { + files: vec![PlannedFile { + src, + dst: dst.clone(), + kind: PlannedKind::Conflict, + top_category: TopCategory::Macros, + }], + mcp_json: None, + }; + + apply_plan(&plan, StickyMode::KeepAll, &mut store, "omc").unwrap(); + + assert_eq!(fs::read_to_string(&dst).unwrap(), "local edit"); + let reloaded = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + let files = &reloaded.get("omc").unwrap().files; + assert_eq!(files.len(), 1); + assert_eq!(files[0].sha256, recorded_sha); + assert_ne!(files[0].sha256, hash_file(&dst).unwrap()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn apply_plan_replace_all_takes_upstream_and_rerecords_hash() { + use crate::config::bundles::hash_bytes; + + let dir = fresh_temp_dir("upd-replace-modified-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let dst = dir.join("macros/owned.yaml"); + write_src(&dir, "macros/owned.yaml", "installed content"); + store + .record_file( + "omc", + FileRecord { + path: provenance_path(&dst), + category: "macros".to_string(), + sha256: hash_file(&dst).unwrap(), + action: FileAction::New, + }, + ) + .unwrap(); + fs::write(&dst, "local edit").unwrap(); + let src = dir.join("upstream/owned.yaml"); + write_src(&dir, "upstream/owned.yaml", "upstream content"); + let plan = InstallPlan { + files: vec![PlannedFile { + src, + dst: dst.clone(), + kind: PlannedKind::Conflict, + top_category: TopCategory::Macros, + }], + mcp_json: None, + }; + + apply_plan(&plan, StickyMode::ReplaceAll, &mut store, "omc").unwrap(); + + assert_eq!(fs::read_to_string(&dst).unwrap(), "upstream content"); + let reloaded = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + let files = &reloaded.get("omc").unwrap().files; + assert_eq!(files.len(), 1); + assert_eq!(files[0].sha256, hash_bytes(b"upstream content")); + assert_eq!(files[0].action, FileAction::Replaced); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn update_bails_non_interactively_on_unowned_conflict() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping update_bails_non_interactively_on_unowned_conflict: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("upd-unowned"); + let src_root = fresh_temp_dir("upd-unowned-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: unowned-bundle\n"); + write_src(&repo, "macros/a.yaml", "a\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + commit_file(&repo, "macros/user.yaml", "upstream\n"); + fs::create_dir_all(paths::macros_dir()).unwrap(); + fs::write(paths::macros_dir().join("user.yaml"), "local\n").unwrap(); + + let err = update_bundle("unowned-bundle", false).unwrap_err(); + + assert!( + err.to_string().contains("Refusing to overwrite"), + "got: {err}" + ); + assert_eq!( + fs::read_to_string(paths::macros_dir().join("user.yaml")).unwrap(), + "local\n" + ); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + fn apply_plan_replace_all_transfers_ownership_between_bundles() { + let dir = fresh_temp_dir("upd-transfer-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("alpha", test_metadata("https://github.com/a/alpha")) + .unwrap(); + store + .upsert_bundle("beta", test_metadata("https://github.com/b/beta")) + .unwrap(); + let dst = dir.join("macros/shared.yaml"); + write_src(&dir, "macros/shared.yaml", "alpha content"); + store + .record_file( + "alpha", + FileRecord { + path: provenance_path(&dst), + category: "macros".to_string(), + sha256: hash_file(&dst).unwrap(), + action: FileAction::New, + }, + ) + .unwrap(); + let src = dir.join("beta-src/shared.yaml"); + write_src(&dir, "beta-src/shared.yaml", "beta content"); + let plan = InstallPlan { + files: vec![PlannedFile { + src, + dst: dst.clone(), + kind: PlannedKind::Conflict, + top_category: TopCategory::Macros, + }], + mcp_json: None, + }; + + apply_plan(&plan, StickyMode::ReplaceAll, &mut store, "beta").unwrap(); + + assert_eq!(fs::read_to_string(&dst).unwrap(), "beta content"); + let reloaded = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + assert!(reloaded.get("alpha").unwrap().files.is_empty()); + let beta_files = &reloaded.get("beta").unwrap().files; + assert_eq!(beta_files.len(), 1); + assert_eq!(beta_files[0].path, provenance_path(&dst)); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn update_keeps_obsolete_files_non_interactively() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping update_keeps_obsolete_files_non_interactively: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("upd-obsolete-keep"); + let src_root = fresh_temp_dir("upd-obsolete-keep-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: obs-keep\n"); + write_src(&repo, "macros/keep.yaml", "k\n"); + write_src(&repo, "macros/gone.yaml", "g\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + fs::remove_file(repo.join("macros/gone.yaml")).unwrap(); + commit_file(&repo, "macros/keep.yaml", "k2\n"); + + update_bundle("obs-keep", false).unwrap(); + + assert_eq!( + fs::read_to_string(paths::macros_dir().join("gone.yaml")).unwrap(), + "g\n" + ); + let store = BundleStore::load().unwrap(); + let record = store.get("obs-keep").unwrap(); + assert!(record.files.iter().any(|f| f.path == "macros/gone.yaml")); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + fn apply_obsolete_delete_removes_file_and_record() { + let dir = fresh_temp_dir("upd-obsolete-delete-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let dst = dir.join("macros/gone.yaml"); + write_src(&dir, "macros/gone.yaml", "g"); + let path = provenance_path(&dst); + store + .record_file( + "omc", + FileRecord { + path: path.clone(), + category: "macros".to_string(), + sha256: hash_file(&dst).unwrap(), + action: FileAction::New, + }, + ) + .unwrap(); + + apply_obsolete_action(&mut store, "omc", &path, &dst, &dir, ObsoleteAction::Delete) + .unwrap(); + + assert!(!dst.exists()); + let reloaded = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + assert!(reloaded.get("omc").unwrap().files.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn handle_obsolete_drops_records_for_missing_files() { + let dir = fresh_temp_dir("upd-obsolete-missing-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + store + .record_file( + "omc", + FileRecord { + path: "macros/ghost-bundle-test.yaml".to_string(), + category: "macros".to_string(), + sha256: "0".repeat(64), + action: FileAction::New, + }, + ) + .unwrap(); + let plan = InstallPlan { + files: Vec::new(), + mcp_json: None, + }; + + handle_obsolete_files(&mut store, "omc", &plan, false).unwrap(); + + assert!(store.get("omc").unwrap().files.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn handle_obsolete_never_touches_suspicious_recorded_paths() { + let dir = fresh_temp_dir("upd-obsolete-suspicious-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + for hostile in ["/etc/nonexistent-coyote-test", "../escape.yaml"] { + store + .record_file( + "omc", + FileRecord { + path: hostile.to_string(), + category: "macros".to_string(), + sha256: "0".repeat(64), + action: FileAction::New, + }, + ) + .unwrap(); + } + let plan = InstallPlan { + files: Vec::new(), + mcp_json: None, + }; + + handle_obsolete_files(&mut store, "omc", &plan, false).unwrap(); + + assert_eq!(store.get("omc").unwrap().files.len(), 2); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn update_refreshes_record_metadata_and_stamps_updated_at() { + let _guard = TestVaultConfigGuard::new("upd-meta"); + let src_root = fresh_temp_dir("upd-meta-src-"); + let repo = src_root.join("bundle"); + write_src( + &repo, + BUNDLE_MANIFEST_FILE, + "name: meta-bundle\n\ + version: \"1.0\"\n\ + description: Old\n\ + homepage: https://example.com/old\n", + ); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + let installed_at = BundleStore::load() + .unwrap() + .get("meta-bundle") + .unwrap() + .installed_at + .clone(); + let new_sha = commit_file( + &repo, + BUNDLE_MANIFEST_FILE, + "name: meta-bundle\n\ + version: \"2.0\"\n\ + description: New\n\ + homepage: https://example.com/new\n", + ); + + update_bundle("meta-bundle", false).unwrap(); + + let store = BundleStore::load().unwrap(); + let record = store.get("meta-bundle").unwrap(); + assert_eq!(record.commit, new_sha); + assert_eq!(record.version.as_deref(), Some("2.0")); + assert_eq!(record.description.as_deref(), Some("New")); + assert_eq!(record.homepage.as_deref(), Some("https://example.com/new")); + assert!(record.updated_at.is_some()); + assert_eq!(record.installed_at, installed_at); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn update_unknown_bundle_lists_installed_names() { + let _guard = TestVaultConfigGuard::new("upd-unknown-name"); + + let err = update_bundle("nope", false).unwrap_err(); + assert!(err.to_string().contains("none are installed"), "got: {err}"); + + let src_root = fresh_temp_dir("upd-unknown-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: known-bundle\n"); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + + let err = update_bundle("nope", false).unwrap_err(); + + assert!( + err.to_string().contains("installed bundles: known-bundle"), + "got: {err}" + ); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn update_honors_recorded_sha_pin() { + let _guard = TestVaultConfigGuard::new("upd-pin"); + let src_root = fresh_temp_dir("upd-pin-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: pin-bundle\n"); + write_src(&repo, "macros/one.yaml", "1\n"); + let pinned = init_bundle_repo(&repo); + install_remote(&format!("{}#{pinned}", repo.display()), None, false).unwrap(); + let newer = commit_file(&repo, "macros/two.yaml", "2\n"); + assert_ne!(pinned, newer); + + update_bundle("pin-bundle", false).unwrap(); + + let store = BundleStore::load().unwrap(); + let record = store.get("pin-bundle").unwrap(); + assert_eq!(record.commit, pinned); + assert_eq!(record.git_ref.as_deref(), Some(pinned.as_str())); + assert!(!paths::macros_dir().join("two.yaml").exists()); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn update_ref_override_moves_the_pin() { + let _guard = TestVaultConfigGuard::new("upd-move-pin"); + let src_root = fresh_temp_dir("upd-move-pin-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: move-bundle\n"); + write_src(&repo, "macros/one.yaml", "1\n"); + let pinned = init_bundle_repo(&repo); + install_remote(&format!("{}#{pinned}", repo.display()), None, false).unwrap(); + let newer = commit_file(&repo, "macros/two.yaml", "2\n"); + + update_bundle(&format!("move-bundle#{newer}"), false).unwrap(); + + let store = BundleStore::load().unwrap(); + let record = store.get("move-bundle").unwrap(); + assert_eq!(record.commit, newer); + assert_eq!(record.git_ref.as_deref(), Some(newer.as_str())); + assert_eq!( + fs::read_to_string(paths::macros_dir().join("two.yaml")).unwrap(), + "2\n" + ); + let _ = fs::remove_dir_all(&src_root); + } + + fn owned_names(names: &[&str]) -> Vec { + names.iter().map(|s| s.to_string()).collect() + } + + #[test] + fn classify_urls_and_paths_as_remote_sources() { + for value in [ + "https://github.com/x/y", + "git@github.com:x/y.git", + "./bundle-dir", + "../up", + "/abs/path", + "~/home-rel", + ] { + assert_eq!( + classify_install_target(value, &[]), + InstallTarget::RemoteSource, + "value: {value}" + ); + } + } + + #[test] + fn classify_categories_win_over_installed_names() { + assert_eq!( + classify_install_target("agents", &owned_names(&["agents"])), + InstallTarget::Category(AssetCategory::Agents) + ); + assert_eq!( + classify_install_target("mcp_config", &[]), + InstallTarget::Category(AssetCategory::McpConfig) + ); + } + + #[test] + fn classify_installed_names_with_optional_ref() { + let installed = owned_names(&["omc"]); + assert_eq!( + classify_install_target("omc", &installed), + InstallTarget::InstalledBundle + ); + assert_eq!( + classify_install_target("omc#v2", &installed), + InstallTarget::InstalledBundle + ); + } + + #[test] + fn classify_bare_names_without_a_match_as_unknown() { + assert_eq!(classify_install_target("omc", &[]), InstallTarget::Unknown); + assert_eq!( + classify_install_target("not-a-bundle", &owned_names(&["omc"])), + InstallTarget::Unknown + ); + } + + #[test] + #[serial] + fn install_or_update_redirects_categories_to_install_builtins() { + let _guard = TestVaultConfigGuard::new("iou-category"); + + let err = install_or_update("agents", None, None, false).unwrap_err(); + + assert!( + err.to_string().contains("--install-builtins agents"), + "got: {err}" + ); + } + + #[test] + #[serial] + fn install_or_update_unknown_name_lists_installed_bundles() { + let _guard = TestVaultConfigGuard::new("iou-unknown"); + + let err = install_or_update("not-a-bundle", None, None, false).unwrap_err(); + assert!(err.to_string().contains("none are installed"), "got: {err}"); + + let src_root = fresh_temp_dir("iou-unknown-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: known-bundle\n"); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + + let err = install_or_update("not-a-bundle", None, None, false).unwrap_err(); + + assert!( + err.to_string().contains("no bundle named 'not-a-bundle'"), + "got: {err}" + ); + assert!( + err.to_string().contains("installed bundles: known-bundle"), + "got: {err}" + ); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + #[serial] + fn install_or_update_category_error_hints_update_for_shadowed_bundle() { + let _guard = TestVaultConfigGuard::new("iou-shadow"); + + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle( + "agents", + InstallMetadata { + source: "https://github.com/x/agents".to_string(), + git_ref: None, + commit: "abc123".to_string(), + version: None, + description: None, + homepage: None, + }, + ) + .unwrap(); + + let err = install_or_update("agents", None, None, false).unwrap_err(); + + assert!( + err.to_string().contains("--install-builtins agents"), + "got: {err}" + ); + assert!( + err.to_string().contains("--update-bundle agents"), + "got: {err}" + ); + } + + #[test] + #[serial] + fn install_or_update_rejects_remote_flags_for_updates() { + let _guard = TestVaultConfigGuard::new("iou-flags"); + let src_root = fresh_temp_dir("iou-flags-src-"); + let repo = src_root.join("bundle"); + write_src(&repo, BUNDLE_MANIFEST_FILE, "name: flag-bundle\n"); + write_src(&repo, "macros/m.yaml", "a: 1\n"); + init_bundle_repo(&repo); + install_remote(repo.to_str().unwrap(), None, false).unwrap(); + + let err = + install_or_update("flag-bundle", None, Some(InstallFilter::Macros), false).unwrap_err(); + assert!( + err.to_string().contains("only apply to remote installs"), + "got: {err}" + ); + + let err = install_or_update("flag-bundle", None, None, true).unwrap_err(); + assert!( + err.to_string().contains("only apply to remote installs"), + "got: {err}" + ); + let _ = fs::remove_dir_all(&src_root); + } + + #[test] + fn shorthand_accepts_owner_repo_and_deeper_paths() { + assert!(is_repo_shorthand("someuser/oh-my-coyote")); + assert!(is_repo_shorthand("group/subgroup/repo")); + assert!(is_repo_shorthand("someuser/repo#v2")); + } + + #[test] + fn shorthand_rejects_urls_paths_and_bare_names() { + assert!(!is_repo_shorthand("https://github.com/x/y")); + assert!(!is_repo_shorthand("git@github.com:x/y.git")); + assert!(!is_repo_shorthand("./local/dir")); + assert!(!is_repo_shorthand("/abs/path")); + assert!(!is_repo_shorthand("~/home/path")); + assert!(!is_repo_shorthand("-flag/like")); + assert!(!is_repo_shorthand("bare-name")); + assert!(!is_repo_shorthand("a//b")); + assert!(!is_repo_shorthand("a/b/")); + assert!(!is_repo_shorthand("a\\b")); + assert!(!is_repo_shorthand("a b/c")); + } + + #[test] + fn shorthand_expands_against_default_and_custom_hosts() { + assert_eq!( + expand_repo_shorthand("someuser/omc", None).unwrap(), + "https://github.com/someuser/omc" + ); + assert_eq!( + expand_repo_shorthand("someuser/omc#v2", Some("git.somedomain.com")).unwrap(), + "https://git.somedomain.com/someuser/omc#v2" + ); + assert_eq!( + expand_repo_shorthand("a/b", Some("https://git.x.com/")).unwrap(), + "https://git.x.com/a/b" + ); + assert!(expand_repo_shorthand("a/b", Some("bad/host")).is_err()); + assert!(expand_repo_shorthand("a/b", Some("")).is_err()); + } + + #[test] + fn classify_prefers_installed_names_over_shorthand() { + assert_eq!( + classify_install_target("someuser/omc", &[]), + InstallTarget::Shorthand + ); + assert_eq!( + classify_install_target("someuser/omc", &["someuser/omc".to_string()]), + InstallTarget::InstalledBundle + ); + } + + #[test] + fn install_or_update_git_host_rejects_non_shorthand_values() { + let err = install_or_update("https://github.com/x/y", Some("git.x.com"), None, false) + .unwrap_err(); + assert!( + err.to_string().contains("--git-host only applies"), + "got: {err}" + ); + + let err = install_or_update("bare-name", Some("git.x.com"), None, false).unwrap_err(); + assert!( + err.to_string().contains("--git-host only applies"), + "got: {err}" + ); + } + + #[test] + fn repl_install_flags_parse_git_host() { + let (filter, force, host) = parse_repl_install_flags( + ".install", + vec!["--git-host".to_string(), "git.x.com".to_string()].into_iter(), + ) + .unwrap(); + assert_eq!(host.as_deref(), Some("git.x.com")); + assert!(filter.is_none() && !force); + + let (_, force, host) = parse_repl_install_flags( + ".install", + vec!["--git-host=git.y.com".to_string(), "--force".to_string()].into_iter(), + ) + .unwrap(); + assert_eq!(host.as_deref(), Some("git.y.com")); + assert!(force); + } + + #[test] + #[serial] + fn uninstall_shorthand_resolves_a_single_source_match() { + let _guard = TestVaultConfigGuard::new("uninst-short-one"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + drop(store); + + uninstall_bundle("someuser/omc", true).unwrap(); + + let store = BundleStore::load().unwrap(); + assert!(store.get("omc").is_none()); + } + + #[test] + #[serial] + fn uninstall_shorthand_with_multiple_matches_bails_non_interactively() { + let _guard = TestVaultConfigGuard::new("uninst-short-multi"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + store + .upsert_bundle( + "omc-fork", + test_metadata("https://git.somedomain.com/someuser/omc"), + ) + .unwrap(); + drop(store); + + let err = uninstall_bundle("someuser/omc", true).unwrap_err(); + + assert!( + err.to_string() + .contains("matches multiple installed bundles"), + "got: {err}" + ); + assert!(err.to_string().contains("omc-fork"), "got: {err}"); + let store = BundleStore::load().unwrap(); + assert!(store.get("omc").is_some()); + assert!(store.get("omc-fork").is_some()); + } + + #[test] + #[serial] + fn uninstall_exact_name_wins_over_shorthand_source_matches() { + let _guard = TestVaultConfigGuard::new("uninst-short-name"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle( + "someuser/omc", + test_metadata("https://git.somedomain.com/other/repo"), + ) + .unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/someuser/omc")) + .unwrap(); + drop(store); + + uninstall_bundle("someuser/omc", true).unwrap(); + + let store = BundleStore::load().unwrap(); + assert!(store.get("someuser/omc").is_none()); + assert!(store.get("omc").is_some()); + } + + fn owned_file_record(path: &str, contents: &str) -> FileRecord { + FileRecord { + path: path.to_string(), + category: "macros".to_string(), + sha256: hash_bytes(contents.as_bytes()), + action: FileAction::New, + } + } + + fn mcp_server_record(name: &str, action: McpAction, sha256: Option) -> McpServerRecord { + McpServerRecord { + name: name.to_string(), + action, + renamed_to: None, + sha256, + } + } + + #[test] + #[serial] + fn uninstall_deletes_intact_files_and_removes_the_bundle_record() { + let _guard = TestVaultConfigGuard::new("uninst-intact"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("gone", test_metadata("https://github.com/x/gone")) + .unwrap(); + let dst = paths::macros_dir().join("hello.yaml"); + fs::create_dir_all(paths::macros_dir()).unwrap(); + fs::write(&dst, "hi\n").unwrap(); + store + .record_file("gone", owned_file_record("macros/hello.yaml", "hi\n")) + .unwrap(); + + uninstall_bundle("gone", true).unwrap(); + + assert!(!dst.exists()); + assert!( + !paths::macros_dir().exists(), + "emptied macros dir should be pruned" + ); + assert!(BundleStore::load().unwrap().get("gone").is_none()); + } + + #[test] + #[serial] + fn uninstall_keeps_modified_files_and_retains_the_record() { + let _guard = TestVaultConfigGuard::new("uninst-modified"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("mods", test_metadata("https://github.com/x/mods")) + .unwrap(); + let dst = paths::macros_dir().join("edited.yaml"); + fs::create_dir_all(paths::macros_dir()).unwrap(); + fs::write(&dst, "user edit\n").unwrap(); + store + .record_file( + "mods", + owned_file_record("macros/edited.yaml", "original\n"), + ) + .unwrap(); + + uninstall_bundle("mods", true).unwrap(); + + assert_eq!(fs::read_to_string(&dst).unwrap(), "user edit\n"); + let store = BundleStore::load().unwrap(); + let record = store.get("mods").unwrap(); + assert_eq!(record.files.len(), 1); + assert_eq!(record.files[0].path, "macros/edited.yaml"); + } + + #[test] + fn apply_uninstall_delete_removes_file_record_and_empty_dirs() { + let dir = fresh_temp_dir("uninst-apply-delete-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let dst = dir.join("macros/gone.yaml"); + write_src(&dir, "macros/gone.yaml", "user edit"); + store + .record_file("omc", owned_file_record("macros/gone.yaml", "original")) + .unwrap(); + let mut summary = UninstallFileSummary::default(); + + apply_uninstall_file_action( + &mut store, + "omc", + "macros/gone.yaml", + &dst, + &dir, + ObsoleteAction::Delete, + &mut summary, + ) + .unwrap(); + + assert!(!dst.exists()); + assert!(!dir.join("macros").exists()); + assert_eq!(summary.deleted, 1); + assert!(store.get("omc").unwrap().files.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_files_drop_records_for_missing_files() { + let dir = fresh_temp_dir("uninst-missing-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + store + .record_file("omc", owned_file_record("macros/ghost.yaml", "gone")) + .unwrap(); + let files = store.get("omc").unwrap().files.clone(); + + let summary = uninstall_owned_files(&mut store, "omc", &files, &dir, true).unwrap(); + + assert_eq!( + summary, + UninstallFileSummary { + missing: 1, + ..UninstallFileSummary::default() + } + ); + assert!(store.get("omc").unwrap().files.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_files_never_touch_unowned_files() { + let dir = fresh_temp_dir("uninst-unowned-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + write_src(&dir, "macros/owned.yaml", "a"); + write_src(&dir, "macros/user.yaml", "mine"); + store + .record_file("omc", owned_file_record("macros/owned.yaml", "a")) + .unwrap(); + let files = store.get("omc").unwrap().files.clone(); + + let summary = uninstall_owned_files(&mut store, "omc", &files, &dir, true).unwrap(); + + assert_eq!(summary.deleted, 1); + assert!(!dir.join("macros/owned.yaml").exists()); + assert_eq!( + fs::read_to_string(dir.join("macros/user.yaml")).unwrap(), + "mine" + ); + let _ = fs::remove_dir_all(&dir); + } + + #[cfg(unix)] + #[test] + fn uninstall_files_keep_records_when_deletion_fails_and_continue() { + use std::os::unix::fs::PermissionsExt; + + struct RestorePerms(PathBuf); + impl Drop for RestorePerms { + fn drop(&mut self) { + let _ = fs::set_permissions(&self.0, fs::Permissions::from_mode(0o755)); + } + } + + let dir = fresh_temp_dir("uninst-fail-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + write_src(&dir, "locked/stuck.yaml", "a"); + write_src(&dir, "macros/ok.yaml", "b"); + store + .record_file("omc", owned_file_record("locked/stuck.yaml", "a")) + .unwrap(); + store + .record_file("omc", owned_file_record("macros/ok.yaml", "b")) + .unwrap(); + let locked = dir.join("locked"); + fs::set_permissions(&locked, fs::Permissions::from_mode(0o555)).unwrap(); + let restore = RestorePerms(locked.clone()); + let probe = locked.join(".write-probe"); + if fs::write(&probe, "x").is_ok() { + // A privileged user bypasses permission bits; the failure path + // cannot be provoked this way. + let _ = fs::remove_file(&probe); + drop(restore); + let _ = fs::remove_dir_all(&dir); + return; + } + let files = store.get("omc").unwrap().files.clone(); + + let summary = uninstall_owned_files(&mut store, "omc", &files, &dir, true).unwrap(); + drop(restore); + + assert_eq!(summary.failed, 1); + assert_eq!(summary.deleted, 1); + assert!(dir.join("locked/stuck.yaml").exists()); + assert!(!dir.join("macros/ok.yaml").exists()); + let paths: Vec<&str> = store + .get("omc") + .unwrap() + .files + .iter() + .map(|f| f.path.as_str()) + .collect(); + assert_eq!(paths, vec!["locked/stuck.yaml"]); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_files_skip_suspicious_recorded_paths() { + let dir = fresh_temp_dir("uninst-suspicious-"); + let config_dir = dir.join("config"); + fs::create_dir_all(&config_dir).unwrap(); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + write_src(&dir, "evil.yaml", "outside"); + let abs_victim = dir.join("abs.yaml"); + fs::write(&abs_victim, "outside").unwrap(); + store + .record_file("omc", owned_file_record("../evil.yaml", "outside")) + .unwrap(); + store + .record_file( + "omc", + owned_file_record(abs_victim.to_str().unwrap(), "outside"), + ) + .unwrap(); + let files = store.get("omc").unwrap().files.clone(); + + let summary = uninstall_owned_files(&mut store, "omc", &files, &config_dir, true).unwrap(); + + assert_eq!(summary.failed, 2); + assert_eq!(summary.deleted, 0); + assert!(dir.join("evil.yaml").exists()); + assert!(abs_victim.exists()); + assert_eq!(store.get("omc").unwrap().files.len(), 2); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_mcp_keeps_replaced_entries_and_drops_the_record() { + let dir = fresh_temp_dir("uninst-mcp-replaced-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + store + .record_mcp_servers( + "omc", + vec![mcp_server_record("srv", McpAction::Replaced, None)], + ) + .unwrap(); + let mcp = dir.join("mcp.json"); + write_mcp( + &mcp, + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "echo"}}}"#, + ); + let servers = store.get("omc").unwrap().mcp_servers.clone(); + + let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap(); + + assert_eq!(summary.kept, vec!["srv"]); + assert!(summary.removed.is_empty()); + let written: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + assert!(written.mcp_servers.contains_key("srv")); + assert!(store.get("omc").unwrap().mcp_servers.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_mcp_removes_intact_entries_and_leaves_unowned_keys() { + let dir = fresh_temp_dir("uninst-mcp-intact-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let mcp = dir.join("mcp.json"); + write_mcp( + &mcp, + r#"{"mcpServers": { + "srv": {"type": "stdio", "command": "echo"}, + "user-srv": {"type": "stdio", "command": "mine"} + }}"#, + ); + let parsed: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + let hash = hash_bytes( + serde_json::to_string(parsed.mcp_servers.get("srv").unwrap()) + .unwrap() + .as_bytes(), + ); + store + .record_mcp_servers( + "omc", + vec![mcp_server_record("srv", McpAction::Added, Some(hash))], + ) + .unwrap(); + let servers = store.get("omc").unwrap().mcp_servers.clone(); + + let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap(); + + assert_eq!(summary.removed, vec!["srv"]); + assert!(summary.kept.is_empty()); + let written: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + assert!(!written.mcp_servers.contains_key("srv")); + assert!(written.mcp_servers.contains_key("user-srv")); + assert!(store.get("omc").unwrap().mcp_servers.is_empty()); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_mcp_reports_referenced_secrets_without_removing_them() { + let dir = fresh_temp_dir("uninst-mcp-secrets-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let mcp = dir.join("mcp.json"); + write_mcp( + &mcp, + r#"{"mcpServers": { + "srv": { + "type": "stdio", + "command": "echo", + "env": {"TOKEN": "{{OMC_TOKEN}}", "ORG": "{{OMC_ORG}}"} + } + }}"#, + ); + let parsed: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + let hash = hash_bytes( + serde_json::to_string(parsed.mcp_servers.get("srv").unwrap()) + .unwrap() + .as_bytes(), + ); + store + .record_mcp_servers( + "omc", + vec![mcp_server_record("srv", McpAction::Added, Some(hash))], + ) + .unwrap(); + let servers = store.get("omc").unwrap().mcp_servers.clone(); + + let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap(); + + assert_eq!(summary.removed, vec!["srv"]); + assert_eq!(summary.secrets, vec!["OMC_ORG", "OMC_TOKEN"]); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_mcp_keeps_modified_entries_and_their_records() { + let dir = fresh_temp_dir("uninst-mcp-modified-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let mcp = dir.join("mcp.json"); + write_mcp( + &mcp, + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "edited"}}}"#, + ); + store + .record_mcp_servers( + "omc", + vec![mcp_server_record( + "srv", + McpAction::Added, + Some("0".repeat(64)), + )], + ) + .unwrap(); + let servers = store.get("omc").unwrap().mcp_servers.clone(); + + let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap(); + + assert_eq!(summary.kept, vec!["srv"]); + assert!(summary.removed.is_empty()); + let written: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + assert!(written.mcp_servers.contains_key("srv")); + assert_eq!(store.get("omc").unwrap().mcp_servers.len(), 1); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + fn uninstall_mcp_keeps_legacy_records_without_a_hash() { + let dir = fresh_temp_dir("uninst-mcp-legacy-"); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let mcp = dir.join("mcp.json"); + write_mcp( + &mcp, + r#"{"mcpServers": {"srv": {"type": "stdio", "command": "echo"}}}"#, + ); + store + .record_mcp_servers( + "omc", + vec![mcp_server_record("srv", McpAction::Added, None)], + ) + .unwrap(); + let servers = store.get("omc").unwrap().mcp_servers.clone(); + + let summary = uninstall_mcp_entries(&mut store, "omc", &servers, &mcp, true).unwrap(); + + assert_eq!(summary.kept, vec!["srv"]); + let written: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&mcp).unwrap()).unwrap(); + assert!(written.mcp_servers.contains_key("srv")); + assert_eq!(store.get("omc").unwrap().mcp_servers.len(), 1); + let _ = fs::remove_dir_all(&dir); + } + + #[test] + #[serial] + fn uninstall_unknown_name_lists_installed_and_url_spelling_resolves() { + let _guard = TestVaultConfigGuard::new("uninst-unknown"); + + let err = uninstall_bundle("nope", true).unwrap_err(); + assert!(err.to_string().contains("none are installed"), "got: {err}"); + + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + let err = uninstall_bundle("nope", true).unwrap_err(); + assert!( + err.to_string().contains("installed bundles: omc"), + "got: {err}" + ); + + uninstall_bundle("git@github.com:x/omc.git", true).unwrap(); + + assert!(BundleStore::load().unwrap().get("omc").is_none()); + } + + #[test] + #[serial] + fn uninstall_refuses_non_interactive_without_yes() { + if *IS_STDOUT_TERMINAL { + eprintln!( + "Skipping uninstall_refuses_non_interactive_without_yes: requires non-TTY stdout" + ); + return; + } + let _guard = TestVaultConfigGuard::new("uninst-non-tty"); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + + let err = uninstall_bundle("omc", false).unwrap_err(); + + assert!(err.to_string().contains("--yes"), "got: {err}"); + assert!(BundleStore::load().unwrap().get("omc").is_some()); + } + + #[test] + #[serial] + fn record_mcp_merge_stores_entry_hashes() { + let _guard = TestVaultConfigGuard::new("uninst-merge-hash"); + let dir = fresh_temp_dir("uninst-merge-hash-"); + let remote = dir.join("remote.json"); + let target = dir.join("target.json"); + write_mcp(&remote, FIXTURE_REMOTE); + let mut store = BundleStore::load_from(dir.join("installed-bundles.yaml")).unwrap(); + store + .upsert_bundle("omc", test_metadata("https://github.com/x/omc")) + .unwrap(); + + let report = merge_mcp_json(None, &remote, &target, false, &HashSet::new(), false).unwrap(); + record_mcp_merge(&mut store, "omc", &report).unwrap(); + + let written: McpServersConfig = + serde_json::from_str(&fs::read_to_string(&target).unwrap()).unwrap(); + let expected = hash_bytes( + serde_json::to_string(written.mcp_servers.get("alpha").unwrap()) + .unwrap() + .as_bytes(), + ); + let record = store.get("omc").unwrap(); + let alpha = record + .mcp_servers + .iter() + .find(|s| s.name == "alpha") + .unwrap(); + assert_eq!(alpha.sha256.as_deref(), Some(expected.as_str())); + let _ = fs::remove_dir_all(&dir); + } } diff --git a/src/config/mod.rs b/src/config/mod.rs index ea90e76..762ca5e 100644 --- a/src/config/mod.rs +++ b/src/config/mod.rs @@ -1,6 +1,7 @@ mod agent; mod app_config; mod app_state; +mod bundles; mod input; mod install_remote; pub(crate) mod instructions; @@ -29,8 +30,11 @@ pub use self::agent::{ pub use self::app_config::AppConfig; #[allow(unused_imports)] pub use self::app_state::AppState; +pub use self::bundles::list_installed_bundles; pub use self::input::Input; -pub use self::install_remote::{install_remote, install_remote_from_repl_args}; +pub use self::install_remote::{ + install_or_update, install_or_update_from_repl_args, uninstall_bundle, update_bundle, +}; pub use self::macro_policy::{ MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, ResolvedMacro, }; @@ -155,6 +159,8 @@ const SBX_KIT_DIR_NAME: &str = "sbx-kit"; const SBX_KIT_HASH_FILE: &str = "kit.sha256"; const SBX_MIXIN_FILE_NAME: &str = "sbx-mixin.yaml"; pub(crate) const VAULT_DATA_FILE_NAME: &str = "vault.yml"; +const INSTALLED_BUNDLES_FILE_NAME: &str = "installed-bundles.yaml"; +const BUNDLE_MANIFEST_FILE: &str = "coyote-bundle.yaml"; const SBX_MIXIN_KITS_DIR_NAME: &str = "sbx-mixin-kits"; const GIT_DIR_NAME: &str = ".git"; const GITIGNORE_FILE_NAME: &str = ".gitignore"; diff --git a/src/config/paths.rs b/src/config/paths.rs index 6a0ecfe..0e2154e 100644 --- a/src/config/paths.rs +++ b/src/config/paths.rs @@ -2,10 +2,10 @@ use super::role::Role; use super::{ AGENT_GRAPH_FILE_NAME, AGENTS_DIR_NAME, BASH_PROMPT_UTILS_FILE_NAME, CONFIG_FILE_NAME, ENV_FILE_NAME, FUNCTIONS_BIN_DIR_NAME, FUNCTIONS_DIR_NAME, GLOBAL_TOOLS_DIR_NAME, - GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, MACROS_DIR_NAME, MCP_FILE_NAME, - MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride, RAGS_DIR_NAME, ROLES_DIR_NAME, - SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, SBX_MIXIN_KITS_DIR_NAME, - SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME, + GLOBAL_TOOLS_UTILS_DIR_NAME, HIDDEN_MCP_FILE_NAME, INSTALLED_BUNDLES_FILE_NAME, + MACROS_DIR_NAME, MCP_FILE_NAME, MEMORY_DIR_NAME, MEMORY_INDEX_FILE_NAME, ModelsOverride, + RAGS_DIR_NAME, ROLES_DIR_NAME, SBX_KIT_DIR_NAME, SBX_KIT_HASH_FILE, SBX_MIXIN_FILE_NAME, + SBX_MIXIN_KITS_DIR_NAME, SKILLS_DIR_NAME, WORKSPACE_COYOTE_DIR_NAME, }; use crate::client::ProviderModels; use crate::config::REPL_HISTORY_DIR_NAME; @@ -169,6 +169,10 @@ pub fn config_file() -> PathBuf { } } +pub fn installed_bundles_file() -> PathBuf { + local_dir(INSTALLED_BUNDLES_FILE_NAME) +} + pub fn roles_dir() -> PathBuf { match env::var(get_env_name("roles_dir")) { Ok(value) => PathBuf::from(value), diff --git a/src/config/request_context.rs b/src/config/request_context.rs index 24597b6..2a0a9a8 100644 --- a/src/config/request_context.rs +++ b/src/config/request_context.rs @@ -1,3 +1,4 @@ +use super::bundles::BundleStore; use super::rag_cache::{RagCache, RagKey}; use super::session::Session; use super::skill::{SKILL_SCAFFOLD, Skill}; @@ -10,7 +11,7 @@ use super::{ Input, InstallFilter, LEFT_PROMPT, LastMessage, MESSAGES_FILE_NAME, MacroAllowlistLevel, MacroPolicy, MacroSource, MacroState, RESERVED_MACRO_NAMES, RIGHT_PROMPT, ResolvedMacro, Role, RoleLike, SESSIONS_DIR_NAME, SUMMARIZATION_PROMPT, SUMMARY_CONTEXT_PROMPT, StateFlags, - TEMP_ROLE_NAME, TEMP_SESSION_NAME, WorkingMode, ensure_parent_exists, + TEMP_ROLE_NAME, TEMP_SESSION_NAME, WorkingMode, bundles, ensure_parent_exists, list_agents_with_descriptions, memory, paths, }; use super::{MessageContentToolCalls, prompts}; @@ -35,6 +36,7 @@ use crate::utils::{ }; use comfy_table::{ContentArrangement, Table, presets::UTF8_FULL}; +use super::install_remote::DEFAULT_GIT_HOST; use super::instructions; use super::memory::{ DEFAULT_MEMORY_CAP_WITH_TOOLS, DEFAULT_MEMORY_CAP_WITHOUT_TOOLS, MemoryStore, WorkspaceMemory, @@ -58,6 +60,22 @@ use std::sync::Arc; use std::time::Duration; use std::{env, fs}; +/// Completion must degrade rather than break the prompt, but a corrupt store +/// should not vanish silently: the failure is logged before returning empty. +fn installed_bundle_names() -> Vec { + match BundleStore::load() { + Ok(store) => store + .bundle_names() + .into_iter() + .map(str::to_string) + .collect(), + Err(e) => { + warn!("skipping bundle-name completion: {e:#}"); + Vec::new() + } + } +} + pub struct AutoContinueConfig { pub enabled: bool, pub max_continues: usize, @@ -112,7 +130,7 @@ fn print_asset_names(kind: &str, names: &[String]) -> Result<()> { Ok(()) } -fn asset_table(header: &[&str]) -> Table { +pub(crate) fn asset_table(header: &[&str]) -> Table { let mut table = Table::new(); table.load_preset(UTF8_FULL); table.set_content_arrangement(ContentArrangement::Dynamic); @@ -2786,8 +2804,9 @@ impl RequestContext { } Ok(()) } + "bundles" => bundles::list_installed_bundles(), _ => bail!( - "Unknown kind '{kind}'. Valid kinds: roles, sessions, agents, rags, macros, skills, tools, mcp-servers" + "Unknown kind '{kind}'. Valid kinds: roles, sessions, agents, rags, macros, skills, tools, mcp-servers, bundles" ), } } @@ -3261,9 +3280,18 @@ impl RequestContext { ".install" => { let mut values: Vec = AssetCategory::NAMES.iter().map(|s| s.to_string()).collect(); - values.push("remote".to_string()); + values.extend(installed_bundle_names()); super::map_completion_values(values) } + ".uninstall" => { + let mut values = super::map_completion_values(installed_bundle_names()); + values.push(( + "--yes".to_string(), + Some("Skip the uninstall confirmation".to_string()), + )); + + values + } ".macro" => { let policy = self.macro_policy(); let mut values: Vec<(String, Option)> = policy @@ -3327,6 +3355,7 @@ impl RequestContext { "skills", "tools", "mcp-servers", + "bundles", ]), ".vault" => { let mut values = vec!["add", "get", "update", "delete", "list"]; @@ -3469,17 +3498,22 @@ impl RequestContext { values = complete_skills_with_descriptions(paths::list_skills()); } else if cmd == ".skill" && args.first() == Some(&"unload") && args.len() == 2 { values = complete_skills_with_descriptions(self.skill_registry.loaded_names()); - } else if cmd == ".install" && args.first() == Some(&"remote") && args.len() >= 2 { + } else if cmd == ".install" && args.len() >= 2 { let prev = args.get(args.len() - 2).copied().unwrap_or(""); if prev == "--filter" { values = super::map_completion_values( InstallFilter::NAMES.iter().map(|s| s.to_string()).collect(), ); + } else if prev == "--git-host" { + values = super::map_completion_values(vec![DEFAULT_GIT_HOST.to_string()]); } else { let has_filter = args.iter().enumerate().any(|(i, a)| { a.starts_with("--filter=") || (*a == "--filter" && i < args.len() - 1) }); let has_force = args.contains(&"--force"); + let has_git_host = args.iter().enumerate().any(|(i, a)| { + a.starts_with("--git-host=") || (*a == "--git-host" && i < args.len() - 1) + }); let mut available: Vec<&str> = vec![]; if !has_filter { @@ -3488,6 +3522,9 @@ impl RequestContext { if !has_force { available.push("--force"); } + if !has_git_host { + available.push("--git-host"); + } values = super::map_completion_values(available); } @@ -4936,6 +4973,64 @@ mod tests { assert!(!ctx.maybe_autoname_session()); } + #[test] + #[serial] + fn repl_complete_uninstall_offers_installed_bundle_names() { + let _guard = TestConfigDirGuard::new(); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle( + "omc", + bundles::InstallMetadata { + source: "https://github.com/x/omc".to_string(), + git_ref: None, + commit: "abc123".to_string(), + version: None, + description: None, + homepage: None, + }, + ) + .unwrap(); + let ctx = create_test_ctx(); + + let values = ctx.repl_complete(".uninstall", &[""], ""); + + assert!( + values.iter().any(|(name, _)| name == "omc"), + "got: {values:?}" + ); + } + + #[test] + #[serial] + fn repl_complete_install_offers_categories_and_bundles() { + let _guard = TestConfigDirGuard::new(); + let mut store = BundleStore::load().unwrap(); + store + .upsert_bundle( + "omc", + bundles::InstallMetadata { + source: "https://github.com/x/omc".to_string(), + git_ref: None, + commit: "abc123".to_string(), + version: None, + description: None, + homepage: None, + }, + ) + .unwrap(); + let ctx = create_test_ctx(); + + let values = ctx.repl_complete(".install", &[""], ""); + + for expected in ["agents", "omc"] { + assert!( + values.iter().any(|(name, _)| name == expected), + "missing '{expected}'; got: {values:?}" + ); + } + } + #[test] #[serial] fn exit_agent_clears_all_agent_state() { diff --git a/src/function/mod.rs b/src/function/mod.rs index fc8d7cb..d17f042 100644 --- a/src/function/mod.rs +++ b/src/function/mod.rs @@ -165,7 +165,16 @@ pub(crate) fn write_file_atomic( std::process::id(), TMP_COUNTER.fetch_add(1, Ordering::Relaxed) )); - fs::write(&tmp, content)?; + let write_synced = || -> io::Result<()> { + use std::io::Write; + let mut file = File::create(&tmp)?; + file.write_all(content.as_bytes())?; + file.sync_all() + }; + if let Err(err) = write_synced() { + let _ = fs::remove_file(&tmp); + return Err(err.into()); + } #[cfg(unix)] if let Some(mode) = mode { @@ -484,9 +493,8 @@ impl Functions { let serialized = serde_json::to_string_pretty(&merged).context("failed to serialize merged mcp.json")?; - let tmp = file_path.with_extension("json.tmp"); - fs::write(&tmp, &serialized).context("failed to write temporary mcp.json")?; - fs::rename(&tmp, &file_path).context("failed to finalize mcp.json")?; + write_file_atomic(&file_path, &serialized, None) + .context("failed to write merged mcp.json")?; if !added.is_empty() { println!(" + new MCP servers: {}", added.join(", ")); diff --git a/src/main.rs b/src/main.rs index 68be6c7..3f98954 100644 --- a/src/main.rs +++ b/src/main.rs @@ -127,14 +127,31 @@ async fn main() -> Result<()> { return sandbox::launch(name.clone(), cli.fresh); } + if cli.list_bundles { + return config::list_installed_bundles(); + } + install_builtins()?; - if let Some(category) = cli.install { + if let Some(category) = cli.install_builtins { return config::install_assets(category); } - if let Some(url) = cli.install_from.as_deref() { - return config::install_remote(url, cli.filter, cli.install_force); + if let Some(value) = cli.install.as_deref() { + return config::install_or_update( + value, + cli.git_host.as_deref(), + cli.filter, + cli.install_force, + ); + } + + if let Some(spec) = cli.update_bundle.as_deref() { + return config::update_bundle(spec, cli.yes); + } + + if let Some(name) = cli.uninstall.as_deref() { + return config::uninstall_bundle(name, cli.yes); } if let Some(client_arg) = &cli.authenticate { diff --git a/src/repl/mod.rs b/src/repl/mod.rs index 6f7bbf2..12c68a9 100644 --- a/src/repl/mod.rs +++ b/src/repl/mod.rs @@ -53,7 +53,7 @@ pub const DEFAULT_CONTINUATION_PROMPT: &str = indoc! {" 4. Continue with the next pending item now. Call tools immediately." }; -static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| { +static REPL_COMMANDS: LazyLock<[ReplCommand; 61]> = LazyLock::new(|| { [ ReplCommand::new(".help", "Show this help guide", AssertState::pass()), ReplCommand::new(".info", "Show system info", AssertState::pass()), @@ -307,7 +307,7 @@ static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| { ), ReplCommand::new( ".list", - "List roles, sessions, agents, RAGs, macros, skills, tools, or MCP servers", + "List roles, sessions, agents, RAGs, macros, skills, tools, MCP servers, or bundles", AssertState::pass(), ), ReplCommand::new( @@ -317,7 +317,12 @@ static REPL_COMMANDS: LazyLock<[ReplCommand; 60]> = LazyLock::new(|| { ), ReplCommand::new( ".install", - "Reinstall bundled assets, or install assets from a remote git repo (.install remote )", + "Reinstall bundled assets, install a bundle from a git repo, or update an installed bundle", + AssertState::pass(), + ), + ReplCommand::new( + ".uninstall", + "Uninstall an installed bundle (delete its owned files and MCP entries)", AssertState::pass(), ), ReplCommand::new( @@ -865,27 +870,17 @@ pub async fn run_repl_command( replay::render(app.as_ref(), &compressed, &active)?; } } - ".install" => { - let trimmed = args.map(str::trim).unwrap_or(""); - let mut parts = trimmed.splitn(2, char::is_whitespace); - match parts.next() { - Some("remote") => { - let rest = parts.next().unwrap_or("").trim(); - config::install_remote_from_repl_args(rest)?; - } - Some(name) if !name.is_empty() => match AssetCategory::parse(name) { - Some(category) => config::install_assets(category)?, - None => println!( - "Unknown asset category '{name}'. Valid categories: {}", - AssetCategory::NAMES.join(", ") - ), - }, - _ => println!( - "Usage: .install <{}> | .install remote ", - AssetCategory::NAMES.join("|") - ), + ".install" => match parse_repl_install(args) { + ReplInstallDispatch::Builtins(category) => config::install_assets(category)?, + ReplInstallDispatch::Unified(value) => { + config::install_or_update_from_repl_args(value)?; } - } + ReplInstallDispatch::Usage => println!( + "Usage: .install <{}> | .install \ + [--git-host ] [--filter ] [--force]", + AssetCategory::NAMES.join("|") + ), + }, ".update" => { if ctx.macro_flag { bail!("Cannot perform this operation because you are in a macro") @@ -1202,13 +1197,17 @@ pub async fn run_repl_command( println!("Usage: .delete ") } }, + ".uninstall" => match parse_repl_uninstall(args) { + Some((name, assume_yes)) => config::uninstall_bundle(&name, assume_yes)?, + None => println!("Usage: .uninstall [--yes]"), + }, ".list" => match args { Some(args) => { ctx.list_assets(args.trim())?; } _ => { println!( - "Usage: .list " + "Usage: .list " ) } }, @@ -1570,6 +1569,45 @@ fn unknown_command() -> Result<()> { bail!(r#"Unknown command. Type ".help" for additional help."#); } +#[derive(Debug, PartialEq)] +enum ReplInstallDispatch<'a> { + Builtins(AssetCategory), + Unified(&'a str), + Usage, +} + +fn parse_repl_install(args: Option<&str>) -> ReplInstallDispatch<'_> { + let trimmed = args.map(str::trim).unwrap_or(""); + let mut parts = trimmed.splitn(2, char::is_whitespace); + match parts.next() { + Some(name) if !name.is_empty() => { + let rest = parts.next().map(str::trim).unwrap_or(""); + match AssetCategory::parse(name) { + Some(category) if rest.is_empty() => ReplInstallDispatch::Builtins(category), + Some(_) => ReplInstallDispatch::Usage, + None => ReplInstallDispatch::Unified(trimmed), + } + } + _ => ReplInstallDispatch::Usage, + } +} + +fn parse_repl_uninstall(args: Option<&str>) -> Option<(String, bool)> { + let mut assume_yes = false; + let mut names = Vec::new(); + for token in args.unwrap_or("").split_whitespace() { + match token { + "--yes" | "-y" => assume_yes = true, + other if other.starts_with('-') => return None, + other => names.push(other), + } + } + match names.as_slice() { + [name] => Some((name.to_string(), assume_yes)), + _ => None, + } +} + pub fn builtin_command_names() -> Vec<&'static str> { let mut names: Vec<&'static str> = REPL_COMMANDS .iter() @@ -1791,8 +1829,46 @@ mod tests { } #[test] - fn repl_commands_has_60_entries() { - assert_eq!(REPL_COMMANDS.len(), 60); + fn repl_commands_has_61_entries() { + assert_eq!(REPL_COMMANDS.len(), 61); + } + + #[test] + fn parse_repl_install_routes_categories_to_builtins() { + assert_eq!( + parse_repl_install(Some("agents")), + ReplInstallDispatch::Builtins(AssetCategory::Agents) + ); + } + + #[test] + fn parse_repl_install_routes_other_values_to_unified_dispatch() { + assert_eq!( + parse_repl_install(Some("https://github.com/x/y")), + ReplInstallDispatch::Unified("https://github.com/x/y") + ); + assert_eq!( + parse_repl_install(Some("my-bundle")), + ReplInstallDispatch::Unified("my-bundle") + ); + } + + #[test] + fn parse_repl_install_empty_args_ask_for_usage() { + assert_eq!(parse_repl_install(None), ReplInstallDispatch::Usage); + assert_eq!(parse_repl_install(Some(" ")), ReplInstallDispatch::Usage); + } + + #[test] + fn parse_repl_install_rejects_extra_tokens_after_a_category() { + assert_eq!( + parse_repl_install(Some("agents --force")), + ReplInstallDispatch::Usage + ); + assert_eq!( + parse_repl_install(Some("agents extra")), + ReplInstallDispatch::Usage + ); } #[test]