diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json index 943b8480f9..cb59ac2a8a 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.schemas.json @@ -10428,6 +10428,44 @@ ], "title": "LocalPluginSource", "type": "object" + }, + { + "properties": { + "path": { + "type": [ + "string", + "null" + ] + }, + "refName": { + "type": [ + "string", + "null" + ] + }, + "sha": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "git" + ], + "title": "GitPluginSourceType", + "type": "string" + }, + "url": { + "type": "string" + } + }, + "required": [ + "type", + "url" + ], + "title": "GitPluginSource", + "type": "object" } ] }, diff --git a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json index 1717dfb393..12eb378e90 100644 --- a/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json +++ b/codex-rs/app-server-protocol/schema/json/codex_app_server_protocol.v2.schemas.json @@ -7200,6 +7200,44 @@ ], "title": "LocalPluginSource", "type": "object" + }, + { + "properties": { + "path": { + "type": [ + "string", + "null" + ] + }, + "refName": { + "type": [ + "string", + "null" + ] + }, + "sha": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "git" + ], + "title": "GitPluginSourceType", + "type": "string" + }, + "url": { + "type": "string" + } + }, + "required": [ + "type", + "url" + ], + "title": "GitPluginSource", + "type": "object" } ] }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json index a9a5960638..ee039060e6 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginListResponse.json @@ -204,6 +204,44 @@ ], "title": "LocalPluginSource", "type": "object" + }, + { + "properties": { + "path": { + "type": [ + "string", + "null" + ] + }, + "refName": { + "type": [ + "string", + "null" + ] + }, + "sha": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "git" + ], + "title": "GitPluginSourceType", + "type": "string" + }, + "url": { + "type": "string" + } + }, + "required": [ + "type", + "url" + ], + "title": "GitPluginSource", + "type": "object" } ] }, diff --git a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json index abe36390c9..43628e28f0 100644 --- a/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json +++ b/codex-rs/app-server-protocol/schema/json/v2/PluginReadResponse.json @@ -224,6 +224,44 @@ ], "title": "LocalPluginSource", "type": "object" + }, + { + "properties": { + "path": { + "type": [ + "string", + "null" + ] + }, + "refName": { + "type": [ + "string", + "null" + ] + }, + "sha": { + "type": [ + "string", + "null" + ] + }, + "type": { + "enum": [ + "git" + ], + "title": "GitPluginSourceType", + "type": "string" + }, + "url": { + "type": "string" + } + }, + "required": [ + "type", + "url" + ], + "title": "GitPluginSource", + "type": "object" } ] }, diff --git a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts index 2f342c5ddd..5c8771aa0b 100644 --- a/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts +++ b/codex-rs/app-server-protocol/schema/typescript/v2/PluginSource.ts @@ -3,4 +3,4 @@ // This file was generated by [ts-rs](https://github.com/Aleph-Alpha/ts-rs). Do not edit this file manually. import type { AbsolutePathBuf } from "../AbsolutePathBuf"; -export type PluginSource = { "type": "local", path: AbsolutePathBuf, }; +export type PluginSource = { "type": "local", path: AbsolutePathBuf, } | { "type": "git", url: string, path: string | null, refName: string | null, sha: string | null, }; diff --git a/codex-rs/app-server-protocol/src/protocol/v2.rs b/codex-rs/app-server-protocol/src/protocol/v2.rs index 155c67e7bf..35718d614a 100644 --- a/codex-rs/app-server-protocol/src/protocol/v2.rs +++ b/codex-rs/app-server-protocol/src/protocol/v2.rs @@ -3633,6 +3633,14 @@ pub enum PluginSource { #[serde(rename_all = "camelCase")] #[ts(rename_all = "camelCase")] Local { path: AbsolutePathBuf }, + #[serde(rename_all = "camelCase")] + #[ts(rename_all = "camelCase")] + Git { + url: String, + path: Option, + ref_name: Option, + sha: Option, + }, } #[derive(Serialize, Deserialize, Debug, Clone, PartialEq, JsonSchema, TS)] diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 2585383cda..e18139f28e 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -8888,6 +8888,17 @@ fn plugin_interface_to_info(interface: PluginManifestInterface) -> PluginInterfa fn marketplace_plugin_source_to_info(source: MarketplacePluginSource) -> PluginSource { match source { MarketplacePluginSource::Local { path } => PluginSource::Local { path }, + MarketplacePluginSource::Git { + url, + path, + ref_name, + sha, + } => PluginSource::Git { + url, + path, + ref_name, + sha, + }, } } diff --git a/codex-rs/core-plugins/src/loader.rs b/codex-rs/core-plugins/src/loader.rs index 206add5b53..4e4879910c 100644 --- a/codex-rs/core-plugins/src/loader.rs +++ b/codex-rs/core-plugins/src/loader.rs @@ -32,7 +32,9 @@ use std::collections::HashMap; use std::collections::HashSet; use std::fs; use std::path::Path; +use std::process::Command; use std::sync::Arc; +use tempfile::TempDir; use tracing::warn; const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; @@ -144,6 +146,14 @@ pub fn refresh_curated_plugin_cache( } let source_path = match plugin.source { MarketplacePluginSource::Local { path } => path, + MarketplacePluginSource::Git { .. } => { + warn!( + plugin = plugin_name, + marketplace = OPENAI_CURATED_MARKETPLACE_NAME, + "skipping remote curated plugin source during cache refresh" + ); + continue; + } }; plugin_sources.insert(plugin_name, source_path); } @@ -198,7 +208,7 @@ pub fn refresh_non_curated_plugin_cache( let store = PluginStore::new(codex_home.to_path_buf()); let marketplace_outcome = list_marketplaces(additional_roots) .map_err(|err| format!("failed to discover marketplaces for cache refresh: {err}"))?; - let mut plugin_sources = HashMap::::new(); + let mut plugin_sources = HashMap::::new(); for marketplace in marketplace_outcome.marketplaces { if marketplace.name == OPENAI_CURATED_MARKETPLACE_NAME { @@ -227,19 +237,14 @@ pub fn refresh_non_curated_plugin_cache( continue; } - let source_path = match plugin.source { - MarketplacePluginSource::Local { path } => path, - }; - let plugin_version = plugin_version_for_source(source_path.as_path()) - .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; - plugin_sources.insert(plugin_key, (source_path, plugin_version)); + plugin_sources.insert(plugin_key, plugin.source); } } let mut cache_refreshed = false; for plugin_id in configured_non_curated_plugin_ids { let plugin_key = plugin_id.as_key(); - let Some((source_path, plugin_version)) = plugin_sources.get(&plugin_key).cloned() else { + let Some(source) = plugin_sources.get(&plugin_key).cloned() else { warn!( plugin = plugin_id.plugin_name, marketplace = plugin_id.marketplace_name, @@ -247,6 +252,13 @@ pub fn refresh_non_curated_plugin_cache( ); continue; }; + let materialized = + materialize_marketplace_plugin_source(codex_home, &source).map_err(|err| { + format!("failed to materialize plugin source for {plugin_key}: {err}") + })?; + let source_path = materialized.path.clone(); + let plugin_version = plugin_version_for_source(source_path.as_path()) + .map_err(|err| format!("failed to read plugin version for {plugin_key}: {err}"))?; if store.active_plugin_version(&plugin_id).as_deref() == Some(plugin_version.as_str()) { continue; @@ -805,3 +817,95 @@ fn normalize_plugin_mcp_server_value( struct PluginMcpDiscovery { mcp_servers: HashMap, } + +#[derive(Debug)] +pub struct MaterializedMarketplacePluginSource { + pub path: AbsolutePathBuf, + _tempdir: Option, +} + +pub fn materialize_marketplace_plugin_source( + codex_home: &Path, + source: &MarketplacePluginSource, +) -> Result { + match source { + MarketplacePluginSource::Local { path } => Ok(MaterializedMarketplacePluginSource { + path: path.clone(), + _tempdir: None, + }), + MarketplacePluginSource::Git { + url, + path, + ref_name, + sha, + } => { + let staging_root = codex_home.join("plugins/.marketplace-plugin-source-staging"); + fs::create_dir_all(&staging_root).map_err(|err| { + format!( + "failed to create marketplace plugin source staging directory {}: {err}", + staging_root.display() + ) + })?; + let tempdir = tempfile::Builder::new() + .prefix("marketplace-plugin-source-") + .tempdir_in(&staging_root) + .map_err(|err| { + format!( + "failed to create marketplace plugin source staging directory in {}: {err}", + staging_root.display() + ) + })?; + clone_git_plugin_source(url, ref_name.as_deref(), sha.as_deref(), tempdir.path())?; + let path = if let Some(path) = path { + AbsolutePathBuf::try_from(tempdir.path().join(path)).map_err(|err| { + format!("failed to resolve materialized plugin source path: {err}") + })? + } else { + AbsolutePathBuf::try_from(tempdir.path().to_path_buf()).map_err(|err| { + format!("failed to resolve materialized plugin source path: {err}") + })? + }; + Ok(MaterializedMarketplacePluginSource { + path, + _tempdir: Some(tempdir), + }) + } + } +} + +fn clone_git_plugin_source( + url: &str, + ref_name: Option<&str>, + sha: Option<&str>, + destination: &Path, +) -> Result<(), String> { + run_git(&["clone", url, destination.to_string_lossy().as_ref()], None)?; + if let Some(target) = sha.or(ref_name) { + run_git(&["checkout", target], Some(destination))?; + } + Ok(()) +} + +fn run_git(args: &[&str], cwd: Option<&Path>) -> Result<(), String> { + 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() + .map_err(|err| format!("failed to run git {}: {err}", args.join(" ")))?; + if output.status.success() { + return Ok(()); + } + + Err(format!( + "git {} failed with status {}\nstdout:\n{}\nstderr:\n{}", + args.join(" "), + output.status, + String::from_utf8_lossy(&output.stdout).trim(), + String::from_utf8_lossy(&output.stderr).trim() + )) +} diff --git a/codex-rs/core-plugins/src/marketplace.rs b/codex-rs/core-plugins/src/marketplace.rs index 79e5237ad0..cc0c4bce3d 100644 --- a/codex-rs/core-plugins/src/marketplace.rs +++ b/codex-rs/core-plugins/src/marketplace.rs @@ -9,7 +9,6 @@ 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; @@ -26,7 +25,7 @@ const MARKETPLACE_MANIFEST_RELATIVE_PATHS: &[&str] = &[ #[derive(Debug, Clone, PartialEq, Eq)] pub struct ResolvedMarketplacePlugin { pub plugin_id: PluginId, - pub source_path: AbsolutePathBuf, + pub source: MarketplacePluginSource, pub auth_policy: MarketplacePluginAuthPolicy, } @@ -65,7 +64,15 @@ pub struct MarketplacePlugin { #[derive(Debug, Clone, PartialEq, Eq)] pub enum MarketplacePluginSource { - Local { path: AbsolutePathBuf }, + Local { + path: AbsolutePathBuf, + }, + Git { + url: String, + path: Option, + ref_name: Option, + sha: Option, + }, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -193,9 +200,7 @@ pub fn resolve_marketplace_plugin( }); } - let Some(source_path) = - resolve_supported_plugin_source_path(marketplace_path, &name, source) - else { + let Some(source) = resolve_supported_plugin_source(marketplace_path, &name, source) else { continue; }; @@ -203,7 +208,7 @@ pub fn resolve_marketplace_plugin( plugin_id: PluginId::new(name, marketplace_name).map_err(|err| match err { PluginIdError::Invalid(message) => MarketplaceError::InvalidPlugin(message), })?, - source_path, + source, auth_policy: policy.authentication, }); } @@ -276,14 +281,15 @@ pub fn load_marketplace(path: &AbsolutePathBuf) -> Result { + load_plugin_manifest(path.as_path()).and_then(|manifest| manifest.interface) + } + MarketplacePluginSource::Git { .. } => None, }; - let mut interface = - load_plugin_manifest(source_path.as_path()).and_then(|manifest| manifest.interface); if let Some(category) = category { // Marketplace taxonomy wins when both sources provide a category. interface @@ -389,27 +395,13 @@ fn load_raw_marketplace_manifest( }) } -fn resolve_supported_plugin_source_path( +fn resolve_supported_plugin_source( marketplace_path: &AbsolutePathBuf, plugin_name: &str, source: RawMarketplaceManifestPluginSource, -) -> Option { +) -> Option { match source { - RawMarketplaceManifestPluginSource::Local { path } => { - 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 - } - } - } - RawMarketplaceManifestPluginSource::Unsupported => { + RawMarketplaceManifestPluginSource::Unsupported(_) => { warn!( path = %marketplace_path.display(), plugin = plugin_name, @@ -417,27 +409,85 @@ fn resolve_supported_plugin_source_path( ); None } + source => match resolve_plugin_source(marketplace_path, source) { + Ok(source) => Some(source), + Err(err) => { + warn!( + path = %marketplace_path.display(), + plugin = plugin_name, + error = %err, + "skipping marketplace plugin that failed to resolve" + ); + None + } + }, + } +} + +fn resolve_plugin_source( + marketplace_path: &AbsolutePathBuf, + source: RawMarketplaceManifestPluginSource, +) -> Result { + match source { + RawMarketplaceManifestPluginSource::Path(path) + | RawMarketplaceManifestPluginSource::Object( + RawMarketplaceManifestPluginSourceObject::Local { path }, + ) => Ok(MarketplacePluginSource::Local { + path: resolve_local_plugin_source_path(marketplace_path, &path)?, + }), + RawMarketplaceManifestPluginSource::Object( + RawMarketplaceManifestPluginSourceObject::Url { + url, + path, + ref_name, + sha, + }, + ) => Ok(MarketplacePluginSource::Git { + url: normalize_git_plugin_source_url(marketplace_path, &url)?, + path: path + .as_deref() + .map(|path| normalize_remote_plugin_subdir(marketplace_path, path)) + .transpose()?, + ref_name: normalize_optional_git_selector(&ref_name), + sha: normalize_optional_git_selector(&sha), + }), + RawMarketplaceManifestPluginSource::Object( + RawMarketplaceManifestPluginSourceObject::GitSubdir { + url, + path, + ref_name, + sha, + }, + ) => Ok(MarketplacePluginSource::Git { + url: normalize_git_plugin_source_url(marketplace_path, &url)?, + path: Some(normalize_remote_plugin_subdir(marketplace_path, &path)?), + ref_name: normalize_optional_git_selector(&ref_name), + sha: normalize_optional_git_selector(&sha), + }), + RawMarketplaceManifestPluginSource::Unsupported(_) => { + unreachable!("unsupported plugin sources should be filtered before resolution") + } } } fn resolve_local_plugin_source_path( marketplace_path: &AbsolutePathBuf, - source_path: &str, + path: &str, ) -> Result { - let Some(source_path) = source_path.strip_prefix("./") else { + 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 source_path.is_empty() { + 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(), }); } - let relative_source_path = Path::new(source_path); + let relative_source_path = Path::new(path); if relative_source_path .components() .any(|component| !matches!(component, Component::Normal(_))) @@ -453,6 +503,98 @@ fn resolve_local_plugin_source_path( Ok(marketplace_root_dir(marketplace_path)?.join(relative_source_path)) } +fn normalize_remote_plugin_subdir( + marketplace_path: &AbsolutePathBuf, + path: &str, +) -> Result { + let path = path.trim(); + if path.is_empty() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "git plugin source path must not be empty".to_string(), + }); + } + let path = path.strip_prefix("./").unwrap_or(path); + let relative_path = Path::new(path); + if relative_path + .components() + .any(|component| !matches!(component, Component::Normal(_))) + { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "git plugin source path must stay within the repository root".to_string(), + }); + } + Ok(path.to_string()) +} + +fn normalize_git_plugin_source_url( + marketplace_path: &AbsolutePathBuf, + url: &str, +) -> Result { + let url = url.trim(); + if url.is_empty() { + return Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: "git plugin source url must not be empty".to_string(), + }); + } + if url.starts_with("http://") || url.starts_with("https://") { + return Ok(normalize_github_git_url(url)); + } + if url.starts_with("file://") + || url.starts_with("./") + || url.starts_with("../") + || url.starts_with('/') + { + return Ok(url.to_string()); + } + if url.starts_with("ssh://") || url.starts_with("git@") && url.contains(':') { + return Ok(url.to_string()); + } + if looks_like_github_shorthand(url) { + return Ok(format!("https://github.com/{url}.git")); + } + + Err(MarketplaceError::InvalidMarketplaceFile { + path: marketplace_path.to_path_buf(), + message: format!("invalid git plugin source url: {url}"), + }) +} + +fn normalize_optional_git_selector(value: &Option) -> Option { + value + .as_deref() + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(str::to_string) +} + +fn normalize_github_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_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, '-' | '_' | '.')) +} + fn marketplace_root_dir( marketplace_path: &AbsolutePathBuf, ) -> Result { @@ -505,33 +647,36 @@ struct RawMarketplaceManifestPluginPolicy { products: Option>, } -#[derive(Debug)] +#[derive(Debug, Deserialize)] +#[serde(untagged)] enum RawMarketplaceManifestPluginSource { - Local { path: String }, - // Mixed-source marketplaces should still contribute the local plugins we can load. - Unsupported, + Path(String), + Object(RawMarketplaceManifestPluginSourceObject), + #[allow(dead_code)] + Unsupported(JsonValue), } -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, - }) - } +#[derive(Debug, Deserialize)] +#[serde(tag = "source", rename_all = "lowercase")] +enum RawMarketplaceManifestPluginSourceObject { + Local { + path: String, + }, + Url { + url: String, + path: Option, + #[serde(rename = "ref")] + ref_name: Option, + sha: Option, + }, + #[serde(rename = "git-subdir")] + GitSubdir { + url: String, + path: String, + #[serde(rename = "ref")] + ref_name: Option, + sha: Option, + }, } fn resolve_marketplace_interface( diff --git a/codex-rs/core-plugins/src/marketplace_tests.rs b/codex-rs/core-plugins/src/marketplace_tests.rs index 3412eff72c..95eaec39e1 100644 --- a/codex-rs/core-plugins/src/marketplace_tests.rs +++ b/codex-rs/core-plugins/src/marketplace_tests.rs @@ -49,7 +49,9 @@ fn resolve_marketplace_plugin_finds_repo_marketplace_plugin() { ResolvedMarketplacePlugin { plugin_id: PluginId::new("local-plugin".to_string(), "codex-curated".to_string()) .unwrap(), - source_path: AbsolutePathBuf::try_from(repo_root.join("plugin-1")).unwrap(), + source: MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugin-1")).unwrap(), + }, auth_policy: MarketplacePluginAuthPolicy::OnInstall, } ); @@ -88,13 +90,99 @@ fn resolve_marketplace_plugin_supports_alternate_layout_and_string_local_source( "alternate-marketplace".to_string() ) .unwrap(), - source_path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin")) - .unwrap(), + source: MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugins/string-source-plugin")) + .unwrap(), + }, auth_policy: MarketplacePluginAuthPolicy::OnInstall, } ); } +#[test] +fn resolve_marketplace_plugin_supports_git_subdir_sources() { + 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": "codex-curated", + "plugins": [ + { + "name": "remote-plugin", + "source": { + "source": "git-subdir", + "url": "openai/joey_marketplace3", + "path": "plugins/toolkit", + "ref": "main", + "sha": "abc123" + } + } + ] +}"#, + ) + .unwrap(); + + let resolved = resolve_marketplace_plugin( + &AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(), + "remote-plugin", + Some(Product::Codex), + ) + .unwrap(); + + assert_eq!( + resolved, + ResolvedMarketplacePlugin { + plugin_id: PluginId::new("remote-plugin".to_string(), "codex-curated".to_string()) + .unwrap(), + source: MarketplacePluginSource::Git { + url: "https://github.com/openai/joey_marketplace3.git".to_string(), + path: Some("plugins/toolkit".to_string()), + ref_name: Some("main".to_string()), + sha: Some("abc123".to_string()), + }, + auth_policy: MarketplacePluginAuthPolicy::OnInstall, + } + ); +} + +#[test] +fn resolve_marketplace_plugin_supports_string_local_sources() { + 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": "codex-curated", + "plugins": [ + { + "name": "local-plugin", + "source": "./plugin-2" + } + ] +}"#, + ) + .unwrap(); + + let resolved = resolve_marketplace_plugin( + &AbsolutePathBuf::try_from(repo_root.join(".agents/plugins/marketplace.json")).unwrap(), + "local-plugin", + Some(Product::Codex), + ) + .unwrap(); + + assert_eq!( + resolved.source, + MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugin-2")).unwrap(), + } + ); +} + #[test] fn resolve_marketplace_plugin_reports_missing_plugin() { let tmp = tempdir().unwrap(); @@ -456,8 +544,10 @@ fn list_marketplaces_keeps_distinct_entries_for_same_name() { .unwrap(); assert_eq!( - resolved.source_path, - AbsolutePathBuf::try_from(repo_root.join("repo-plugin")).unwrap() + resolved.source, + MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("repo-plugin")).unwrap(), + } ); } @@ -675,7 +765,7 @@ fn list_marketplaces_reports_marketplace_load_errors() { } #[test] -fn list_marketplaces_skips_unsupported_plugin_sources_but_keeps_local_plugins() { +fn list_marketplaces_keeps_remote_and_local_plugin_sources() { let tmp = tempdir().unwrap(); let repo_root = tmp.path().join("repo"); @@ -693,7 +783,7 @@ fn list_marketplaces_skips_unsupported_plugin_sources_but_keeps_local_plugins() "name": "url-plugin", "source": { "source": "url", - "url": "https://github.com/example/plugin.git" + "url": "https://github.com/example/plugin" } }, { @@ -702,7 +792,8 @@ fn list_marketplaces_skips_unsupported_plugin_sources_but_keeps_local_plugins() "source": "git-subdir", "url": "owner/repo", "path": "plugins/example", - "ref": "main" + "ref": "main", + "sha": "abc123" } } ] @@ -717,14 +808,53 @@ fn list_marketplaces_skips_unsupported_plugin_sources_but_keeps_local_plugins() .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(), - } + marketplaces[0].plugins, + vec![ + MarketplacePlugin { + name: "local-plugin".to_string(), + source: MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("plugins/local-plugin")) + .unwrap(), + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + }, + MarketplacePlugin { + name: "url-plugin".to_string(), + source: MarketplacePluginSource::Git { + url: "https://github.com/example/plugin.git".to_string(), + path: None, + ref_name: None, + sha: None, + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + }, + MarketplacePlugin { + name: "git-subdir-plugin".to_string(), + source: MarketplacePluginSource::Git { + url: "https://github.com/owner/repo.git".to_string(), + path: Some("plugins/example".to_string()), + ref_name: Some("main".to_string()), + sha: Some("abc123".to_string()), + }, + policy: MarketplacePluginPolicy { + installation: MarketplacePluginInstallPolicy::Available, + authentication: MarketplacePluginAuthPolicy::OnInstall, + products: None, + }, + interface: None, + }, + ] ); } @@ -940,7 +1070,7 @@ fn list_marketplaces_ignores_plugin_interface_assets_without_dot_slash() { } #[test] -fn resolve_marketplace_plugin_skips_invalid_local_paths() { +fn resolve_marketplace_plugin_skips_non_relative_local_paths() { let tmp = tempdir().unwrap(); let repo_root = tmp.path().join("repo"); fs::create_dir_all(repo_root.join(".git")).unwrap(); @@ -962,46 +1092,14 @@ fn resolve_marketplace_plugin_skips_invalid_local_paths() { ) .unwrap(); - 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)) + 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)) .unwrap_err(); assert_eq!( err.to_string(), - "plugin `remote-plugin` was not found in marketplace `alternate-marketplace`" + "plugin `local-plugin` was not found in marketplace `codex-curated`" ); } @@ -1043,8 +1141,10 @@ fn resolve_marketplace_plugin_uses_first_duplicate_entry() { .unwrap(); assert_eq!( - resolved.source_path, - AbsolutePathBuf::try_from(repo_root.join("first")).unwrap() + resolved.source, + MarketplacePluginSource::Local { + path: AbsolutePathBuf::try_from(repo_root.join("first")).unwrap(), + } ); } diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index 50c62d5823..85532c5575 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -22,6 +22,7 @@ use codex_core_plugins::loader::load_plugin_mcp_servers; use codex_core_plugins::loader::load_plugin_skills; use codex_core_plugins::loader::load_plugins_from_layer_stack; use codex_core_plugins::loader::log_plugin_load_errors; +use codex_core_plugins::loader::materialize_marketplace_plugin_source; use codex_core_plugins::loader::plugin_telemetry_metadata_from_root; use codex_core_plugins::loader::refresh_curated_plugin_cache; use codex_core_plugins::loader::refresh_non_curated_plugin_cache; @@ -560,11 +561,16 @@ impl PluginsManager { None }; let store = self.store.clone(); + let codex_home = self.codex_home.clone(); let result: StorePluginInstallResult = tokio::task::spawn_blocking(move || { + let materialized = + materialize_marketplace_plugin_source(codex_home.as_path(), &resolved.source) + .map_err(PluginStoreError::Invalid)?; + let source_path = materialized.path; if let Some(plugin_version) = plugin_version { - store.install_with_version(resolved.source_path, resolved.plugin_id, plugin_version) + store.install_with_version(source_path, resolved.plugin_id, plugin_version) } else { - store.install(resolved.source_path, resolved.plugin_id) + store.install(source_path, resolved.plugin_id) } }) .await @@ -725,6 +731,14 @@ impl PluginsManager { let plugin_key = plugin_id.as_key(); let source_path = match plugin.source { MarketplacePluginSource::Local { path } => path, + MarketplacePluginSource::Git { .. } => { + warn!( + plugin = plugin_name, + marketplace = %marketplace_name, + "skipping remote plugin source during remote sync" + ); + continue; + } }; let current_enabled = configured_plugins .get(&plugin_key) @@ -1018,9 +1032,17 @@ impl PluginsManager { } })?; let plugin_key = plugin_id.as_key(); - let source_path = match &plugin.source { - MarketplacePluginSource::Local { path } => path.clone(), - }; + let codex_home = self.codex_home.clone(); + let source = plugin.source.clone(); + let materialized = tokio::task::spawn_blocking(move || { + materialize_marketplace_plugin_source(codex_home.as_path(), &source) + }) + .await + .map_err(|err| { + MarketplaceError::InvalidPlugin(format!("failed to materialize plugin source: {err}")) + })? + .map_err(MarketplaceError::InvalidPlugin)?; + let source_path = materialized.path.clone(); if !source_path.as_path().is_dir() { return Err(MarketplaceError::InvalidPlugin( "path does not exist or is not a directory".to_string(), diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index f74b8fd117..cf499ce396 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -64,6 +64,31 @@ fn write_plugin(root: &Path, dir_name: &str, manifest_name: &str) { ); } +fn init_git_repo(repo: &Path) { + run_git(repo, &["init"]); + run_git(repo, &["config", "user.email", "codex-test@example.com"]); + run_git(repo, &["config", "user.name", "Codex Test"]); + run_git(repo, &["add", "."]); + run_git(repo, &["commit", "-m", "initial"]); +} + +fn run_git(repo: &Path, args: &[&str]) { + let output = std::process::Command::new("git") + .arg("-C") + .arg(repo) + .args(args) + .output() + .unwrap_or_else(|err| panic!("git should run: {err}")); + assert!( + output.status.success(), + "git -C {} {} failed\nstdout:\n{}\nstderr:\n{}", + repo.display(), + args.join(" "), + String::from_utf8_lossy(&output.stdout), + String::from_utf8_lossy(&output.stderr) + ); +} + fn plugin_config_toml(enabled: bool, plugins_feature_enabled: bool) -> String { let mut root = toml::map::Map::new(); @@ -1040,6 +1065,60 @@ async fn install_plugin_uses_manifest_version_for_non_curated_plugins() { ); } +#[tokio::test] +async fn install_plugin_supports_git_subdir_marketplace_sources() { + let tmp = tempfile::tempdir().unwrap(); + let repo_root = tmp.path().join("marketplace"); + let remote_repo = tmp.path().join("remote-plugin-repo"); + fs::create_dir_all(repo_root.join(".git")).unwrap(); + fs::create_dir_all(repo_root.join(".agents/plugins")).unwrap(); + write_plugin(&remote_repo, "plugins/toolkit", "toolkit"); + init_git_repo(&remote_repo); + fs::write( + repo_root.join(".agents/plugins/marketplace.json"), + format!( + r#"{{ + "name": "debug", + "plugins": [ + {{ + "name": "toolkit", + "source": {{ + "source": "git-subdir", + "url": "{}", + "path": "plugins/toolkit" + }} + }} + ] +}}"#, + remote_repo.display() + ), + ) + .unwrap(); + + let result = PluginsManager::new(tmp.path().to_path_buf()) + .install_plugin(PluginInstallRequest { + plugin_name: "toolkit".to_string(), + marketplace_path: AbsolutePathBuf::try_from( + repo_root.join(".agents/plugins/marketplace.json"), + ) + .unwrap(), + }) + .await + .unwrap(); + + let installed_path = tmp.path().join("plugins/cache/debug/toolkit/local"); + assert_eq!( + result, + PluginInstallOutcome { + plugin_id: PluginId::new("toolkit".to_string(), "debug".to_string()).unwrap(), + plugin_version: "local".to_string(), + installed_path: AbsolutePathBuf::try_from(installed_path.clone()).unwrap(), + auth_policy: MarketplacePluginAuthPolicy::OnInstall, + } + ); + assert!(installed_path.join(".codex-plugin/plugin.json").is_file()); +} + #[tokio::test] async fn uninstall_plugin_removes_cache_and_config_entry() { let tmp = tempfile::tempdir().unwrap();