[codex] Add marketplace remove command and shared logic (#17752)

## Summary

Move the marketplace remove implementation into shared core logic so
both the CLI command and follow-up app-server RPC can reuse the same
behavior.

This change:
- adds a shared `codex_core::plugins::remove_marketplace(...)` flow
- moves validation, config removal, and installed-root deletion out of
the CLI
- keeps the CLI as a thin wrapper over the shared implementation
- adds focused core coverage for the shared remove path

## Validation

- `just fmt`
- focused local coverage for the shared remove path
- heavier follow-up validation deferred to stacked PR CI
This commit is contained in:
xli-oai
2026-04-17 21:44:47 -07:00
committed by GitHub
parent 6b39d0c657
commit e9c70fff3f
7 changed files with 629 additions and 0 deletions

View File

@@ -63,7 +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;

View File

@@ -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,
@@ -31,6 +38,36 @@ 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<bool> {
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<RemoveMarketplaceConfigOutcome> {
let config_path = codex_home.join(CONFIG_TOML_FILE);
let mut doc = match fs::read_to_string(&config_path) {
Ok(raw) => raw
.parse::<DocumentMut>()
.map_err(|err| std::io::Error::new(ErrorKind::InvalidData, err))?,
Err(err) if err.kind() == ErrorKind::NotFound => {
return Ok(RemoveMarketplaceConfigOutcome::NotFound);
}
Err(err) => return Err(err),
};
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(RemoveMarketplaceConfigOutcome::Removed)
}
fn read_or_create_document(config_path: &Path) -> std::io::Result<DocumentMut> {
match fs::read_to_string(config_path) {
Ok(raw) => raw
@@ -80,8 +117,160 @@ fn upsert_marketplace(
marketplaces.insert(marketplace_name, TomlItem::Table(entry));
}
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 RemoveMarketplaceConfigOutcome::NotFound;
};
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 outcome == RemoveMarketplaceConfigOutcome::Removed && remove_marketplaces {
root.remove("marketplaces");
}
outcome
}
fn case_mismatched_key<'a>(
mut keys: impl Iterator<Item = &'a str>,
requested_name: &str,
) -> Option<String> {
keys.find(|key| *key != requested_name && key.eq_ignore_ascii_case(requested_name))
.map(str::to_string)
}
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",
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();
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);
}
#[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"));
}
}