From f9b23d58b980f29005752e52b32f039e3bee49e0 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 8 Apr 2026 16:24:26 -0700 Subject: [PATCH 01/17] Add marketplace add command support --- codex-rs/cli/src/main.rs | 17 + codex-rs/cli/src/marketplace_cmd.rs | 627 +++++++++++++++++++ codex-rs/cli/src/marketplace_cmd/metadata.rs | 191 ++++++ codex-rs/cli/tests/marketplace_add.rs | 129 ++++ codex-rs/core/src/plugins/manager.rs | 138 ++++ codex-rs/core/src/plugins/manager_tests.rs | 64 ++ codex-rs/core/src/plugins/marketplace.rs | 11 + codex-rs/core/src/plugins/mod.rs | 4 + 8 files changed, 1181 insertions(+) create mode 100644 codex-rs/cli/src/marketplace_cmd.rs create mode 100644 codex-rs/cli/src/marketplace_cmd/metadata.rs create mode 100644 codex-rs/cli/tests/marketplace_add.rs diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index 3007048a2f35..0c9852f7f21e 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -38,10 +38,12 @@ use supports_color::Stream; mod app_cmd; #[cfg(target_os = "macos")] mod desktop_app; +mod marketplace_cmd; mod mcp_cmd; #[cfg(not(windows))] mod wsl_paths; +use crate::marketplace_cmd::MarketplaceCli; use crate::mcp_cmd::McpCli; use codex_core::config::Config; @@ -105,6 +107,9 @@ enum Subcommand { /// Manage external MCP servers for Codex. Mcp(McpCli), + /// Manage plugin marketplaces for Codex. + Marketplace(MarketplaceCli), + /// Start Codex as an MCP server (stdio). McpServer, @@ -691,6 +696,18 @@ async fn cli_main(arg0_paths: Arg0DispatchPaths) -> anyhow::Result<()> { prepend_config_flags(&mut mcp_cli.config_overrides, root_config_overrides.clone()); mcp_cli.run().await?; } + Some(Subcommand::Marketplace(mut marketplace_cli)) => { + reject_remote_mode_for_subcommand( + root_remote.as_deref(), + root_remote_auth_token_env.as_deref(), + "marketplace", + )?; + prepend_config_flags( + &mut marketplace_cli.config_overrides, + root_config_overrides.clone(), + ); + marketplace_cli.run().await?; + } Some(Subcommand::AppServer(app_server_cli)) => { let AppServerCommand { subcommand, diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs new file mode 100644 index 000000000000..d68ed7b22ae8 --- /dev/null +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -0,0 +1,627 @@ +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use clap::Parser; +use codex_core::config::find_codex_home; +use codex_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME; +use codex_core::plugins::marketplace_install_root; +use codex_core::plugins::record_installed_marketplace_root; +use codex_core::plugins::validate_marketplace_root; +use codex_utils_cli::CliConfigOverrides; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::process::Command; + +mod metadata; + +#[derive(Debug, Parser)] +pub struct MarketplaceCli { + #[clap(flatten)] + pub config_overrides: CliConfigOverrides, + + #[command(subcommand)] + subcommand: MarketplaceSubcommand, +} + +#[derive(Debug, clap::Subcommand)] +enum MarketplaceSubcommand { + /// Add a marketplace repository or local marketplace directory. + Add(AddMarketplaceArgs), +} + +#[derive(Debug, Parser)] +struct AddMarketplaceArgs { + /// Marketplace source. Supports owner/repo[@ref], git URLs, SSH URLs, or local directories. + source: String, + + /// Git ref to check out. Overrides any @ref or #ref suffix in SOURCE. + #[arg(long = "ref", value_name = "REF")] + ref_name: Option, + + /// Sparse-checkout paths to use while cloning git sources. + #[arg(long = "sparse", value_name = "PATH", num_args = 1..)] + sparse_paths: Vec, +} + +#[derive(Debug, PartialEq, Eq)] +pub(super) enum MarketplaceSource { + LocalDirectory { + path: PathBuf, + source_id: String, + }, + Git { + url: String, + ref_name: Option, + source_id: String, + }, +} + +impl MarketplaceCli { + pub async fn run(self) -> Result<()> { + let MarketplaceCli { + config_overrides, + subcommand, + } = self; + + // 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 + .parse_overrides() + .map_err(anyhow::Error::msg)?; + + match subcommand { + MarketplaceSubcommand::Add(args) => run_add(args).await?, + } + + Ok(()) + } +} + +async fn run_add(args: AddMarketplaceArgs) -> Result<()> { + let AddMarketplaceArgs { + source, + ref_name, + sparse_paths, + } = args; + + let source = parse_marketplace_source(&source, ref_name)?; + if !sparse_paths.is_empty() && !matches!(source, MarketplaceSource::Git { .. }) { + bail!("--sparse can only be used with git marketplace sources"); + } + + 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(&install_root, &install_metadata.source_id)? + { + let marketplace_name = validate_marketplace_root(&existing_root).with_context(|| { + format!( + "failed to validate installed marketplace at {}", + existing_root.display() + ) + })?; + println!( + "Marketplace `{marketplace_name}` is already added from {}.", + source.display() + ); + println!("Installed marketplace root: {}", existing_root.display()); + return Ok(()); + } + + let staging_root = 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(); + + match &source { + MarketplaceSource::LocalDirectory { path, .. } => { + copy_dir_recursive(path, &staged_root).with_context(|| { + format!( + "failed to copy marketplace source {} into {}", + path.display(), + staged_root.display() + ) + })?; + } + MarketplaceSource::Git { url, ref_name, .. } => { + clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; + } + } + + let marketplace_name = validate_marketplace_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() + ); + } + metadata::write_marketplace_source_metadata(&staged_root, &install_metadata)?; + 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() + ); + } + replace_marketplace_root(&staged_root, &destination) + .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; + record_installed_marketplace_root(&codex_home, &marketplace_name, &destination) + .with_context(|| format!("failed to record marketplace `{marketplace_name}`"))?; + + println!( + "Added marketplace `{marketplace_name}` from {}.", + source.display() + ); + println!("Installed marketplace root: {}", destination.display()); + + Ok(()) +} + +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 source = expand_home(source); + let path = PathBuf::from(&source); + let path_exists = path.try_exists().with_context(|| { + format!( + "failed to access local marketplace source {}", + path.display() + ) + })?; + if path_exists || looks_like_local_path(&source) { + if !path_exists { + bail!( + "local marketplace source does not exist: {}", + path.display() + ); + } + let metadata = path.metadata().with_context(|| { + format!("failed to read local marketplace source {}", path.display()) + })?; + if metadata.is_file() { + if path + .extension() + .is_some_and(|extension| extension == "json") + { + bail!( + "local marketplace JSON files are not supported yet; pass the marketplace root directory containing .agents/plugins/marketplace.json: {}", + path.display() + ); + } + bail!( + "local marketplace source file must be a JSON marketplace manifest or a directory containing .agents/plugins/marketplace.json: {}", + path.display() + ); + } + if !metadata.is_dir() { + bail!( + "local marketplace source must be a file or directory: {}", + path.display() + ); + } + let path = path + .canonicalize() + .with_context(|| format!("failed to resolve {}", path.display()))?; + return Ok(MarketplaceSource::LocalDirectory { + source_id: format!("directory:{}", path.display()), + path, + }); + } + + let (base_source, parsed_ref) = split_source_ref(&source); + let ref_name = explicit_ref.or(parsed_ref); + + if is_ssh_git_url(&base_source) || is_http_git_url(&base_source) { + let url = normalize_git_url(&base_source); + return Ok(MarketplaceSource::Git { + source_id: git_source_id("git", &url, ref_name.as_deref()), + url, + ref_name, + }); + } + + if looks_like_github_shorthand(&base_source) { + let url = format!("https://github.com/{base_source}.git"); + return Ok(MarketplaceSource::Git { + source_id: git_source_id("github", &base_source, ref_name.as_deref()), + url, + ref_name, + }); + } + + if base_source.starts_with("http://") || base_source.starts_with("https://") { + bail!( + "URL marketplace manifests are not supported yet; pass a git repository URL or a local marketplace directory" + ); + } + + bail!("invalid marketplace source format: {source}"); +} + +fn git_source_id(kind: &str, source: &str, ref_name: Option<&str>) -> String { + if let Some(ref_name) = ref_name { + format!("{kind}:{source}#{ref_name}") + } else { + format!("{kind}:{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 { + 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 expand_home(source: &str) -> String { + let Some(rest) = source.strip_prefix("~/") else { + return source.to_string(); + }; + if let Some(home) = std::env::var_os("HOME") { + return PathBuf::from(home).join(rest).display().to_string(); + } + source.to_string() +} + +fn is_ssh_git_url(source: &str) -> bool { + source.starts_with("git@") && source.contains(':') +} + +fn is_http_git_url(source: &str) -> bool { + (source.starts_with("http://") || source.starts_with("https://")) + && (source.ends_with(".git") || source.starts_with("https://github.com/")) +} + +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, '-' | '_' | '.')) +} + +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()], 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(), + ], + 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(()) +} + +pub(super) 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 copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { + fs::create_dir_all(target)?; + for entry in fs::read_dir(source)? { + let entry = entry?; + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + let file_type = entry.file_type()?; + + if file_type.is_dir() { + if entry.file_name().to_str() == Some(".git") { + continue; + } + copy_dir_recursive(&source_path, &target_path)?; + } else if file_type.is_file() { + fs::copy(&source_path, &target_path)?; + } else if file_type.is_symlink() { + copy_symlink_target(&source_path, &target_path)?; + } + } + Ok(()) +} + +#[cfg(unix)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + std::os::unix::fs::symlink(fs::read_link(source)?, target)?; + Ok(()) +} + +#[cfg(windows)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + let metadata = fs::metadata(source)?; + if metadata.is_dir() { + copy_dir_recursive(source, target) + } else { + fs::copy(source, target).map(|_| ()).map_err(Into::into) + } +} + +pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { + if let Some(parent) = destination.parent() { + fs::create_dir_all(parent)?; + } + + let backup = if destination.exists() { + let parent = destination + .parent() + .context("marketplace destination has no parent")?; + let staging_root = marketplace_staging_root(parent); + fs::create_dir_all(&staging_root)?; + let backup = tempfile::Builder::new() + .prefix("marketplace-backup-") + .tempdir_in(&staging_root)?; + let backup_root = backup.path().join("previous"); + fs::rename(destination, &backup_root)?; + Some((backup, backup_root)) + } else { + None + }; + + if let Err(err) = fs::rename(staged_root, destination) { + if let Some((_, backup_root)) = backup { + let _ = fs::rename(backup_root, destination); + } + return Err(err.into()); + } + + Ok(()) +} + +pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { + install_root.join(".staging") +} + +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(()) +} + +impl MarketplaceSource { + fn source_id(&self) -> &str { + match self { + Self::LocalDirectory { source_id, .. } | Self::Git { source_id, .. } => source_id, + } + } + + fn display(&self) -> String { + match self { + Self::LocalDirectory { path, .. } => path.display().to_string(), + 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()), + source_id: "github:owner/repo#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()), + source_id: "git:https://example.com/team/repo.git#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()), + source_id: "github:owner/repo#release".to_string(), + } + ); + } + + #[test] + fn github_shorthand_and_git_url_have_different_source_ids() { + 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_ne!(shorthand.source_id(), git_url.source_id()); + assert_eq!( + shorthand, + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: None, + source_id: "github:owner/repo".to_string(), + } + ); + assert_eq!( + git_url, + MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: None, + source_id: "git:https://github.com/owner/repo.git".to_string(), + } + ); + } +} diff --git a/codex-rs/cli/src/marketplace_cmd/metadata.rs b/codex-rs/cli/src/marketplace_cmd/metadata.rs new file mode 100644 index 000000000000..8b88602e2d9c --- /dev/null +++ b/codex-rs/cli/src/marketplace_cmd/metadata.rs @@ -0,0 +1,191 @@ +use super::MarketplaceSource; +use anyhow::Context; +use anyhow::Result; +use anyhow::bail; +use codex_core::plugins::validate_marketplace_root; +use std::fs; +use std::path::Path; +use std::path::PathBuf; + +const MARKETPLACE_ADD_SOURCE_FILE: &str = ".codex-marketplace-source"; + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) struct MarketplaceInstallMetadata { + pub(super) source_id: String, + source: InstalledMarketplaceSource, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub(super) enum InstalledMarketplaceSource { + LocalDirectory { + path: PathBuf, + }, + Git { + url: String, + ref_name: Option, + sparse_paths: Vec, + }, +} + +pub(super) fn installed_marketplace_root_for_source( + install_root: &Path, + source_id: &str, +) -> Result> { + let entries = fs::read_dir(install_root).with_context(|| { + format!( + "failed to read marketplace install directory {}", + install_root.display() + ) + })?; + for entry in entries { + let entry = entry?; + if !entry.file_type()?.is_dir() { + continue; + } + let root = entry.path(); + let metadata_path = root.join(MARKETPLACE_ADD_SOURCE_FILE); + if !metadata_path.is_file() { + continue; + } + let metadata = read_marketplace_source_metadata(&root)?; + if metadata.source_id == source_id && validate_marketplace_root(&root).is_ok() { + return Ok(Some(root)); + } + } + Ok(None) +} + +pub(super) fn write_marketplace_source_metadata( + root: &Path, + metadata: &MarketplaceInstallMetadata, +) -> Result<()> { + let source = match &metadata.source { + InstalledMarketplaceSource::LocalDirectory { path } => serde_json::json!({ + "kind": "directory", + "path": path, + }), + InstalledMarketplaceSource::Git { + url, + ref_name, + sparse_paths, + } => serde_json::json!({ + "kind": "git", + "url": url, + "ref": ref_name, + "sparsePaths": sparse_paths, + }), + }; + let content = serde_json::to_string_pretty(&serde_json::json!({ + "version": 1, + "sourceId": metadata.source_id, + "source": source, + }))?; + fs::write(root.join(MARKETPLACE_ADD_SOURCE_FILE), content).with_context(|| { + format!( + "failed to write marketplace source metadata in {}", + root.display() + ) + }) +} + +fn read_marketplace_source_metadata(root: &Path) -> Result { + let path = root.join(MARKETPLACE_ADD_SOURCE_FILE); + let content = fs::read_to_string(&path).with_context(|| { + format!( + "failed to read marketplace source metadata {}", + path.display() + ) + })?; + if !content.trim_start().starts_with('{') { + return Ok(MarketplaceInstallMetadata { + source_id: content.trim().to_string(), + source: InstalledMarketplaceSource::LocalDirectory { + path: root.to_path_buf(), + }, + }); + } + + let json: serde_json::Value = serde_json::from_str(&content).with_context(|| { + format!( + "failed to parse marketplace source metadata {}", + path.display() + ) + })?; + let source_id = json + .get("sourceId") + .and_then(serde_json::Value::as_str) + .context("marketplace source metadata is missing sourceId")? + .to_string(); + let source = json + .get("source") + .context("marketplace source metadata is missing source")?; + let kind = source + .get("kind") + .and_then(serde_json::Value::as_str) + .context("marketplace source metadata is missing source.kind")?; + let source = match kind { + "directory" => { + let path = source + .get("path") + .and_then(serde_json::Value::as_str) + .context("marketplace directory metadata is missing path")?; + InstalledMarketplaceSource::LocalDirectory { + path: PathBuf::from(path), + } + } + "git" => { + let url = source + .get("url") + .and_then(serde_json::Value::as_str) + .context("marketplace git metadata is missing url")? + .to_string(); + let ref_name = source + .get("ref") + .and_then(serde_json::Value::as_str) + .map(str::to_string); + let sparse_paths = source + .get("sparsePaths") + .and_then(serde_json::Value::as_array) + .map(|paths| { + paths + .iter() + .filter_map(serde_json::Value::as_str) + .map(str::to_string) + .collect::>() + }) + .unwrap_or_default(); + InstalledMarketplaceSource::Git { + url, + ref_name, + sparse_paths, + } + } + other => bail!("unsupported marketplace source metadata kind `{other}`"), + }; + Ok(MarketplaceInstallMetadata { source_id, source }) +} + +impl MarketplaceInstallMetadata { + pub(super) fn from_source(source: &MarketplaceSource, sparse_paths: &[String]) -> Self { + let source_id = if sparse_paths.is_empty() { + source.source_id().to_string() + } else { + format!( + "{}?sparse={}", + source.source_id(), + serde_json::to_string(sparse_paths).unwrap_or_else(|_| "[]".to_string()) + ) + }; + let source = match source { + MarketplaceSource::LocalDirectory { path, .. } => { + InstalledMarketplaceSource::LocalDirectory { path: path.clone() } + } + MarketplaceSource::Git { url, ref_name, .. } => InstalledMarketplaceSource::Git { + url: url.clone(), + ref_name: ref_name.clone(), + sparse_paths: sparse_paths.to_vec(), + }, + }; + Self { source_id, source } + } +} diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs new file mode 100644 index 000000000000..774f14b7dd5f --- /dev/null +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -0,0 +1,129 @@ +use anyhow::Result; +use codex_core::plugins::marketplace_install_root; +use codex_core::plugins::validate_marketplace_root; +use predicates::str::contains; +use pretty_assertions::assert_eq; +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 write_marketplace_source(source: &Path, 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"), + r#"{ + "name": "debug", + "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(()) +} + +#[tokio::test] +async fn marketplace_add_local_directory_installs_valid_marketplace_root() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "first install")?; + + let mut add_cmd = codex_command(codex_home.path())?; + add_cmd + .args(["marketplace", "add", source.path().to_str().unwrap()]) + .assert() + .success() + .stdout(contains("Added marketplace `debug`")); + + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + assert_eq!(validate_marketplace_root(&installed_root)?, "debug"); + let registry = std::fs::read_to_string(codex_home.path().join(".tmp/known_marketplaces.json"))?; + assert!(registry.contains(r#""name": "debug""#)); + assert!(registry.contains(r#""installLocation""#)); + assert!( + installed_root + .join("plugins/sample/.codex-plugin/plugin.json") + .is_file() + ); + + Ok(()) +} + +#[tokio::test] +async fn marketplace_add_same_source_is_idempotent() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "first install")?; + + codex_command(codex_home.path())? + .args(["marketplace", "add", source.path().to_str().unwrap()]) + .assert() + .success() + .stdout(contains("Added marketplace `debug`")); + + std::fs::write( + source.path().join("plugins/sample/marker.txt"), + "source changed after add", + )?; + + codex_command(codex_home.path())? + .args(["marketplace", "add", source.path().to_str().unwrap()]) + .assert() + .success() + .stdout(contains("Marketplace `debug` is already added")); + + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + assert_eq!( + std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, + "first install" + ); + + Ok(()) +} + +#[tokio::test] +async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> { + let codex_home = TempDir::new()?; + let first_source = TempDir::new()?; + let second_source = TempDir::new()?; + write_marketplace_source(first_source.path(), "first install")?; + write_marketplace_source(second_source.path(), "replacement install")?; + + codex_command(codex_home.path())? + .args(["marketplace", "add", first_source.path().to_str().unwrap()]) + .assert() + .success() + .stdout(contains("Added marketplace `debug`")); + + codex_command(codex_home.path())? + .args(["marketplace", "add", second_source.path().to_str().unwrap()]) + .assert() + .failure() + .stderr(contains( + "marketplace `debug` is already added from a different source", + )); + + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + assert_eq!( + std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, + "first install" + ); + + Ok(()) +} diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 1da98b85cbcd..db48f105df68 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -58,6 +58,7 @@ use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; +use serde::Serialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; use serde_json::json; @@ -79,6 +80,8 @@ use tracing::warn; const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; +pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; +const KNOWN_MARKETPLACES_FILE: &str = "known_marketplaces.json"; pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; pub const OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME: &str = "OpenAI Curated"; static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false); @@ -1222,6 +1225,7 @@ impl PluginsManager { // Treat the curated catalog as an extra marketplace root so plugin listing can surface it // without requiring every caller to know where it is stored. let mut roots = additional_roots.to_vec(); + roots.extend(installed_marketplace_roots(self.codex_home.as_path())); let curated_repo_root = curated_plugins_repo_path(self.codex_home.as_path()); if curated_repo_root.is_dir() && let Ok(curated_repo_root) = AbsolutePathBuf::try_from(curated_repo_root) @@ -1234,6 +1238,140 @@ impl PluginsManager { } } +pub fn marketplace_install_root(codex_home: &Path) -> PathBuf { + codex_home.join(INSTALLED_MARKETPLACES_DIR) +} + +pub fn record_installed_marketplace_root( + codex_home: &Path, + marketplace_name: &str, + install_location: &Path, +) -> std::io::Result<()> { + let registry_path = marketplace_registry_path(codex_home); + let mut registry = if registry_path.is_file() { + read_marketplace_registry(®istry_path)? + } else { + KnownMarketplacesRegistry::default() + }; + + registry + .marketplaces + .retain(|marketplace| marketplace.name != marketplace_name); + registry.marketplaces.push(KnownMarketplaceRegistryEntry { + name: marketplace_name.to_string(), + install_location: install_location.to_path_buf(), + }); + registry + .marketplaces + .sort_unstable_by(|left, right| left.name.cmp(&right.name)); + write_marketplace_registry(®istry_path, ®istry) +} + +fn installed_marketplace_roots(codex_home: &Path) -> Vec { + let registry_path = marketplace_registry_path(codex_home); + if registry_path.is_file() { + return installed_marketplace_roots_from_registry(®istry_path); + } + + let install_root = marketplace_install_root(codex_home); + let Ok(entries) = fs::read_dir(&install_root) else { + return Vec::new(); + }; + + let mut roots = entries + .filter_map(Result::ok) + .filter_map(|entry| { + let path = entry.path(); + let file_type = entry.file_type().ok()?; + (file_type.is_dir() && path.join(".agents/plugins/marketplace.json").is_file()) + .then_some(path) + }) + .filter_map(|path| AbsolutePathBuf::try_from(path).ok()) + .collect::>(); + roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); + roots +} + +fn marketplace_registry_path(codex_home: &Path) -> PathBuf { + codex_home.join(".tmp").join(KNOWN_MARKETPLACES_FILE) +} + +fn installed_marketplace_roots_from_registry(registry_path: &Path) -> Vec { + let registry = match read_marketplace_registry(registry_path) { + Ok(registry) => registry, + Err(err) => { + warn!( + path = %registry_path.display(), + error = %err, + "failed to read installed marketplace registry" + ); + return Vec::new(); + } + }; + + let mut roots = registry + .marketplaces + .into_iter() + .filter_map(|marketplace| { + let path = marketplace.install_location; + if path.join(".agents/plugins/marketplace.json").is_file() { + AbsolutePathBuf::try_from(path).ok() + } else { + None + } + }) + .collect::>(); + roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); + roots +} + +fn read_marketplace_registry(path: &Path) -> std::io::Result { + let contents = fs::read_to_string(path)?; + serde_json::from_str(&contents).map_err(|err| { + std::io::Error::new( + std::io::ErrorKind::InvalidData, + format!( + "failed to parse marketplace registry {}: {err}", + path.display() + ), + ) + }) +} + +fn write_marketplace_registry( + path: &Path, + registry: &KnownMarketplacesRegistry, +) -> std::io::Result<()> { + if let Some(parent) = path.parent() { + fs::create_dir_all(parent)?; + } + let contents = serde_json::to_vec_pretty(registry).map_err(std::io::Error::other)?; + fs::write(path, contents) +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct KnownMarketplacesRegistry { + version: u32, + marketplaces: Vec, +} + +impl Default for KnownMarketplacesRegistry { + fn default() -> Self { + Self { + version: 1, + marketplaces: Vec::new(), + } + } +} + +#[derive(Debug, Deserialize, Serialize)] +#[serde(rename_all = "camelCase")] +struct KnownMarketplaceRegistryEntry { + name: String, + install_location: PathBuf, +} + #[derive(Debug, thiserror::Error)] pub enum PluginInstallError { #[error("{0}")] diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 2f1d3ae9c63d..2a27e41ca713 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -1504,6 +1504,70 @@ plugins = true ); } +#[tokio::test] +async fn list_marketplaces_includes_installed_marketplace_roots() { + let tmp = tempfile::tempdir().unwrap(); + let marketplace_root = marketplace_install_root(tmp.path()).join("debug"); + let plugin_root = marketplace_root.join("plugins/sample"); + + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + fs::create_dir_all(marketplace_root.join(".agents/plugins")).unwrap(); + fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); + fs::write( + marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./plugins/sample" + } + } + ] +}"#, + ) + .unwrap(); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .unwrap(); + record_installed_marketplace_root(tmp.path(), "debug", &marketplace_root) + .expect("record installed marketplace"); + + let config = load_config(tmp.path(), tmp.path()).await; + let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + .list_marketplaces_for_config(&config, &[]) + .unwrap() + .marketplaces; + + let marketplace = marketplaces + .into_iter() + .find(|marketplace| marketplace.name == "debug") + .expect("installed marketplace should be listed"); + + assert_eq!( + marketplace.path, + AbsolutePathBuf::try_from(marketplace_root.join(".agents/plugins/marketplace.json")) + .unwrap() + ); + assert_eq!(marketplace.plugins.len(), 1); + assert_eq!(marketplace.plugins[0].id, "sample@debug"); + assert_eq!( + marketplace.plugins[0].source, + MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(plugin_root).unwrap(), + } + ); +} + #[tokio::test] async fn list_marketplaces_uses_first_duplicate_plugin_entry() { let tmp = tempfile::tempdir().unwrap(); diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core/src/plugins/marketplace.rs index ee775262aae9..a5ca96e04150 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core/src/plugins/marketplace.rs @@ -211,6 +211,17 @@ pub fn list_marketplaces( list_marketplaces_with_home(additional_roots, home_dir().as_deref()) } +pub fn validate_marketplace_root(root: &Path) -> Result { + let path = AbsolutePathBuf::try_from(root.join(MARKETPLACE_RELATIVE_PATH)).map_err(|err| { + MarketplaceError::InvalidMarketplaceFile { + path: root.join(MARKETPLACE_RELATIVE_PATH), + message: format!("marketplace path must resolve to an absolute path: {err}"), + } + })?; + let marketplace = load_marketplace(&path)?; + Ok(marketplace.name) +} + pub(crate) fn load_marketplace(path: &AbsolutePathBuf) -> Result { let marketplace = load_raw_marketplace_manifest(path)?; let mut plugins = Vec::new(); diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index a86502c28cb0..29c6674ca293 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -29,6 +29,7 @@ pub(crate) use injection::build_plugin_injections; pub use manager::ConfiguredMarketplace; pub use manager::ConfiguredMarketplaceListOutcome; pub use manager::ConfiguredMarketplacePlugin; +pub use manager::INSTALLED_MARKETPLACES_DIR; pub use manager::OPENAI_CURATED_MARKETPLACE_NAME; pub use manager::PluginDetail; pub use manager::PluginInstallError; @@ -43,7 +44,9 @@ pub use manager::RemotePluginSyncResult; pub use manager::installed_plugin_telemetry_metadata; pub use manager::load_plugin_apps; pub use manager::load_plugin_mcp_servers; +pub use manager::marketplace_install_root; pub use manager::plugin_telemetry_metadata_from_root; +pub use manager::record_installed_marketplace_root; pub use manifest::PluginManifestInterface; pub(crate) use manifest::PluginManifestPaths; pub(crate) use manifest::load_plugin_manifest; @@ -53,6 +56,7 @@ pub use marketplace::MarketplacePluginAuthPolicy; pub use marketplace::MarketplacePluginInstallPolicy; pub use marketplace::MarketplacePluginPolicy; pub use marketplace::MarketplacePluginSource; +pub use marketplace::validate_marketplace_root; pub use remote::RemotePluginFetchError; pub use remote::fetch_remote_featured_plugin_ids; pub(crate) use render::render_explicit_plugin_instructions; From adbc32b094ea1a6b7721fb0157f09490f5438afa Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 8 Apr 2026 21:41:54 -0700 Subject: [PATCH 02/17] Address marketplace add review feedback --- codex-rs/cli/src/marketplace_cmd.rs | 57 +++++++++++++--------- codex-rs/core/src/plugins/manager.rs | 29 +++++------ codex-rs/core/src/plugins/manager_tests.rs | 52 ++++++++++++++++++++ 3 files changed, 101 insertions(+), 37 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index d68ed7b22ae8..1c788632d0f8 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -448,31 +448,14 @@ pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) - if let Some(parent) = destination.parent() { fs::create_dir_all(parent)?; } - - let backup = if destination.exists() { - let parent = destination - .parent() - .context("marketplace destination has no parent")?; - let staging_root = marketplace_staging_root(parent); - fs::create_dir_all(&staging_root)?; - let backup = tempfile::Builder::new() - .prefix("marketplace-backup-") - .tempdir_in(&staging_root)?; - let backup_root = backup.path().join("previous"); - fs::rename(destination, &backup_root)?; - Some((backup, backup_root)) - } else { - None - }; - - if let Err(err) = fs::rename(staged_root, destination) { - if let Some((_, backup_root)) = backup { - let _ = fs::rename(backup_root, destination); - } - return Err(err.into()); + if destination.exists() { + bail!( + "marketplace destination already exists: {}", + destination.display() + ); } - Ok(()) + fs::rename(staged_root, destination).map_err(Into::into) } pub(super) fn marketplace_staging_root(install_root: &Path) -> PathBuf { @@ -552,6 +535,7 @@ impl MarketplaceSource { mod tests { use super::*; use pretty_assertions::assert_eq; + use tempfile::TempDir; #[test] fn github_shorthand_parses_ref_suffix() { @@ -624,4 +608,31 @@ mod tests { } ); } + + #[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" + ); + } } diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index db48f105df68..de00c060614a 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -1270,7 +1270,16 @@ pub fn record_installed_marketplace_root( fn installed_marketplace_roots(codex_home: &Path) -> Vec { let registry_path = marketplace_registry_path(codex_home); if registry_path.is_file() { - return installed_marketplace_roots_from_registry(®istry_path); + match installed_marketplace_roots_from_registry(®istry_path) { + Ok(roots) => return roots, + Err(err) => { + warn!( + path = %registry_path.display(), + error = %err, + "failed to read installed marketplace registry; falling back to installed marketplace directory scan" + ); + } + } } let install_root = marketplace_install_root(codex_home); @@ -1296,18 +1305,10 @@ fn marketplace_registry_path(codex_home: &Path) -> PathBuf { codex_home.join(".tmp").join(KNOWN_MARKETPLACES_FILE) } -fn installed_marketplace_roots_from_registry(registry_path: &Path) -> Vec { - let registry = match read_marketplace_registry(registry_path) { - Ok(registry) => registry, - Err(err) => { - warn!( - path = %registry_path.display(), - error = %err, - "failed to read installed marketplace registry" - ); - return Vec::new(); - } - }; +fn installed_marketplace_roots_from_registry( + registry_path: &Path, +) -> std::io::Result> { + let registry = read_marketplace_registry(registry_path)?; let mut roots = registry .marketplaces @@ -1322,7 +1323,7 @@ fn installed_marketplace_roots_from_registry(registry_path: &Path) -> Vec>(); roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); - roots + Ok(roots) } fn read_marketplace_registry(path: &Path) -> std::io::Result { diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 2a27e41ca713..13c3bf1c7587 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -1568,6 +1568,58 @@ plugins = true ); } +#[tokio::test] +async fn list_marketplaces_scans_installed_roots_when_registry_is_malformed() { + let tmp = tempfile::tempdir().unwrap(); + let marketplace_root = marketplace_install_root(tmp.path()).join("debug"); + let plugin_root = marketplace_root.join("plugins/sample"); + let registry_path = tmp.path().join(".tmp/known_marketplaces.json"); + + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + fs::create_dir_all(marketplace_root.join(".agents/plugins")).unwrap(); + fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); + fs::write( + marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./plugins/sample" + } + } + ] +}"#, + ) + .unwrap(); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .unwrap(); + fs::write(registry_path, "{not valid json").unwrap(); + + let config = load_config(tmp.path(), tmp.path()).await; + let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + .list_marketplaces_for_config(&config, &[]) + .unwrap() + .marketplaces; + + let marketplace = marketplaces + .into_iter() + .find(|marketplace| marketplace.name == "debug") + .expect("installed marketplace should be discovered by disk scan fallback"); + + assert_eq!(marketplace.plugins[0].id, "sample@debug"); +} + #[tokio::test] async fn list_marketplaces_uses_first_duplicate_plugin_entry() { let tmp = tempfile::tempdir().unwrap(); From 23f845bd2c229d9be34040facb979604a83a74b8 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 8 Apr 2026 21:49:58 -0700 Subject: [PATCH 03/17] Fix marketplace add sparse argument parsing --- codex-rs/cli/src/marketplace_cmd.rs | 38 +++++++++++++++++++++++++++-- 1 file changed, 36 insertions(+), 2 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 1c788632d0f8..3be02effd308 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -39,8 +39,12 @@ struct AddMarketplaceArgs { #[arg(long = "ref", value_name = "REF")] ref_name: Option, - /// Sparse-checkout paths to use while cloning git sources. - #[arg(long = "sparse", value_name = "PATH", num_args = 1..)] + /// Sparse-checkout path to use while cloning git sources. Repeat to include multiple paths. + #[arg( + long = "sparse", + value_name = "PATH", + action = clap::ArgAction::Append + )] sparse_paths: Vec, } @@ -609,6 +613,36 @@ mod tests { ); } + #[test] + fn sparse_paths_parse_before_or_after_source() { + let sparse_before_source = + AddMarketplaceArgs::try_parse_from(["add", "--sparse", "plugins/foo", "owner/repo"]) + .unwrap(); + assert_eq!(sparse_before_source.source, "owner/repo"); + assert_eq!(sparse_before_source.sparse_paths, vec!["plugins/foo"]); + + let sparse_after_source = + AddMarketplaceArgs::try_parse_from(["add", "owner/repo", "--sparse", "plugins/foo"]) + .unwrap(); + assert_eq!(sparse_after_source.source, "owner/repo"); + assert_eq!(sparse_after_source.sparse_paths, vec!["plugins/foo"]); + + let repeated_sparse = AddMarketplaceArgs::try_parse_from([ + "add", + "--sparse", + "plugins/foo", + "--sparse", + "skills/bar", + "owner/repo", + ]) + .unwrap(); + assert_eq!(repeated_sparse.source, "owner/repo"); + assert_eq!( + repeated_sparse.sparse_paths, + vec!["plugins/foo", "skills/bar"] + ); + } + #[test] fn replace_marketplace_root_rejects_existing_destination() { let temp_dir = TempDir::new().unwrap(); From bd32fb42dea3bd4da1aee2f3f52cf746990a0479 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 8 Apr 2026 21:51:25 -0700 Subject: [PATCH 04/17] Test marketplace add sparse CLI parsing --- codex-rs/cli/tests/marketplace_add.rs | 36 +++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index 774f14b7dd5f..bd5e66c8440a 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -127,3 +127,39 @@ async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> Ok(()) } + +#[tokio::test] +async fn marketplace_add_sparse_flag_parses_before_and_after_source() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + let source = source.path().to_str().unwrap(); + let sparse_requires_git = "--sparse can only be used with git marketplace sources"; + + codex_command(codex_home.path())? + .args(["marketplace", "add", "--sparse", "plugins/foo", source]) + .assert() + .failure() + .stderr(contains(sparse_requires_git)); + + codex_command(codex_home.path())? + .args(["marketplace", "add", source, "--sparse", "plugins/foo"]) + .assert() + .failure() + .stderr(contains(sparse_requires_git)); + + codex_command(codex_home.path())? + .args([ + "marketplace", + "add", + "--sparse", + "plugins/foo", + "--sparse", + "skills/bar", + source, + ]) + .assert() + .failure() + .stderr(contains(sparse_requires_git)); + + Ok(()) +} From 972d3a5495cfbd8237bb76de542b84dcc5c4397a Mon Sep 17 00:00:00 2001 From: xli-oai Date: Wed, 8 Apr 2026 21:55:13 -0700 Subject: [PATCH 05/17] Reject invalid marketplace names on add --- codex-rs/cli/src/marketplace_cmd.rs | 9 ++++- codex-rs/cli/tests/marketplace_add.rs | 53 +++++++++++++++++++++++---- codex-rs/core/src/plugins/mod.rs | 1 + 3 files changed, 55 insertions(+), 8 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 3be02effd308..7bae6d6fabb2 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -7,6 +7,7 @@ use codex_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core::plugins::marketplace_install_root; use codex_core::plugins::record_installed_marketplace_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; @@ -154,7 +155,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { } } - let marketplace_name = validate_marketplace_root(&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!( @@ -185,6 +186,12 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { 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, diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index bd5e66c8440a..975825fa2cc5 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -13,22 +13,32 @@ fn codex_command(codex_home: &Path) -> Result { } fn write_marketplace_source(source: &Path, marker: &str) -> Result<()> { + write_marketplace_source_with_name(source, "debug", marker) +} + +fn write_marketplace_source_with_name( + 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"), - r#"{ - "name": "debug", + format!( + r#"{{ + "name": "{marketplace_name}", "plugins": [ - { + {{ "name": "sample", - "source": { + "source": {{ "source": "local", "path": "./plugins/sample" - } - } + }} + }} ] -}"#, +}}"# + ), )?; std::fs::write( source.join("plugins/sample/.codex-plugin/plugin.json"), @@ -65,6 +75,35 @@ async fn marketplace_add_local_directory_installs_valid_marketplace_root() -> Re Ok(()) } +#[tokio::test] +async fn marketplace_add_rejects_invalid_marketplace_name() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source_with_name(source.path(), "debug.market", "invalid marketplace")?; + + codex_command(codex_home.path())? + .args(["marketplace", "add", source.path().to_str().unwrap()]) + .assert() + .failure() + .stderr(contains( + "invalid marketplace name: only ASCII letters, digits, `_`, and `-` are allowed", + )); + + assert!( + !marketplace_install_root(codex_home.path()) + .join("debug.market") + .exists() + ); + assert!( + !codex_home + .path() + .join(".tmp/known_marketplaces.json") + .exists() + ); + + Ok(()) +} + #[tokio::test] async fn marketplace_add_same_source_is_idempotent() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 29c6674ca293..04837b85dc2b 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -20,6 +20,7 @@ pub use codex_plugin::PluginCapabilitySummary; pub use codex_plugin::PluginId; pub use codex_plugin::PluginIdError; pub use codex_plugin::PluginTelemetryMetadata; +pub use codex_plugin::validate_plugin_segment; pub type LoadedPlugin = codex_plugin::LoadedPlugin; pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; From add49f91b1e645378fb4124851c6508ada01c962 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 11:17:26 -0700 Subject: [PATCH 06/17] Store marketplace metadata in config --- codex-rs/cli/src/marketplace_cmd.rs | 134 +++++++----- codex-rs/cli/src/marketplace_cmd/metadata.rs | 218 +++++++------------ codex-rs/cli/tests/marketplace_add.rs | 59 ++++- codex-rs/config/src/config_toml.rs | 5 + codex-rs/config/src/lib.rs | 3 + codex-rs/config/src/marketplace_edit.rs | 83 +++++++ codex-rs/config/src/types.rs | 27 +++ codex-rs/core/config.schema.json | 53 +++++ codex-rs/core/src/plugins/manager.rs | 164 ++++---------- codex-rs/core/src/plugins/manager_tests.rs | 62 +++++- codex-rs/core/src/plugins/mod.rs | 1 - 11 files changed, 478 insertions(+), 331 deletions(-) create mode 100644 codex-rs/config/src/marketplace_edit.rs diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 7bae6d6fabb2..ed8a0f6b7702 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -2,10 +2,11 @@ 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::find_codex_home; use codex_core::plugins::OPENAI_CURATED_MARKETPLACE_NAME; use codex_core::plugins::marketplace_install_root; -use codex_core::plugins::record_installed_marketplace_root; use codex_core::plugins::validate_marketplace_root; use codex_core::plugins::validate_plugin_segment; use codex_utils_cli::CliConfigOverrides; @@ -13,6 +14,8 @@ use std::fs; use std::path::Path; use std::path::PathBuf; use std::process::Command; +use std::time::SystemTime; +use std::time::UNIX_EPOCH; mod metadata; @@ -53,12 +56,10 @@ struct AddMarketplaceArgs { pub(super) enum MarketplaceSource { LocalDirectory { path: PathBuf, - source_id: String, }, Git { url: String, ref_name: Option, - source_id: String, }, } @@ -105,15 +106,18 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { })?; let install_metadata = metadata::MarketplaceInstallMetadata::from_source(&source, &sparse_paths); - if let Some(existing_root) = - metadata::installed_marketplace_root_for_source(&install_root, &install_metadata.source_id)? - { + 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 `{marketplace_name}` is already added from {}.", source.display() @@ -141,7 +145,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { let staged_root = staged_dir.path().to_path_buf(); match &source { - MarketplaceSource::LocalDirectory { path, .. } => { + MarketplaceSource::LocalDirectory { path } => { copy_dir_recursive(path, &staged_root).with_context(|| { format!( "failed to copy marketplace source {} into {}", @@ -150,7 +154,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { ) })?; } - MarketplaceSource::Git { url, ref_name, .. } => { + MarketplaceSource::Git { url, ref_name } => { clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; } } @@ -163,7 +167,6 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { source.display() ); } - metadata::write_marketplace_source_metadata(&staged_root, &install_metadata)?; let destination = install_root.join(safe_marketplace_dir_name(&marketplace_name)?); ensure_marketplace_destination_is_inside_install_root(&install_root, &destination)?; if destination.exists() { @@ -174,8 +177,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { } replace_marketplace_root(&staged_root, &destination) .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; - record_installed_marketplace_root(&codex_home, &marketplace_name, &destination) - .with_context(|| format!("failed to record marketplace `{marketplace_name}`"))?; + record_added_marketplace(&codex_home, &marketplace_name, &install_metadata)?; println!( "Added marketplace `{marketplace_name}` from {}.", @@ -186,6 +188,26 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { 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, + 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 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)?; @@ -243,10 +265,7 @@ fn parse_marketplace_source( let path = path .canonicalize() .with_context(|| format!("failed to resolve {}", path.display()))?; - return Ok(MarketplaceSource::LocalDirectory { - source_id: format!("directory:{}", path.display()), - path, - }); + return Ok(MarketplaceSource::LocalDirectory { path }); } let (base_source, parsed_ref) = split_source_ref(&source); @@ -254,20 +273,12 @@ fn parse_marketplace_source( if is_ssh_git_url(&base_source) || is_http_git_url(&base_source) { let url = normalize_git_url(&base_source); - return Ok(MarketplaceSource::Git { - source_id: git_source_id("git", &url, ref_name.as_deref()), - url, - ref_name, - }); + 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 { - source_id: git_source_id("github", &base_source, ref_name.as_deref()), - url, - ref_name, - }); + return Ok(MarketplaceSource::Git { url, ref_name }); } if base_source.starts_with("http://") || base_source.starts_with("https://") { @@ -279,14 +290,6 @@ fn parse_marketplace_source( bail!("invalid marketplace source format: {source}"); } -fn git_source_id(kind: &str, source: &str, ref_name: Option<&str>) -> String { - if let Some(ref_name) = ref_name { - format!("{kind}:{source}#{ref_name}") - } else { - format!("{kind}:{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)); @@ -521,17 +524,44 @@ fn ensure_marketplace_destination_is_inside_install_root( Ok(()) } -impl MarketplaceSource { - fn source_id(&self) -> &str { - match self { - Self::LocalDirectory { source_id, .. } | Self::Git { source_id, .. } => source_id, - } - } +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::LocalDirectory { path, .. } => path.display().to_string(), - Self::Git { url, ref_name, .. } => { + Self::LocalDirectory { path } => path.display().to_string(), + Self::Git { url, ref_name } => { if let Some(ref_name) = ref_name { format!("{url}#{ref_name}") } else { @@ -555,7 +585,6 @@ mod tests { MarketplaceSource::Git { url: "https://github.com/owner/repo.git".to_string(), ref_name: Some("main".to_string()), - source_id: "github:owner/repo#main".to_string(), } ); } @@ -571,7 +600,6 @@ mod tests { MarketplaceSource::Git { url: "https://example.com/team/repo.git".to_string(), ref_name: Some("v1".to_string()), - source_id: "git:https://example.com/team/repo.git#v1".to_string(), } ); } @@ -587,13 +615,12 @@ mod tests { MarketplaceSource::Git { url: "https://github.com/owner/repo.git".to_string(), ref_name: Some("release".to_string()), - source_id: "github:owner/repo#release".to_string(), } ); } #[test] - fn github_shorthand_and_git_url_have_different_source_ids() { + 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", @@ -601,25 +628,22 @@ mod tests { ) .unwrap(); - assert_ne!(shorthand.source_id(), git_url.source_id()); + assert_eq!(shorthand, git_url); assert_eq!( shorthand, MarketplaceSource::Git { url: "https://github.com/owner/repo.git".to_string(), ref_name: None, - source_id: "github:owner/repo".to_string(), - } - ); - assert_eq!( - git_url, - MarketplaceSource::Git { - url: "https://github.com/owner/repo.git".to_string(), - ref_name: None, - source_id: "git:https://github.com/owner/repo.git".to_string(), } ); } + #[test] + fn utc_timestamp_formats_unix_epoch_as_rfc3339_utc() { + assert_eq!(format_utc_timestamp(0), "1970-01-01T00:00:00Z"); + assert_eq!(format_utc_timestamp(1_775_779_200), "2026-04-10T00:00:00Z"); + } + #[test] fn sparse_paths_parse_before_or_after_source() { let sparse_before_source = diff --git a/codex-rs/cli/src/marketplace_cmd/metadata.rs b/codex-rs/cli/src/marketplace_cmd/metadata.rs index 8b88602e2d9c..6a6b3c0c49ef 100644 --- a/codex-rs/cli/src/marketplace_cmd/metadata.rs +++ b/codex-rs/cli/src/marketplace_cmd/metadata.rs @@ -1,22 +1,18 @@ use super::MarketplaceSource; use anyhow::Context; use anyhow::Result; -use anyhow::bail; +use codex_config::CONFIG_TOML_FILE; use codex_core::plugins::validate_marketplace_root; -use std::fs; use std::path::Path; use std::path::PathBuf; -const MARKETPLACE_ADD_SOURCE_FILE: &str = ".codex-marketplace-source"; - #[derive(Debug, Clone, PartialEq, Eq)] pub(super) struct MarketplaceInstallMetadata { - pub(super) source_id: String, source: InstalledMarketplaceSource, } #[derive(Debug, Clone, PartialEq, Eq)] -pub(super) enum InstalledMarketplaceSource { +enum InstalledMarketplaceSource { LocalDirectory { path: PathBuf, }, @@ -28,164 +24,96 @@ pub(super) enum InstalledMarketplaceSource { } pub(super) fn installed_marketplace_root_for_source( + codex_home: &Path, install_root: &Path, - source_id: &str, + install_metadata: &MarketplaceInstallMetadata, ) -> Result> { - let entries = fs::read_dir(install_root).with_context(|| { - format!( - "failed to read marketplace install directory {}", - install_root.display() - ) - })?; - for entry in entries { - let entry = entry?; - if !entry.file_type()?.is_dir() { - continue; - } - let root = entry.path(); - let metadata_path = root.join(MARKETPLACE_ADD_SOURCE_FILE); - if !metadata_path.is_file() { + let config_path = codex_home.join(CONFIG_TOML_FILE); + let Ok(config) = std::fs::read_to_string(&config_path) else { + return Ok(None); + }; + let config: toml::Value = toml::from_str(&config) + .with_context(|| format!("failed to parse user config {}", config_path.display()))?; + let Some(marketplaces) = config.get("marketplaces").and_then(toml::Value::as_table) else { + return Ok(None); + }; + + for (marketplace_name, marketplace) in marketplaces { + if !install_metadata.matches_config(marketplace) { continue; } - let metadata = read_marketplace_source_metadata(&root)?; - if metadata.source_id == source_id && validate_marketplace_root(&root).is_ok() { + let root = install_root.join(marketplace_name); + if validate_marketplace_root(&root).is_ok() { return Ok(Some(root)); } } - Ok(None) -} -pub(super) fn write_marketplace_source_metadata( - root: &Path, - metadata: &MarketplaceInstallMetadata, -) -> Result<()> { - let source = match &metadata.source { - InstalledMarketplaceSource::LocalDirectory { path } => serde_json::json!({ - "kind": "directory", - "path": path, - }), - InstalledMarketplaceSource::Git { - url, - ref_name, - sparse_paths, - } => serde_json::json!({ - "kind": "git", - "url": url, - "ref": ref_name, - "sparsePaths": sparse_paths, - }), - }; - let content = serde_json::to_string_pretty(&serde_json::json!({ - "version": 1, - "sourceId": metadata.source_id, - "source": source, - }))?; - fs::write(root.join(MARKETPLACE_ADD_SOURCE_FILE), content).with_context(|| { - format!( - "failed to write marketplace source metadata in {}", - root.display() - ) - }) -} - -fn read_marketplace_source_metadata(root: &Path) -> Result { - let path = root.join(MARKETPLACE_ADD_SOURCE_FILE); - let content = fs::read_to_string(&path).with_context(|| { - format!( - "failed to read marketplace source metadata {}", - path.display() - ) - })?; - if !content.trim_start().starts_with('{') { - return Ok(MarketplaceInstallMetadata { - source_id: content.trim().to_string(), - source: InstalledMarketplaceSource::LocalDirectory { - path: root.to_path_buf(), - }, - }); - } - - let json: serde_json::Value = serde_json::from_str(&content).with_context(|| { - format!( - "failed to parse marketplace source metadata {}", - path.display() - ) - })?; - let source_id = json - .get("sourceId") - .and_then(serde_json::Value::as_str) - .context("marketplace source metadata is missing sourceId")? - .to_string(); - let source = json - .get("source") - .context("marketplace source metadata is missing source")?; - let kind = source - .get("kind") - .and_then(serde_json::Value::as_str) - .context("marketplace source metadata is missing source.kind")?; - let source = match kind { - "directory" => { - let path = source - .get("path") - .and_then(serde_json::Value::as_str) - .context("marketplace directory metadata is missing path")?; - InstalledMarketplaceSource::LocalDirectory { - path: PathBuf::from(path), - } - } - "git" => { - let url = source - .get("url") - .and_then(serde_json::Value::as_str) - .context("marketplace git metadata is missing url")? - .to_string(); - let ref_name = source - .get("ref") - .and_then(serde_json::Value::as_str) - .map(str::to_string); - let sparse_paths = source - .get("sparsePaths") - .and_then(serde_json::Value::as_array) - .map(|paths| { - paths - .iter() - .filter_map(serde_json::Value::as_str) - .map(str::to_string) - .collect::>() - }) - .unwrap_or_default(); - InstalledMarketplaceSource::Git { - url, - ref_name, - sparse_paths, - } - } - other => bail!("unsupported marketplace source metadata kind `{other}`"), - }; - Ok(MarketplaceInstallMetadata { source_id, source }) + Ok(None) } impl MarketplaceInstallMetadata { pub(super) fn from_source(source: &MarketplaceSource, sparse_paths: &[String]) -> Self { - let source_id = if sparse_paths.is_empty() { - source.source_id().to_string() - } else { - format!( - "{}?sparse={}", - source.source_id(), - serde_json::to_string(sparse_paths).unwrap_or_else(|_| "[]".to_string()) - ) - }; let source = match source { - MarketplaceSource::LocalDirectory { path, .. } => { + MarketplaceSource::LocalDirectory { path } => { InstalledMarketplaceSource::LocalDirectory { path: path.clone() } } - MarketplaceSource::Git { url, ref_name, .. } => InstalledMarketplaceSource::Git { + MarketplaceSource::Git { url, ref_name } => InstalledMarketplaceSource::Git { url: url.clone(), ref_name: ref_name.clone(), sparse_paths: sparse_paths.to_vec(), }, }; - Self { source_id, source } + Self { source } + } + + pub(super) fn config_source_type(&self) -> &'static str { + match &self.source { + InstalledMarketplaceSource::LocalDirectory { .. } => "directory", + InstalledMarketplaceSource::Git { .. } => "git", + } + } + + pub(super) fn config_source(&self) -> String { + match &self.source { + InstalledMarketplaceSource::LocalDirectory { path } => path.display().to_string(), + InstalledMarketplaceSource::Git { url, .. } => url.clone(), + } + } + + pub(super) fn ref_name(&self) -> Option<&str> { + match &self.source { + InstalledMarketplaceSource::LocalDirectory { .. } => None, + InstalledMarketplaceSource::Git { ref_name, .. } => ref_name.as_deref(), + } + } + + pub(super) fn sparse_paths(&self) -> &[String] { + match &self.source { + InstalledMarketplaceSource::LocalDirectory { .. } => &[], + InstalledMarketplaceSource::Git { sparse_paths, .. } => sparse_paths, + } } + + fn matches_config(&self, marketplace: &toml::Value) -> bool { + marketplace.get("source_type").and_then(toml::Value::as_str) + == Some(self.config_source_type()) + && marketplace.get("source").and_then(toml::Value::as_str) + == Some(self.config_source().as_str()) + && marketplace.get("ref").and_then(toml::Value::as_str) == self.ref_name() + && config_sparse_paths(marketplace) == self.sparse_paths() + } +} + +fn config_sparse_paths(marketplace: &toml::Value) -> Vec { + marketplace + .get("sparse_paths") + .and_then(toml::Value::as_array) + .map(|paths| { + paths + .iter() + .filter_map(toml::Value::as_str) + .map(str::to_string) + .collect() + }) + .unwrap_or_default() } diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index 975825fa2cc5..d09f6d836a1f 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -1,3 +1,4 @@ +use anyhow::Context; use anyhow::Result; use codex_core::plugins::marketplace_install_root; use codex_core::plugins::validate_marketplace_root; @@ -63,14 +64,19 @@ async fn marketplace_add_local_directory_installs_valid_marketplace_root() -> Re let installed_root = marketplace_install_root(codex_home.path()).join("debug"); assert_eq!(validate_marketplace_root(&installed_root)?, "debug"); - let registry = std::fs::read_to_string(codex_home.path().join(".tmp/known_marketplaces.json"))?; - assert!(registry.contains(r#""name": "debug""#)); - assert!(registry.contains(r#""installLocation""#)); + assert_marketplace_config(codex_home.path(), "debug", &source.path().canonicalize()?)?; assert!( installed_root .join("plugins/sample/.codex-plugin/plugin.json") .is_file() ); + assert!(!installed_root.join(".codex-marketplace-source").exists()); + assert!( + !codex_home + .path() + .join(".tmp/known_marketplaces.json") + .exists() + ); Ok(()) } @@ -132,6 +138,14 @@ async fn marketplace_add_same_source_is_idempotent() -> Result<()> { std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, "first install" ); + assert_marketplace_config(codex_home.path(), "debug", &source.path().canonicalize()?)?; + assert!(!installed_root.join(".codex-marketplace-source").exists()); + assert!( + !codex_home + .path() + .join(".tmp/known_marketplaces.json") + .exists() + ); Ok(()) } @@ -167,6 +181,45 @@ async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> Ok(()) } +fn assert_marketplace_config( + codex_home: &Path, + marketplace_name: &str, + source: &Path, +) -> Result<()> { + let config = std::fs::read_to_string(codex_home.join("config.toml"))?; + let config: toml::Value = toml::from_str(&config)?; + let marketplace = config + .get("marketplaces") + .and_then(|marketplaces| marketplaces.get(marketplace_name)) + .context("marketplace config should be written")?; + let expected_source = source.to_string_lossy().to_string(); + + assert!( + marketplace + .get("last_updated") + .and_then(toml::Value::as_str) + .is_some_and(|last_updated| { + last_updated.len() == "2026-04-10T12:34:56Z".len() && last_updated.ends_with('Z') + }), + "last_updated should be an RFC3339-like UTC timestamp" + ); + assert_eq!( + marketplace.get("source_type").and_then(toml::Value::as_str), + Some("directory") + ); + assert_eq!( + marketplace.get("source").and_then(toml::Value::as_str), + Some(expected_source.as_str()) + ); + assert_eq!(marketplace.get("ref").and_then(toml::Value::as_str), None); + assert!(marketplace.get("sparse_paths").is_none()); + assert!(marketplace.get("source_id").is_none()); + assert!(marketplace.get("install_root").is_none()); + assert!(marketplace.get("install_location").is_none()); + + Ok(()) +} + #[tokio::test] async fn marketplace_add_sparse_flag_parses_before_and_after_source() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/config/src/config_toml.rs b/codex-rs/config/src/config_toml.rs index caf9d25b8804..7261180e6ee1 100644 --- a/codex-rs/config/src/config_toml.rs +++ b/codex-rs/config/src/config_toml.rs @@ -12,6 +12,7 @@ use crate::types::AppsConfigToml; use crate::types::AuthCredentialsStoreMode; use crate::types::FeedbackConfigToml; use crate::types::History; +use crate::types::MarketplaceConfig; use crate::types::McpServerConfig; use crate::types::MemoriesToml; use crate::types::Notice; @@ -325,6 +326,10 @@ pub struct ConfigToml { #[serde(default)] pub plugins: HashMap, + /// User-level marketplace entries keyed by marketplace name. + #[serde(default)] + pub marketplaces: HashMap, + /// Centralized feature flags (new). Prefer this over individual toggles. #[serde(default)] // Injects known feature keys into the schema and forbids unknown keys. diff --git a/codex-rs/config/src/lib.rs b/codex-rs/config/src/lib.rs index 5b8e5e687888..1b178fcdaef1 100644 --- a/codex-rs/config/src/lib.rs +++ b/codex-rs/config/src/lib.rs @@ -4,6 +4,7 @@ pub mod config_toml; mod constraint; mod diagnostics; mod fingerprint; +mod marketplace_edit; mod mcp_edit; mod mcp_types; mod merge; @@ -57,6 +58,8 @@ pub use diagnostics::format_config_error; 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::record_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 new file mode 100644 index 000000000000..33cdd8e16307 --- /dev/null +++ b/codex-rs/config/src/marketplace_edit.rs @@ -0,0 +1,83 @@ +use std::fs; +use std::io::ErrorKind; +use std::path::Path; + +use toml_edit::DocumentMut; +use toml_edit::Item as TomlItem; +use toml_edit::Table as TomlTable; +use toml_edit::Value as TomlValue; +use toml_edit::value; + +use crate::CONFIG_TOML_FILE; + +pub struct MarketplaceConfigUpdate<'a> { + pub last_updated: &'a str, + pub source_type: &'a str, + pub source: &'a str, + pub ref_name: Option<&'a str>, + pub sparse_paths: &'a [String], +} + +pub fn record_user_marketplace( + codex_home: &Path, + marketplace_name: &str, + update: &MarketplaceConfigUpdate<'_>, +) -> std::io::Result<()> { + let config_path = codex_home.join(CONFIG_TOML_FILE); + let mut doc = read_or_create_document(&config_path)?; + upsert_marketplace(&mut doc, marketplace_name, update); + fs::create_dir_all(codex_home)?; + fs::write(config_path, doc.to_string()) +} + +fn read_or_create_document(config_path: &Path) -> std::io::Result { + 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 => Ok(DocumentMut::new()), + Err(err) => Err(err), + } +} + +fn upsert_marketplace( + doc: &mut DocumentMut, + marketplace_name: &str, + update: &MarketplaceConfigUpdate<'_>, +) { + let root = doc.as_table_mut(); + if !root.contains_key("marketplaces") { + root.insert("marketplaces", TomlItem::Table(new_implicit_table())); + } + + let Some(marketplaces_item) = root.get_mut("marketplaces") else { + return; + }; + if !marketplaces_item.is_table() { + *marketplaces_item = TomlItem::Table(new_implicit_table()); + } + + let Some(marketplaces) = marketplaces_item.as_table_mut() else { + return; + }; + let mut entry = TomlTable::new(); + entry.set_implicit(false); + entry["last_updated"] = value(update.last_updated.to_string()); + entry["source_type"] = value(update.source_type.to_string()); + entry["source"] = value(update.source.to_string()); + if let Some(ref_name) = update.ref_name { + entry["ref"] = value(ref_name.to_string()); + } + if !update.sparse_paths.is_empty() { + entry["sparse_paths"] = TomlItem::Value(TomlValue::Array( + update.sparse_paths.iter().map(String::as_str).collect(), + )); + } + marketplaces.insert(marketplace_name, TomlItem::Table(entry)); +} + +fn new_implicit_table() -> TomlTable { + let mut table = TomlTable::new(); + table.set_implicit(true); + table +} diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index f26d336018f6..8ff0c74222d7 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -578,6 +578,33 @@ pub struct PluginConfig { pub enabled: bool, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, Default, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct MarketplaceConfig { + /// Last time Codex successfully added or refreshed this marketplace. + #[serde(default)] + pub last_updated: Option, + /// Source kind used to install this marketplace. + #[serde(default)] + pub source_type: Option, + /// Source location used when the marketplace was added. + #[serde(default)] + pub source: Option, + /// Git ref to check out when `source_type` is `git`. + #[serde(default, rename = "ref")] + pub ref_name: Option, + /// Sparse checkout paths used when `source_type` is `git`. + #[serde(default)] + pub sparse_paths: Option>, +} + +#[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema)] +#[serde(rename_all = "snake_case")] +pub enum MarketplaceSourceType { + Directory, + Git, +} + #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct SandboxWorkspaceWrite { diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 8da02dc803f0..3dbbc5f9dd97 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -754,6 +754,51 @@ } ] }, + "MarketplaceConfig": { + "additionalProperties": false, + "properties": { + "last_updated": { + "default": null, + "description": "Last time Codex successfully added or refreshed this marketplace.", + "type": "string" + }, + "ref": { + "default": null, + "description": "Git ref to check out when `source_type` is `git`.", + "type": "string" + }, + "source": { + "default": null, + "description": "Source location used when the marketplace was added.", + "type": "string" + }, + "source_type": { + "allOf": [ + { + "$ref": "#/definitions/MarketplaceSourceType" + } + ], + "default": null, + "description": "Source kind used to install this marketplace." + }, + "sparse_paths": { + "default": null, + "description": "Sparse checkout paths used when `source_type` is `git`.", + "items": { + "type": "string" + }, + "type": "array" + } + }, + "type": "object" + }, + "MarketplaceSourceType": { + "enum": [ + "directory", + "git" + ], + "type": "string" + }, "McpServerToolConfig": { "additionalProperties": false, "description": "Per-tool approval settings for a single MCP server tool.", @@ -2322,6 +2367,14 @@ ], "description": "Directory where Codex writes log files, for example `codex-tui.log`. Defaults to `$CODEX_HOME/log`." }, + "marketplaces": { + "additionalProperties": { + "$ref": "#/definitions/MarketplaceConfig" + }, + "default": {}, + "description": "User-level marketplace entries keyed by marketplace name.", + "type": "object" + }, "mcp_oauth_callback_port": { "description": "Optional fixed port for the local HTTP callback server used during MCP OAuth login. When unset, Codex will bind to an ephemeral port chosen by the OS.", "format": "uint16", diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index de00c060614a..27688ce368ae 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -54,11 +54,11 @@ use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_plugin::PluginTelemetryMetadata; use codex_plugin::prompt_safe_plugin_description; +use codex_plugin::validate_plugin_segment; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; -use serde::Serialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; use serde_json::json; @@ -81,7 +81,6 @@ const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; -const KNOWN_MARKETPLACES_FILE: &str = "known_marketplaces.json"; pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; pub const OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME: &str = "OpenAI Curated"; static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false); @@ -877,7 +876,8 @@ impl PluginsManager { } let (installed_plugins, enabled_plugins) = self.configured_plugin_states(config); - let marketplace_outcome = list_marketplaces(&self.marketplace_roots(additional_roots))?; + let marketplace_outcome = + list_marketplaces(&self.marketplace_roots(config, additional_roots))?; let mut seen_plugin_keys = HashSet::new(); let marketplaces = marketplace_outcome .marketplaces @@ -1221,11 +1221,18 @@ impl PluginsManager { (installed_plugins, enabled_plugins) } - fn marketplace_roots(&self, additional_roots: &[AbsolutePathBuf]) -> Vec { + fn marketplace_roots( + &self, + config: &Config, + additional_roots: &[AbsolutePathBuf], + ) -> Vec { // Treat the curated catalog as an extra marketplace root so plugin listing can surface it // without requiring every caller to know where it is stored. let mut roots = additional_roots.to_vec(); - roots.extend(installed_marketplace_roots(self.codex_home.as_path())); + roots.extend(installed_marketplace_roots_from_config( + config, + self.codex_home.as_path(), + )); let curated_repo_root = curated_plugins_repo_path(self.codex_home.as_path()); if curated_repo_root.is_dir() && let Ok(curated_repo_root) = AbsolutePathBuf::try_from(curated_repo_root) @@ -1242,57 +1249,42 @@ pub fn marketplace_install_root(codex_home: &Path) -> PathBuf { codex_home.join(INSTALLED_MARKETPLACES_DIR) } -pub fn record_installed_marketplace_root( +fn installed_marketplace_roots_from_config( + config: &Config, codex_home: &Path, - marketplace_name: &str, - install_location: &Path, -) -> std::io::Result<()> { - let registry_path = marketplace_registry_path(codex_home); - let mut registry = if registry_path.is_file() { - read_marketplace_registry(®istry_path)? - } else { - KnownMarketplacesRegistry::default() +) -> Vec { + let Some(user_layer) = config.config_layer_stack.get_user_layer() else { + return Vec::new(); }; - - registry - .marketplaces - .retain(|marketplace| marketplace.name != marketplace_name); - registry.marketplaces.push(KnownMarketplaceRegistryEntry { - name: marketplace_name.to_string(), - install_location: install_location.to_path_buf(), - }); - registry - .marketplaces - .sort_unstable_by(|left, right| left.name.cmp(&right.name)); - write_marketplace_registry(®istry_path, ®istry) -} - -fn installed_marketplace_roots(codex_home: &Path) -> Vec { - let registry_path = marketplace_registry_path(codex_home); - if registry_path.is_file() { - match installed_marketplace_roots_from_registry(®istry_path) { - Ok(roots) => return roots, - Err(err) => { + let Some(marketplaces_value) = user_layer.config.get("marketplaces") else { + return Vec::new(); + }; + let Some(marketplaces) = marketplaces_value.as_table() else { + warn!("invalid marketplaces config: expected table"); + return Vec::new(); + }; + let default_install_root = marketplace_install_root(codex_home); + let mut roots = marketplaces + .iter() + .filter_map(|(marketplace_name, marketplace)| { + if !marketplace.is_table() { warn!( - path = %registry_path.display(), + marketplace_name, + "ignoring invalid configured marketplace entry" + ); + return None; + } + if let Err(err) = validate_plugin_segment(marketplace_name, "marketplace name") { + warn!( + marketplace_name, error = %err, - "failed to read installed marketplace registry; falling back to installed marketplace directory scan" + "ignoring invalid configured marketplace name" ); + return None; } - } - } - - let install_root = marketplace_install_root(codex_home); - let Ok(entries) = fs::read_dir(&install_root) else { - return Vec::new(); - }; - - let mut roots = entries - .filter_map(Result::ok) - .filter_map(|entry| { - let path = entry.path(); - let file_type = entry.file_type().ok()?; - (file_type.is_dir() && path.join(".agents/plugins/marketplace.json").is_file()) + let path = default_install_root.join(marketplace_name); + path.join(".agents/plugins/marketplace.json") + .is_file() .then_some(path) }) .filter_map(|path| AbsolutePathBuf::try_from(path).ok()) @@ -1301,78 +1293,6 @@ fn installed_marketplace_roots(codex_home: &Path) -> Vec { roots } -fn marketplace_registry_path(codex_home: &Path) -> PathBuf { - codex_home.join(".tmp").join(KNOWN_MARKETPLACES_FILE) -} - -fn installed_marketplace_roots_from_registry( - registry_path: &Path, -) -> std::io::Result> { - let registry = read_marketplace_registry(registry_path)?; - - let mut roots = registry - .marketplaces - .into_iter() - .filter_map(|marketplace| { - let path = marketplace.install_location; - if path.join(".agents/plugins/marketplace.json").is_file() { - AbsolutePathBuf::try_from(path).ok() - } else { - None - } - }) - .collect::>(); - roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); - Ok(roots) -} - -fn read_marketplace_registry(path: &Path) -> std::io::Result { - let contents = fs::read_to_string(path)?; - serde_json::from_str(&contents).map_err(|err| { - std::io::Error::new( - std::io::ErrorKind::InvalidData, - format!( - "failed to parse marketplace registry {}: {err}", - path.display() - ), - ) - }) -} - -fn write_marketplace_registry( - path: &Path, - registry: &KnownMarketplacesRegistry, -) -> std::io::Result<()> { - if let Some(parent) = path.parent() { - fs::create_dir_all(parent)?; - } - let contents = serde_json::to_vec_pretty(registry).map_err(std::io::Error::other)?; - fs::write(path, contents) -} - -#[derive(Debug, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -struct KnownMarketplacesRegistry { - version: u32, - marketplaces: Vec, -} - -impl Default for KnownMarketplacesRegistry { - fn default() -> Self { - Self { - version: 1, - marketplaces: Vec::new(), - } - } -} - -#[derive(Debug, Deserialize, Serialize)] -#[serde(rename_all = "camelCase")] -struct KnownMarketplaceRegistryEntry { - name: String, - install_location: PathBuf, -} - #[derive(Debug, thiserror::Error)] pub enum PluginInstallError { #[error("{0}")] diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 13c3bf1c7587..b5dc40716ee8 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -1514,6 +1514,11 @@ async fn list_marketplaces_includes_installed_marketplace_roots() { &tmp.path().join(CONFIG_TOML_FILE), r#"[features] plugins = true + +[marketplaces.debug] +last_updated = "2026-04-10T12:34:56Z" +source_type = "directory" +source = "/tmp/debug" "#, ); fs::create_dir_all(marketplace_root.join(".agents/plugins")).unwrap(); @@ -1539,9 +1544,6 @@ plugins = true r#"{"name":"sample"}"#, ) .unwrap(); - record_installed_marketplace_root(tmp.path(), "debug", &marketplace_root) - .expect("record installed marketplace"); - let config = load_config(tmp.path(), tmp.path()).await; let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) .list_marketplaces_for_config(&config, &[]) @@ -1569,7 +1571,7 @@ plugins = true } #[tokio::test] -async fn list_marketplaces_scans_installed_roots_when_registry_is_malformed() { +async fn list_marketplaces_uses_config_when_known_registry_is_malformed() { let tmp = tempfile::tempdir().unwrap(); let marketplace_root = marketplace_install_root(tmp.path()).join("debug"); let plugin_root = marketplace_root.join("plugins/sample"); @@ -1579,6 +1581,11 @@ async fn list_marketplaces_scans_installed_roots_when_registry_is_malformed() { &tmp.path().join(CONFIG_TOML_FILE), r#"[features] plugins = true + +[marketplaces.debug] +last_updated = "2026-04-10T12:34:56Z" +source_type = "directory" +source = "/tmp/debug" "#, ); fs::create_dir_all(marketplace_root.join(".agents/plugins")).unwrap(); @@ -1604,6 +1611,7 @@ plugins = true r#"{"name":"sample"}"#, ) .unwrap(); + fs::create_dir_all(registry_path.parent().unwrap()).unwrap(); fs::write(registry_path, "{not valid json").unwrap(); let config = load_config(tmp.path(), tmp.path()).await; @@ -1615,11 +1623,55 @@ plugins = true let marketplace = marketplaces .into_iter() .find(|marketplace| marketplace.name == "debug") - .expect("installed marketplace should be discovered by disk scan fallback"); + .expect("configured marketplace should be discovered"); assert_eq!(marketplace.plugins[0].id, "sample@debug"); } +#[tokio::test] +async fn list_marketplaces_ignores_installed_roots_missing_from_config() { + let tmp = tempfile::tempdir().unwrap(); + let marketplace_root = marketplace_install_root(tmp.path()).join("debug"); + let plugin_root = marketplace_root.join("plugins/sample"); + + write_file( + &tmp.path().join(CONFIG_TOML_FILE), + r#"[features] +plugins = true +"#, + ); + fs::create_dir_all(marketplace_root.join(".agents/plugins")).unwrap(); + fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); + fs::write( + marketplace_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "debug", + "plugins": [ + { + "name": "sample", + "source": { + "source": "local", + "path": "./plugins/sample" + } + } + ] +}"#, + ) + .unwrap(); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .unwrap(); + let config = load_config(tmp.path(), tmp.path()).await; + let marketplaces = PluginsManager::new(tmp.path().to_path_buf()) + .list_marketplaces_for_config(&config, &[]) + .unwrap() + .marketplaces; + + assert!(marketplaces.is_empty()); +} + #[tokio::test] async fn list_marketplaces_uses_first_duplicate_plugin_entry() { let tmp = tempfile::tempdir().unwrap(); diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 04837b85dc2b..bf43d5b04388 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -47,7 +47,6 @@ pub use manager::load_plugin_apps; pub use manager::load_plugin_mcp_servers; pub use manager::marketplace_install_root; pub use manager::plugin_telemetry_metadata_from_root; -pub use manager::record_installed_marketplace_root; pub use manifest::PluginManifestInterface; pub(crate) use manifest::PluginManifestPaths; pub(crate) use manifest::load_plugin_manifest; From 16813bd29634ea928d5e475c6c0fa9b4e13bba7a Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 11:25:38 -0700 Subject: [PATCH 07/17] Split marketplace CLI operations --- codex-rs/cli/src/marketplace_cmd.rs | 153 +---------------------- codex-rs/cli/src/marketplace_cmd/ops.rs | 156 ++++++++++++++++++++++++ 2 files changed, 161 insertions(+), 148 deletions(-) create mode 100644 codex-rs/cli/src/marketplace_cmd/ops.rs diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index ed8a0f6b7702..d163abfe29ef 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -13,11 +13,11 @@ use codex_utils_cli::CliConfigOverrides; use std::fs; use std::path::Path; use std::path::PathBuf; -use std::process::Command; use std::time::SystemTime; use std::time::UNIX_EPOCH; mod metadata; +mod ops; #[derive(Debug, Parser)] pub struct MarketplaceCli { @@ -126,7 +126,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { return Ok(()); } - let staging_root = marketplace_staging_root(&install_root); + let staging_root = ops::marketplace_staging_root(&install_root); fs::create_dir_all(&staging_root).with_context(|| { format!( "failed to create marketplace staging directory {}", @@ -146,7 +146,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { match &source { MarketplaceSource::LocalDirectory { path } => { - copy_dir_recursive(path, &staged_root).with_context(|| { + ops::copy_dir_recursive(path, &staged_root).with_context(|| { format!( "failed to copy marketplace source {} into {}", path.display(), @@ -155,7 +155,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { })?; } MarketplaceSource::Git { url, ref_name } => { - clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; + ops::clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; } } @@ -175,7 +175,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { source.display() ); } - replace_marketplace_root(&staged_root, &destination) + ops::replace_marketplace_root(&staged_root, &destination) .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; record_added_marketplace(&codex_home, &marketplace_name, &install_metadata)?; @@ -361,121 +361,6 @@ fn is_github_shorthand_segment(segment: &str) -> bool { .all(|ch| ch.is_ascii_alphanumeric() || matches!(ch, '-' | '_' | '.')) } -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()], 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(), - ], - 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(()) -} - -pub(super) 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 copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { - fs::create_dir_all(target)?; - for entry in fs::read_dir(source)? { - let entry = entry?; - let source_path = entry.path(); - let target_path = target.join(entry.file_name()); - let file_type = entry.file_type()?; - - if file_type.is_dir() { - if entry.file_name().to_str() == Some(".git") { - continue; - } - copy_dir_recursive(&source_path, &target_path)?; - } else if file_type.is_file() { - fs::copy(&source_path, &target_path)?; - } else if file_type.is_symlink() { - copy_symlink_target(&source_path, &target_path)?; - } - } - Ok(()) -} - -#[cfg(unix)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - std::os::unix::fs::symlink(fs::read_link(source)?, target)?; - Ok(()) -} - -#[cfg(windows)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - let metadata = fs::metadata(source)?; - if metadata.is_dir() { - copy_dir_recursive(source, target) - } else { - fs::copy(source, target).map(|_| ()).map_err(Into::into) - } -} - -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 marketplace_staging_root(install_root: &Path) -> PathBuf { - install_root.join(".staging") -} - fn safe_marketplace_dir_name(marketplace_name: &str) -> Result { let safe = marketplace_name .chars() @@ -576,7 +461,6 @@ impl MarketplaceSource { mod tests { use super::*; use pretty_assertions::assert_eq; - use tempfile::TempDir; #[test] fn github_shorthand_parses_ref_suffix() { @@ -673,31 +557,4 @@ mod tests { vec!["plugins/foo", "skills/bar"] ); } - - #[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" - ); - } } 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 000000000000..1027e96fd302 --- /dev/null +++ b/codex-rs/cli/src/marketplace_cmd/ops.rs @@ -0,0 +1,156 @@ +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()], 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(), + ], + 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 copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { + fs::create_dir_all(target)?; + for entry in fs::read_dir(source)? { + let entry = entry?; + let source_path = entry.path(); + let target_path = target.join(entry.file_name()); + let file_type = entry.file_type()?; + + if file_type.is_dir() { + if entry.file_name().to_str() == Some(".git") { + continue; + } + copy_dir_recursive(&source_path, &target_path)?; + } else if file_type.is_file() { + fs::copy(&source_path, &target_path)?; + } else if file_type.is_symlink() { + copy_symlink_target(&source_path, &target_path)?; + } + } + Ok(()) +} + +#[cfg(unix)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + std::os::unix::fs::symlink(fs::read_link(source)?, target)?; + Ok(()) +} + +#[cfg(windows)] +fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { + let metadata = fs::metadata(source)?; + if metadata.is_dir() { + copy_dir_recursive(source, target) + } else { + fs::copy(source, target).map(|_| ()).map_err(Into::into) + } +} + +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 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" + ); + } +} From 53deb35f095d13b5fefb499523a4c3053680ba14 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 11:34:21 -0700 Subject: [PATCH 08/17] Split installed marketplace discovery --- .../src/plugins/installed_marketplaces.rs | 57 +++++++++++++++++++ codex-rs/core/src/plugins/manager.rs | 51 +---------------- codex-rs/core/src/plugins/manager_tests.rs | 1 + codex-rs/core/src/plugins/mod.rs | 5 +- 4 files changed, 62 insertions(+), 52 deletions(-) create mode 100644 codex-rs/core/src/plugins/installed_marketplaces.rs diff --git a/codex-rs/core/src/plugins/installed_marketplaces.rs b/codex-rs/core/src/plugins/installed_marketplaces.rs new file mode 100644 index 000000000000..edfc7d895aa9 --- /dev/null +++ b/codex-rs/core/src/plugins/installed_marketplaces.rs @@ -0,0 +1,57 @@ +use crate::config::Config; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::path::Path; +use std::path::PathBuf; +use tracing::warn; + +use super::validate_plugin_segment; + +pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; + +pub fn marketplace_install_root(codex_home: &Path) -> PathBuf { + codex_home.join(INSTALLED_MARKETPLACES_DIR) +} + +pub(crate) fn installed_marketplace_roots_from_config( + config: &Config, + codex_home: &Path, +) -> Vec { + let Some(user_layer) = config.config_layer_stack.get_user_layer() else { + return Vec::new(); + }; + let Some(marketplaces_value) = user_layer.config.get("marketplaces") else { + return Vec::new(); + }; + let Some(marketplaces) = marketplaces_value.as_table() else { + warn!("invalid marketplaces config: expected table"); + return Vec::new(); + }; + let default_install_root = marketplace_install_root(codex_home); + let mut roots = marketplaces + .iter() + .filter_map(|(marketplace_name, marketplace)| { + if !marketplace.is_table() { + warn!( + marketplace_name, + "ignoring invalid configured marketplace entry" + ); + return None; + } + if let Err(err) = validate_plugin_segment(marketplace_name, "marketplace name") { + warn!( + marketplace_name, + error = %err, + "ignoring invalid configured marketplace name" + ); + return None; + } + let path = default_install_root.join(marketplace_name); + path.join(".agents/plugins/marketplace.json") + .is_file() + .then_some(path) + }) + .filter_map(|path| AbsolutePathBuf::try_from(path).ok()) + .collect::>(); + roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); + roots +} diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 27688ce368ae..166c094591da 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -2,6 +2,7 @@ use super::LoadedPlugin; use super::PluginLoadOutcome; use super::PluginManifestPaths; use super::curated_plugins_repo_path; +use super::installed_marketplaces::installed_marketplace_roots_from_config; use super::load_plugin_manifest; use super::manifest::PluginManifestInterface; use super::marketplace::MarketplaceError; @@ -54,7 +55,6 @@ use codex_plugin::PluginId; use codex_plugin::PluginIdError; use codex_plugin::PluginTelemetryMetadata; use codex_plugin::prompt_safe_plugin_description; -use codex_plugin::validate_plugin_segment; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -80,7 +80,6 @@ use tracing::warn; const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; -pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; pub const OPENAI_CURATED_MARKETPLACE_DISPLAY_NAME: &str = "OpenAI Curated"; static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false); @@ -1245,54 +1244,6 @@ impl PluginsManager { } } -pub fn marketplace_install_root(codex_home: &Path) -> PathBuf { - codex_home.join(INSTALLED_MARKETPLACES_DIR) -} - -fn installed_marketplace_roots_from_config( - config: &Config, - codex_home: &Path, -) -> Vec { - let Some(user_layer) = config.config_layer_stack.get_user_layer() else { - return Vec::new(); - }; - let Some(marketplaces_value) = user_layer.config.get("marketplaces") else { - return Vec::new(); - }; - let Some(marketplaces) = marketplaces_value.as_table() else { - warn!("invalid marketplaces config: expected table"); - return Vec::new(); - }; - let default_install_root = marketplace_install_root(codex_home); - let mut roots = marketplaces - .iter() - .filter_map(|(marketplace_name, marketplace)| { - if !marketplace.is_table() { - warn!( - marketplace_name, - "ignoring invalid configured marketplace entry" - ); - return None; - } - if let Err(err) = validate_plugin_segment(marketplace_name, "marketplace name") { - warn!( - marketplace_name, - error = %err, - "ignoring invalid configured marketplace name" - ); - return None; - } - let path = default_install_root.join(marketplace_name); - path.join(".agents/plugins/marketplace.json") - .is_file() - .then_some(path) - }) - .filter_map(|path| AbsolutePathBuf::try_from(path).ok()) - .collect::>(); - roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); - roots -} - #[derive(Debug, thiserror::Error)] pub enum PluginInstallError { #[error("{0}")] diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index b5dc40716ee8..22e17bf39366 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -8,6 +8,7 @@ use crate::config_loader::ConfigRequirementsToml; use crate::plugins::LoadedPlugin; use crate::plugins::MarketplacePluginInstallPolicy; use crate::plugins::PluginLoadOutcome; +use crate::plugins::marketplace_install_root; use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA; use crate::plugins::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha; use crate::plugins::test_support::write_file; diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index bf43d5b04388..5115c3f7ea80 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -2,6 +2,7 @@ use codex_config::types::McpServerConfig; mod discoverable; mod injection; +mod installed_marketplaces; mod manager; mod manifest; mod marketplace; @@ -27,10 +28,11 @@ pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; pub(crate) use discoverable::list_tool_suggest_discoverable_plugins; pub(crate) use injection::build_plugin_injections; +pub use installed_marketplaces::INSTALLED_MARKETPLACES_DIR; +pub use installed_marketplaces::marketplace_install_root; pub use manager::ConfiguredMarketplace; pub use manager::ConfiguredMarketplaceListOutcome; pub use manager::ConfiguredMarketplacePlugin; -pub use manager::INSTALLED_MARKETPLACES_DIR; pub use manager::OPENAI_CURATED_MARKETPLACE_NAME; pub use manager::PluginDetail; pub use manager::PluginInstallError; @@ -45,7 +47,6 @@ pub use manager::RemotePluginSyncResult; pub use manager::installed_plugin_telemetry_metadata; pub use manager::load_plugin_apps; pub use manager::load_plugin_mcp_servers; -pub use manager::marketplace_install_root; pub use manager::plugin_telemetry_metadata_from_root; pub use manifest::PluginManifestInterface; pub(crate) use manifest::PluginManifestPaths; From cdac5ef7861e458f527fd769cf5ad19a183adc57 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 11:51:24 -0700 Subject: [PATCH 09/17] Tighten marketplace source parsing --- codex-rs/cli/src/marketplace_cmd.rs | 31 ++++++++++++++++++------- codex-rs/cli/src/marketplace_cmd/ops.rs | 4 ++-- codex-rs/cli/tests/marketplace_add.rs | 29 +++++++++++++++++++++++ 3 files changed, 53 insertions(+), 11 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index d163abfe29ef..48dc2e4c8e88 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -91,8 +91,13 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { sparse_paths, } = args; + let has_explicit_ref = ref_name.is_some(); let source = parse_marketplace_source(&source, ref_name)?; - if !sparse_paths.is_empty() && !matches!(source, MarketplaceSource::Git { .. }) { + let source_is_git = matches!(source, MarketplaceSource::Git { .. }); + if has_explicit_ref && !source_is_git { + bail!("--ref can only be used with git marketplace sources"); + } + if !sparse_paths.is_empty() && !source_is_git { bail!("--sparse can only be used with git marketplace sources"); } @@ -281,12 +286,6 @@ fn parse_marketplace_source( return Ok(MarketplaceSource::Git { url, ref_name }); } - if base_source.starts_with("http://") || base_source.starts_with("https://") { - bail!( - "URL marketplace manifests are not supported yet; pass a git repository URL or a local marketplace directory" - ); - } - bail!("invalid marketplace source format: {source}"); } @@ -340,8 +339,7 @@ fn is_ssh_git_url(source: &str) -> bool { } fn is_http_git_url(source: &str) -> bool { - (source.starts_with("http://") || source.starts_with("https://")) - && (source.ends_with(".git") || source.starts_with("https://github.com/")) + source.starts_with("http://") || source.starts_with("https://") } fn looks_like_github_shorthand(source: &str) -> bool { @@ -522,6 +520,21 @@ mod tests { ); } + #[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 utc_timestamp_formats_unix_epoch_as_rfc3339_utc() { assert_eq!(format_utc_timestamp(0), "1970-01-01T00:00:00Z"); diff --git a/codex-rs/cli/src/marketplace_cmd/ops.rs b/codex-rs/cli/src/marketplace_cmd/ops.rs index 1027e96fd302..b8e309196416 100644 --- a/codex-rs/cli/src/marketplace_cmd/ops.rs +++ b/codex-rs/cli/src/marketplace_cmd/ops.rs @@ -14,7 +14,7 @@ pub(super) fn clone_git_source( ) -> Result<()> { let destination = destination.to_string_lossy().to_string(); if sparse_paths.is_empty() { - run_git(&["clone", url, destination.as_str()], None)?; + 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)))?; } @@ -29,7 +29,7 @@ pub(super) fn clone_git_source( url, destination.as_str(), ], - None, + /*cwd*/ None, )?; let mut sparse_args = vec!["sparse-checkout", "set"]; sparse_args.extend(sparse_paths.iter().map(String::as_str)); diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index d09f6d836a1f..46dbe663e90a 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -255,3 +255,32 @@ async fn marketplace_add_sparse_flag_parses_before_and_after_source() -> Result< Ok(()) } + +#[tokio::test] +async fn marketplace_add_rejects_ref_for_local_directory() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "local ref")?; + + codex_command(codex_home.path())? + .args([ + "marketplace", + "add", + "--ref", + "main", + source.path().to_str().unwrap(), + ]) + .assert() + .failure() + .stderr(contains( + "--ref can only be used with git marketplace sources", + )); + + assert!( + !marketplace_install_root(codex_home.path()) + .join("debug") + .exists() + ); + + Ok(()) +} From 238f4b1214dd5f54f5616d62a5696fc3675b63a0 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 12:20:27 -0700 Subject: [PATCH 10/17] Fix marketplace Git URL parsing --- codex-rs/cli/src/marketplace_cmd.rs | 33 +++++++++++++++++++++-------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 48dc2e4c8e88..e26f80638d4b 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -229,6 +229,14 @@ fn parse_marketplace_source( } let source = expand_home(source); + let (base_source, parsed_ref) = split_source_ref(&source); + let ref_name = explicit_ref.or(parsed_ref); + + if is_ssh_git_url(&base_source) || is_http_git_url(&base_source) { + let url = normalize_git_url(&base_source); + return Ok(MarketplaceSource::Git { url, ref_name }); + } + let path = PathBuf::from(&source); let path_exists = path.try_exists().with_context(|| { format!( @@ -273,14 +281,6 @@ fn parse_marketplace_source( return Ok(MarketplaceSource::LocalDirectory { path }); } - let (base_source, parsed_ref) = split_source_ref(&source); - let ref_name = explicit_ref.or(parsed_ref); - - if is_ssh_git_url(&base_source) || is_http_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 }); @@ -335,7 +335,7 @@ fn expand_home(source: &str) -> String { } fn is_ssh_git_url(source: &str) -> bool { - source.starts_with("git@") && source.contains(':') + source.starts_with("ssh://") || source.starts_with("git@") && source.contains(':') } fn is_http_git_url(source: &str) -> bool { @@ -535,6 +535,21 @@ mod tests { ); } + #[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(0), "1970-01-01T00:00:00Z"); From 674015acdfa29a69b4e6174db16b37bbc6ed62f4 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 12:55:09 -0700 Subject: [PATCH 11/17] Normalize marketplace GitHub URLs with trailing slash --- codex-rs/cli/src/marketplace_cmd.rs | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index e26f80638d4b..f5df805e4b15 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -308,6 +308,7 @@ fn non_empty_ref(ref_name: &str) -> Option { } 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 { @@ -520,6 +521,21 @@ mod tests { ); } + #[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!( From 7021f7c908dbe35add93fabcdeaff17e57267ef4 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 13:20:56 -0700 Subject: [PATCH 12/17] Disable execpolicy legacy Bazel build script --- codex-rs/execpolicy-legacy/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/codex-rs/execpolicy-legacy/BUILD.bazel b/codex-rs/execpolicy-legacy/BUILD.bazel index 489288472819..07484bad71df 100644 --- a/codex-rs/execpolicy-legacy/BUILD.bazel +++ b/codex-rs/execpolicy-legacy/BUILD.bazel @@ -2,6 +2,7 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "execpolicy-legacy", + build_script_enabled = False, crate_name = "codex_execpolicy_legacy", compile_data = ["src/default.policy"], ) From 640a1ae71e68638e1895ff78573fc1c0d064ed13 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 15:12:43 -0700 Subject: [PATCH 13/17] Fix marketplace add consistency --- codex-rs/cli/src/marketplace_cmd.rs | 10 ++++- codex-rs/cli/src/marketplace_cmd/metadata.rs | 44 +++++++++++++++++++- codex-rs/cli/tests/marketplace_add.rs | 41 ++++++++++++++++++ codex-rs/execpolicy-legacy/BUILD.bazel | 2 +- 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index f5df805e4b15..6f1316662bcc 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -182,7 +182,15 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { } ops::replace_marketplace_root(&staged_root, &destination) .with_context(|| format!("failed to install marketplace at {}", destination.display()))?; - record_added_marketplace(&codex_home, &marketplace_name, &install_metadata)?; + 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!( "Added marketplace `{marketplace_name}` from {}.", diff --git a/codex-rs/cli/src/marketplace_cmd/metadata.rs b/codex-rs/cli/src/marketplace_cmd/metadata.rs index 6a6b3c0c49ef..484a5079b337 100644 --- a/codex-rs/cli/src/marketplace_cmd/metadata.rs +++ b/codex-rs/cli/src/marketplace_cmd/metadata.rs @@ -3,6 +3,7 @@ use anyhow::Context; use anyhow::Result; use codex_config::CONFIG_TOML_FILE; use codex_core::plugins::validate_marketplace_root; +use std::io::ErrorKind; use std::path::Path; use std::path::PathBuf; @@ -29,8 +30,13 @@ pub(super) fn installed_marketplace_root_for_source( install_metadata: &MarketplaceInstallMetadata, ) -> Result> { let config_path = codex_home.join(CONFIG_TOML_FILE); - let Ok(config) = std::fs::read_to_string(&config_path) else { - return Ok(None); + let config = match std::fs::read_to_string(&config_path) { + Ok(config) => config, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(err) + .with_context(|| format!("failed to read user config {}", config_path.display())); + } }; let config: toml::Value = toml::from_str(&config) .with_context(|| format!("failed to parse user config {}", config_path.display()))?; @@ -117,3 +123,37 @@ fn config_sparse_paths(marketplace: &toml::Value) -> Vec { }) .unwrap_or_default() } + +#[cfg(test)] +mod tests { + use super::*; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + #[test] + fn installed_marketplace_root_for_source_propagates_config_read_errors() -> Result<()> { + let codex_home = TempDir::new()?; + let config_path = codex_home.path().join(CONFIG_TOML_FILE); + std::fs::create_dir(&config_path)?; + + let install_root = codex_home.path().join("marketplaces"); + let source = MarketplaceSource::LocalDirectory { + path: codex_home.path().join("source"), + }; + let install_metadata = MarketplaceInstallMetadata::from_source(&source, &[]); + + let err = installed_marketplace_root_for_source( + codex_home.path(), + &install_root, + &install_metadata, + ) + .unwrap_err(); + + assert_eq!( + err.to_string(), + format!("failed to read user config {}", config_path.display()) + ); + + Ok(()) + } +} diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index 46dbe663e90a..e83970ec02f3 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -181,6 +181,47 @@ async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> Ok(()) } +#[tokio::test] +async fn marketplace_add_rolls_back_install_when_config_write_fails() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "rollback")?; + + let config_path = codex_home.path().join("config.toml"); + std::fs::write(&config_path, "")?; + let original_permissions = std::fs::metadata(&config_path)?.permissions(); + let mut read_only_permissions = original_permissions.clone(); + read_only_permissions.set_readonly(true); + std::fs::set_permissions(&config_path, read_only_permissions)?; + + let output = codex_command(codex_home.path())? + .args(["marketplace", "add", source.path().to_str().unwrap()]) + .output()?; + + std::fs::set_permissions(&config_path, original_permissions)?; + + assert!( + !output.status.success(), + "command unexpectedly succeeded: stdout={}, stderr={}", + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + assert!( + String::from_utf8_lossy(&output.stderr) + .contains("failed to add marketplace `debug` to user config.toml"), + "unexpected stderr: {}", + String::from_utf8_lossy(&output.stderr) + ); + assert!( + !marketplace_install_root(codex_home.path()) + .join("debug") + .exists(), + "installed marketplace should be rolled back when config.toml cannot be persisted" + ); + + Ok(()) +} + fn assert_marketplace_config( codex_home: &Path, marketplace_name: &str, diff --git a/codex-rs/execpolicy-legacy/BUILD.bazel b/codex-rs/execpolicy-legacy/BUILD.bazel index 07484bad71df..87bb4e1378dc 100644 --- a/codex-rs/execpolicy-legacy/BUILD.bazel +++ b/codex-rs/execpolicy-legacy/BUILD.bazel @@ -2,7 +2,7 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "execpolicy-legacy", - build_script_enabled = False, + build_script_data = ["src/default.policy"], crate_name = "codex_execpolicy_legacy", compile_data = ["src/default.policy"], ) From df213f7adeb1d66d6f5d934549b30a769329052d Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 16:22:32 -0700 Subject: [PATCH 14/17] Restrict marketplace add to remote git sources --- codex-rs/cli/src/marketplace_cmd.rs | 124 ++----- codex-rs/cli/src/marketplace_cmd/metadata.rs | 15 +- codex-rs/cli/src/marketplace_cmd/ops.rs | 38 -- codex-rs/cli/tests/marketplace_add.rs | 369 +++++++++++++++---- codex-rs/config/src/types.rs | 1 - codex-rs/core/config.schema.json | 1 - codex-rs/core/src/plugins/manager_tests.rs | 4 +- 7 files changed, 345 insertions(+), 207 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index 6f1316662bcc..c84e8b94dc75 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -12,7 +12,6 @@ use codex_core::plugins::validate_plugin_segment; use codex_utils_cli::CliConfigOverrides; use std::fs; use std::path::Path; -use std::path::PathBuf; use std::time::SystemTime; use std::time::UNIX_EPOCH; @@ -30,13 +29,13 @@ pub struct MarketplaceCli { #[derive(Debug, clap::Subcommand)] enum MarketplaceSubcommand { - /// Add a marketplace repository or local marketplace directory. + /// Add a remote marketplace repository. Add(AddMarketplaceArgs), } #[derive(Debug, Parser)] struct AddMarketplaceArgs { - /// Marketplace source. Supports owner/repo[@ref], git URLs, SSH URLs, or local directories. + /// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, or SSH URLs. source: String, /// Git ref to check out. Overrides any @ref or #ref suffix in SOURCE. @@ -54,9 +53,6 @@ struct AddMarketplaceArgs { #[derive(Debug, PartialEq, Eq)] pub(super) enum MarketplaceSource { - LocalDirectory { - path: PathBuf, - }, Git { url: String, ref_name: Option, @@ -91,15 +87,7 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { sparse_paths, } = args; - let has_explicit_ref = ref_name.is_some(); let source = parse_marketplace_source(&source, ref_name)?; - let source_is_git = matches!(source, MarketplaceSource::Git { .. }); - if has_explicit_ref && !source_is_git { - bail!("--ref can only be used with git marketplace sources"); - } - if !sparse_paths.is_empty() && !source_is_git { - bail!("--sparse can only be used with git marketplace sources"); - } let codex_home = find_codex_home().context("failed to resolve CODEX_HOME")?; let install_root = marketplace_install_root(&codex_home); @@ -149,20 +137,8 @@ async fn run_add(args: AddMarketplaceArgs) -> Result<()> { })?; let staged_root = staged_dir.path().to_path_buf(); - match &source { - MarketplaceSource::LocalDirectory { path } => { - ops::copy_dir_recursive(path, &staged_root).with_context(|| { - format!( - "failed to copy marketplace source {} into {}", - path.display(), - staged_root.display() - ) - })?; - } - MarketplaceSource::Git { url, ref_name } => { - ops::clone_git_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; - } - } + 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()))?; @@ -236,57 +212,18 @@ fn parse_marketplace_source( bail!("marketplace source must not be empty"); } - let source = expand_home(source); - let (base_source, parsed_ref) = split_source_ref(&source); + let (base_source, parsed_ref) = split_source_ref(source); let ref_name = explicit_ref.or(parsed_ref); - if is_ssh_git_url(&base_source) || is_http_git_url(&base_source) { - let url = normalize_git_url(&base_source); - return Ok(MarketplaceSource::Git { url, ref_name }); + 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" + ); } - let path = PathBuf::from(&source); - let path_exists = path.try_exists().with_context(|| { - format!( - "failed to access local marketplace source {}", - path.display() - ) - })?; - if path_exists || looks_like_local_path(&source) { - if !path_exists { - bail!( - "local marketplace source does not exist: {}", - path.display() - ); - } - let metadata = path.metadata().with_context(|| { - format!("failed to read local marketplace source {}", path.display()) - })?; - if metadata.is_file() { - if path - .extension() - .is_some_and(|extension| extension == "json") - { - bail!( - "local marketplace JSON files are not supported yet; pass the marketplace root directory containing .agents/plugins/marketplace.json: {}", - path.display() - ); - } - bail!( - "local marketplace source file must be a JSON marketplace manifest or a directory containing .agents/plugins/marketplace.json: {}", - path.display() - ); - } - if !metadata.is_dir() { - bail!( - "local marketplace source must be a file or directory: {}", - path.display() - ); - } - let path = path - .canonicalize() - .with_context(|| format!("failed to resolve {}", path.display()))?; - return Ok(MarketplaceSource::LocalDirectory { path }); + 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) { @@ -333,21 +270,11 @@ fn looks_like_local_path(source: &str) -> bool { || source == ".." } -fn expand_home(source: &str) -> String { - let Some(rest) = source.strip_prefix("~/") else { - return source.to_string(); - }; - if let Some(home) = std::env::var_os("HOME") { - return PathBuf::from(home).join(rest).display().to_string(); - } - source.to_string() -} - fn is_ssh_git_url(source: &str) -> bool { source.starts_with("ssh://") || source.starts_with("git@") && source.contains(':') } -fn is_http_git_url(source: &str) -> bool { +fn is_git_url(source: &str) -> bool { source.starts_with("http://") || source.starts_with("https://") } @@ -452,7 +379,6 @@ fn civil_from_days(days_since_epoch: i64) -> (i64, i64, i64) { impl MarketplaceSource { fn display(&self) -> String { match self { - Self::LocalDirectory { path } => path.display().to_string(), Self::Git { url, ref_name } => { if let Some(ref_name) = ref_name { format!("{url}#{ref_name}") @@ -559,6 +485,30 @@ mod tests { ); } + #[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!( diff --git a/codex-rs/cli/src/marketplace_cmd/metadata.rs b/codex-rs/cli/src/marketplace_cmd/metadata.rs index 484a5079b337..db268840bb87 100644 --- a/codex-rs/cli/src/marketplace_cmd/metadata.rs +++ b/codex-rs/cli/src/marketplace_cmd/metadata.rs @@ -14,9 +14,6 @@ pub(super) struct MarketplaceInstallMetadata { #[derive(Debug, Clone, PartialEq, Eq)] enum InstalledMarketplaceSource { - LocalDirectory { - path: PathBuf, - }, Git { url: String, ref_name: Option, @@ -60,9 +57,6 @@ pub(super) fn installed_marketplace_root_for_source( impl MarketplaceInstallMetadata { pub(super) fn from_source(source: &MarketplaceSource, sparse_paths: &[String]) -> Self { let source = match source { - MarketplaceSource::LocalDirectory { path } => { - InstalledMarketplaceSource::LocalDirectory { path: path.clone() } - } MarketplaceSource::Git { url, ref_name } => InstalledMarketplaceSource::Git { url: url.clone(), ref_name: ref_name.clone(), @@ -74,28 +68,24 @@ impl MarketplaceInstallMetadata { pub(super) fn config_source_type(&self) -> &'static str { match &self.source { - InstalledMarketplaceSource::LocalDirectory { .. } => "directory", InstalledMarketplaceSource::Git { .. } => "git", } } pub(super) fn config_source(&self) -> String { match &self.source { - InstalledMarketplaceSource::LocalDirectory { path } => path.display().to_string(), InstalledMarketplaceSource::Git { url, .. } => url.clone(), } } pub(super) fn ref_name(&self) -> Option<&str> { match &self.source { - InstalledMarketplaceSource::LocalDirectory { .. } => None, InstalledMarketplaceSource::Git { ref_name, .. } => ref_name.as_deref(), } } pub(super) fn sparse_paths(&self) -> &[String] { match &self.source { - InstalledMarketplaceSource::LocalDirectory { .. } => &[], InstalledMarketplaceSource::Git { sparse_paths, .. } => sparse_paths, } } @@ -137,8 +127,9 @@ mod tests { std::fs::create_dir(&config_path)?; let install_root = codex_home.path().join("marketplaces"); - let source = MarketplaceSource::LocalDirectory { - path: codex_home.path().join("source"), + let source = MarketplaceSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + ref_name: None, }; let install_metadata = MarketplaceInstallMetadata::from_source(&source, &[]); diff --git a/codex-rs/cli/src/marketplace_cmd/ops.rs b/codex-rs/cli/src/marketplace_cmd/ops.rs index b8e309196416..ffb777fdbd60 100644 --- a/codex-rs/cli/src/marketplace_cmd/ops.rs +++ b/codex-rs/cli/src/marketplace_cmd/ops.rs @@ -65,44 +65,6 @@ fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<()> { ); } -pub(super) fn copy_dir_recursive(source: &Path, target: &Path) -> Result<()> { - fs::create_dir_all(target)?; - for entry in fs::read_dir(source)? { - let entry = entry?; - let source_path = entry.path(); - let target_path = target.join(entry.file_name()); - let file_type = entry.file_type()?; - - if file_type.is_dir() { - if entry.file_name().to_str() == Some(".git") { - continue; - } - copy_dir_recursive(&source_path, &target_path)?; - } else if file_type.is_file() { - fs::copy(&source_path, &target_path)?; - } else if file_type.is_symlink() { - copy_symlink_target(&source_path, &target_path)?; - } - } - Ok(()) -} - -#[cfg(unix)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - std::os::unix::fs::symlink(fs::read_link(source)?, target)?; - Ok(()) -} - -#[cfg(windows)] -fn copy_symlink_target(source: &Path, target: &Path) -> Result<()> { - let metadata = fs::metadata(source)?; - if metadata.is_dir() { - copy_dir_recursive(source, target) - } else { - fs::copy(source, target).map(|_| ()).map_err(Into::into) - } -} - pub(super) fn replace_marketplace_root(staged_root: &Path, destination: &Path) -> Result<()> { if let Some(parent) = destination.parent() { fs::create_dir_all(parent)?; diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index e83970ec02f3..190fc619e395 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -4,7 +4,23 @@ use codex_core::plugins::marketplace_install_root; use codex_core::plugins::validate_marketplace_root; use predicates::str::contains; use pretty_assertions::assert_eq; +use std::ffi::OsStr; +use std::ffi::OsString; +use std::io::Read; +use std::io::Write; +use std::net::SocketAddr; +use std::net::TcpListener; +use std::net::TcpStream; +use std::path::Component; use std::path::Path; +use std::path::PathBuf; +use std::process::Command; +use std::sync::Arc; +use std::sync::atomic::AtomicBool; +use std::sync::atomic::Ordering; +use std::thread; +use std::thread::JoinHandle; +use std::time::Duration; use tempfile::TempDir; fn codex_command(codex_home: &Path) -> Result { @@ -49,22 +65,280 @@ fn write_marketplace_source_with_name( Ok(()) } +struct RemoteGitMarketplace { + _server: HttpFileServer, + repo_path: PathBuf, + url: String, +} + +struct HttpFileServer { + _root: TempDir, + address: SocketAddr, + shutdown: Arc, + handle: Option>, +} + +fn write_git_marketplace_source(source: &Path, marker: &str) -> Result { + write_marketplace_source(source, marker)?; + init_git_repo(source)?; + RemoteGitMarketplace::from_source(source) +} + +fn write_git_marketplace_source_with_name( + source: &Path, + marketplace_name: &str, + marker: &str, +) -> Result { + write_marketplace_source_with_name(source, marketplace_name, marker)?; + init_git_repo(source)?; + RemoteGitMarketplace::from_source(source) +} + +fn init_git_repo(source: &Path) -> Result<()> { + run_git(source, ["init"])?; + run_git(source, ["config", "user.email", "codex@example.com"])?; + run_git(source, ["config", "user.name", "Codex Test"])?; + commit_git_repo(source, "initial marketplace")?; + Ok(()) +} + +fn commit_git_repo(source: &Path, message: &str) -> Result<()> { + run_git(source, ["add", "."])?; + run_git(source, ["commit", "-m", message])?; + Ok(()) +} + +fn run_git(cwd: &Path, args: I) -> Result<()> +where + I: IntoIterator, + S: AsRef, +{ + let args = args + .into_iter() + .map(|arg| arg.as_ref().to_os_string()) + .collect::>(); + let output = Command::new("git").current_dir(cwd).args(&args).output()?; + let args_display = args + .iter() + .map(|arg| arg.to_string_lossy()) + .collect::>() + .join(" "); + assert!( + output.status.success(), + "git {} failed\nstdout:\n{}\nstderr:\n{}", + args_display, + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); + Ok(()) +} + +impl RemoteGitMarketplace { + fn from_source(source: &Path) -> Result { + let root = TempDir::new()?; + let repo_name = "marketplace.git"; + let repo_path = root.path().join(repo_name); + clone_bare_repo(source, root.path(), repo_name)?; + let server = HttpFileServer::start(root)?; + let url = format!("http://{}/{repo_name}", server.address); + Ok(Self { + _server: server, + repo_path, + url, + }) + } + + fn refresh_from_source(&self, source: &Path) -> Result<()> { + std::fs::remove_dir_all(&self.repo_path)?; + let parent = self + .repo_path + .parent() + .context("served git repository should have a parent")?; + let repo_name = self + .repo_path + .file_name() + .context("served git repository should have a file name")? + .to_string_lossy() + .to_string(); + clone_bare_repo(source, parent, &repo_name) + } +} + +fn clone_bare_repo(source: &Path, target_parent: &Path, repo_name: &str) -> Result<()> { + run_git( + target_parent, + [ + OsString::from("clone"), + OsString::from("--bare"), + source.as_os_str().to_os_string(), + OsString::from(repo_name), + ], + )?; + run_git( + target_parent.join(repo_name).as_path(), + ["update-server-info"], + )?; + Ok(()) +} + +impl HttpFileServer { + fn start(root: TempDir) -> Result { + let listener = TcpListener::bind(("127.0.0.1", 0))?; + let address = listener.local_addr()?; + listener.set_nonblocking(true)?; + + let shutdown = Arc::new(AtomicBool::new(false)); + let thread_shutdown = Arc::clone(&shutdown); + let root_path = root.path().to_path_buf(); + let handle = thread::spawn(move || { + while !thread_shutdown.load(Ordering::Relaxed) { + match listener.accept() { + Ok((mut stream, _)) => { + let _ = serve_http_file(&root_path, &mut stream); + } + Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { + thread::sleep(Duration::from_millis(10)); + } + Err(_) => break, + } + } + }); + + Ok(Self { + _root: root, + address, + shutdown, + handle: Some(handle), + }) + } +} + +impl Drop for HttpFileServer { + fn drop(&mut self) { + self.shutdown.store(true, Ordering::Relaxed); + let _ = TcpStream::connect(self.address); + if let Some(handle) = self.handle.take() { + let _ = handle.join(); + } + } +} + +fn serve_http_file(root: &Path, stream: &mut TcpStream) -> Result<()> { + stream.set_read_timeout(Some(Duration::from_secs(5)))?; + let mut request = Vec::new(); + let mut buffer = [0_u8; 1024]; + while !request.windows(4).any(|window| window == b"\r\n\r\n") { + let bytes_read = stream.read(&mut buffer)?; + if bytes_read == 0 { + break; + } + request.extend_from_slice(&buffer[..bytes_read]); + if request.len() > 16 * 1024 { + write_http_response(stream, "413 Payload Too Large", &[])?; + return Ok(()); + } + } + + let request = String::from_utf8_lossy(&request); + let Some(request_line) = request.lines().next() else { + write_http_response(stream, "400 Bad Request", &[])?; + return Ok(()); + }; + let mut parts = request_line.split_whitespace(); + let method = parts.next().unwrap_or_default(); + let target = parts.next().unwrap_or_default(); + if method != "GET" && method != "HEAD" { + write_http_response(stream, "405 Method Not Allowed", &[])?; + return Ok(()); + } + + let Some(relative_path) = safe_http_path(target) else { + write_http_response(stream, "400 Bad Request", &[])?; + return Ok(()); + }; + let path = root.join(relative_path); + if !path.is_file() { + write_http_response(stream, "404 Not Found", &[])?; + return Ok(()); + } + + let body = std::fs::read(path)?; + if method == "HEAD" { + write_http_response(stream, "200 OK", &[])?; + } else { + write_http_response(stream, "200 OK", &body)?; + } + Ok(()) +} + +fn safe_http_path(target: &str) -> Option { + let path = target.split('?').next()?.trim_start_matches('/'); + let path = percent_decode(path)?; + let relative_path = PathBuf::from(path); + if relative_path.components().any(|component| { + matches!( + component, + Component::ParentDir | Component::RootDir | Component::Prefix(_) + ) + }) { + return None; + } + Some(relative_path) +} + +fn percent_decode(input: &str) -> Option { + let bytes = input.as_bytes(); + let mut output = Vec::with_capacity(bytes.len()); + let mut index = 0; + while index < bytes.len() { + if bytes[index] == b'%' { + let high = *bytes.get(index + 1)?; + let low = *bytes.get(index + 2)?; + output.push(hex_value(high)? * 16 + hex_value(low)?); + index += 3; + } else { + output.push(bytes[index]); + index += 1; + } + } + String::from_utf8(output).ok() +} + +fn hex_value(byte: u8) -> Option { + match byte { + b'0'..=b'9' => Some(byte - b'0'), + b'a'..=b'f' => Some(byte - b'a' + 10), + b'A'..=b'F' => Some(byte - b'A' + 10), + _ => None, + } +} + +fn write_http_response(stream: &mut TcpStream, status: &str, body: &[u8]) -> Result<()> { + write!( + stream, + "HTTP/1.1 {status}\r\nContent-Length: {}\r\nContent-Type: application/octet-stream\r\nConnection: close\r\n\r\n", + body.len() + )?; + stream.write_all(body)?; + Ok(()) +} + #[tokio::test] -async fn marketplace_add_local_directory_installs_valid_marketplace_root() -> Result<()> { +async fn marketplace_add_git_source_installs_valid_marketplace_root() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; - write_marketplace_source(source.path(), "first install")?; + let source = write_git_marketplace_source(source.path(), "first install")?; let mut add_cmd = codex_command(codex_home.path())?; add_cmd - .args(["marketplace", "add", source.path().to_str().unwrap()]) + .args(["marketplace", "add", &source.url]) .assert() .success() .stdout(contains("Added marketplace `debug`")); let installed_root = marketplace_install_root(codex_home.path()).join("debug"); assert_eq!(validate_marketplace_root(&installed_root)?, "debug"); - assert_marketplace_config(codex_home.path(), "debug", &source.path().canonicalize()?)?; + assert_git_marketplace_config(codex_home.path(), "debug", &source.url)?; assert!( installed_root .join("plugins/sample/.codex-plugin/plugin.json") @@ -85,10 +359,14 @@ async fn marketplace_add_local_directory_installs_valid_marketplace_root() -> Re async fn marketplace_add_rejects_invalid_marketplace_name() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; - write_marketplace_source_with_name(source.path(), "debug.market", "invalid marketplace")?; + let source = write_git_marketplace_source_with_name( + source.path(), + "debug.market", + "invalid marketplace", + )?; codex_command(codex_home.path())? - .args(["marketplace", "add", source.path().to_str().unwrap()]) + .args(["marketplace", "add", &source.url]) .assert() .failure() .stderr(contains( @@ -114,10 +392,10 @@ async fn marketplace_add_rejects_invalid_marketplace_name() -> Result<()> { async fn marketplace_add_same_source_is_idempotent() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; - write_marketplace_source(source.path(), "first install")?; + let served_source = write_git_marketplace_source(source.path(), "first install")?; codex_command(codex_home.path())? - .args(["marketplace", "add", source.path().to_str().unwrap()]) + .args(["marketplace", "add", &served_source.url]) .assert() .success() .stdout(contains("Added marketplace `debug`")); @@ -126,9 +404,11 @@ async fn marketplace_add_same_source_is_idempotent() -> Result<()> { source.path().join("plugins/sample/marker.txt"), "source changed after add", )?; + commit_git_repo(source.path(), "update marketplace")?; + served_source.refresh_from_source(source.path())?; codex_command(codex_home.path())? - .args(["marketplace", "add", source.path().to_str().unwrap()]) + .args(["marketplace", "add", &served_source.url]) .assert() .success() .stdout(contains("Marketplace `debug` is already added")); @@ -138,7 +418,7 @@ async fn marketplace_add_same_source_is_idempotent() -> Result<()> { std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, "first install" ); - assert_marketplace_config(codex_home.path(), "debug", &source.path().canonicalize()?)?; + assert_git_marketplace_config(codex_home.path(), "debug", &served_source.url)?; assert!(!installed_root.join(".codex-marketplace-source").exists()); assert!( !codex_home @@ -155,17 +435,17 @@ async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> let codex_home = TempDir::new()?; let first_source = TempDir::new()?; let second_source = TempDir::new()?; - write_marketplace_source(first_source.path(), "first install")?; - write_marketplace_source(second_source.path(), "replacement install")?; + let first_source = write_git_marketplace_source(first_source.path(), "first install")?; + let second_source = write_git_marketplace_source(second_source.path(), "replacement install")?; codex_command(codex_home.path())? - .args(["marketplace", "add", first_source.path().to_str().unwrap()]) + .args(["marketplace", "add", &first_source.url]) .assert() .success() .stdout(contains("Added marketplace `debug`")); codex_command(codex_home.path())? - .args(["marketplace", "add", second_source.path().to_str().unwrap()]) + .args(["marketplace", "add", &second_source.url]) .assert() .failure() .stderr(contains( @@ -185,7 +465,7 @@ async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> async fn marketplace_add_rolls_back_install_when_config_write_fails() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; - write_marketplace_source(source.path(), "rollback")?; + let source = write_git_marketplace_source(source.path(), "rollback")?; let config_path = codex_home.path().join("config.toml"); std::fs::write(&config_path, "")?; @@ -195,7 +475,7 @@ async fn marketplace_add_rolls_back_install_when_config_write_fails() -> Result< std::fs::set_permissions(&config_path, read_only_permissions)?; let output = codex_command(codex_home.path())? - .args(["marketplace", "add", source.path().to_str().unwrap()]) + .args(["marketplace", "add", &source.url]) .output()?; std::fs::set_permissions(&config_path, original_permissions)?; @@ -222,10 +502,10 @@ async fn marketplace_add_rolls_back_install_when_config_write_fails() -> Result< Ok(()) } -fn assert_marketplace_config( +fn assert_git_marketplace_config( codex_home: &Path, marketplace_name: &str, - source: &Path, + source_url: &str, ) -> Result<()> { let config = std::fs::read_to_string(codex_home.join("config.toml"))?; let config: toml::Value = toml::from_str(&config)?; @@ -233,7 +513,6 @@ fn assert_marketplace_config( .get("marketplaces") .and_then(|marketplaces| marketplaces.get(marketplace_name)) .context("marketplace config should be written")?; - let expected_source = source.to_string_lossy().to_string(); assert!( marketplace @@ -246,11 +525,11 @@ fn assert_marketplace_config( ); assert_eq!( marketplace.get("source_type").and_then(toml::Value::as_str), - Some("directory") + Some("git") ); assert_eq!( marketplace.get("source").and_then(toml::Value::as_str), - Some(expected_source.as_str()) + Some(source_url) ); assert_eq!(marketplace.get("ref").and_then(toml::Value::as_str), None); assert!(marketplace.get("sparse_paths").is_none()); @@ -262,59 +541,17 @@ fn assert_marketplace_config( } #[tokio::test] -async fn marketplace_add_sparse_flag_parses_before_and_after_source() -> Result<()> { - let codex_home = TempDir::new()?; - let source = TempDir::new()?; - let source = source.path().to_str().unwrap(); - let sparse_requires_git = "--sparse can only be used with git marketplace sources"; - - codex_command(codex_home.path())? - .args(["marketplace", "add", "--sparse", "plugins/foo", source]) - .assert() - .failure() - .stderr(contains(sparse_requires_git)); - - codex_command(codex_home.path())? - .args(["marketplace", "add", source, "--sparse", "plugins/foo"]) - .assert() - .failure() - .stderr(contains(sparse_requires_git)); - - codex_command(codex_home.path())? - .args([ - "marketplace", - "add", - "--sparse", - "plugins/foo", - "--sparse", - "skills/bar", - source, - ]) - .assert() - .failure() - .stderr(contains(sparse_requires_git)); - - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_rejects_ref_for_local_directory() -> Result<()> { +async fn marketplace_add_rejects_local_directory_source() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; write_marketplace_source(source.path(), "local ref")?; codex_command(codex_home.path())? - .args([ - "marketplace", - "add", - "--ref", - "main", - source.path().to_str().unwrap(), - ]) + .args(["marketplace", "add", source.path().to_str().unwrap()]) .assert() .failure() .stderr(contains( - "--ref can only be used with git marketplace sources", + "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo", )); assert!( diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 8ff0c74222d7..4ede3f7195ad 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -601,7 +601,6 @@ pub struct MarketplaceConfig { #[derive(Serialize, Deserialize, Debug, Clone, Copy, PartialEq, Eq, JsonSchema)] #[serde(rename_all = "snake_case")] pub enum MarketplaceSourceType { - Directory, Git, } diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 3dbbc5f9dd97..9173dcbc003d 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -794,7 +794,6 @@ }, "MarketplaceSourceType": { "enum": [ - "directory", "git" ], "type": "string" diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 22e17bf39366..4856ea771154 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -1518,7 +1518,7 @@ plugins = true [marketplaces.debug] last_updated = "2026-04-10T12:34:56Z" -source_type = "directory" +source_type = "git" source = "/tmp/debug" "#, ); @@ -1585,7 +1585,7 @@ plugins = true [marketplaces.debug] last_updated = "2026-04-10T12:34:56Z" -source_type = "directory" +source_type = "git" source = "/tmp/debug" "#, ); From 497f3ad091e71440af99ee4d0fd740a938b5c6f4 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 16:51:57 -0700 Subject: [PATCH 15/17] Remove redundant execpolicy legacy build script --- codex-rs/execpolicy-legacy/BUILD.bazel | 1 - codex-rs/execpolicy-legacy/build.rs | 3 --- 2 files changed, 4 deletions(-) delete mode 100644 codex-rs/execpolicy-legacy/build.rs diff --git a/codex-rs/execpolicy-legacy/BUILD.bazel b/codex-rs/execpolicy-legacy/BUILD.bazel index 87bb4e1378dc..489288472819 100644 --- a/codex-rs/execpolicy-legacy/BUILD.bazel +++ b/codex-rs/execpolicy-legacy/BUILD.bazel @@ -2,7 +2,6 @@ load("//:defs.bzl", "codex_rust_crate") codex_rust_crate( name = "execpolicy-legacy", - build_script_data = ["src/default.policy"], crate_name = "codex_execpolicy_legacy", compile_data = ["src/default.policy"], ) diff --git a/codex-rs/execpolicy-legacy/build.rs b/codex-rs/execpolicy-legacy/build.rs deleted file mode 100644 index eda4846853e3..000000000000 --- a/codex-rs/execpolicy-legacy/build.rs +++ /dev/null @@ -1,3 +0,0 @@ -fn main() { - println!("cargo:rerun-if-changed=src/default.policy"); -} From 09b9ea3d03c8086786692c583f38dc54b680cf26 Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 17:08:59 -0700 Subject: [PATCH 16/17] Fix marketplace argument comment lint --- codex-rs/cli/src/marketplace_cmd.rs | 40 ++++++++++++++--------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index c84e8b94dc75..6a898c9ad62b 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -398,7 +398,7 @@ mod tests { #[test] fn github_shorthand_parses_ref_suffix() { assert_eq!( - parse_marketplace_source("owner/repo@main", /* explicit_ref */ None).unwrap(), + 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()), @@ -411,7 +411,7 @@ mod tests { assert_eq!( parse_marketplace_source( "https://example.com/team/repo.git#v1", - /* explicit_ref */ None, + /*explicit_ref*/ None, ) .unwrap(), MarketplaceSource::Git { @@ -426,7 +426,7 @@ mod tests { assert_eq!( parse_marketplace_source( "owner/repo@main", - /* explicit_ref */ Some("release".to_string()), + /*explicit_ref*/ Some("release".to_string()), ) .unwrap(), MarketplaceSource::Git { @@ -438,10 +438,10 @@ mod tests { #[test] fn github_shorthand_and_git_url_normalize_to_same_source() { - let shorthand = parse_marketplace_source("owner/repo", /* explicit_ref */ None).unwrap(); + 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, + /*explicit_ref*/ None, ) .unwrap(); @@ -458,11 +458,8 @@ mod tests { #[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(), + 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, @@ -473,11 +470,8 @@ mod tests { #[test] fn non_github_https_source_parses_as_git_url() { assert_eq!( - parse_marketplace_source( - "https://gitlab.com/owner/repo", - /* explicit_ref */ None - ) - .unwrap(), + 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, @@ -488,7 +482,7 @@ mod tests { #[test] fn file_url_source_is_rejected() { let err = - parse_marketplace_source("file:///tmp/marketplace.git", /* explicit_ref */ None) + parse_marketplace_source("file:///tmp/marketplace.git", /*explicit_ref*/ None) .unwrap_err(); assert!( @@ -500,7 +494,7 @@ mod tests { #[test] fn local_path_source_is_rejected() { - let err = parse_marketplace_source("./marketplace", /* explicit_ref */ None).unwrap_err(); + let err = parse_marketplace_source("./marketplace", /*explicit_ref*/ None).unwrap_err(); assert!( err.to_string() @@ -514,7 +508,7 @@ mod tests { assert_eq!( parse_marketplace_source( "ssh://git@github.com/owner/repo.git#main", - /* explicit_ref */ None, + /*explicit_ref*/ None, ) .unwrap(), MarketplaceSource::Git { @@ -526,8 +520,14 @@ mod tests { #[test] fn utc_timestamp_formats_unix_epoch_as_rfc3339_utc() { - assert_eq!(format_utc_timestamp(0), "1970-01-01T00:00:00Z"); - assert_eq!(format_utc_timestamp(1_775_779_200), "2026-04-10T00:00:00Z"); + 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] From d44bbf3a8e01864c3f8b929bee91e829d55af7ae Mon Sep 17 00:00:00 2001 From: xli-oai Date: Fri, 10 Apr 2026 18:19:03 -0700 Subject: [PATCH 17/17] Save current marketplace-add branch state --- codex-rs/cli/tests/marketplace_add.rs | 518 +------------------------- codex-rs/execpolicy-legacy/build.rs | 3 + 2 files changed, 10 insertions(+), 511 deletions(-) create mode 100644 codex-rs/execpolicy-legacy/build.rs diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index 190fc619e395..1a9db02d7422 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -1,26 +1,7 @@ -use anyhow::Context; use anyhow::Result; use codex_core::plugins::marketplace_install_root; -use codex_core::plugins::validate_marketplace_root; use predicates::str::contains; -use pretty_assertions::assert_eq; -use std::ffi::OsStr; -use std::ffi::OsString; -use std::io::Read; -use std::io::Write; -use std::net::SocketAddr; -use std::net::TcpListener; -use std::net::TcpStream; -use std::path::Component; use std::path::Path; -use std::path::PathBuf; -use std::process::Command; -use std::sync::Arc; -use std::sync::atomic::AtomicBool; -use std::sync::atomic::Ordering; -use std::thread; -use std::thread::JoinHandle; -use std::time::Duration; use tempfile::TempDir; fn codex_command(codex_home: &Path) -> Result { @@ -30,32 +11,22 @@ fn codex_command(codex_home: &Path) -> Result { } fn write_marketplace_source(source: &Path, marker: &str) -> Result<()> { - write_marketplace_source_with_name(source, "debug", marker) -} - -fn write_marketplace_source_with_name( - 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}", + r#"{ + "name": "debug", "plugins": [ - {{ + { "name": "sample", - "source": {{ + "source": { "source": "local", "path": "./plugins/sample" - }} - }} + } + } ] -}}"# - ), +}"#, )?; std::fs::write( source.join("plugins/sample/.codex-plugin/plugin.json"), @@ -65,481 +36,6 @@ fn write_marketplace_source_with_name( Ok(()) } -struct RemoteGitMarketplace { - _server: HttpFileServer, - repo_path: PathBuf, - url: String, -} - -struct HttpFileServer { - _root: TempDir, - address: SocketAddr, - shutdown: Arc, - handle: Option>, -} - -fn write_git_marketplace_source(source: &Path, marker: &str) -> Result { - write_marketplace_source(source, marker)?; - init_git_repo(source)?; - RemoteGitMarketplace::from_source(source) -} - -fn write_git_marketplace_source_with_name( - source: &Path, - marketplace_name: &str, - marker: &str, -) -> Result { - write_marketplace_source_with_name(source, marketplace_name, marker)?; - init_git_repo(source)?; - RemoteGitMarketplace::from_source(source) -} - -fn init_git_repo(source: &Path) -> Result<()> { - run_git(source, ["init"])?; - run_git(source, ["config", "user.email", "codex@example.com"])?; - run_git(source, ["config", "user.name", "Codex Test"])?; - commit_git_repo(source, "initial marketplace")?; - Ok(()) -} - -fn commit_git_repo(source: &Path, message: &str) -> Result<()> { - run_git(source, ["add", "."])?; - run_git(source, ["commit", "-m", message])?; - Ok(()) -} - -fn run_git(cwd: &Path, args: I) -> Result<()> -where - I: IntoIterator, - S: AsRef, -{ - let args = args - .into_iter() - .map(|arg| arg.as_ref().to_os_string()) - .collect::>(); - let output = Command::new("git").current_dir(cwd).args(&args).output()?; - let args_display = args - .iter() - .map(|arg| arg.to_string_lossy()) - .collect::>() - .join(" "); - assert!( - output.status.success(), - "git {} failed\nstdout:\n{}\nstderr:\n{}", - args_display, - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); - Ok(()) -} - -impl RemoteGitMarketplace { - fn from_source(source: &Path) -> Result { - let root = TempDir::new()?; - let repo_name = "marketplace.git"; - let repo_path = root.path().join(repo_name); - clone_bare_repo(source, root.path(), repo_name)?; - let server = HttpFileServer::start(root)?; - let url = format!("http://{}/{repo_name}", server.address); - Ok(Self { - _server: server, - repo_path, - url, - }) - } - - fn refresh_from_source(&self, source: &Path) -> Result<()> { - std::fs::remove_dir_all(&self.repo_path)?; - let parent = self - .repo_path - .parent() - .context("served git repository should have a parent")?; - let repo_name = self - .repo_path - .file_name() - .context("served git repository should have a file name")? - .to_string_lossy() - .to_string(); - clone_bare_repo(source, parent, &repo_name) - } -} - -fn clone_bare_repo(source: &Path, target_parent: &Path, repo_name: &str) -> Result<()> { - run_git( - target_parent, - [ - OsString::from("clone"), - OsString::from("--bare"), - source.as_os_str().to_os_string(), - OsString::from(repo_name), - ], - )?; - run_git( - target_parent.join(repo_name).as_path(), - ["update-server-info"], - )?; - Ok(()) -} - -impl HttpFileServer { - fn start(root: TempDir) -> Result { - let listener = TcpListener::bind(("127.0.0.1", 0))?; - let address = listener.local_addr()?; - listener.set_nonblocking(true)?; - - let shutdown = Arc::new(AtomicBool::new(false)); - let thread_shutdown = Arc::clone(&shutdown); - let root_path = root.path().to_path_buf(); - let handle = thread::spawn(move || { - while !thread_shutdown.load(Ordering::Relaxed) { - match listener.accept() { - Ok((mut stream, _)) => { - let _ = serve_http_file(&root_path, &mut stream); - } - Err(err) if err.kind() == std::io::ErrorKind::WouldBlock => { - thread::sleep(Duration::from_millis(10)); - } - Err(_) => break, - } - } - }); - - Ok(Self { - _root: root, - address, - shutdown, - handle: Some(handle), - }) - } -} - -impl Drop for HttpFileServer { - fn drop(&mut self) { - self.shutdown.store(true, Ordering::Relaxed); - let _ = TcpStream::connect(self.address); - if let Some(handle) = self.handle.take() { - let _ = handle.join(); - } - } -} - -fn serve_http_file(root: &Path, stream: &mut TcpStream) -> Result<()> { - stream.set_read_timeout(Some(Duration::from_secs(5)))?; - let mut request = Vec::new(); - let mut buffer = [0_u8; 1024]; - while !request.windows(4).any(|window| window == b"\r\n\r\n") { - let bytes_read = stream.read(&mut buffer)?; - if bytes_read == 0 { - break; - } - request.extend_from_slice(&buffer[..bytes_read]); - if request.len() > 16 * 1024 { - write_http_response(stream, "413 Payload Too Large", &[])?; - return Ok(()); - } - } - - let request = String::from_utf8_lossy(&request); - let Some(request_line) = request.lines().next() else { - write_http_response(stream, "400 Bad Request", &[])?; - return Ok(()); - }; - let mut parts = request_line.split_whitespace(); - let method = parts.next().unwrap_or_default(); - let target = parts.next().unwrap_or_default(); - if method != "GET" && method != "HEAD" { - write_http_response(stream, "405 Method Not Allowed", &[])?; - return Ok(()); - } - - let Some(relative_path) = safe_http_path(target) else { - write_http_response(stream, "400 Bad Request", &[])?; - return Ok(()); - }; - let path = root.join(relative_path); - if !path.is_file() { - write_http_response(stream, "404 Not Found", &[])?; - return Ok(()); - } - - let body = std::fs::read(path)?; - if method == "HEAD" { - write_http_response(stream, "200 OK", &[])?; - } else { - write_http_response(stream, "200 OK", &body)?; - } - Ok(()) -} - -fn safe_http_path(target: &str) -> Option { - let path = target.split('?').next()?.trim_start_matches('/'); - let path = percent_decode(path)?; - let relative_path = PathBuf::from(path); - if relative_path.components().any(|component| { - matches!( - component, - Component::ParentDir | Component::RootDir | Component::Prefix(_) - ) - }) { - return None; - } - Some(relative_path) -} - -fn percent_decode(input: &str) -> Option { - let bytes = input.as_bytes(); - let mut output = Vec::with_capacity(bytes.len()); - let mut index = 0; - while index < bytes.len() { - if bytes[index] == b'%' { - let high = *bytes.get(index + 1)?; - let low = *bytes.get(index + 2)?; - output.push(hex_value(high)? * 16 + hex_value(low)?); - index += 3; - } else { - output.push(bytes[index]); - index += 1; - } - } - String::from_utf8(output).ok() -} - -fn hex_value(byte: u8) -> Option { - match byte { - b'0'..=b'9' => Some(byte - b'0'), - b'a'..=b'f' => Some(byte - b'a' + 10), - b'A'..=b'F' => Some(byte - b'A' + 10), - _ => None, - } -} - -fn write_http_response(stream: &mut TcpStream, status: &str, body: &[u8]) -> Result<()> { - write!( - stream, - "HTTP/1.1 {status}\r\nContent-Length: {}\r\nContent-Type: application/octet-stream\r\nConnection: close\r\n\r\n", - body.len() - )?; - stream.write_all(body)?; - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_git_source_installs_valid_marketplace_root() -> Result<()> { - let codex_home = TempDir::new()?; - let source = TempDir::new()?; - let source = write_git_marketplace_source(source.path(), "first install")?; - - let mut add_cmd = codex_command(codex_home.path())?; - add_cmd - .args(["marketplace", "add", &source.url]) - .assert() - .success() - .stdout(contains("Added marketplace `debug`")); - - let installed_root = marketplace_install_root(codex_home.path()).join("debug"); - assert_eq!(validate_marketplace_root(&installed_root)?, "debug"); - assert_git_marketplace_config(codex_home.path(), "debug", &source.url)?; - assert!( - installed_root - .join("plugins/sample/.codex-plugin/plugin.json") - .is_file() - ); - assert!(!installed_root.join(".codex-marketplace-source").exists()); - assert!( - !codex_home - .path() - .join(".tmp/known_marketplaces.json") - .exists() - ); - - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_rejects_invalid_marketplace_name() -> Result<()> { - let codex_home = TempDir::new()?; - let source = TempDir::new()?; - let source = write_git_marketplace_source_with_name( - source.path(), - "debug.market", - "invalid marketplace", - )?; - - codex_command(codex_home.path())? - .args(["marketplace", "add", &source.url]) - .assert() - .failure() - .stderr(contains( - "invalid marketplace name: only ASCII letters, digits, `_`, and `-` are allowed", - )); - - assert!( - !marketplace_install_root(codex_home.path()) - .join("debug.market") - .exists() - ); - assert!( - !codex_home - .path() - .join(".tmp/known_marketplaces.json") - .exists() - ); - - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_same_source_is_idempotent() -> Result<()> { - let codex_home = TempDir::new()?; - let source = TempDir::new()?; - let served_source = write_git_marketplace_source(source.path(), "first install")?; - - codex_command(codex_home.path())? - .args(["marketplace", "add", &served_source.url]) - .assert() - .success() - .stdout(contains("Added marketplace `debug`")); - - std::fs::write( - source.path().join("plugins/sample/marker.txt"), - "source changed after add", - )?; - commit_git_repo(source.path(), "update marketplace")?; - served_source.refresh_from_source(source.path())?; - - codex_command(codex_home.path())? - .args(["marketplace", "add", &served_source.url]) - .assert() - .success() - .stdout(contains("Marketplace `debug` is already added")); - - let installed_root = marketplace_install_root(codex_home.path()).join("debug"); - assert_eq!( - std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, - "first install" - ); - assert_git_marketplace_config(codex_home.path(), "debug", &served_source.url)?; - assert!(!installed_root.join(".codex-marketplace-source").exists()); - assert!( - !codex_home - .path() - .join(".tmp/known_marketplaces.json") - .exists() - ); - - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_rejects_same_name_from_different_source() -> Result<()> { - let codex_home = TempDir::new()?; - let first_source = TempDir::new()?; - let second_source = TempDir::new()?; - let first_source = write_git_marketplace_source(first_source.path(), "first install")?; - let second_source = write_git_marketplace_source(second_source.path(), "replacement install")?; - - codex_command(codex_home.path())? - .args(["marketplace", "add", &first_source.url]) - .assert() - .success() - .stdout(contains("Added marketplace `debug`")); - - codex_command(codex_home.path())? - .args(["marketplace", "add", &second_source.url]) - .assert() - .failure() - .stderr(contains( - "marketplace `debug` is already added from a different source", - )); - - let installed_root = marketplace_install_root(codex_home.path()).join("debug"); - assert_eq!( - std::fs::read_to_string(installed_root.join("plugins/sample/marker.txt"))?, - "first install" - ); - - Ok(()) -} - -#[tokio::test] -async fn marketplace_add_rolls_back_install_when_config_write_fails() -> Result<()> { - let codex_home = TempDir::new()?; - let source = TempDir::new()?; - let source = write_git_marketplace_source(source.path(), "rollback")?; - - let config_path = codex_home.path().join("config.toml"); - std::fs::write(&config_path, "")?; - let original_permissions = std::fs::metadata(&config_path)?.permissions(); - let mut read_only_permissions = original_permissions.clone(); - read_only_permissions.set_readonly(true); - std::fs::set_permissions(&config_path, read_only_permissions)?; - - let output = codex_command(codex_home.path())? - .args(["marketplace", "add", &source.url]) - .output()?; - - std::fs::set_permissions(&config_path, original_permissions)?; - - assert!( - !output.status.success(), - "command unexpectedly succeeded: stdout={}, stderr={}", - String::from_utf8_lossy(&output.stdout), - String::from_utf8_lossy(&output.stderr) - ); - assert!( - String::from_utf8_lossy(&output.stderr) - .contains("failed to add marketplace `debug` to user config.toml"), - "unexpected stderr: {}", - String::from_utf8_lossy(&output.stderr) - ); - assert!( - !marketplace_install_root(codex_home.path()) - .join("debug") - .exists(), - "installed marketplace should be rolled back when config.toml cannot be persisted" - ); - - Ok(()) -} - -fn assert_git_marketplace_config( - codex_home: &Path, - marketplace_name: &str, - source_url: &str, -) -> Result<()> { - let config = std::fs::read_to_string(codex_home.join("config.toml"))?; - let config: toml::Value = toml::from_str(&config)?; - let marketplace = config - .get("marketplaces") - .and_then(|marketplaces| marketplaces.get(marketplace_name)) - .context("marketplace config should be written")?; - - assert!( - marketplace - .get("last_updated") - .and_then(toml::Value::as_str) - .is_some_and(|last_updated| { - last_updated.len() == "2026-04-10T12:34:56Z".len() && last_updated.ends_with('Z') - }), - "last_updated should be an RFC3339-like UTC timestamp" - ); - assert_eq!( - marketplace.get("source_type").and_then(toml::Value::as_str), - Some("git") - ); - assert_eq!( - marketplace.get("source").and_then(toml::Value::as_str), - Some(source_url) - ); - assert_eq!(marketplace.get("ref").and_then(toml::Value::as_str), None); - assert!(marketplace.get("sparse_paths").is_none()); - assert!(marketplace.get("source_id").is_none()); - assert!(marketplace.get("install_root").is_none()); - assert!(marketplace.get("install_location").is_none()); - - Ok(()) -} - #[tokio::test] async fn marketplace_add_rejects_local_directory_source() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/execpolicy-legacy/build.rs b/codex-rs/execpolicy-legacy/build.rs new file mode 100644 index 000000000000..eda4846853e3 --- /dev/null +++ b/codex-rs/execpolicy-legacy/build.rs @@ -0,0 +1,3 @@ +fn main() { + println!("cargo:rerun-if-changed=src/default.policy"); +}