From aef1efbaefcaad22319fb74024b3ecfc7e604665 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Mon, 13 Apr 2026 04:30:35 -0700 Subject: [PATCH 1/4] Add marketplace remove command --- codex-rs/cli/src/marketplace_cmd.rs | 51 +++++++++ codex-rs/cli/src/marketplace_cmd/ops.rs | 136 +++++++++++++++++++++++ codex-rs/cli/tests/marketplace_remove.rs | 69 ++++++++++++ codex-rs/config/src/lib.rs | 1 + codex-rs/config/src/marketplace_edit.rs | 80 +++++++++++++ 5 files changed, 337 insertions(+) create mode 100644 codex-rs/cli/src/marketplace_cmd/ops.rs create mode 100644 codex-rs/cli/tests/marketplace_remove.rs diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index ce1f99390aa..bce3c69def7 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -2,14 +2,19 @@ use anyhow::Context; use anyhow::Result; use anyhow::bail; use clap::Parser; +use codex_config::remove_user_marketplace; use codex_core::config::Config; use codex_core::config::find_codex_home; use codex_core::plugins::MarketplaceAddRequest; use codex_core::plugins::PluginMarketplaceUpgradeOutcome; use codex_core::plugins::PluginsManager; use codex_core::plugins::add_marketplace; +use codex_core::plugins::marketplace_install_root; +use codex_core::plugins::validate_plugin_segment; use codex_utils_cli::CliConfigOverrides; +mod ops; + #[derive(Debug, Parser)] pub struct MarketplaceCli { #[clap(flatten)] @@ -23,6 +28,7 @@ pub struct MarketplaceCli { enum MarketplaceSubcommand { Add(AddMarketplaceArgs), Upgrade(UpgradeMarketplaceArgs), + Remove(RemoveMarketplaceArgs), } #[derive(Debug, Parser)] @@ -47,6 +53,12 @@ struct UpgradeMarketplaceArgs { marketplace_name: Option, } +#[derive(Debug, Parser)] +struct RemoveMarketplaceArgs { + /// Configured marketplace name to remove. + marketplace_name: String, +} + impl MarketplaceCli { pub async fn run(self) -> Result<()> { let MarketplaceCli { @@ -61,6 +73,7 @@ impl MarketplaceCli { match subcommand { MarketplaceSubcommand::Add(args) => run_add(args).await?, MarketplaceSubcommand::Upgrade(args) => run_upgrade(overrides, args).await?, + MarketplaceSubcommand::Remove(args) => run_remove(args).await?, } Ok(()) @@ -120,6 +133,38 @@ async fn run_upgrade( print_upgrade_outcome(&outcome, marketplace_name.as_deref()) } +async fn run_remove(args: RemoveMarketplaceArgs) -> Result<()> { + let RemoveMarketplaceArgs { marketplace_name } = args; + validate_plugin_segment(&marketplace_name, "marketplace name").map_err(anyhow::Error::msg)?; + + let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; + let install_root = marketplace_install_root(&codex_home); + let destination = install_root.join(&marketplace_name); + let removed_root = ops::remove_marketplace_root(&destination).with_context(|| { + format!( + "failed to remove installed marketplace root {}", + destination.display() + ) + })?; + let removed_config = + remove_user_marketplace(&codex_home, &marketplace_name).with_context(|| { + format!("failed to remove marketplace `{marketplace_name}` from user config.toml") + })?; + if !removed_root && !removed_config { + bail!("marketplace `{marketplace_name}` is not configured or installed"); + } + + println!("Removed marketplace `{marketplace_name}`."); + if removed_root { + println!( + "Removed installed marketplace root: {}", + destination.display() + ); + } + + Ok(()) +} + fn print_upgrade_outcome( outcome: &PluginMarketplaceUpgradeOutcome, marketplace_name: Option<&str>, @@ -201,4 +246,10 @@ mod tests { let upgrade_one = UpgradeMarketplaceArgs::try_parse_from(["upgrade", "debug"]).unwrap(); assert_eq!(upgrade_one.marketplace_name.as_deref(), Some("debug")); } + + #[test] + fn remove_subcommand_parses_marketplace_name() { + let remove = RemoveMarketplaceArgs::try_parse_from(["remove", "debug"]).unwrap(); + assert_eq!(remove.marketplace_name, "debug"); + } } diff --git a/codex-rs/cli/src/marketplace_cmd/ops.rs b/codex-rs/cli/src/marketplace_cmd/ops.rs new file mode 100644 index 00000000000..ac5c487434a --- /dev/null +++ b/codex-rs/cli/src/marketplace_cmd/ops.rs @@ -0,0 +1,136 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +pub(super) fn clone_git_source( + url: &str, + ref_name: Option<&str>, + sparse_paths: &[String], + destination: &Path, +) -> Result<()> { + let destination = destination.to_string_lossy().to_string(); + if sparse_paths.is_empty() { + run_git(&["clone", url, destination.as_str()], /*cwd*/ None)?; + if let Some(ref_name) = ref_name { + run_git(&["checkout", ref_name], Some(Path::new(&destination)))?; + } + return Ok(()); + } + + run_git( + &[ + "clone", + "--filter=blob:none", + "--no-checkout", + url, + destination.as_str(), + ], + /*cwd*/ None, + )?; + let mut sparse_args = vec!["sparse-checkout", "set"]; + sparse_args.extend(sparse_paths.iter().map(String::as_str)); + let destination = Path::new(&destination); + run_git(&sparse_args, Some(destination))?; + run_git(&["checkout", ref_name.unwrap_or("HEAD")], Some(destination))?; + Ok(()) +} + +fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<()> { + let mut command = Command::new("git"); + command.args(args); + command.env("GIT_TERMINAL_PROMPT", "0"); + if let Some(cwd) = cwd { + command.current_dir(cwd); + } + + let output = command + .output() + .with_context(|| format!("failed to run git {}", args.join(" ")))?; + if output.status.success() { + return Ok(()); + } + + let stderr = String::from_utf8_lossy(&output.stderr); + let stdout = String::from_utf8_lossy(&output.stdout); + bail!( + "git {} failed with status {}\nstdout:\n{}\nstderr:\n{}", + args.join(" "), + output.status, + stdout.trim(), + stderr.trim() + ); +} + +pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { + if let Some(parent) = destination.parent() { + fs::create_dir_all(parent)?; + } + if destination.exists() { + bail!( + "marketplace destination already exists: {}", + destination.display() + ); + } + + fs::rename(staged_root, destination).map_err(Into::into) +} + +pub(super) fn remove_marketplace_root(root: &Path) -> Result { + if !root.exists() { + return Ok(false); + } + + fs::remove_dir_all(root)?; + Ok(true) +} + +pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { + install_root.join(".staging") +} + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + #[test] + fn replace_marketplace_root_rejects_existing_destination() { + let temp_dir = TempDir::new().unwrap(); + let staged_root = temp_dir.path().join("staged"); + let destination = temp_dir.path().join("destination"); + fs::create_dir_all(&staged_root).unwrap(); + fs::write(staged_root.join("marker.txt"), "staged").unwrap(); + fs::create_dir_all(&destination).unwrap(); + fs::write(destination.join("marker.txt"), "installed").unwrap(); + + let err = replace_marketplace_root(&staged_root, &destination).unwrap_err(); + + assert!( + err.to_string() + .contains("marketplace destination already exists"), + "unexpected error: {err}" + ); + assert_eq!( + fs::read_to_string(staged_root.join("marker.txt")).unwrap(), + "staged" + ); + assert_eq!( + fs::read_to_string(destination.join("marker.txt")).unwrap(), + "installed" + ); + } + + #[test] + fn remove_marketplace_root_returns_false_when_missing() { + let temp_dir = TempDir::new().unwrap(); + + let removed = remove_marketplace_root(&temp_dir.path().join("missing")).unwrap(); + + assert!(!removed); + } +} diff --git a/codex-rs/cli/tests/marketplace_remove.rs b/codex-rs/cli/tests/marketplace_remove.rs new file mode 100644 index 00000000000..6a47a010080 --- /dev/null +++ b/codex-rs/cli/tests/marketplace_remove.rs @@ -0,0 +1,69 @@ +use anyhow::Result; +use codex_config::MarketplaceConfigUpdate; +use codex_config::record_user_marketplace; +use codex_core::plugins::marketplace_install_root; +use predicates::str::contains; +use std::path::Path; +use tempfile::TempDir; + +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 configured_marketplace_update() -> MarketplaceConfigUpdate<'static> { + MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + } +} + +fn write_installed_marketplace(codex_home: &Path, marketplace_name: &str) -> Result<()> { + let root = marketplace_install_root(codex_home).join(marketplace_name); + std::fs::create_dir_all(root.join(".agents/plugins"))?; + std::fs::write(root.join(".agents/plugins/marketplace.json"), "{}")?; + std::fs::write(root.join("marker.txt"), "installed")?; + Ok(()) +} + +#[tokio::test] +async fn marketplace_remove_deletes_config_and_installed_root() -> Result<()> { + let codex_home = TempDir::new()?; + record_user_marketplace(codex_home.path(), "debug", &configured_marketplace_update())?; + write_installed_marketplace(codex_home.path(), "debug")?; + + codex_command(codex_home.path())? + .args(["marketplace", "remove", "debug"]) + .assert() + .success() + .stdout(contains("Removed marketplace `debug`.")); + + let config_path = codex_home.path().join("config.toml"); + let config = std::fs::read_to_string(config_path)?; + assert!(!config.contains("[marketplaces.debug]")); + assert!( + !marketplace_install_root(codex_home.path()) + .join("debug") + .exists() + ); + Ok(()) +} + +#[tokio::test] +async fn marketplace_remove_rejects_unknown_marketplace() -> Result<()> { + let codex_home = TempDir::new()?; + + codex_command(codex_home.path())? + .args(["marketplace", "remove", "debug"]) + .assert() + .failure() + .stderr(contains( + "marketplace `debug` is not configured or installed", + )); + + Ok(()) +} diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 66a6fc22fce..c4cf8ad041a 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -61,6 +61,7 @@ pub use diagnostics::io_error_from_config_error; pub use fingerprint::version_for_toml; pub use marketplace_edit::MarketplaceConfigUpdate; pub use marketplace_edit::record_user_marketplace; +pub use marketplace_edit::remove_user_marketplace; pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; pub use mcp_types::AppToolApproval; diff --git a/codex-rs/config/src/marketplace_edit.rs b/codex-rs/config/src/marketplace_edit.rs index e20a75a02e1..1b2f076e522 100644 --- a/codex-rs/config/src/marketplace_edit.rs +++ b/codex-rs/config/src/marketplace_edit.rs @@ -31,6 +31,26 @@ pub fn record_user_marketplace( fs::write(config_path, doc.to_string()) } +pub fn remove_user_marketplace(codex_home: &Path, marketplace_name: &str) -> std::io::Result { + let config_path = codex_home.join(CONFIG_TOML_FILE); + let mut doc = match fs::read_to_string(&config_path) { + Ok(raw) => raw + .parse::() + .map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err))?, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(false), + Err(err) => return Err(err), + }; + + let removed = remove_marketplace(&mut doc, marketplace_name); + if !removed { + return Ok(false); + } + + fs::create_dir_all(codex_home)?; + fs::write(config_path, doc.to_string())?; + Ok(true) +} + fn read_or_create_document(config_path: &Path) -> std::io::Result { match fs::read_to_string(config_path) { Ok(raw) => raw @@ -80,8 +100,68 @@ fn upsert_marketplace( marketplaces.insert(marketplace_name, TomlItem::Table(entry)); } +fn remove_marketplace(doc: &mut DocumentMut, marketplace_name: &str) -> bool { + let root = doc.as_table_mut(); + let Some(marketplaces_item) = root.get_mut("marketplaces") else { + return false; + }; + let Some(marketplaces) = marketplaces_item.as_table_mut() else { + return false; + }; + if marketplaces.remove(marketplace_name).is_none() { + return false; + } + if marketplaces.is_empty() { + root.remove("marketplaces"); + } + true +} + fn new_implicit_table() -> TomlTable { let mut table = TomlTable::new(); table.set_implicit(true); table } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + #[test] + fn remove_user_marketplace_removes_requested_entry() { + let codex_home = TempDir::new().unwrap(); + let update = MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + }; + record_user_marketplace(codex_home.path(), "debug", &update).unwrap(); + record_user_marketplace(codex_home.path(), "other", &update).unwrap(); + + let removed = remove_user_marketplace(codex_home.path(), "debug").unwrap(); + + assert!(removed); + let config: toml::Value = + toml::from_str(&fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).unwrap()) + .unwrap(); + let marketplaces = config + .get("marketplaces") + .and_then(toml::Value::as_table) + .unwrap(); + assert_eq!(marketplaces.len(), 1); + assert!(marketplaces.contains_key("other")); + } + + #[test] + fn remove_user_marketplace_returns_false_when_missing() { + let codex_home = TempDir::new().unwrap(); + + let removed = remove_user_marketplace(codex_home.path(), "debug").unwrap(); + + assert!(!removed); + } +} From 908d13c432896a911ea752278ac9360ff06885c0 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Mon, 13 Apr 2026 20:40:20 -0700 Subject: [PATCH 2/4] [codex] Refactor marketplace/remove for shared core use --- codex-rs/cli/src/main.rs | 13 ++ codex-rs/cli/src/marketplace_cmd.rs | 35 ++-- codex-rs/cli/src/marketplace_cmd/ops.rs | 136 --------------- codex-rs/cli/tests/marketplace_remove.rs | 5 +- .../core/src/plugins/marketplace_remove.rs | 155 ++++++++++++++++++ codex-rs/core/src/plugins/mod.rs | 5 + 6 files changed, 186 insertions(+), 163 deletions(-) delete mode 100644 codex-rs/cli/src/marketplace_cmd/ops.rs create mode 100644 codex-rs/core/src/plugins/marketplace_remove.rs diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 32fb25ad3d1..7ed8262fdb2 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -1729,6 +1729,15 @@ mod tests { assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_)))); } + #[test] + fn plugin_marketplace_remove_parses_under_plugin() { + let cli = + MultitoolCli::try_parse_from(["codex", "plugin", "marketplace", "remove", "debug"]) + .expect("parse"); + + assert!(matches!(cli.subcommand, Some(Subcommand::Plugin(_)))); + } + #[test] fn marketplace_no_longer_parses_at_top_level() { let add_result = @@ -1738,6 +1747,10 @@ mod tests { let upgrade_result = MultitoolCli::try_parse_from(["codex", "marketplace", "upgrade", "debug"]); assert!(upgrade_result.is_err()); + + let remove_result = + MultitoolCli::try_parse_from(["codex", "marketplace", "remove", "debug"]); + assert!(remove_result.is_err()); } fn sample_exit_info(conversation_id: Option<&str>, thread_name: Option<&str>) -> AppExitInfo { diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index bce3c69def7..a29f9ba76fb 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -2,19 +2,16 @@ use anyhow::Context; use anyhow::Result; use anyhow::bail; use clap::Parser; -use codex_config::remove_user_marketplace; use codex_core::config::Config; use codex_core::config::find_codex_home; use codex_core::plugins::MarketplaceAddRequest; +use codex_core::plugins::MarketplaceRemoveRequest; use codex_core::plugins::PluginMarketplaceUpgradeOutcome; use codex_core::plugins::PluginsManager; use codex_core::plugins::add_marketplace; -use codex_core::plugins::marketplace_install_root; -use codex_core::plugins::validate_plugin_segment; +use codex_core::plugins::remove_marketplace; use codex_utils_cli::CliConfigOverrides; -mod ops; - #[derive(Debug, Parser)] pub struct MarketplaceCli { #[clap(flatten)] @@ -135,30 +132,18 @@ async fn run_upgrade( async fn run_remove(args: RemoveMarketplaceArgs) -> Result<()> { let RemoveMarketplaceArgs { marketplace_name } = args; - validate_plugin_segment(&marketplace_name, "marketplace name").map_err(anyhow::Error::msg)?; - let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; - let install_root = marketplace_install_root(&codex_home); - let destination = install_root.join(&marketplace_name); - let removed_root = ops::remove_marketplace_root(&destination).with_context(|| { - format!( - "failed to remove installed marketplace root {}", - destination.display() - ) - })?; - let removed_config = - remove_user_marketplace(&codex_home, &marketplace_name).with_context(|| { - format!("failed to remove marketplace `{marketplace_name}` from user config.toml") - })?; - if !removed_root && !removed_config { - bail!("marketplace `{marketplace_name}` is not configured or installed"); - } + let outcome = remove_marketplace( + codex_home.to_path_buf(), + MarketplaceRemoveRequest { marketplace_name }, + ) + .await?; - println!("Removed marketplace `{marketplace_name}`."); - if removed_root { + println!("Removed marketplace `{}`.", outcome.marketplace_name); + if let Some(installed_root) = outcome.removed_installed_root { println!( "Removed installed marketplace root: {}", - destination.display() + installed_root.as_path().display() ); } diff --git a/codex-rs/cli/src/marketplace_cmd/ops.rs b/codex-rs/cli/src/marketplace_cmd/ops.rs deleted file mode 100644 index ac5c487434a..00000000000 --- a/codex-rs/cli/src/marketplace_cmd/ops.rs +++ /dev/null @@ -1,136 +0,0 @@ -use anyhow::Context; -use anyhow::Result; -use anyhow::bail; -use std::fs; -use std::path::Path; -use std::path::PathBuf; -use std::process::Command; - -pub(super) fn clone_git_source( - url: &str, - ref_name: Option<&str>, - sparse_paths: &[String], - destination: &Path, -) -> Result<()> { - let destination = destination.to_string_lossy().to_string(); - if sparse_paths.is_empty() { - run_git(&["clone", url, destination.as_str()], /*cwd*/ None)?; - if let Some(ref_name) = ref_name { - run_git(&["checkout", ref_name], Some(Path::new(&destination)))?; - } - return Ok(()); - } - - run_git( - &[ - "clone", - "--filter=blob:none", - "--no-checkout", - url, - destination.as_str(), - ], - /*cwd*/ None, - )?; - let mut sparse_args = vec!["sparse-checkout", "set"]; - sparse_args.extend(sparse_paths.iter().map(String::as_str)); - let destination = Path::new(&destination); - run_git(&sparse_args, Some(destination))?; - run_git(&["checkout", ref_name.unwrap_or("HEAD")], Some(destination))?; - Ok(()) -} - -fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<()> { - let mut command = Command::new("git"); - command.args(args); - command.env("GIT_TERMINAL_PROMPT", "0"); - if let Some(cwd) = cwd { - command.current_dir(cwd); - } - - let output = command - .output() - .with_context(|| format!("failed to run git {}", args.join(" ")))?; - if output.status.success() { - return Ok(()); - } - - let stderr = String::from_utf8_lossy(&output.stderr); - let stdout = String::from_utf8_lossy(&output.stdout); - bail!( - "git {} failed with status {}\nstdout:\n{}\nstderr:\n{}", - args.join(" "), - output.status, - stdout.trim(), - stderr.trim() - ); -} - -pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { - if let Some(parent) = destination.parent() { - fs::create_dir_all(parent)?; - } - if destination.exists() { - bail!( - "marketplace destination already exists: {}", - destination.display() - ); - } - - fs::rename(staged_root, destination).map_err(Into::into) -} - -pub(super) fn remove_marketplace_root(root: &Path) -> Result { - if !root.exists() { - return Ok(false); - } - - fs::remove_dir_all(root)?; - Ok(true) -} - -pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { - install_root.join(".staging") -} - -#[cfg(test)] -mod tests { - use super::*; - use pretty_assertions::assert_eq; - use tempfile::TempDir; - - #[test] - fn replace_marketplace_root_rejects_existing_destination() { - let temp_dir = TempDir::new().unwrap(); - let staged_root = temp_dir.path().join("staged"); - let destination = temp_dir.path().join("destination"); - fs::create_dir_all(&staged_root).unwrap(); - fs::write(staged_root.join("marker.txt"), "staged").unwrap(); - fs::create_dir_all(&destination).unwrap(); - fs::write(destination.join("marker.txt"), "installed").unwrap(); - - let err = replace_marketplace_root(&staged_root, &destination).unwrap_err(); - - assert!( - err.to_string() - .contains("marketplace destination already exists"), - "unexpected error: {err}" - ); - assert_eq!( - fs::read_to_string(staged_root.join("marker.txt")).unwrap(), - "staged" - ); - assert_eq!( - fs::read_to_string(destination.join("marker.txt")).unwrap(), - "installed" - ); - } - - #[test] - fn remove_marketplace_root_returns_false_when_missing() { - let temp_dir = TempDir::new().unwrap(); - - let removed = remove_marketplace_root(&temp_dir.path().join("missing")).unwrap(); - - assert!(!removed); - } -} diff --git a/codex-rs/cli/tests/marketplace_remove.rs b/codex-rs/cli/tests/marketplace_remove.rs index 6a47a010080..06e213bae67 100644 --- a/codex-rs/cli/tests/marketplace_remove.rs +++ b/codex-rs/cli/tests/marketplace_remove.rs @@ -15,6 +15,7 @@ fn codex_command(codex_home: &Path) -> Result { fn configured_marketplace_update() -> MarketplaceConfigUpdate<'static> { MarketplaceConfigUpdate { last_updated: "2026-04-13T00:00:00Z", + last_revision: None, source_type: "git", source: "https://github.com/owner/repo.git", ref_name: Some("main"), @@ -37,7 +38,7 @@ async fn marketplace_remove_deletes_config_and_installed_root() -> Result<()> { write_installed_marketplace(codex_home.path(), "debug")?; codex_command(codex_home.path())? - .args(["marketplace", "remove", "debug"]) + .args(["plugin", "marketplace", "remove", "debug"]) .assert() .success() .stdout(contains("Removed marketplace `debug`.")); @@ -58,7 +59,7 @@ async fn marketplace_remove_rejects_unknown_marketplace() -> Result<()> { let codex_home = TempDir::new()?; codex_command(codex_home.path())? - .args(["marketplace", "remove", "debug"]) + .args(["plugin", "marketplace", "remove", "debug"]) .assert() .failure() .stderr(contains( diff --git a/codex-rs/core/src/plugins/marketplace_remove.rs b/codex-rs/core/src/plugins/marketplace_remove.rs new file mode 100644 index 00000000000..ae7136467a4 --- /dev/null +++ b/codex-rs/core/src/plugins/marketplace_remove.rs @@ -0,0 +1,155 @@ +use crate::plugins::marketplace_install_root; +use crate::plugins::validate_plugin_segment; +use codex_config::remove_user_marketplace; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::fs; +use std::path::Path; +use std::path::PathBuf; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MarketplaceRemoveRequest { + pub marketplace_name: String, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct MarketplaceRemoveOutcome { + pub marketplace_name: String, + pub removed_installed_root: Option, +} + +#[derive(Debug, thiserror::Error)] +pub enum MarketplaceRemoveError { + #[error("{0}")] + InvalidRequest(String), + #[error("{0}")] + Internal(String), +} + +pub async fn remove_marketplace( + codex_home: PathBuf, + request: MarketplaceRemoveRequest, +) -> Result { + tokio::task::spawn_blocking(move || remove_marketplace_sync(codex_home.as_path(), request)) + .await + .map_err(|err| { + MarketplaceRemoveError::Internal(format!("failed to remove marketplace: {err}")) + })? +} + +fn remove_marketplace_sync( + codex_home: &Path, + request: MarketplaceRemoveRequest, +) -> Result { + let marketplace_name = request.marketplace_name; + validate_plugin_segment(&marketplace_name, "marketplace name") + .map_err(MarketplaceRemoveError::InvalidRequest)?; + + let destination = marketplace_install_root(codex_home).join(&marketplace_name); + let removed_installed_root = remove_marketplace_root(&destination)?; + let removed_config = remove_user_marketplace(codex_home, &marketplace_name).map_err(|err| { + MarketplaceRemoveError::Internal(format!( + "failed to remove marketplace '{marketplace_name}' from user config.toml: {err}" + )) + })?; + + if removed_installed_root.is_none() && !removed_config { + return Err(MarketplaceRemoveError::InvalidRequest(format!( + "marketplace `{marketplace_name}` is not configured or installed" + ))); + } + + Ok(MarketplaceRemoveOutcome { + marketplace_name, + removed_installed_root, + }) +} + +fn remove_marketplace_root(root: &Path) -> Result, MarketplaceRemoveError> { + if !root.exists() { + return Ok(None); + } + + let removed_root = AbsolutePathBuf::try_from(root.to_path_buf()).map_err(|err| { + MarketplaceRemoveError::Internal(format!( + "failed to resolve installed marketplace root {}: {err}", + root.display() + )) + })?; + fs::remove_dir_all(root).map_err(|err| { + MarketplaceRemoveError::Internal(format!( + "failed to remove installed marketplace root {}: {err}", + root.display() + )) + })?; + Ok(Some(removed_root)) +} + +#[cfg(test)] +mod tests { + use super::*; + use codex_config::MarketplaceConfigUpdate; + use codex_config::record_user_marketplace; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + #[test] + fn remove_marketplace_sync_removes_config_and_installed_root() { + let codex_home = TempDir::new().unwrap(); + record_user_marketplace( + codex_home.path(), + "debug", + &MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + last_revision: None, + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + }, + ) + .unwrap(); + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + fs::create_dir_all(installed_root.join(".agents/plugins")).unwrap(); + fs::write( + installed_root.join(".agents/plugins/marketplace.json"), + "{}", + ) + .unwrap(); + + let outcome = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "debug".to_string(), + }, + ) + .unwrap(); + + assert_eq!(outcome.marketplace_name, "debug"); + assert_eq!( + outcome.removed_installed_root, + Some(AbsolutePathBuf::try_from(installed_root.clone()).unwrap()) + ); + let config = + fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE)).unwrap(); + assert!(!config.contains("[marketplaces.debug]")); + assert!(!installed_root.exists()); + } + + #[test] + fn remove_marketplace_sync_rejects_unknown_marketplace() { + let codex_home = TempDir::new().unwrap(); + + let err = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "debug".to_string(), + }, + ) + .unwrap_err(); + + assert_eq!( + err.to_string(), + "marketplace `debug` is not configured or installed" + ); + } +} diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 16cc8d08c4d..a8b85c8339a 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -5,6 +5,7 @@ mod injection; mod installed_marketplaces; mod manager; mod marketplace_add; +mod marketplace_remove; mod mentions; mod render; mod startup_sync; @@ -49,6 +50,10 @@ pub use marketplace_add::MarketplaceAddOutcome; pub use marketplace_add::MarketplaceAddRequest; pub use marketplace_add::add_marketplace; pub(crate) use marketplace_add::parse_marketplace_source; +pub use marketplace_remove::MarketplaceRemoveError; +pub use marketplace_remove::MarketplaceRemoveOutcome; +pub use marketplace_remove::MarketplaceRemoveRequest; +pub use marketplace_remove::remove_marketplace; pub(crate) use render::render_explicit_plugin_instructions; pub(crate) use render::render_plugins_section; pub(crate) use startup_sync::curated_plugins_repo_path; From a5ab48e9ed6594b315513615064748a866513ecf Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 17 Apr 2026 03:30:01 -0700 Subject: [PATCH 3/4] Fix marketplace config test build --- codex-rs/config/src/marketplace_edit.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/config/src/marketplace_edit.rs b/codex-rs/config/src/marketplace_edit.rs index 1b2f076e522..2b60e42ba14 100644 --- a/codex-rs/config/src/marketplace_edit.rs +++ b/codex-rs/config/src/marketplace_edit.rs @@ -134,6 +134,7 @@ mod tests { let codex_home = TempDir::new().unwrap(); let update = MarketplaceConfigUpdate { last_updated: "2026-04-13T00:00:00Z", + last_revision: None, source_type: "git", source: "https://github.com/owner/repo.git", ref_name: Some("main"), From e53c56038dba47031a987f28a637950d18fd133a Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 17 Apr 2026 15:01:24 -0700 Subject: [PATCH 4/4] Fix marketplace remove edge cases --- codex-rs/config/src/lib.rs | 2 + codex-rs/config/src/marketplace_edit.rs | 136 ++++++++++++-- .../core/src/plugins/marketplace_remove.rs | 172 +++++++++++++++++- 3 files changed, 289 insertions(+), 21 deletions(-) diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 8854dd5e1bc..c1d90416988 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -63,8 +63,10 @@ pub use diagnostics::format_config_error_with_source; pub use diagnostics::io_error_from_config_error; pub use fingerprint::version_for_toml; pub use marketplace_edit::MarketplaceConfigUpdate; +pub use marketplace_edit::RemoveMarketplaceConfigOutcome; pub use marketplace_edit::record_user_marketplace; pub use marketplace_edit::remove_user_marketplace; +pub use marketplace_edit::remove_user_marketplace_config; pub use mcp_edit::ConfigEditsBuilder; pub use mcp_edit::load_global_mcp_servers; pub use mcp_types::AppToolApproval; diff --git a/codex-rs/config/src/marketplace_edit.rs b/codex-rs/config/src/marketplace_edit.rs index 2b60e42ba14..2aad2486ebf 100644 --- a/codex-rs/config/src/marketplace_edit.rs +++ b/codex-rs/config/src/marketplace_edit.rs @@ -19,6 +19,13 @@ pub struct MarketplaceConfigUpdate<'a> { pub sparse_paths: &'a [String], } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum RemoveMarketplaceConfigOutcome { + Removed, + NotFound, + NameCaseMismatch { configured_name: String }, +} + pub fn record_user_marketplace( codex_home: &Path, marketplace_name: &str, @@ -32,23 +39,33 @@ pub fn record_user_marketplace( } pub fn remove_user_marketplace(codex_home: &Path, marketplace_name: &str) -> std::io::Result { + let outcome = remove_user_marketplace_config(codex_home, marketplace_name)?; + Ok(outcome == RemoveMarketplaceConfigOutcome::Removed) +} + +pub fn remove_user_marketplace_config( + codex_home: &Path, + marketplace_name: &str, +) -> std::io::Result { let config_path = codex_home.join(CONFIG_TOML_FILE); let mut doc = match fs::read_to_string(&config_path) { Ok(raw) => raw .parse::() .map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err))?, - Err(err) if err.kind() == ErrorKind::NotFound => return Ok(false), + Err(err) if err.kind() == ErrorKind::NotFound => { + return Ok(RemoveMarketplaceConfigOutcome::NotFound); + } Err(err) => return Err(err), }; - let removed = remove_marketplace(&mut doc, marketplace_name); - if !removed { - return Ok(false); + let outcome = remove_marketplace(&mut doc, marketplace_name); + if outcome != RemoveMarketplaceConfigOutcome::Removed { + return Ok(outcome); } fs::create_dir_all(codex_home)?; fs::write(config_path, doc.to_string())?; - Ok(true) + Ok(RemoveMarketplaceConfigOutcome::Removed) } fn read_or_create_document(config_path: &Path) -> std::io::Result { @@ -100,21 +117,61 @@ fn upsert_marketplace( marketplaces.insert(marketplace_name, TomlItem::Table(entry)); } -fn remove_marketplace(doc: &mut DocumentMut, marketplace_name: &str) -> bool { +fn remove_marketplace( + doc: &mut DocumentMut, + marketplace_name: &str, +) -> RemoveMarketplaceConfigOutcome { let root = doc.as_table_mut(); let Some(marketplaces_item) = root.get_mut("marketplaces") else { - return false; + return RemoveMarketplaceConfigOutcome::NotFound; }; - let Some(marketplaces) = marketplaces_item.as_table_mut() else { - return false; + + let mut remove_marketplaces = false; + let outcome = match marketplaces_item { + TomlItem::Table(marketplaces) => { + let outcome = if marketplaces.remove(marketplace_name).is_some() { + RemoveMarketplaceConfigOutcome::Removed + } else if let Some(configured_name) = + case_mismatched_key(marketplaces.iter().map(|(key, _)| key), marketplace_name) + { + RemoveMarketplaceConfigOutcome::NameCaseMismatch { configured_name } + } else { + RemoveMarketplaceConfigOutcome::NotFound + }; + remove_marketplaces = marketplaces.is_empty(); + outcome + } + TomlItem::Value(value) => { + let Some(marketplaces) = value.as_inline_table_mut() else { + return RemoveMarketplaceConfigOutcome::NotFound; + }; + let outcome = if marketplaces.remove(marketplace_name).is_some() { + RemoveMarketplaceConfigOutcome::Removed + } else if let Some(configured_name) = + case_mismatched_key(marketplaces.iter().map(|(key, _)| key), marketplace_name) + { + RemoveMarketplaceConfigOutcome::NameCaseMismatch { configured_name } + } else { + RemoveMarketplaceConfigOutcome::NotFound + }; + remove_marketplaces = marketplaces.is_empty(); + outcome + } + _ => RemoveMarketplaceConfigOutcome::NotFound, }; - if marketplaces.remove(marketplace_name).is_none() { - return false; - } - if marketplaces.is_empty() { + + if outcome == RemoveMarketplaceConfigOutcome::Removed && remove_marketplaces { root.remove("marketplaces"); } - true + outcome +} + +fn case_mismatched_key<'a>( + mut keys: impl Iterator, + requested_name: &str, +) -> Option { + keys.find(|key| *key != requested_name && key.eq_ignore_ascii_case(requested_name)) + .map(str::to_string) } fn new_implicit_table() -> TomlTable { @@ -165,4 +222,55 @@ mod tests { assert!(!removed); } + + #[test] + fn remove_user_marketplace_config_reports_case_mismatch() { + let codex_home = TempDir::new().unwrap(); + let update = MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + last_revision: None, + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + }; + record_user_marketplace(codex_home.path(), "debug", &update).unwrap(); + + let outcome = remove_user_marketplace_config(codex_home.path(), "Debug").unwrap(); + + assert_eq!( + outcome, + RemoveMarketplaceConfigOutcome::NameCaseMismatch { + configured_name: "debug".to_string() + } + ); + } + + #[test] + fn remove_user_marketplace_config_removes_inline_table_entry() { + let codex_home = TempDir::new().unwrap(); + fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + r#" +marketplaces = { + debug = { source_type = "git", source = "https://github.com/owner/repo.git" }, + other = { source_type = "local", source = "/tmp/marketplace" }, +} +"#, + ) + .unwrap(); + + let outcome = remove_user_marketplace_config(codex_home.path(), "debug").unwrap(); + + assert_eq!(outcome, RemoveMarketplaceConfigOutcome::Removed); + let config: toml::Value = + toml::from_str(&fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE)).unwrap()) + .unwrap(); + let marketplaces = config + .get("marketplaces") + .and_then(toml::Value::as_table) + .unwrap(); + assert_eq!(marketplaces.len(), 1); + assert!(marketplaces.contains_key("other")); + } } diff --git a/codex-rs/core/src/plugins/marketplace_remove.rs b/codex-rs/core/src/plugins/marketplace_remove.rs index ae7136467a4..490b16f95d9 100644 --- a/codex-rs/core/src/plugins/marketplace_remove.rs +++ b/codex-rs/core/src/plugins/marketplace_remove.rs @@ -1,6 +1,7 @@ use crate::plugins::marketplace_install_root; use crate::plugins::validate_plugin_segment; -use codex_config::remove_user_marketplace; +use codex_config::RemoveMarketplaceConfigOutcome; +use codex_config::remove_user_marketplace_config; use codex_utils_absolute_path::AbsolutePathBuf; use std::fs; use std::path::Path; @@ -45,12 +46,20 @@ fn remove_marketplace_sync( .map_err(MarketplaceRemoveError::InvalidRequest)?; let destination = marketplace_install_root(codex_home).join(&marketplace_name); + let config_outcome = + remove_user_marketplace_config(codex_home, &marketplace_name).map_err(|err| { + MarketplaceRemoveError::Internal(format!( + "failed to remove marketplace '{marketplace_name}' from user config.toml: {err}" + )) + })?; + if let RemoveMarketplaceConfigOutcome::NameCaseMismatch { configured_name } = &config_outcome { + return Err(MarketplaceRemoveError::InvalidRequest(format!( + "marketplace `{marketplace_name}` does not match configured marketplace `{configured_name}` exactly" + ))); + } + + let removed_config = config_outcome == RemoveMarketplaceConfigOutcome::Removed; let removed_installed_root = remove_marketplace_root(&destination)?; - let removed_config = remove_user_marketplace(codex_home, &marketplace_name).map_err(|err| { - MarketplaceRemoveError::Internal(format!( - "failed to remove marketplace '{marketplace_name}' from user config.toml: {err}" - )) - })?; if removed_installed_root.is_none() && !removed_config { return Err(MarketplaceRemoveError::InvalidRequest(format!( @@ -75,7 +84,18 @@ fn remove_marketplace_root(root: &Path) -> Result, Marke root.display() )) })?; - fs::remove_dir_all(root).map_err(|err| { + let metadata = fs::symlink_metadata(root).map_err(|err| { + MarketplaceRemoveError::Internal(format!( + "failed to inspect installed marketplace root {}: {err}", + root.display() + )) + })?; + let remove_result = if metadata.is_dir() { + fs::remove_dir_all(root) + } else { + fs::remove_file(root) + }; + remove_result.map_err(|err| { MarketplaceRemoveError::Internal(format!( "failed to remove installed marketplace root {}: {err}", root.display() @@ -152,4 +172,142 @@ mod tests { "marketplace `debug` is not configured or installed" ); } + + #[test] + fn remove_marketplace_sync_rejects_case_mismatched_configured_name() { + let codex_home = TempDir::new().unwrap(); + record_user_marketplace( + codex_home.path(), + "debug", + &MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + last_revision: None, + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + }, + ) + .unwrap(); + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + fs::create_dir_all(&installed_root).unwrap(); + + let err = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "Debug".to_string(), + }, + ) + .unwrap_err(); + + assert_eq!( + err.to_string(), + "marketplace `Debug` does not match configured marketplace `debug` exactly" + ); + assert!(installed_root.exists()); + let config = + fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE)).unwrap(); + assert!(config.contains("[marketplaces.debug]")); + } + + #[test] + fn remove_marketplace_sync_keeps_installed_root_when_config_removal_fails() { + let codex_home = TempDir::new().unwrap(); + fs::write( + codex_home.path().join(codex_config::CONFIG_TOML_FILE), + "[marketplaces.debug\n", + ) + .unwrap(); + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + fs::create_dir_all(&installed_root).unwrap(); + + let err = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "debug".to_string(), + }, + ) + .unwrap_err(); + + assert!( + err.to_string() + .contains("failed to remove marketplace 'debug' from user config.toml") + ); + assert!(installed_root.exists()); + } + + #[test] + fn remove_marketplace_sync_removes_file_installed_root() { + let codex_home = TempDir::new().unwrap(); + record_user_marketplace( + codex_home.path(), + "debug", + &MarketplaceConfigUpdate { + last_updated: "2026-04-13T00:00:00Z", + last_revision: None, + source_type: "git", + source: "https://github.com/owner/repo.git", + ref_name: Some("main"), + sparse_paths: &[], + }, + ) + .unwrap(); + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + fs::create_dir_all(installed_root.parent().unwrap()).unwrap(); + fs::write(&installed_root, "corrupt install root").unwrap(); + + let outcome = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "debug".to_string(), + }, + ) + .unwrap(); + + assert_eq!( + outcome, + MarketplaceRemoveOutcome { + marketplace_name: "debug".to_string(), + removed_installed_root: Some( + AbsolutePathBuf::try_from(installed_root.clone()).unwrap() + ), + } + ); + assert!(!installed_root.exists()); + let config = + fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE)).unwrap(); + assert!(!config.contains("[marketplaces.debug]")); + } + + #[test] + fn remove_marketplace_sync_removes_inline_config_entry() { + let codex_home = TempDir::new().unwrap(); + fs::write( + codex_home.path().join(codex_config::CONFIG_TOML_FILE), + r#" +marketplaces = { debug = { source_type = "git", source = "https://github.com/owner/repo.git" } } +"#, + ) + .unwrap(); + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + fs::create_dir_all(&installed_root).unwrap(); + + let outcome = remove_marketplace_sync( + codex_home.path(), + MarketplaceRemoveRequest { + marketplace_name: "debug".to_string(), + }, + ) + .unwrap(); + + assert_eq!(outcome.marketplace_name, "debug"); + assert_eq!( + outcome.removed_installed_root, + Some(AbsolutePathBuf::try_from(installed_root.clone()).unwrap()) + ); + assert!(!installed_root.exists()); + let config = + fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE)).unwrap(); + assert!(!config.contains("debug")); + } }