diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index b21e25a393..17adecccbb 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -1,10 +1,25 @@ use anyhow::Context; use anyhow::Result; +use anyhow::bail; use clap::Parser; +use codex_config::MarketplaceConfigUpdate; +use codex_config::record_user_marketplace; +use codex_core::config::Config; use codex_core::config::find_codex_home; -use codex_core::plugins::MarketplaceAddRequest; -use codex_core::plugins::add_marketplace; +use codex_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME; +use codex_core::plugins::PluginMarketplaceUpgradeOutcome; +use codex_core::plugins::PluginsManager; +use codex_core::plugins::marketplace_install_root; +use codex_core::plugins::validate_marketplace_root; +use codex_core::plugins::validate_plugin_segment; use codex_utils_cli::CliConfigOverrides; +use std::fs; +use std::path::Path; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; + +mod metadata; +mod ops; #[derive(Debug, Parser)] pub struct MarketplaceCli { @@ -19,6 +34,9 @@ pub struct MarketplaceCli { enum MarketplaceSubcommand { /// Add a remote marketplace repository. Add(AddMarketplaceArgs), + + /// Upgrade configured Git marketplaces. + Upgrade(UpgradeMarketplaceArgs), } #[derive(Debug, Parser)] @@ -39,6 +57,20 @@ struct AddMarketplaceArgs { sparse_paths: Vec, } +#[derive(Debug, Parser)] +struct UpgradeMarketplaceArgs { + /// Upgrade only one configured marketplace. When omitted, upgrades all configured Git marketplaces. + marketplace_name: Option, +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum MarketplaceSource { + Git { + url: String, + ref_name: Option, + }, +} + impl MarketplaceCli { pub async fn run(self) -> Result<()> { let MarketplaceCli { @@ -48,12 +80,13 @@ impl MarketplaceCli { // Validate overrides now. This command writes to CODEX_HOME only; marketplace discovery // happens from that cache root after the next plugin/list or app-server start. - config_overrides + let overrides = config_overrides .parse_overrides() .map_err(anyhow::Error::msg)?; match subcommand { MarketplaceSubcommand::Add(args) => run_add(args).await?, + MarketplaceSubcommand::Upgrade(args) => run_upgrade(overrides, args).await?, } Ok(()) @@ -67,41 +100,505 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { sparse_paths, } = args; - let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; - let outcome = add_marketplace( - codex_home.to_path_buf(), - MarketplaceAddRequest { - source, - ref_name, - sparse_paths, - }, - ) - .await?; + let source = parse_marketplace_source(&source, ref_name)?; - if outcome.already_added { + let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; + let install_root = marketplace_install_root(&codex_home); + fs::create_dir_all(&install_root).with_context(|| { + format!( + "failed to create marketplace install directory {}", + install_root.display() + ) + })?; + let install_metadata = + metadata::MarketplaceInstallMetadata::from_source(&source, &sparse_paths); + if let Some(existing_root) = metadata::installed_marketplace_root_for_source( + &codex_home, + &install_root, + &install_metadata, + )? { + let marketplace_name = validate_marketplace_root(&existing_root).with_context(|| { + format!( + "failed to validate installed marketplace at {}", + existing_root.display() + ) + })?; + record_added_marketplace(&codex_home, &marketplace_name, &install_metadata)?; println!( - "Marketplace `{}` is already added from {}.", - outcome.marketplace_name, outcome.source_display + "Marketplace `{marketplace_name}` is already added from {}.", + source.display() ); - } else { - println!( - "Added marketplace `{}` from {}.", - outcome.marketplace_name, outcome.source_display + println!("Installed marketplace root: {}", existing_root.display()); + return Ok(()); + } + + let staging_root = ops::marketplace_staging_root(&install_root); + fs::create_dir_all(&staging_root).with_context(|| { + format!( + "failed to create marketplace staging directory {}", + staging_root.display() + ) + })?; + let staged_dir = tempfile::Builder::new() + .prefix("marketplace-add-") + .tempdir_in(&staging_root) + .with_context(|| { + format!( + "failed to create temporary marketplace directory in {}", + staging_root.display() + ) + })?; + let staged_root = staged_dir.path().to_path_buf(); + + let MarketplaceSource::Git { url, ref_name } = &source; + ops::clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; + + let marketplace_name = validate_marketplace_source_root(&staged_root) + .with_context(|| format!("failed to validate marketplace from {}", source.display()))?; + if marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME { + bail!( + "marketplace `{OPENAI_CURATED_MARKETPLACE_NAME}` is reserved and cannot be added from {}", + source.display() ); } + let destination = install_root.join(safe_marketplace_dir_name(&marketplace_name)?); + ensure_marketplace_destination_is_inside_install_root(&install_root, &destination)?; + if destination.exists() { + bail!( + "marketplace `{marketplace_name}` is already added from a different source; remove it before adding {}", + source.display() + ); + } + ops::replace_marketplace_root(&staged_root, &destination) + .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; + if let Err(err) = record_added_marketplace(&codex_home, &marketplace_name, &install_metadata) { + if let Err(rollback_err) = fs::rename(&destination, &staged_root) { + bail!( + "{err}; additionally failed to roll back installed marketplace at {}: {rollback_err}", + destination.display() + ); + } + return Err(err); + } + println!( - "Installed marketplace root: {}", - outcome.installed_root.as_path().display() + "Added marketplace `{marketplace_name}` from {}.", + source.display() ); + println!("Installed marketplace root: {}", destination.display()); Ok(()) } +async fn run_upgrade( + overrides: Vec<(String, toml::Value)>, + args: UpgradeMarketplaceArgs, +) -> Result<()> { + let UpgradeMarketplaceArgs { marketplace_name } = args; + let config = Config::load_with_cli_overrides(overrides) + .await + .context("failed to load configuration")?; + let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; + let manager = PluginsManager::new(codex_home); + let outcome = manager + .upgrade_configured_marketplaces_for_config(&config, marketplace_name.as_deref()) + .map_err(anyhow::Error::msg)?; + print_upgrade_outcome(&outcome, marketplace_name.as_deref())?; + Ok(()) +} + +fn record_added_marketplace( + codex_home: &Path, + marketplace_name: &str, + install_metadata: &metadata::MarketplaceInstallMetadata, +) -> Result<()> { + let source = install_metadata.config_source(); + let last_updated = utc_timestamp_now()?; + let update = MarketplaceConfigUpdate { + last_updated: &last_updated, + last_revision: None, + source_type: install_metadata.config_source_type(), + source: &source, + ref_name: install_metadata.ref_name(), + sparse_paths: install_metadata.sparse_paths(), + }; + record_user_marketplace(codex_home, marketplace_name, &update).with_context(|| { + format!("failed to add marketplace `{marketplace_name}` to user config.toml") + })?; + Ok(()) +} + +fn print_upgrade_outcome( + outcome: &PluginMarketplaceUpgradeOutcome, + marketplace_name: Option<&str>, +) -> Result<()> { + for error in &outcome.errors { + eprintln!( + "Failed to upgrade marketplace `{}`: {}", + error.marketplace_name, error.message + ); + } + if !outcome.all_succeeded() { + bail!("{} upgrade failure(s) occurred.", outcome.errors.len()); + } + + let selection_label = marketplace_name.unwrap_or("all configured Git marketplaces"); + if outcome.selected_marketplaces.is_empty() { + println!("No configured Git marketplaces to upgrade."); + } else if outcome.upgraded_roots.is_empty() { + if marketplace_name.is_some() { + println!("Marketplace `{selection_label}` is already up to date."); + } else { + println!("All configured Git marketplaces are already up to date."); + } + } else if marketplace_name.is_some() { + println!("Upgraded marketplace `{selection_label}` to the latest configured revision."); + for root in &outcome.upgraded_roots { + println!("Installed marketplace root: {}", root.display()); + } + } else { + println!("Upgraded {} marketplace(s).", outcome.upgraded_roots.len()); + for root in &outcome.upgraded_roots { + println!("Installed marketplace root: {}", root.display()); + } + } + + Ok(()) +} + +fn validate_marketplace_source_root(root: &Path) -> Result { + let marketplace_name = validate_marketplace_root(root)?; + validate_plugin_segment(&marketplace_name, "marketplace name").map_err(anyhow::Error::msg)?; + Ok(marketplace_name) +} + +fn parse_marketplace_source( + source: &str, + explicit_ref: Option, +) -> Result { + let source = source.trim(); + if source.is_empty() { + bail!("marketplace source must not be empty"); + } + + let (base_source, parsed_ref) = split_source_ref(source); + let ref_name = explicit_ref.or(parsed_ref); + + if looks_like_local_path(&base_source) { + bail!( + "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo" + ); + } + + if is_ssh_git_url(&base_source) || is_git_url(&base_source) { + let url = normalize_git_url(&base_source); + return Ok(MarketplaceSource::Git { url, ref_name }); + } + + if looks_like_github_shorthand(&base_source) { + let url = format!("https://github.com/{base_source}.git"); + return Ok(MarketplaceSource::Git { url, ref_name }); + } + + bail!("invalid marketplace source format: {source}"); +} + +fn split_source_ref(source: &str) -> (String, Option) { + if let Some((base, ref_name)) = source.rsplit_once('#') { + return (base.to_string(), non_empty_ref(ref_name)); + } + if !source.contains("://") + && !is_ssh_git_url(source) + && let Some((base, ref_name)) = source.rsplit_once('@') + { + return (base.to_string(), non_empty_ref(ref_name)); + } + (source.to_string(), None) +} + +fn non_empty_ref(ref_name: &str) -> Option { + let ref_name = ref_name.trim(); + (!ref_name.is_empty()).then(|| ref_name.to_string()) +} + +fn normalize_git_url(url: &str) -> String { + let url = url.trim_end_matches('/'); + if url.starts_with("https://github.com/") && !url.ends_with(".git") { + format!("{url}.git") + } else { + url.to_string() + } +} + +fn looks_like_local_path(source: &str) -> bool { + source.starts_with("./") + || source.starts_with("../") + || source.starts_with('/') + || source.starts_with("~/") + || source == "." + || source == ".." +} + +fn is_ssh_git_url(source: &str) -> bool { + source.starts_with("ssh://") || source.starts_with("git@") && source.contains(':') +} + +fn is_git_url(source: &str) -> bool { + source.starts_with("http://") || source.starts_with("https://") +} + +fn looks_like_github_shorthand(source: &str) -> bool { + let mut segments = source.split('/'); + let owner = segments.next(); + let repo = segments.next(); + let extra = segments.next(); + owner.is_some_and(is_github_shorthand_segment) + && repo.is_some_and(is_github_shorthand_segment) + && extra.is_none() +} + +fn is_github_shorthand_segment(segment: &str) -> bool { + !segment.is_empty() + && segment + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.')) +} + +fn safe_marketplace_dir_name(marketplace_name: &str) -> Result { + let safe = marketplace_name + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.') { + ch + } else { + '-' + } + }) + .collect::(); + let safe = safe.trim_matches('.').to_string(); + if safe.is_empty() || safe == ".." { + bail!("marketplace name `{marketplace_name}` cannot be used as an install directory"); + } + Ok(safe) +} + +fn ensure_marketplace_destination_is_inside_install_root( + install_root: &Path, + destination: &Path, +) -> Result<()> { + let install_root = install_root.canonicalize().with_context(|| { + format!( + "failed to resolve marketplace install root {}", + install_root.display() + ) + })?; + let destination_parent = destination + .parent() + .context("marketplace destination has no parent")? + .canonicalize() + .with_context(|| { + format!( + "failed to resolve marketplace destination parent {}", + destination.display() + ) + })?; + if !destination_parent.starts_with(&install_root) { + bail!( + "marketplace destination {} is outside install root {}", + destination.display(), + install_root.display() + ); + } + Ok(()) +} + +fn utc_timestamp_now() -> Result { + let duration = SystemTime::now() + .duration_since(UNIX_EPOCH) + .context("system clock is before Unix epoch")?; + Ok(format_utc_timestamp(duration.as_secs() as i64)) +} + +fn format_utc_timestamp(seconds_since_epoch: i64) -> String { + const SECONDS_PER_DAY: i64 = 86_400; + let days = seconds_since_epoch.div_euclid(SECONDS_PER_DAY); + let seconds_of_day = seconds_since_epoch.rem_euclid(SECONDS_PER_DAY); + let (year, month, day) = civil_from_days(days); + let hour = seconds_of_day / 3_600; + let minute = (seconds_of_day % 3_600) / 60; + let second = seconds_of_day % 60; + format!("{year:04}-{month:02}-{day:02}T{hour:02}:{minute:02}:{second:02}Z") +} + +fn civil_from_days(days_since_epoch: i64) -> (i64, i64, i64) { + let days = days_since_epoch + 719_468; + let era = if days >= 0 { days } else { days - 146_096 } / 146_097; + let day_of_era = days - era * 146_097; + let year_of_era = + (day_of_era - day_of_era / 1_460 + day_of_era / 36_524 - day_of_era / 146_096) / 365; + let mut year = year_of_era + era * 400; + let day_of_year = day_of_era - (365 * year_of_era + year_of_era / 4 - year_of_era / 100); + let month_prime = (5 * day_of_year + 2) / 153; + let day = day_of_year - (153 * month_prime + 2) / 5 + 1; + let month = month_prime + if month_prime < 10 { 3 } else { -9 }; + year += if month <= 2 { 1 } else { 0 }; + (year, month, day) +} + +impl MarketplaceSource { + fn display(&self) -> String { + match self { + Self::Git { url, ref_name } => { + if let Some(ref_name) = ref_name { + format!("{url}#{ref_name}") + } else { + url.clone() + } + } + } + } +} + #[cfg(test)] mod tests { use super::*; use pretty_assertions::assert_eq; + #[test] + fn github_shorthand_parses_ref_suffix() { + assert_eq!( + parse_marketplace_source("owner/repo@main", /*explicit_ref*/ None).unwrap(), + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: Some("main".to_string()), + } + ); + } + + #[test] + fn git_url_parses_fragment_ref() { + assert_eq!( + parse_marketplace_source( + "https://example.com/team/repo.git#v1", + /*explicit_ref*/ None, + ) + .unwrap(), + MarketplaceSource::Git { + url: "https://example.com/team/repo.git".to_string(), + ref_name: Some("v1".to_string()), + } + ); + } + + #[test] + fn explicit_ref_overrides_source_ref() { + assert_eq!( + parse_marketplace_source( + "owner/repo@main", + /*explicit_ref*/ Some("release".to_string()), + ) + .unwrap(), + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: Some("release".to_string()), + } + ); + } + + #[test] + fn github_shorthand_and_git_url_normalize_to_same_source() { + let shorthand = parse_marketplace_source("owner/repo", /*explicit_ref*/ None).unwrap(); + let git_url = parse_marketplace_source( + "https://github.com/owner/repo.git", + /*explicit_ref*/ None, + ) + .unwrap(); + + assert_eq!(shorthand, git_url); + assert_eq!( + shorthand, + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: None, + } + ); + } + + #[test] + fn github_url_with_trailing_slash_normalizes_without_extra_path_segment() { + assert_eq!( + parse_marketplace_source("https://github.com/owner/repo/", /*explicit_ref*/ None) + .unwrap(), + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: None, + } + ); + } + + #[test] + fn non_github_https_source_parses_as_git_url() { + assert_eq!( + parse_marketplace_source("https://gitlab.com/owner/repo", /*explicit_ref*/ None) + .unwrap(), + MarketplaceSource::Git { + url: "https://gitlab.com/owner/repo".to_string(), + ref_name: None, + } + ); + } + + #[test] + fn file_url_source_is_rejected() { + let err = + parse_marketplace_source("file:///tmp/marketplace.git", /*explicit_ref*/ None) + .unwrap_err(); + + assert!( + err.to_string() + .contains("invalid marketplace source format"), + "unexpected error: {err}" + ); + } + + #[test] + fn local_path_source_is_rejected() { + let err = parse_marketplace_source("./marketplace", /*explicit_ref*/ None).unwrap_err(); + + assert!( + err.to_string() + .contains("local marketplace sources are not supported yet"), + "unexpected error: {err}" + ); + } + + #[test] + fn ssh_url_parses_as_git_url() { + assert_eq!( + parse_marketplace_source( + "ssh://git@github.com/owner/repo.git#main", + /*explicit_ref*/ None, + ) + .unwrap(), + MarketplaceSource::Git { + url: "ssh://git@github.com/owner/repo.git".to_string(), + ref_name: Some("main".to_string()), + } + ); + } + + #[test] + fn utc_timestamp_formats_unix_epoch_as_rfc3339_utc() { + assert_eq!( + format_utc_timestamp(/*seconds_since_epoch*/ 0), + "1970-01-01T00:00:00Z" + ); + assert_eq!( + format_utc_timestamp(/*seconds_since_epoch*/ 1_775_779_200), + "2026-04-10T00:00:00Z" + ); + } + #[test] fn sparse_paths_parse_before_or_after_source() { let sparse_before_source = @@ -131,4 +628,13 @@ mod tests { vec!["plugins/foo", "skills/bar"] ); } + + #[test] + fn upgrade_subcommand_parses_optional_marketplace_name() { + let upgrade_all = UpgradeMarketplaceArgs::try_parse_from(["upgrade"]).unwrap(); + assert_eq!(upgrade_all.marketplace_name, None); + + let upgrade_one = UpgradeMarketplaceArgs::try_parse_from(["upgrade", "debug"]).unwrap(); + assert_eq!(upgrade_one.marketplace_name.as_deref(), Some("debug")); + } } diff --git a/codex-rs/cli/tests/marketplace_upgrade.rs b/codex-rs/cli/tests/marketplace_upgrade.rs new file mode 100644 index 0000000000..2fb3669b1c --- /dev/null +++ b/codex-rs/cli/tests/marketplace_upgrade.rs @@ -0,0 +1,200 @@ +use anyhow::Result; +use codex_core::plugins::marketplace_install_root; +use predicates::str::contains; +use std::path::Path; +use std::process::Command; +use tempfile::TempDir; +use toml::Value; + +fn codex_command(codex_home: &Path) -> Result { + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + cmd.env("CODEX_HOME", codex_home); + Ok(cmd) +} + +fn write_marketplace_source(source: &Path, marketplace_name: &str, marker: &str) -> Result<()> { + std::fs::create_dir_all(source.join(".agents/plugins"))?; + std::fs::create_dir_all(source.join("plugins/sample/.codex-plugin"))?; + std::fs::write( + source.join(".agents/plugins/marketplace.json"), + format!( + r#"{{ + "name": "{marketplace_name}", + "plugins": [ + {{ + "name": "sample", + "source": {{ + "source": "local", + "path": "./plugins/sample" + }} + }} + ] +}}"# + ), + )?; + std::fs::write( + source.join("plugins/sample/.codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + )?; + std::fs::write(source.join("plugins/sample/marker.txt"), marker)?; + Ok(()) +} + +fn init_git_repo(repo: &Path) { + git(repo, &["init"]); + git(repo, &["config", "user.email", "codex-test@example.com"]); + git(repo, &["config", "user.name", "Codex Test"]); + git(repo, &["add", "."]); + git(repo, &["commit", "-m", "initial marketplace"]); +} + +fn git(repo: &Path, args: &[&str]) { + let output = Command::new("git") + .arg("-C") + .arg(repo) + .args(args) + .output() + .unwrap_or_else(|err| panic!("git should run: {err}")); + assert!( + output.status.success(), + "git -C {} {} failed\nstdout:\n{}\nstderr:\n{}", + repo.display(), + args.join(" "), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + +fn write_marketplaces_config(codex_home: &Path, entries: &[(&str, &Path)]) -> Result<()> { + let mut root = toml::map::Map::new(); + let mut features = toml::map::Map::new(); + features.insert("plugins".to_string(), Value::Boolean(true)); + root.insert("features".to_string(), Value::Table(features)); + + let mut marketplaces = toml::map::Map::new(); + for (name, source) in entries { + let mut marketplace = toml::map::Map::new(); + marketplace.insert( + "last_updated".to_string(), + Value::String("2026-04-10T00:00:00Z".to_string()), + ); + marketplace.insert( + "last_revision".to_string(), + Value::String("old-revision".to_string()), + ); + marketplace.insert("source_type".to_string(), Value::String("git".to_string())); + marketplace.insert( + "source".to_string(), + Value::String(source.display().to_string()), + ); + marketplaces.insert((*name).to_string(), Value::Table(marketplace)); + } + root.insert("marketplaces".to_string(), Value::Table(marketplaces)); + + std::fs::write( + codex_home.join("config.toml"), + toml::to_string(&Value::Table(root))?, + )?; + Ok(()) +} + +fn installed_marker(codex_home: &Path, marketplace_name: &str) -> String { + std::fs::read_to_string( + marketplace_install_root(codex_home) + .join(marketplace_name) + .join("plugins/sample/marker.txt"), + ) + .unwrap_or_else(|err| panic!("installed marker should read: {err}")) +} + +#[tokio::test] +async fn marketplace_upgrade_all_upgrades_every_configured_git_marketplace() -> Result<()> { + let codex_home = TempDir::new()?; + let alpha_source = TempDir::new()?; + let beta_source = TempDir::new()?; + write_marketplace_source(alpha_source.path(), "alpha", "alpha-new")?; + write_marketplace_source(beta_source.path(), "beta", "beta-new")?; + init_git_repo(alpha_source.path()); + init_git_repo(beta_source.path()); + write_marketplaces_config( + codex_home.path(), + &[("alpha", alpha_source.path()), ("beta", beta_source.path())], + )?; + write_marketplace_source( + &marketplace_install_root(codex_home.path()).join("alpha"), + "alpha", + "alpha-old", + )?; + write_marketplace_source( + &marketplace_install_root(codex_home.path()).join("beta"), + "beta", + "beta-old", + )?; + + codex_command(codex_home.path())? + .args(["marketplace", "upgrade"]) + .assert() + .success() + .stdout(contains("Upgraded 2 marketplace(s).")); + + assert_eq!(installed_marker(codex_home.path(), "alpha"), "alpha-new"); + assert_eq!(installed_marker(codex_home.path(), "beta"), "beta-new"); + Ok(()) +} + +#[tokio::test] +async fn marketplace_upgrade_single_marketplace_only_upgrades_requested_marketplace() -> Result<()> +{ + let codex_home = TempDir::new()?; + let alpha_source = TempDir::new()?; + let beta_source = TempDir::new()?; + write_marketplace_source(alpha_source.path(), "alpha", "alpha-new")?; + write_marketplace_source(beta_source.path(), "beta", "beta-new")?; + init_git_repo(alpha_source.path()); + init_git_repo(beta_source.path()); + write_marketplaces_config( + codex_home.path(), + &[("alpha", alpha_source.path()), ("beta", beta_source.path())], + )?; + write_marketplace_source( + &marketplace_install_root(codex_home.path()).join("alpha"), + "alpha", + "alpha-old", + )?; + write_marketplace_source( + &marketplace_install_root(codex_home.path()).join("beta"), + "beta", + "beta-old", + )?; + + codex_command(codex_home.path())? + .args(["marketplace", "upgrade", "alpha"]) + .assert() + .success() + .stdout(contains( + "Upgraded marketplace `alpha` to the latest configured revision.", + )); + + assert_eq!(installed_marker(codex_home.path(), "alpha"), "alpha-new"); + assert_eq!(installed_marker(codex_home.path(), "beta"), "beta-old"); + Ok(()) +} + +#[tokio::test] +async fn marketplace_upgrade_rejects_unknown_marketplace_name() -> Result<()> { + let codex_home = TempDir::new()?; + let alpha_source = TempDir::new()?; + write_marketplace_source(alpha_source.path(), "alpha", "alpha-new")?; + init_git_repo(alpha_source.path()); + write_marketplaces_config(codex_home.path(), &[("alpha", alpha_source.path())])?; + + codex_command(codex_home.path())? + .args(["marketplace", "upgrade", "missing"]) + .assert() + .failure() + .stderr(contains( + "marketplace `missing` is not configured as a Git marketplace", + )); + + Ok(()) +} diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 62ec4ba6ad..2969c7a651 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -15,6 +15,9 @@ use super::marketplace::ResolvedMarketplacePlugin; use super::marketplace::list_marketplaces; use super::marketplace::load_marketplace; use super::marketplace::resolve_marketplace_plugin; +use super::marketplace_upgrade::ConfiguredMarketplaceUpgradeError; +use super::marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome; +use super::marketplace_upgrade::configured_git_marketplace_names; use super::marketplace_upgrade::upgrade_configured_git_marketplaces; use super::read_curated_plugins_sha; use super::remote::RemotePluginFetchError; @@ -1144,16 +1147,24 @@ impl PluginsManager { } let manager = Arc::clone(self); - let codex_home = self.codex_home.clone(); let config = config.clone(); if let Err(err) = std::thread::Builder::new() .name("plugins-marketplace-auto-upgrade".to_string()) .spawn(move || { - let outcome = upgrade_configured_git_marketplaces(codex_home.as_path(), &config); - if !outcome.upgraded_roots.is_empty() { - manager.maybe_start_non_curated_plugin_cache_force_reinstall_for_roots( - &outcome.upgraded_roots, - ); + let outcome = manager.upgrade_configured_marketplaces_for_config(&config, None); + match outcome { + Ok(outcome) => { + for error in outcome.errors { + warn!( + marketplace = error.marketplace_name, + error = %error.message, + "failed to auto-upgrade configured marketplace" + ); + } + } + Err(err) => { + warn!("failed to auto-upgrade configured marketplaces: {err}"); + } } let mut state = match manager.configured_marketplace_upgrade_state.write() { @@ -1172,6 +1183,37 @@ impl PluginsManager { } } + pub fn upgrade_configured_marketplaces_for_config( + &self, + config: &Config, + marketplace_name: Option<&str>, + ) -> Result { + if let Some(marketplace_name) = marketplace_name + && !configured_git_marketplace_names(config) + .iter() + .any(|name| name == marketplace_name) + { + return Err(format!( + "marketplace `{marketplace_name}` is not configured as a Git marketplace" + )); + } + + let mut outcome = upgrade_configured_git_marketplaces( + self.codex_home.as_path(), + config, + marketplace_name, + ); + if let Err(err) = self.refresh_upgraded_marketplace_plugin_cache(&outcome.upgraded_roots) { + outcome.errors.push(ConfiguredMarketplaceUpgradeError { + marketplace_name: marketplace_name + .unwrap_or("all configured marketplaces") + .to_string(), + message: err, + }); + } + Ok(outcome) + } + pub fn maybe_start_non_curated_plugin_cache_refresh_for_roots( self: &Arc, roots: &[AbsolutePathBuf], @@ -1182,16 +1224,6 @@ impl PluginsManager { ); } - fn maybe_start_non_curated_plugin_cache_force_reinstall_for_roots( - self: &Arc, - roots: &[AbsolutePathBuf], - ) { - self.maybe_start_non_curated_plugin_cache_refresh_for_roots_with_mode( - roots, - NonCuratedCacheRefreshMode::ForceReinstall, - ); - } - fn maybe_start_non_curated_plugin_cache_refresh_for_roots_with_mode( self: &Arc, roots: &[AbsolutePathBuf], @@ -1256,6 +1288,34 @@ impl PluginsManager { } } + fn refresh_upgraded_marketplace_plugin_cache( + &self, + roots: &[AbsolutePathBuf], + ) -> Result<(), String> { + if roots.is_empty() { + return Ok(()); + } + + match refresh_non_curated_plugin_cache( + self.codex_home.as_path(), + roots, + NonCuratedCacheRefreshMode::ForceReinstall, + ) { + Ok(cache_refreshed) => { + if cache_refreshed { + self.clear_cache(); + } + Ok(()) + } + Err(err) => { + self.clear_cache(); + Err(format!( + "failed to refresh installed plugin cache after marketplace upgrade: {err}" + )) + } + } + } + fn start_curated_repo_sync(self: &Arc) { if CURATED_REPO_SYNC_STARTED.swap(true, Ordering::SeqCst) { return; diff --git a/codex-rs/core/src/plugins/marketplace_upgrade.rs b/codex-rs/core/src/plugins/marketplace_upgrade.rs index 62e960eac3..7c0fcce0ac 100644 --- a/codex-rs/core/src/plugins/marketplace_upgrade.rs +++ b/codex-rs/core/src/plugins/marketplace_upgrade.rs @@ -23,10 +23,17 @@ use crate::config::Config; const MARKETPLACE_UPGRADE_GIT_TIMEOUT: Duration = Duration::from_secs(30); -#[derive(Debug, Default, PartialEq, Eq)] -pub(super) struct ConfiguredMarketplaceUpgradeOutcome { +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ConfiguredMarketplaceUpgradeError { + pub marketplace_name: String, + pub message: String, +} + +#[derive(Debug, Default, Clone, PartialEq, Eq)] +pub struct ConfiguredMarketplaceUpgradeOutcome { + pub selected_marketplaces: Vec, pub upgraded_roots: Vec, - pub all_succeeded: bool, + pub errors: Vec, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -38,40 +45,58 @@ struct ConfiguredGitMarketplace { last_revision: Option, } -pub(super) fn upgrade_configured_git_marketplaces( +impl ConfiguredMarketplaceUpgradeOutcome { + pub fn all_succeeded(&self) -> bool { + self.errors.is_empty() + } +} + +pub fn configured_git_marketplace_names(config: &Config) -> Vec { + let mut names = configured_git_marketplaces(config) + .into_iter() + .map(|marketplace| marketplace.name) + .collect::>(); + names.sort_unstable(); + names +} + +pub fn upgrade_configured_git_marketplaces( codex_home: &Path, config: &Config, + marketplace_name: Option<&str>, ) -> ConfiguredMarketplaceUpgradeOutcome { - let marketplaces = configured_git_marketplaces(config); + let marketplaces = configured_git_marketplaces(config) + .into_iter() + .filter(|marketplace| marketplace_name.is_none_or(|name| marketplace.name.as_str() == name)) + .collect::>(); if marketplaces.is_empty() { - return ConfiguredMarketplaceUpgradeOutcome { - all_succeeded: true, - ..Default::default() - }; + return ConfiguredMarketplaceUpgradeOutcome::default(); } let install_root = marketplace_install_root(codex_home); + let selected_marketplaces = marketplaces + .iter() + .map(|marketplace| marketplace.name.clone()) + .collect(); let mut upgraded_roots = Vec::new(); - let mut all_succeeded = true; + let mut errors = Vec::new(); for marketplace in marketplaces { match upgrade_configured_git_marketplace(codex_home, &install_root, &marketplace) { Ok(Some(upgraded_root)) => upgraded_roots.push(upgraded_root), Ok(None) => {} Err(err) => { - all_succeeded = false; - warn!( - marketplace = marketplace.name, - source = marketplace.source, - error = %err, - "failed to auto-upgrade configured marketplace" - ); + errors.push(ConfiguredMarketplaceUpgradeError { + marketplace_name: marketplace.name, + message: err, + }); } } } ConfiguredMarketplaceUpgradeOutcome { + selected_marketplaces, upgraded_roots, - all_succeeded, + errors, } } @@ -289,18 +314,19 @@ mod tests { ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); assert_eq!( outcome, ConfiguredMarketplaceUpgradeOutcome { + selected_marketplaces: vec!["debug".to_string()], upgraded_roots: vec![ AbsolutePathBuf::try_from( marketplace_install_root(codex_home.path()).join("debug") ) .unwrap() ], - all_succeeded: true, + errors: Vec::new(), } ); assert_eq!( @@ -330,13 +356,14 @@ mod tests { ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); assert_eq!( outcome, ConfiguredMarketplaceUpgradeOutcome { + selected_marketplaces: vec!["debug".to_string()], upgraded_roots: Vec::new(), - all_succeeded: true, + errors: Vec::new(), } ); assert_eq!( @@ -361,18 +388,19 @@ mod tests { ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); assert_eq!( outcome, ConfiguredMarketplaceUpgradeOutcome { + selected_marketplaces: vec!["debug".to_string()], upgraded_roots: vec![ AbsolutePathBuf::try_from( marketplace_install_root(codex_home.path()).join("debug") ) .unwrap() ], - all_succeeded: true, + errors: Vec::new(), } ); assert_eq!( @@ -395,15 +423,12 @@ mod tests { ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); - assert_eq!( - outcome, - ConfiguredMarketplaceUpgradeOutcome { - upgraded_roots: Vec::new(), - all_succeeded: false, - } - ); + assert_eq!(outcome.selected_marketplaces, vec!["debug".to_string()]); + assert!(outcome.upgraded_roots.is_empty()); + assert_eq!(outcome.errors.len(), 1); + assert_eq!(outcome.errors[0].marketplace_name, "debug"); assert_eq!( std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt")).unwrap(), "old" @@ -422,15 +447,12 @@ mod tests { ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); - assert_eq!( - outcome, - ConfiguredMarketplaceUpgradeOutcome { - upgraded_roots: Vec::new(), - all_succeeded: false, - } - ); + assert_eq!(outcome.selected_marketplaces, vec!["debug".to_string()]); + assert!(outcome.upgraded_roots.is_empty()); + assert_eq!(outcome.errors.len(), 1); + assert_eq!(outcome.errors[0].marketplace_name, "debug"); assert_eq!( std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt")).unwrap(), "old" @@ -458,15 +480,12 @@ mod tests { &marketplace_config(changed_source_repo.path(), "changed-revision"), ); - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); - assert_eq!( - outcome, - ConfiguredMarketplaceUpgradeOutcome { - upgraded_roots: Vec::new(), - all_succeeded: false, - } - ); + assert_eq!(outcome.selected_marketplaces, vec!["debug".to_string()]); + assert!(outcome.upgraded_roots.is_empty()); + assert_eq!(outcome.errors.len(), 1); + assert_eq!(outcome.errors[0].marketplace_name, "debug"); assert_eq!( std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt")).unwrap(), "old" @@ -488,13 +507,14 @@ plugins = true ); let config = load_plugins_config(codex_home.path()).await; - let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config); + let outcome = upgrade_configured_git_marketplaces(codex_home.path(), &config, None); assert_eq!( outcome, ConfiguredMarketplaceUpgradeOutcome { + selected_marketplaces: Vec::new(), upgraded_roots: Vec::new(), - all_succeeded: true, + errors: Vec::new(), } ); assert!( diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 13e065561d..37e9436ba2 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -6,7 +6,6 @@ mod installed_marketplaces; mod manager; mod manifest; mod marketplace; -mod marketplace_add; mod marketplace_upgrade; mod mentions; mod remote; @@ -60,10 +59,8 @@ pub use marketplace::MarketplacePluginInstallPolicy; pub use marketplace::MarketplacePluginPolicy; pub use marketplace::MarketplacePluginSource; pub use marketplace::validate_marketplace_root; -pub use marketplace_add::MarketplaceAddError; -pub use marketplace_add::MarketplaceAddOutcome; -pub use marketplace_add::MarketplaceAddRequest; -pub use marketplace_add::add_marketplace; +pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; +pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; pub use remote::RemotePluginFetchError; pub use remote::fetch_remote_featured_plugin_ids; pub(crate) use render::render_explicit_plugin_instructions;