diff --git a/codex-rs/app-server/tests/suite/v2/marketplace_add.rs b/codex-rs/app-server/tests/suite/v2/marketplace_add.rs index 8e81d6865497..cf3c57360ffd 100644 --- a/codex-rs/app-server/tests/suite/v2/marketplace_add.rs +++ b/codex-rs/app-server/tests/suite/v2/marketplace_add.rs @@ -1,7 +1,11 @@ use anyhow::Result; use app_test_support::McpProcess; +use app_test_support::to_response; +use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::MarketplaceAddParams; +use codex_app_server_protocol::MarketplaceAddResponse; use codex_app_server_protocol::RequestId; +use pretty_assertions::assert_eq; use tempfile::TempDir; use tokio::time::Duration; use tokio::time::timeout; @@ -9,8 +13,20 @@ use tokio::time::timeout; const DEFAULT_TIMEOUT: Duration = Duration::from_secs(10); #[tokio::test] -async fn marketplace_add_rejects_local_directory_source() -> Result<()> { +async fn marketplace_add_local_directory_source() -> Result<()> { let codex_home = TempDir::new()?; + let source = codex_home.path().join("marketplace"); + 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":[]}"#, + )?; + std::fs::write( + source.join("plugins/sample/.codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + )?; + std::fs::write(source.join("plugins/sample/marker.txt"), "local ref")?; let mut mcp = McpProcess::new(codex_home.path()).await?; timeout(DEFAULT_TIMEOUT, mcp.initialize()).await??; @@ -22,19 +38,24 @@ async fn marketplace_add_rejects_local_directory_source() -> Result<()> { }) .await?; - let err = timeout( + let response: JSONRPCResponse = timeout( DEFAULT_TIMEOUT, - mcp.read_stream_until_error_message(RequestId::Integer(request_id)), + mcp.read_stream_until_response_message(RequestId::Integer(request_id)), ) .await??; + let MarketplaceAddResponse { + marketplace_name, + installed_root, + already_added, + } = to_response(response)?; + let expected_root = source.canonicalize()?; - assert_eq!(err.error.code, -32600); - assert!( - err.error.message.contains( - "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo" - ), - "unexpected error: {}", - err.error.message + assert_eq!(marketplace_name, "debug"); + assert_eq!(installed_root.as_path(), expected_root.as_path()); + assert!(!already_added); + assert_eq!( + std::fs::read_to_string(installed_root.as_path().join("plugins/sample/marker.txt"))?, + "local ref" ); Ok(()) } diff --git a/codex-rs/cli/src/marketplace_cmd.rs b/codex-rs/cli/src/marketplace_cmd.rs index b21e25a3934c..cde50a2bf4ae 100644 --- a/codex-rs/cli/src/marketplace_cmd.rs +++ b/codex-rs/cli/src/marketplace_cmd.rs @@ -23,7 +23,8 @@ enum MarketplaceSubcommand { #[derive(Debug, Parser)] struct AddMarketplaceArgs { - /// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, or SSH URLs. + /// Marketplace source. Supports owner/repo[@ref], HTTP(S) Git URLs, SSH URLs, + /// or local marketplace root directories. source: String, /// Git ref to check out. Overrides any @ref or #ref suffix in SOURCE. diff --git a/codex-rs/cli/tests/marketplace_add.rs b/codex-rs/cli/tests/marketplace_add.rs index 9cc5c65a5cdd..e04cfb579168 100644 --- a/codex-rs/cli/tests/marketplace_add.rs +++ b/codex-rs/cli/tests/marketplace_add.rs @@ -1,6 +1,8 @@ use anyhow::Result; +use codex_config::CONFIG_TOML_FILE; use codex_core::plugins::marketplace_install_root; use predicates::str::contains; +use pretty_assertions::assert_eq; use std::path::Path; use tempfile::TempDir; @@ -37,7 +39,7 @@ fn write_marketplace_source(source: &Path, marker: &str) -> Result<()> { } #[tokio::test] -async fn marketplace_add_rejects_local_directory_source() -> Result<()> { +async fn marketplace_add_local_directory_source() -> Result<()> { let codex_home = TempDir::new()?; let source = TempDir::new()?; write_marketplace_source(source.path(), "local ref")?; @@ -48,16 +50,40 @@ async fn marketplace_add_rejects_local_directory_source() -> Result<()> { .current_dir(source_parent) .args(["marketplace", "add", source_arg.as_str()]) .assert() + .success(); + + let installed_root = marketplace_install_root(codex_home.path()).join("debug"); + assert!(!installed_root.exists()); + + let config = std::fs::read_to_string(codex_home.path().join(CONFIG_TOML_FILE))?; + let config: toml::Value = toml::from_str(&config)?; + let expected_source = source.path().canonicalize()?.display().to_string(); + assert_eq!( + config["marketplaces"]["debug"]["source_type"].as_str(), + Some("local") + ); + assert_eq!( + config["marketplaces"]["debug"]["source"].as_str(), + Some(expected_source.as_str()) + ); + + Ok(()) +} + +#[tokio::test] +async fn marketplace_add_rejects_local_manifest_file_source() -> Result<()> { + let codex_home = TempDir::new()?; + let source = TempDir::new()?; + write_marketplace_source(source.path(), "local ref")?; + let manifest_path = source.path().join(".agents/plugins/marketplace.json"); + + codex_command(codex_home.path())? + .args(["marketplace", "add", manifest_path.to_str().unwrap()]) + .assert() .failure() .stderr(contains( - "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo", + "local marketplace source must be a directory, not a file", )); - assert!( - !marketplace_install_root(codex_home.path()) - .join("debug") - .exists() - ); - Ok(()) } diff --git a/codex-rs/config/src/types.rs b/codex-rs/config/src/types.rs index 64cfe85d085c..73a5763fe1e2 100644 --- a/codex-rs/config/src/types.rs +++ b/codex-rs/config/src/types.rs @@ -632,6 +632,7 @@ pub struct MarketplaceConfig { #[serde(rename_all = "snake_case")] pub enum MarketplaceSourceType { Git, + Local, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Default, JsonSchema)] diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 7c3f6dd99a74..d5d832d6cb16 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -810,7 +810,8 @@ }, "MarketplaceSourceType": { "enum": [ - "git" + "git", + "local" ], "type": "string" }, diff --git a/codex-rs/core/src/plugins/installed_marketplaces.rs b/codex-rs/core/src/plugins/installed_marketplaces.rs index edfc7d895aa9..df69111a2736 100644 --- a/codex-rs/core/src/plugins/installed_marketplaces.rs +++ b/codex-rs/core/src/plugins/installed_marketplaces.rs @@ -45,7 +45,11 @@ pub(crate) fn installed_marketplace_roots_from_config( ); return None; } - let path = default_install_root.join(marketplace_name); + let path = resolve_configured_marketplace_root( + marketplace_name, + marketplace, + &default_install_root, + )?; path.join(".agents/plugins/marketplace.json") .is_file() .then_some(path) @@ -55,3 +59,18 @@ pub(crate) fn installed_marketplace_roots_from_config( roots.sort_unstable_by(|left, right| left.as_path().cmp(right.as_path())); roots } + +pub(crate) fn resolve_configured_marketplace_root( + marketplace_name: &str, + marketplace: &toml::Value, + default_install_root: &Path, +) -> Option { + match marketplace.get("source_type").and_then(toml::Value::as_str) { + Some("local") => marketplace + .get("source") + .and_then(toml::Value::as_str) + .filter(|source| !source.is_empty()) + .map(PathBuf::from), + _ => Some(default_install_root.join(marketplace_name)), + } +} diff --git a/codex-rs/core/src/plugins/marketplace_add.rs b/codex-rs/core/src/plugins/marketplace_add.rs index 55c50099a7a2..49c1c2e565be 100644 --- a/codex-rs/core/src/plugins/marketplace_add.rs +++ b/codex-rs/core/src/plugins/marketplace_add.rs @@ -16,10 +16,12 @@ use install::marketplace_staging_root; use install::replace_marketplace_root; use install::safe_marketplace_dir_name; use metadata::MarketplaceInstallMetadata; +use metadata::find_marketplace_root_by_name; use metadata::installed_marketplace_root_for_source; use metadata::record_added_marketplace_entry; use source::MarketplaceSource; use source::parse_marketplace_source; +use source::stage_marketplace_source; use source::validate_marketplace_source_root; #[derive(Debug, Clone, PartialEq, Eq)] @@ -102,6 +104,33 @@ where }); } + if let MarketplaceSource::Local { path } = &source { + let marketplace_name = validate_marketplace_source_root(path)?; + if marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME { + return Err(MarketplaceAddError::InvalidRequest(format!( + "marketplace '{OPENAI_CURATED_MARKETPLACE_NAME}' is reserved and cannot be added from {}", + source.display() + ))); + } + if find_marketplace_root_by_name(codex_home, &install_root, &marketplace_name)?.is_some() { + return Err(MarketplaceAddError::InvalidRequest(format!( + "marketplace '{marketplace_name}' is already added from a different source; remove it before adding {}", + source.display() + ))); + } + record_added_marketplace_entry(codex_home, &marketplace_name, &install_metadata)?; + return Ok(MarketplaceAddOutcome { + marketplace_name, + source_display: source.display(), + installed_root: AbsolutePathBuf::try_from(path.clone()).map_err(|err| { + MarketplaceAddError::Internal(format!( + "failed to resolve installed marketplace root: {err}" + )) + })?, + already_added: false, + }); + } + let staging_root = marketplace_staging_root(&install_root); fs::create_dir_all(&staging_root).map_err(|err| { MarketplaceAddError::Internal(format!( @@ -120,8 +149,7 @@ where })?; let staged_root = staged_root.keep(); - let MarketplaceSource::Git { url, ref_name } = &source; - clone_source(url, ref_name.as_deref(), &sparse_paths, &staged_root)?; + stage_marketplace_source(&source, &sparse_paths, &staged_root, clone_source)?; let marketplace_name = validate_marketplace_source_root(&staged_root)?; if marketplace_name == OPENAI_CURATED_MARKETPLACE_NAME { @@ -214,6 +242,75 @@ mod tests { Ok(()) } + #[test] + fn add_marketplace_sync_installs_local_directory_source_and_updates_config() -> Result<()> { + let codex_home = TempDir::new()?; + let source_root = TempDir::new()?; + write_marketplace_source(source_root.path(), "local copy")?; + + let result = add_marketplace_sync_with_cloner( + codex_home.path(), + MarketplaceAddRequest { + source: source_root.path().display().to_string(), + ref_name: None, + sparse_paths: Vec::new(), + }, + |_url, _ref_name, _sparse_paths, _destination| { + panic!("git cloner should not be called for local marketplace sources") + }, + )?; + + let expected_source = source_root.path().canonicalize()?.display().to_string(); + assert_eq!(result.marketplace_name, "debug"); + assert_eq!(result.source_display, expected_source); + assert_eq!( + result.installed_root.as_path(), + source_root.path().canonicalize()? + ); + assert!(!result.already_added); + assert!( + !marketplace_install_root(codex_home.path()) + .join("debug") + .exists() + ); + + let config = fs::read_to_string(codex_home.path().join(codex_config::CONFIG_TOML_FILE))?; + assert!(config.contains("[marketplaces.debug]")); + assert!(config.contains("source_type = \"local\"")); + assert!(config.contains(&format!("source = \"{expected_source}\""))); + Ok(()) + } + + #[test] + fn add_marketplace_sync_treats_existing_local_directory_source_as_already_added() -> Result<()> + { + let codex_home = TempDir::new()?; + let source_root = TempDir::new()?; + write_marketplace_source(source_root.path(), "local copy")?; + + let request = MarketplaceAddRequest { + source: source_root.path().display().to_string(), + ref_name: None, + sparse_paths: Vec::new(), + }; + let first_result = add_marketplace_sync_with_cloner(codex_home.path(), request.clone(), { + |_url, _ref_name, _sparse_paths, _destination| { + panic!("git cloner should not be called for local marketplace sources") + } + })?; + let second_result = add_marketplace_sync_with_cloner(codex_home.path(), request, { + |_url, _ref_name, _sparse_paths, _destination| { + panic!("git cloner should not be called for local marketplace sources") + } + })?; + + assert!(!first_result.already_added); + assert!(second_result.already_added); + assert_eq!(second_result.installed_root, first_result.installed_root); + + Ok(()) + } + fn write_marketplace_source(source: &Path, marker: &str) -> std::io::Result<()> { fs::create_dir_all(source.join(".agents/plugins"))?; fs::create_dir_all(source.join("plugins/sample/.codex-plugin"))?; diff --git a/codex-rs/core/src/plugins/marketplace_add/metadata.rs b/codex-rs/core/src/plugins/marketplace_add/metadata.rs index 9ee27a9df57c..ce271b116d80 100644 --- a/codex-rs/core/src/plugins/marketplace_add/metadata.rs +++ b/codex-rs/core/src/plugins/marketplace_add/metadata.rs @@ -1,5 +1,6 @@ use super::MarketplaceAddError; use super::MarketplaceSource; +use crate::plugins::installed_marketplaces::resolve_configured_marketplace_root; use crate::plugins::validate_marketplace_root; use codex_config::CONFIG_TOML_FILE; use codex_config::MarketplaceConfigUpdate; @@ -23,6 +24,9 @@ enum InstalledMarketplaceSource { ref_name: Option, sparse_paths: Vec, }, + Local { + path: String, + }, } pub(super) fn record_added_marketplace_entry( @@ -77,7 +81,11 @@ pub(super) fn installed_marketplace_root_for_source( if !install_metadata.matches_config(marketplace) { continue; } - let root = install_root.join(marketplace_name); + let Some(root) = + resolve_configured_marketplace_root(marketplace_name, marketplace, install_root) + else { + continue; + }; if validate_marketplace_root(&root).is_ok() { return Ok(Some(root)); } @@ -86,6 +94,48 @@ pub(super) fn installed_marketplace_root_for_source( Ok(None) } +pub(super) fn find_marketplace_root_by_name( + codex_home: &Path, + install_root: &Path, + marketplace_name: &str, +) -> Result, MarketplaceAddError> { + let config_path = codex_home.join(CONFIG_TOML_FILE); + let config = match fs::read_to_string(&config_path) { + Ok(config) => config, + Err(err) if err.kind() == ErrorKind::NotFound => return Ok(None), + Err(err) => { + return Err(MarketplaceAddError::Internal(format!( + "failed to read user config {}: {err}", + config_path.display() + ))); + } + }; + let config: toml::Value = toml::from_str(&config).map_err(|err| { + MarketplaceAddError::Internal(format!( + "failed to parse user config {}: {err}", + config_path.display() + )) + })?; + let Some(marketplace) = config + .get("marketplaces") + .and_then(toml::Value::as_table) + .and_then(|marketplaces| marketplaces.get(marketplace_name)) + else { + return Ok(None); + }; + + let Some(root) = + resolve_configured_marketplace_root(marketplace_name, marketplace, install_root) + else { + return Ok(None); + }; + if validate_marketplace_root(&root).is_ok() { + Ok(Some(root)) + } else { + Ok(None) + } +} + impl MarketplaceInstallMetadata { pub(super) fn from_source(source: &MarketplaceSource, sparse_paths: &[String]) -> Self { let source = match source { @@ -94,6 +144,9 @@ impl MarketplaceInstallMetadata { ref_name: ref_name.clone(), sparse_paths: sparse_paths.to_vec(), }, + MarketplaceSource::Local { path } => InstalledMarketplaceSource::Local { + path: path.display().to_string(), + }, }; Self { source } } @@ -101,24 +154,28 @@ impl MarketplaceInstallMetadata { fn config_source_type(&self) -> &'static str { match &self.source { InstalledMarketplaceSource::Git { .. } => "git", + InstalledMarketplaceSource::Local { .. } => "local", } } fn config_source(&self) -> String { match &self.source { InstalledMarketplaceSource::Git { url, .. } => url.clone(), + InstalledMarketplaceSource::Local { path } => path.clone(), } } fn ref_name(&self) -> Option<&str> { match &self.source { InstalledMarketplaceSource::Git { ref_name, .. } => ref_name.as_deref(), + InstalledMarketplaceSource::Local { .. } => None, } } fn sparse_paths(&self) -> &[String] { match &self.source { InstalledMarketplaceSource::Git { sparse_paths, .. } => sparse_paths, + InstalledMarketplaceSource::Local { .. } => &[], } } @@ -227,4 +284,39 @@ mod tests { "unexpected error: {err}" ); } + + #[test] + fn installed_marketplace_root_for_source_uses_local_source_root() { + let codex_home = TempDir::new().unwrap(); + let install_root = codex_home.path().join("marketplaces"); + let source_root = codex_home.path().join("source"); + fs::create_dir_all(source_root.join(".agents/plugins")).unwrap(); + fs::write( + source_root.join(".agents/plugins/marketplace.json"), + r#"{"name":"debug","plugins":[]}"#, + ) + .unwrap(); + fs::write( + codex_home.path().join(CONFIG_TOML_FILE), + format!( + "[marketplaces.debug]\nsource_type = \"local\"\nsource = \"{}\"\n", + source_root.display() + ), + ) + .unwrap(); + + let source = MarketplaceSource::Local { + path: source_root.clone(), + }; + let install_metadata = MarketplaceInstallMetadata::from_source(&source, &[]); + + let root = installed_marketplace_root_for_source( + codex_home.path(), + &install_root, + &install_metadata, + ) + .unwrap(); + + assert_eq!(root, Some(source_root)); + } } diff --git a/codex-rs/core/src/plugins/marketplace_add/source.rs b/codex-rs/core/src/plugins/marketplace_add/source.rs index bb3d746a01d3..19fec57ade4c 100644 --- a/codex-rs/core/src/plugins/marketplace_add/source.rs +++ b/codex-rs/core/src/plugins/marketplace_add/source.rs @@ -2,6 +2,7 @@ use super::MarketplaceAddError; use crate::plugins::validate_marketplace_root; use crate::plugins::validate_plugin_segment; use std::path::Path; +use std::path::PathBuf; #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum MarketplaceSource { @@ -9,6 +10,9 @@ pub(super) enum MarketplaceSource { url: String, ref_name: Option, }, + Local { + path: PathBuf, + }, } pub(super) fn parse_marketplace_source( @@ -26,9 +30,18 @@ pub(super) fn parse_marketplace_source( let ref_name = explicit_ref.or(parsed_ref); if looks_like_local_path(&base_source) { - return Err(MarketplaceAddError::InvalidRequest( - "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo".to_string(), - )); + if ref_name.is_some() { + return Err(MarketplaceAddError::InvalidRequest( + "--ref is only supported for git marketplace sources".to_string(), + )); + } + let path = resolve_local_source_path(&base_source)?; + if path.is_file() { + return Err(MarketplaceAddError::InvalidRequest( + "local marketplace source must be a directory, not a file".to_string(), + )); + } + return Ok(MarketplaceSource::Local { path }); } if is_ssh_git_url(&base_source) || is_git_url(&base_source) { @@ -50,6 +63,31 @@ pub(super) fn parse_marketplace_source( ))) } +pub(super) fn stage_marketplace_source( + source: &MarketplaceSource, + sparse_paths: &[String], + staged_root: &Path, + clone_source: F, +) -> Result<(), MarketplaceAddError> +where + F: Fn(&str, Option<&str>, &[String], &Path) -> Result<(), MarketplaceAddError>, +{ + if !sparse_paths.is_empty() && !matches!(source, MarketplaceSource::Git { .. }) { + return Err(MarketplaceAddError::InvalidRequest( + "--sparse is only supported for git marketplace sources".to_string(), + )); + } + + match source { + MarketplaceSource::Git { url, ref_name } => { + clone_source(url, ref_name.as_deref(), sparse_paths, staged_root) + } + MarketplaceSource::Local { .. } => unreachable!( + "local marketplace sources are added without staging a copied install root" + ), + } +} + pub(super) fn validate_marketplace_source_root(root: &Path) -> Result { let marketplace_name = validate_marketplace_root(root) .map_err(|err| MarketplaceAddError::InvalidRequest(err.to_string()))?; @@ -94,6 +132,38 @@ fn looks_like_local_path(source: &str) -> bool { || source == ".." } +fn resolve_local_source_path(source: &str) -> Result { + let path = expand_tilde_path(source); + let path = if path.is_absolute() { + path + } else { + std::env::current_dir() + .map_err(|err| { + MarketplaceAddError::Internal(format!( + "failed to read current working directory for local marketplace source: {err}" + )) + })? + .join(path) + }; + + path.canonicalize().map_err(|err| { + MarketplaceAddError::InvalidRequest(format!( + "failed to resolve local marketplace source {}: {err}", + path.display() + )) + }) +} + +fn expand_tilde_path(source: &str) -> PathBuf { + let Some(rest) = source.strip_prefix("~/") else { + return PathBuf::from(source); + }; + let Some(home) = std::env::var_os("HOME").or_else(|| std::env::var_os("USERPROFILE")) else { + return PathBuf::from(source); + }; + PathBuf::from(home).join(rest) +} + fn is_ssh_git_url(source: &str) -> bool { source.starts_with("ssh://") || source.starts_with("git@") && source.contains(':') } @@ -126,6 +196,7 @@ impl MarketplaceSource { Some(ref_name) => format!("{url}#{ref_name}"), None => url.clone(), }, + Self::Local { path } => path.display().to_string(), } } } @@ -134,6 +205,7 @@ impl MarketplaceSource { mod tests { use super::*; use pretty_assertions::assert_eq; + use tempfile::TempDir; #[test] fn github_shorthand_parses_ref_suffix() { @@ -229,12 +301,57 @@ mod tests { } #[test] - fn parse_marketplace_source_rejects_local_directory_source() { - let err = parse_marketplace_source("./marketplace", /*explicit_ref*/ None).unwrap_err(); + fn local_path_source_parses() { + let source = parse_marketplace_source(".", /*explicit_ref*/ None).unwrap(); - assert_eq!( - err.to_string(), - "local marketplace sources are not supported yet; use an HTTP(S) Git URL, SSH Git URL, or GitHub owner/repo" + let MarketplaceSource::Local { path } = source else { + panic!("expected local path source"); + }; + assert!(path.is_absolute()); + } + + #[test] + fn local_file_source_is_rejected() { + let tempdir = TempDir::new().unwrap(); + let file = tempdir.path().join("marketplace.json"); + std::fs::write(&file, "{}").unwrap(); + + let err = + parse_marketplace_source(file.to_str().unwrap(), /*explicit_ref*/ None).unwrap_err(); + + assert!( + err.to_string() + .contains("local marketplace source must be a directory, not a file"), + "unexpected error: {err}" + ); + } + + #[test] + fn non_git_sources_reject_ref_override() { + let err = parse_marketplace_source("./marketplace", Some("main".to_string())).unwrap_err(); + + assert!( + err.to_string() + .contains("--ref is only supported for git marketplace sources"), + "unexpected error: {err}" + ); + } + + #[test] + fn non_git_sources_reject_sparse_checkout() { + let path = std::env::current_dir().unwrap(); + let err = stage_marketplace_source( + &MarketplaceSource::Local { path }, + &["plugins/foo".to_string()], + Path::new("/tmp"), + |_url, _ref_name, _sparse_paths, _staged_root| Ok(()), + ) + .unwrap_err(); + + assert!( + err.to_string() + .contains("--sparse is only supported for git marketplace sources"), + "unexpected error: {err}" ); }