diff --git a/codex-rs/core/src/plugins/installed_marketplaces.rs b/codex-rs/core/src/plugins/installed_marketplaces.rs index df69111a2..ad1918d51 100644 --- a/codex-rs/core/src/plugins/installed_marketplaces.rs +++ b/codex-rs/core/src/plugins/installed_marketplaces.rs @@ -4,6 +4,7 @@ use std::path::Path; use std::path::PathBuf; use tracing::warn; +use super::marketplace::find_marketplace_manifest_path; use super::validate_plugin_segment; pub const INSTALLED_MARKETPLACES_DIR: &str = ".tmp/marketplaces"; @@ -50,9 +51,7 @@ pub(crate) fn installed_marketplace_roots_from_config( marketplace, &default_install_root, )?; - path.join(".agents/plugins/marketplace.json") - .is_file() - .then_some(path) + find_marketplace_manifest_path(&path).map(|_| path) }) .filter_map(|path| AbsolutePathBuf::try_from(path).ok()) .collect::>(); diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core/src/plugins/marketplace.rs index 3cb1756f8..96c73e8e1 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core/src/plugins/marketplace.rs @@ -9,6 +9,8 @@ use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; use dirs::home_dir; use serde::Deserialize; +use serde::Deserializer; +use serde_json::Value as JsonValue; use std::fs; use std::io; use std::path::Component; @@ -16,7 +18,10 @@ use std::path::Path; use std::path::PathBuf; use tracing::warn; -const MARKETPLACE_RELATIVE_PATH: &str = ".agents/plugins/marketplace.json"; +const MARKETPLACE_MANIFEST_RELATIVE_PATHS: &[&str] = &[ + ".agents/plugins/marketplace.json", + ".claude-plugin/marketplace.json", +]; #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolvedMarketplacePlugin { @@ -162,46 +167,50 @@ pub fn resolve_marketplace_plugin( ) -> Result { let marketplace = load_raw_marketplace_manifest(marketplace_path)?; let marketplace_name = marketplace.name; - let plugin = marketplace - .plugins - .into_iter() - .find(|plugin| plugin.name == plugin_name); - - let Some(plugin) = plugin else { - return Err(MarketplaceError::PluginNotFound { - plugin_name: plugin_name.to_string(), - marketplace_name, - }); - }; - - let RawMarketplaceManifestPlugin { - name, - source, - policy, - .. - } = plugin; - let install_policy = policy.installation; - let product_allowed = match policy.products.as_deref() { - None => true, - Some([]) => false, - Some(products) => { - restriction_product.is_some_and(|product| product.matches_product_restriction(products)) + let marketplace_name_for_not_found = marketplace_name.clone(); + for plugin in marketplace.plugins { + if plugin.name != plugin_name { + continue; } - }; - if install_policy == MarketplacePluginInstallPolicy::NotAvailable || !product_allowed { - return Err(MarketplaceError::PluginNotAvailable { - plugin_name: name, - marketplace_name, + + let RawMarketplaceManifestPlugin { + name, + source, + policy, + .. + } = plugin; + let install_policy = policy.installation; + let product_allowed = match policy.products.as_deref() { + None => true, + Some([]) => false, + Some(products) => restriction_product + .is_some_and(|product| product.matches_product_restriction(products)), + }; + if install_policy == MarketplacePluginInstallPolicy::NotAvailable || !product_allowed { + return Err(MarketplaceError::PluginNotAvailable { + plugin_name: name, + marketplace_name, + }); + } + + let Some(source_path) = + resolve_supported_plugin_source_path(marketplace_path, &name, source) + else { + continue; + }; + + return Ok(ResolvedMarketplacePlugin { + plugin_id: PluginId::new(name, marketplace_name).map_err(|err| match err { + PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message), + })?, + source_path, + auth_policy: policy.authentication, }); } - let plugin_id = PluginId::new(name, marketplace_name).map_err(|err| match err { - PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message), - })?; - Ok(ResolvedMarketplacePlugin { - plugin_id, - source_path: resolve_plugin_source_path(marketplace_path, source)?, - auth_policy: policy.authentication, + Err(MarketplaceError::PluginNotFound { + plugin_name: plugin_name.to_string(), + marketplace_name: marketplace_name_for_not_found, }) } @@ -212,16 +221,50 @@ pub fn list_marketplaces( } 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 Some(path) = find_marketplace_manifest_path(root) else { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: root.to_path_buf(), + message: "marketplace root does not contain a supported manifest".to_string(), + }); + }; let marketplace = load_marketplace(&path)?; Ok(marketplace.name) } +pub(crate) fn find_marketplace_manifest_path(root: &Path) -> Option { + MARKETPLACE_MANIFEST_RELATIVE_PATHS + .iter() + .find_map(|relative_path| { + let path = root.join(relative_path); + if !path.is_file() { + return None; + } + AbsolutePathBuf::try_from(path).ok() + }) +} + +fn invalid_marketplace_layout_error(path: &AbsolutePathBuf) -> MarketplaceError { + MarketplaceError::InvalidMarketplaceFile { + path: path.to_path_buf(), + message: "marketplace file is not in a supported location".to_string(), + } +} + +fn marketplace_root_from_layout(marketplace_path: &Path, relative_path: &str) -> Option { + let mut current = marketplace_path; + for component in Path::new(relative_path).components().rev() { + let expected = match component { + Component::Normal(expected) => expected, + _ => return None, + }; + if current.file_name() != Some(expected) { + return None; + } + current = current.parent()?; + } + Some(current.to_path_buf()) +} + pub(crate) fn load_marketplace(path: &AbsolutePathBuf) -> Result { let marketplace = load_raw_marketplace_manifest(path)?; let mut plugins = Vec::new(); @@ -233,7 +276,9 @@ pub(crate) fn load_marketplace(path: &AbsolutePathBuf) -> Result Vec { let mut paths = Vec::new(); - if let Some(home) = home_dir { - let path = home.join(MARKETPLACE_RELATIVE_PATH); - if path.is_file() - && let Ok(path) = AbsolutePathBuf::try_from(path) - { - paths.push(path); - } + if let Some(home) = home_dir + && let Some(path) = find_marketplace_manifest_path(home) + { + paths.push(path); } for root in additional_roots { // Curated marketplaces can now come from an HTTP-downloaded directory that is not a git // checkout, so check the root directly before falling back to repo-root discovery. - let path = root.join(MARKETPLACE_RELATIVE_PATH); - if path.as_path().is_file() && !paths.contains(&path) { + if let Some(path) = find_marketplace_manifest_path(root.as_path()) + && !paths.contains(&path) + { paths.push(path); continue; } if let Some(repo_root) = get_git_repo_root(root.as_path()) && let Ok(repo_root) = AbsolutePathBuf::try_from(repo_root) + && let Some(path) = find_marketplace_manifest_path(repo_root.as_path()) + && !paths.contains(&path) { - let path = repo_root.join(MARKETPLACE_RELATIVE_PATH); - if path.as_path().is_file() && !paths.contains(&path) { - paths.push(path); - } + paths.push(path); } } @@ -346,80 +388,83 @@ fn load_raw_marketplace_manifest( }) } -fn resolve_plugin_source_path( +fn resolve_supported_plugin_source_path( marketplace_path: &AbsolutePathBuf, + plugin_name: &str, source: RawMarketplaceManifestPluginSource, -) -> Result { +) -> Option { match source { RawMarketplaceManifestPluginSource::Local { path } => { - let Some(path) = path.strip_prefix("./") else { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "local plugin source path must start with `./`".to_string(), - }); - }; - if path.is_empty() { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "local plugin source path must not be empty".to_string(), - }); + match resolve_local_plugin_source_path(marketplace_path, &path) { + Ok(path) => Some(path), + Err(err) => { + warn!( + path = %marketplace_path.display(), + plugin = plugin_name, + error = %err, + "skipping marketplace plugin that failed to resolve" + ); + None + } } - - let relative_source_path = Path::new(path); - if relative_source_path - .components() - .any(|component| !matches!(component, Component::Normal(_))) - { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "local plugin source path must stay within the marketplace root" - .to_string(), - }); - } - - // `marketplace.json` lives under `/.agents/plugins/`, but local plugin paths - // are resolved relative to ``, not relative to the `plugins/` directory. - Ok(marketplace_root_dir(marketplace_path)?.join(relative_source_path)) + } + RawMarketplaceManifestPluginSource::Unsupported => { + warn!( + path = %marketplace_path.display(), + plugin = plugin_name, + "skipping marketplace plugin with unsupported source" + ); + None } } } +fn resolve_local_plugin_source_path( + marketplace_path: &AbsolutePathBuf, + source_path: &str, +) -> Result { + let Some(source_path) = source_path.strip_prefix("./") else { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "local plugin source path must start with `./`".to_string(), + }); + }; + if source_path.is_empty() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "local plugin source path must not be empty".to_string(), + }); + } + + let relative_source_path = Path::new(source_path); + if relative_source_path + .components() + .any(|component| !matches!(component, Component::Normal(_))) + { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "local plugin source path must stay within the marketplace root".to_string(), + }); + } + + // `marketplace.json` lives under a supported marketplace layout beneath ``, + // but local plugin paths are resolved relative to ``. + Ok(marketplace_root_dir(marketplace_path)?.join(relative_source_path)) +} + fn marketplace_root_dir( marketplace_path: &AbsolutePathBuf, ) -> Result { - let Some(plugins_dir) = marketplace_path.parent() else { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "marketplace file must live under `/.agents/plugins/`".to_string(), - }); - }; - let Some(dot_agents_dir) = plugins_dir.parent() else { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "marketplace file must live under `/.agents/plugins/`".to_string(), - }); - }; - let Some(marketplace_root) = dot_agents_dir.parent() else { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "marketplace file must live under `/.agents/plugins/`".to_string(), - }); - }; - - if plugins_dir.as_path().file_name().and_then(|s| s.to_str()) != Some("plugins") - || dot_agents_dir - .as_path() - .file_name() - .and_then(|s| s.to_str()) - != Some(".agents") - { - return Err(MarketplaceError::InvalidMarketplaceFile { - path: marketplace_path.to_path_buf(), - message: "marketplace file must live under `/.agents/plugins/`".to_string(), - }); + for relative_path in MARKETPLACE_MANIFEST_RELATIVE_PATHS { + if let Some(marketplace_root) = + marketplace_root_from_layout(marketplace_path.as_path(), relative_path) + { + return AbsolutePathBuf::try_from(marketplace_root) + .map_err(|_| invalid_marketplace_layout_error(marketplace_path)); + } } - Ok(marketplace_root) + Err(invalid_marketplace_layout_error(marketplace_path)) } #[derive(Debug, Deserialize)] @@ -459,10 +504,33 @@ struct RawMarketplaceManifestPluginPolicy { products: Option>, } -#[derive(Debug, Deserialize)] -#[serde(tag = "source", rename_all = "lowercase")] +#[derive(Debug)] enum RawMarketplaceManifestPluginSource { Local { path: String }, + // Mixed-source marketplaces should still contribute the local plugins we can load. + Unsupported, +} + +impl<'de> Deserialize<'de> for RawMarketplaceManifestPluginSource { + fn deserialize(deserializer: D) -> Result + where + D: Deserializer<'de>, + { + let source = JsonValue::deserialize(deserializer)?; + Ok(match source { + JsonValue::String(path) => Self::Local { path }, + JsonValue::Object(object) => match object.get("source").and_then(JsonValue::as_str) { + Some("local") => match object.get("path").and_then(JsonValue::as_str) { + Some(path) => Self::Local { + path: path.to_string(), + }, + None => Self::Unsupported, + }, + _ => Self::Unsupported, + }, + _ => Self::Unsupported, + }) + } } fn resolve_marketplace_interface( diff --git a/codex-rs/core/src/plugins/marketplace_tests.rs b/codex-rs/core/src/plugins/marketplace_tests.rs index 226ae3ff6..3412eff72 100644 --- a/codex-rs/core/src/plugins/marketplace_tests.rs +++ b/codex-rs/core/src/plugins/marketplace_tests.rs @@ -1,8 +1,18 @@ use super::*; use codex_protocol::protocol::Product; use pretty_assertions::assert_eq; +use std::path::Path; use tempfile::tempdir; +const ALTERNATE_MARKETPLACE_RELATIVE_PATH: &str = ".claude-plugin/marketplace.json"; + +fn write_alternate_marketplace(repo_root: &Path, contents: &str) -> AbsolutePathBuf { + let marketplace_path = repo_root.join(ALTERNATE_MARKETPLACE_RELATIVE_PATH); + fs::create_dir_all(marketplace_path.parent().unwrap()).unwrap(); + fs::write(&marketplace_path, contents).unwrap(); + AbsolutePathBuf::try_from(marketplace_path).unwrap() +} + #[test] fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() { let tmp = tempdir().unwrap(); @@ -45,6 +55,46 @@ fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() { ); } +#[test] +fn resolve_marketplace_plugin_supports_alternate_layout_and_string_local_source() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + let marketplace_path = write_alternate_marketplace( + &repo_root, + r#"{ + "name": "alternate-marketplace", + "plugins": [ + { + "name": "string-source-plugin", + "source": "./plugins/string-source-plugin" + } + ] +}"#, + ); + + let resolved = resolve_marketplace_plugin( + &marketplace_path, + "string-source-plugin", + Some(Product::Codex), + ) + .unwrap(); + + assert_eq!( + resolved, + ResolvedMarketplacePlugin { + plugin_id: PluginId::new( + "string-source-plugin".to_string(), + "alternate-marketplace".to_string() + ) + .unwrap(), + source_path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin")) + .unwrap(), + auth_policy: MarketplacePluginAuthPolicy::OnInstall, + } + ); +} + #[test] fn resolve_marketplace_plugin_reports_missing_plugin() { let tmp = tempdir().unwrap(); @@ -70,6 +120,106 @@ fn resolve_marketplace_plugin_reports_missing_plugin() { ); } +#[test] +fn list_marketplaces_supports_alternate_manifest_layout() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + + fs::create_dir_all(repo_root.join(".git")).unwrap(); + let marketplace_path = write_alternate_marketplace( + &repo_root, + r#"{ + "name": "alternate-marketplace", + "plugins": [ + { + "name": "string-source-plugin", + "source": "./plugins/string-source-plugin" + } + ] +}"#, + ); + + let marketplaces = list_marketplaces_with_home( + &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], + /*home_dir*/ None, + ) + .unwrap() + .marketplaces; + + assert_eq!( + marketplaces, + vec![Marketplace { + name: "alternate-marketplace".to_string(), + path: marketplace_path, + interface: None, + plugins: vec![MarketplacePlugin { + name: "string-source-plugin".to_string(), + source: MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin")) + .unwrap(), + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + }], + }] + ); +} + +#[test] +fn list_marketplaces_prefers_first_supported_manifest_layout() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + r#"{ + "name": "agents-marketplace", + "plugins": [ + { + "name": "agents-plugin", + "source": { + "source": "local", + "path": "./plugins/agents-plugin" + } + } + ] +}"#, + ) + .unwrap(); + write_alternate_marketplace( + &repo_root, + r#"{ + "name": "alternate-marketplace", + "plugins": [ + { + "name": "string-source-plugin", + "source": "./plugins/string-source-plugin" + } + ] +}"#, + ); + + let marketplaces = list_marketplaces_with_home( + &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], + /*home_dir*/ None, + ) + .unwrap() + .marketplaces; + + assert_eq!(marketplaces.len(), 1); + assert_eq!(marketplaces[0].name, "agents-marketplace"); + assert_eq!( + marketplaces[0].path, + AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap() + ); +} + #[test] fn list_marketplaces_returns_home_and_repo_marketplaces() { let tmp = tempdir().unwrap(); @@ -413,7 +563,7 @@ fn list_marketplaces_reads_marketplace_display_name() { } #[test] -fn list_marketplaces_skips_marketplaces_that_fail_to_load() { +fn list_marketplaces_skips_invalid_plugins_but_keeps_marketplace() { let tmp = tempdir().unwrap(); let valid_repo_root = tmp.path().join("valid-repo"); let invalid_repo_root = tmp.path().join("invalid-repo"); @@ -465,8 +615,10 @@ fn list_marketplaces_skips_marketplaces_that_fail_to_load() { .unwrap() .marketplaces; - assert_eq!(marketplaces.len(), 1); + assert_eq!(marketplaces.len(), 2); assert_eq!(marketplaces[0].name, "valid-marketplace"); + assert_eq!(marketplaces[1].name, "invalid-marketplace"); + assert!(marketplaces[1].plugins.is_empty()); } #[test] @@ -522,6 +674,60 @@ fn list_marketplaces_reports_marketplace_load_errors() { ); } +#[test] +fn list_marketplaces_skips_unsupported_plugin_sources_but_keeps_local_plugins() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + + fs::create_dir_all(repo_root.join(".git")).unwrap(); + write_alternate_marketplace( + &repo_root, + r#"{ + "name": "mixed-source-marketplace", + "plugins": [ + { + "name": "local-plugin", + "source": "./plugins/local-plugin" + }, + { + "name": "url-plugin", + "source": { + "source": "url", + "url": "https://github.com/example/plugin.git" + } + }, + { + "name": "git-subdir-plugin", + "source": { + "source": "git-subdir", + "url": "owner/repo", + "path": "plugins/example", + "ref": "main" + } + } + ] +}"#, + ); + + let marketplaces = list_marketplaces_with_home( + &[AbsolutePathBuf::try_from(repo_root.clone()).unwrap()], + /*home_dir*/ None, + ) + .unwrap() + .marketplaces; + + assert_eq!(marketplaces.len(), 1); + assert_eq!(marketplaces[0].name, "mixed-source-marketplace"); + assert_eq!(marketplaces[0].plugins.len(), 1); + assert_eq!(marketplaces[0].plugins[0].name, "local-plugin"); + assert_eq!( + marketplaces[0].plugins[0].source, + MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugins/local-plugin")).unwrap(), + } + ); +} + #[test] fn list_marketplaces_resolves_plugin_interface_paths_to_absolute() { let tmp = tempdir().unwrap(); @@ -734,7 +940,7 @@ fn list_marketplaces_ignores_plugin_interface_assets_without_dot_slash() { } #[test] -fn resolve_marketplace_plugin_rejects_non_relative_local_paths() { +fn resolve_marketplace_plugin_skips_invalid_local_paths() { let tmp = tempdir().unwrap(); let repo_root = tmp.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).unwrap(); @@ -756,17 +962,46 @@ fn resolve_marketplace_plugin_rejects_non_relative_local_paths() { ) .unwrap(); - let marketplace_path = - AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(); - let err = resolve_marketplace_plugin(&marketplace_path, "local-plugin", Some(Product::Codex)) + let err = resolve_marketplace_plugin( + &AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(), + "local-plugin", + Some(Product::Codex), + ) + .unwrap_err(); + + assert_eq!( + err.to_string(), + "plugin `local-plugin` was not found in marketplace `codex-curated`" + ); +} + +#[test] +fn resolve_marketplace_plugin_skips_unsupported_sources() { + let tmp = tempdir().unwrap(); + let repo_root = tmp.path().join("repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + let marketplace_path = write_alternate_marketplace( + &repo_root, + r#"{ + "name": "alternate-marketplace", + "plugins": [ + { + "name": "remote-plugin", + "source": { + "source": "url", + "url": "https://github.com/example/plugin.git" + } + } + ] +}"#, + ); + + let err = resolve_marketplace_plugin(&marketplace_path, "remote-plugin", Some(Product::Codex)) .unwrap_err(); assert_eq!( err.to_string(), - format!( - "invalid marketplace file `{}`: local plugin source path must start with `./`", - marketplace_path.display() - ) + "plugin `remote-plugin` was not found in marketplace `alternate-marketplace`" ); }