From 267eacfca2de761b2c1aa79b51b6b2bbdad4fab0 Mon Sep 17 00:00:00 2001 From: jif Date: Fri, 12 Jun 2026 12:37:33 +0100 Subject: [PATCH] Add executor-owned plugin resolution (#27692) ## Why CCA can select a capability root that lives in an executor environment, but Codex only had a host-filesystem plugin loader. Before selected executor plugins can contribute MCP servers, we need a small package boundary that can answer: > Does this selected root contain a plugin, and if so, what does its manifest > declare? The answer must come from the selected environment's filesystem. A failed executor lookup must never fall back to the orchestrator filesystem. ## What this changes This PR introduces: ```rust PluginProvider::resolve(root) -> Result, Error> ``` `ExecutorPluginProvider` resolves one `SelectedCapabilityRoot` through its exact `environment_id`. It checks the recognized manifest locations, reads the manifest through that environment's `ExecutorFileSystem`, and returns an inert `ResolvedPlugin` containing: - the opaque selected-root ID; - the environment-bound plugin root; - the authority-bound manifest resource; - parsed metadata and authority-bound component locators. Descriptor construction rejects manifest or component paths outside the selected package root, so consumers cannot accidentally lose the package boundary when they receive a resolved plugin. If the root has no plugin manifest, resolution returns `None`, allowing the caller to treat it as a standalone capability such as a skill. ```text selected root: repo -> env-1:/workspace/repo | | env-1 filesystem only v .codex-plugin/plugin.json | v ResolvedPlugin { authority, root, manifest } ``` The existing host loader and the new executor provider now share the same manifest parser. Existing `codex-core-plugins::manifest` type paths remain available through re-exports, so host behavior and callers are unchanged. ## Scope This is intentionally a non-user-visible package-resolution PR. It does not: - parse or register plugin MCP server configurations; - activate skills, connectors, hooks, or MCP servers; - change app-server wiring; - introduce host fallback, caching, or lifecycle behavior. #27670 has merged, and this PR is now based directly on `main`. Together with the resolved MCP catalog from #27634, it establishes the inputs needed for the executor stdio MCP vertical without changing the existing MCP runtime. ## Follow-up The next PR will consume `ResolvedPlugin`, read its declared/default MCP config through the same executor filesystem, bind supported stdio servers to that environment, and feed those registrations into the resolved MCP catalog. An app-server E2E will prove that selecting an executor plugin exposes and invokes its tool on the owning executor. Resume/fork semantics, dynamic environment replacement, and non-stdio placement remain separate lifecycle decisions. ## Validation - `just fmt` - `cargo check --tests -p codex-plugin -p codex-core-plugins` - `just bazel-lock-check` - `git diff --check` Test targets were compiled but not executed locally; CI will run the test and Clippy suites. --- codex-rs/Cargo.lock | 3 + codex-rs/core-plugins/Cargo.toml | 1 + codex-rs/core-plugins/src/lib.rs | 3 + codex-rs/core-plugins/src/manifest.rs | 350 +++++++++--------- codex-rs/core-plugins/src/provider.rs | 232 ++++++++++++ codex-rs/core-plugins/src/provider_tests.rs | 332 +++++++++++++++++ codex-rs/plugin/Cargo.toml | 4 + codex-rs/plugin/src/lib.rs | 9 +- codex-rs/plugin/src/manifest.rs | 157 ++++++++ codex-rs/plugin/src/provider.rs | 128 +++++++ codex-rs/plugin/src/provider_tests.rs | 126 +++++++ codex-rs/utils/plugins/src/lib.rs | 1 + .../utils/plugins/src/plugin_namespace.rs | 3 +- 13 files changed, 1177 insertions(+), 172 deletions(-) create mode 100644 codex-rs/core-plugins/src/provider.rs create mode 100644 codex-rs/core-plugins/src/provider_tests.rs create mode 100644 codex-rs/plugin/src/manifest.rs create mode 100644 codex-rs/plugin/src/provider.rs create mode 100644 codex-rs/plugin/src/provider_tests.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index 4ae2d33c4..db8b8e396 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -2744,6 +2744,7 @@ dependencies = [ "codex-plugin", "codex-protocol", "codex-utils-absolute-path", + "codex-utils-path-uri", "codex-utils-plugins", "dirs", "flate2", @@ -3558,8 +3559,10 @@ name = "codex-plugin" version = "0.0.0" dependencies = [ "codex-config", + "codex-protocol", "codex-utils-absolute-path", "codex-utils-plugins", + "pretty_assertions", "thiserror 2.0.18", ] diff --git a/codex-rs/core-plugins/Cargo.toml b/codex-rs/core-plugins/Cargo.toml index d0096e445..7e0b1bf91 100644 --- a/codex-rs/core-plugins/Cargo.toml +++ b/codex-rs/core-plugins/Cargo.toml @@ -27,6 +27,7 @@ codex-otel = { workspace = true } codex-plugin = { workspace = true } codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } +codex-utils-path-uri = { workspace = true } codex-utils-plugins = { workspace = true } chrono = { workspace = true } dirs = { workspace = true } diff --git a/codex-rs/core-plugins/src/lib.rs b/codex-rs/core-plugins/src/lib.rs index 84eeea5dc..620ec2086 100644 --- a/codex-rs/core-plugins/src/lib.rs +++ b/codex-rs/core-plugins/src/lib.rs @@ -8,6 +8,7 @@ pub mod marketplace_add; pub mod marketplace_remove; pub mod marketplace_upgrade; mod plugin_bundle_archive; +mod provider; pub mod remote; pub mod remote_bundle; pub mod remote_legacy; @@ -42,3 +43,5 @@ pub use manager::PluginsConfigInput; pub use manager::PluginsManager; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeError as PluginMarketplaceUpgradeError; pub use marketplace_upgrade::ConfiguredMarketplaceUpgradeOutcome as PluginMarketplaceUpgradeOutcome; +pub use provider::ExecutorPluginProvider; +pub use provider::ExecutorPluginProviderError; diff --git a/codex-rs/core-plugins/src/manifest.rs b/codex-rs/core-plugins/src/manifest.rs index 606ca8147..28a200ab8 100644 --- a/codex-rs/core-plugins/src/manifest.rs +++ b/codex-rs/core-plugins/src/manifest.rs @@ -9,6 +9,11 @@ use std::path::Path; const MAX_DEFAULT_PROMPT_COUNT: usize = 3; const MAX_DEFAULT_PROMPT_LEN: usize = 128; +pub type PluginManifest = codex_plugin::manifest::PluginManifest; +pub type PluginManifestHooks = codex_plugin::manifest::PluginManifestHooks; +pub type PluginManifestInterface = codex_plugin::manifest::PluginManifestInterface; +pub type PluginManifestPaths = codex_plugin::manifest::PluginManifestPaths; + #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] struct RawPluginManifest { @@ -34,48 +39,6 @@ struct RawPluginManifest { interface: Option, } -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginManifest { - pub name: String, - pub version: Option, - pub description: Option, - pub keywords: Vec, - pub paths: PluginManifestPaths, - pub interface: Option, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginManifestPaths { - pub skills: Option, - pub mcp_servers: Option, - pub apps: Option, - pub hooks: Option, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub enum PluginManifestHooks { - Paths(Vec), - Inline(Vec), -} - -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct PluginManifestInterface { - pub display_name: Option, - pub short_description: Option, - pub long_description: Option, - pub developer_name: Option, - pub category: Option, - pub capabilities: Vec, - pub website_url: Option, - pub privacy_policy_url: Option, - pub terms_of_service_url: Option, - pub default_prompt: Option>, - pub brand_color: Option, - pub composer_icon: Option, - pub logo: Option, - pub screenshots: Vec, -} - #[derive(Debug, Default, Deserialize)] #[serde(rename_all = "camelCase")] struct RawPluginManifestInterface { @@ -144,119 +107,12 @@ enum RawPluginManifestHooks { Invalid(JsonValue), } +/// Loads a plugin manifest from the local host filesystem. pub fn load_plugin_manifest(plugin_root: &Path) -> Option { let manifest_path = find_plugin_manifest_path(plugin_root)?; let contents = fs::read_to_string(&manifest_path).ok()?; - match serde_json::from_str::(&contents) { - Ok(manifest) => { - let RawPluginManifest { - name: raw_name, - version, - description, - keywords, - skills, - mcp_servers, - apps, - hooks, - interface, - } = manifest; - let name = plugin_root - .file_name() - .and_then(|entry| entry.to_str()) - .filter(|_| raw_name.trim().is_empty()) - .unwrap_or(&raw_name) - .to_string(); - let version = version.and_then(|version| { - let version = version.trim(); - (!version.is_empty()).then(|| version.to_string()) - }); - let interface = interface.and_then(|interface| { - let RawPluginManifestInterface { - display_name, - short_description, - long_description, - developer_name, - category, - capabilities, - website_url, - privacy_policy_url, - terms_of_service_url, - default_prompt, - brand_color, - composer_icon, - logo, - screenshots, - } = interface; - - let interface = PluginManifestInterface { - display_name, - short_description, - long_description, - developer_name, - category, - capabilities, - website_url, - privacy_policy_url, - terms_of_service_url, - default_prompt: resolve_default_prompts(plugin_root, default_prompt.as_ref()), - brand_color, - composer_icon: resolve_interface_asset_path( - plugin_root, - "interface.composerIcon", - composer_icon.as_deref(), - ), - logo: resolve_interface_asset_path( - plugin_root, - "interface.logo", - logo.as_deref(), - ), - screenshots: screenshots - .iter() - .filter_map(|screenshot| { - resolve_interface_asset_path( - plugin_root, - "interface.screenshots", - Some(screenshot), - ) - }) - .collect(), - }; - - let has_fields = interface.display_name.is_some() - || interface.short_description.is_some() - || interface.long_description.is_some() - || interface.developer_name.is_some() - || interface.category.is_some() - || !interface.capabilities.is_empty() - || interface.website_url.is_some() - || interface.privacy_policy_url.is_some() - || interface.terms_of_service_url.is_some() - || interface.default_prompt.is_some() - || interface.brand_color.is_some() - || interface.composer_icon.is_some() - || interface.logo.is_some() - || !interface.screenshots.is_empty(); - - has_fields.then_some(interface) - }); - Some(PluginManifest { - name, - version, - description, - keywords, - paths: PluginManifestPaths { - skills: resolve_manifest_path_value(plugin_root, "skills", skills.as_ref()), - mcp_servers: resolve_manifest_path( - plugin_root, - "mcpServers", - mcp_servers.as_deref(), - ), - apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), - hooks: resolve_manifest_hooks(plugin_root, hooks), - }, - interface, - }) - } + match parse_plugin_manifest(plugin_root, &manifest_path, &contents) { + Ok(manifest) => Some(manifest), Err(err) => { tracing::warn!( path = %manifest_path.display(), @@ -267,6 +123,112 @@ pub fn load_plugin_manifest(plugin_root: &Path) -> Option { } } +pub(crate) fn parse_plugin_manifest( + plugin_root: &Path, + manifest_path: &Path, + contents: &str, +) -> Result { + let RawPluginManifest { + name: raw_name, + version, + description, + keywords, + skills, + mcp_servers, + apps, + hooks, + interface, + } = serde_json::from_str::(contents)?; + let name = plugin_root + .file_name() + .and_then(|entry| entry.to_str()) + .filter(|_| raw_name.trim().is_empty()) + .unwrap_or(&raw_name) + .to_string(); + let version = version.and_then(|version| { + let version = version.trim(); + (!version.is_empty()).then(|| version.to_string()) + }); + let interface = interface.and_then(|interface| { + let RawPluginManifestInterface { + display_name, + short_description, + long_description, + developer_name, + category, + capabilities, + website_url, + privacy_policy_url, + terms_of_service_url, + default_prompt, + brand_color, + composer_icon, + logo, + screenshots, + } = interface; + + let interface = PluginManifestInterface { + display_name, + short_description, + long_description, + developer_name, + category, + capabilities, + website_url, + privacy_policy_url, + terms_of_service_url, + default_prompt: resolve_default_prompts(manifest_path, default_prompt.as_ref()), + brand_color, + composer_icon: resolve_interface_asset_path( + plugin_root, + "interface.composerIcon", + composer_icon.as_deref(), + ), + logo: resolve_interface_asset_path(plugin_root, "interface.logo", logo.as_deref()), + screenshots: screenshots + .iter() + .filter_map(|screenshot| { + resolve_interface_asset_path( + plugin_root, + "interface.screenshots", + Some(screenshot), + ) + }) + .collect(), + }; + + let has_fields = interface.display_name.is_some() + || interface.short_description.is_some() + || interface.long_description.is_some() + || interface.developer_name.is_some() + || interface.category.is_some() + || !interface.capabilities.is_empty() + || interface.website_url.is_some() + || interface.privacy_policy_url.is_some() + || interface.terms_of_service_url.is_some() + || interface.default_prompt.is_some() + || interface.brand_color.is_some() + || interface.composer_icon.is_some() + || interface.logo.is_some() + || !interface.screenshots.is_empty(); + + has_fields.then_some(interface) + }); + Ok(PluginManifest { + name, + version, + description, + keywords, + paths: PluginManifestPaths { + skills: resolve_manifest_path_value(plugin_root, "skills", skills.as_ref()), + mcp_servers: resolve_manifest_path(plugin_root, "mcpServers", mcp_servers.as_deref()), + apps: resolve_manifest_path(plugin_root, "apps", apps.as_deref()), + hooks: resolve_manifest_hooks(plugin_root, hooks), + }, + interface, + }) +} + fn resolve_manifest_hooks( plugin_root: &Path, hooks: Option, @@ -306,12 +268,12 @@ fn resolve_interface_asset_path( } fn resolve_default_prompts( - plugin_root: &Path, + manifest_path: &Path, value: Option<&RawPluginManifestDefaultPrompt>, ) -> Option> { match value? { RawPluginManifestDefaultPrompt::String(prompt) => { - resolve_default_prompt_str(plugin_root, "interface.defaultPrompt", prompt) + resolve_default_prompt_str(manifest_path, "interface.defaultPrompt", prompt) .map(|prompt| vec![prompt]) } RawPluginManifestDefaultPrompt::List(values) => { @@ -319,7 +281,7 @@ fn resolve_default_prompts( for (index, item) in values.iter().enumerate() { if prompts.len() >= MAX_DEFAULT_PROMPT_COUNT { warn_invalid_default_prompt( - plugin_root, + manifest_path, "interface.defaultPrompt", &format!("maximum of {MAX_DEFAULT_PROMPT_COUNT} prompts is supported"), ); @@ -330,7 +292,7 @@ fn resolve_default_prompts( RawPluginManifestDefaultPromptEntry::String(prompt) => { let field = format!("interface.defaultPrompt[{index}]"); if let Some(prompt) = - resolve_default_prompt_str(plugin_root, &field, prompt) + resolve_default_prompt_str(manifest_path, &field, prompt) { prompts.push(prompt); } @@ -338,7 +300,7 @@ fn resolve_default_prompts( RawPluginManifestDefaultPromptEntry::Invalid(value) => { let field = format!("interface.defaultPrompt[{index}]"); warn_invalid_default_prompt( - plugin_root, + manifest_path, &field, &format!("expected a string, found {}", json_value_type(value)), ); @@ -350,7 +312,7 @@ fn resolve_default_prompts( } RawPluginManifestDefaultPrompt::Invalid(value) => { warn_invalid_default_prompt( - plugin_root, + manifest_path, "interface.defaultPrompt", &format!( "expected a string or array of strings, found {}", @@ -362,15 +324,15 @@ fn resolve_default_prompts( } } -fn resolve_default_prompt_str(plugin_root: &Path, field: &str, prompt: &str) -> Option { +fn resolve_default_prompt_str(manifest_path: &Path, field: &str, prompt: &str) -> Option { let prompt = prompt.split_whitespace().collect::>().join(" "); if prompt.is_empty() { - warn_invalid_default_prompt(plugin_root, field, "prompt must not be empty"); + warn_invalid_default_prompt(manifest_path, field, "prompt must not be empty"); return None; } if prompt.chars().count() > MAX_DEFAULT_PROMPT_LEN { warn_invalid_default_prompt( - plugin_root, + manifest_path, field, &format!("prompt must be at most {MAX_DEFAULT_PROMPT_LEN} characters"), ); @@ -379,15 +341,11 @@ fn resolve_default_prompt_str(plugin_root: &Path, field: &str, prompt: &str) -> Some(prompt) } -fn warn_invalid_default_prompt(plugin_root: &Path, field: &str, message: &str) { - if let Some(manifest_path) = find_plugin_manifest_path(plugin_root) { - tracing::warn!( - path = %manifest_path.display(), - "ignoring {field}: {message}" - ); - } else { - tracing::warn!("ignoring {field}: {message}"); - } +fn warn_invalid_default_prompt(manifest_path: &Path, field: &str, message: &str) { + tracing::warn!( + path = %manifest_path.display(), + "ignoring {field}: {message}" + ); } fn json_value_type(value: &JsonValue) -> &'static str { @@ -466,11 +424,21 @@ mod tests { use super::MAX_DEFAULT_PROMPT_LEN; use super::PluginManifest; use super::load_plugin_manifest; + use codex_exec_server::EnvironmentManager; + use codex_exec_server::LOCAL_ENVIRONMENT_ID; + use codex_plugin::PluginProvider; + use codex_plugin::ResolvedPlugin; + use codex_protocol::capabilities::CapabilityRootLocation; + use codex_protocol::capabilities::SelectedCapabilityRoot; + use codex_utils_absolute_path::AbsolutePathBuf; use pretty_assertions::assert_eq; use std::fs; use std::path::Path; + use std::sync::Arc; use tempfile::tempdir; + use crate::ExecutorPluginProvider; + const ALTERNATE_PLUGIN_MANIFEST_RELATIVE_PATH: &str = ".claude-plugin/plugin.json"; fn write_manifest(plugin_root: &Path, version: Option<&str>, interface: &str) { @@ -645,4 +613,46 @@ mod tests { Some("Fallback Plugin") ); } + + #[tokio::test] + async fn host_and_executor_sources_parse_the_same_manifest() { + let temp_dir = tempdir().expect("tempdir"); + let plugin_root = temp_dir.path().join("demo-plugin"); + write_manifest( + &plugin_root, + Some(" 1.2.3 "), + r#"{ + "displayName": "Demo Plugin", + "composerIcon": "./assets/icon.svg" + }"#, + ); + let host_manifest = load_plugin_manifest(&plugin_root).expect("host manifest"); + let provider = + ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); + let selected_root = SelectedCapabilityRoot { + id: "selected-demo".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), + path: plugin_root.to_string_lossy().into_owned(), + }, + }; + + let executor_plugin = provider + .resolve(&selected_root) + .await + .expect("resolve executor plugin") + .expect("plugin descriptor"); + let plugin_root = + AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let expected_plugin = ResolvedPlugin::from_environment( + "selected-demo".to_string(), + LOCAL_ENVIRONMENT_ID.to_string(), + plugin_root.clone(), + plugin_root.join(".codex-plugin/plugin.json"), + host_manifest, + ) + .expect("valid expected descriptor"); + + assert_eq!(executor_plugin, expected_plugin); + } } diff --git a/codex-rs/core-plugins/src/provider.rs b/codex-rs/core-plugins/src/provider.rs new file mode 100644 index 000000000..2ba1ecca6 --- /dev/null +++ b/codex-rs/core-plugins/src/provider.rs @@ -0,0 +1,232 @@ +use crate::manifest::parse_plugin_manifest; +use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecutorFileSystem; +use codex_plugin::PluginProvider; +use codex_plugin::ResolvedPlugin; +use codex_plugin::ResolvedPluginError; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +use codex_utils_plugins::DISCOVERABLE_PLUGIN_MANIFEST_PATHS; +use std::io; +use std::path::PathBuf; +use std::sync::Arc; +use thiserror::Error; + +/// Failure to resolve an environment-owned capability root as a plugin package. +#[derive(Debug, Error)] +pub enum ExecutorPluginProviderError { + #[error("selected capability root `{root_id}` has invalid path `{path}`: {message}")] + InvalidRootPath { + root_id: String, + path: String, + message: String, + }, + #[error( + "selected capability root `{root_id}` references unavailable environment `{environment_id}`" + )] + UnavailableEnvironment { + root_id: String, + environment_id: String, + }, + #[error("failed to inspect selected capability root `{root_id}` at {path}: {source}")] + InspectRoot { + root_id: String, + path: AbsolutePathBuf, + #[source] + source: io::Error, + }, + #[error("selected capability root `{root_id}` path {path} is not a directory")] + RootNotDirectory { + root_id: String, + path: AbsolutePathBuf, + }, + #[error("failed to inspect plugin manifest for `{root_id}` at {path}: {source}")] + InspectManifest { + root_id: String, + path: AbsolutePathBuf, + #[source] + source: io::Error, + }, + #[error("failed to read plugin manifest for `{root_id}` at {path}: {source}")] + ReadManifest { + root_id: String, + path: AbsolutePathBuf, + #[source] + source: io::Error, + }, + #[error("failed to parse plugin manifest for `{root_id}` at {path}: {source}")] + ParseManifest { + root_id: String, + path: AbsolutePathBuf, + #[source] + source: serde_json::Error, + }, + #[error("failed to construct plugin descriptor for `{root_id}`: {source}")] + ConstructDescriptor { + root_id: String, + #[source] + source: ResolvedPluginError, + }, +} + +/// Resolves plugin packages through the filesystem owned by an execution environment. +#[derive(Clone, Debug)] +pub struct ExecutorPluginProvider { + environment_manager: Arc, +} + +impl ExecutorPluginProvider { + /// Creates a provider backed by the active execution environments. + pub fn new(environment_manager: Arc) -> Self { + Self { + environment_manager, + } + } +} + +impl PluginProvider for ExecutorPluginProvider { + type Error = ExecutorPluginProviderError; + + async fn resolve( + &self, + selected_root: &SelectedCapabilityRoot, + ) -> Result, Self::Error> { + let root_id = &selected_root.id; + let plugin_root = selected_plugin_root(selected_root)?; + let CapabilityRootLocation::Environment { environment_id, .. } = &selected_root.location; + let environment = self + .environment_manager + .get_environment(environment_id) + .ok_or_else(|| ExecutorPluginProviderError::UnavailableEnvironment { + root_id: root_id.clone(), + environment_id: environment_id.clone(), + })?; + let file_system = environment.get_filesystem(); + + resolve_plugin_root(selected_root, plugin_root, file_system.as_ref()).await + } +} + +fn selected_plugin_root( + selected_root: &SelectedCapabilityRoot, +) -> Result { + let root_id = &selected_root.id; + let CapabilityRootLocation::Environment { path, .. } = &selected_root.location; + let plugin_root = PathBuf::from(path); + if !plugin_root.is_absolute() { + return Err(ExecutorPluginProviderError::InvalidRootPath { + root_id: root_id.clone(), + path: path.clone(), + message: "executor path must be absolute".to_string(), + }); + } + AbsolutePathBuf::from_absolute_path_checked(plugin_root).map_err(|err| { + ExecutorPluginProviderError::InvalidRootPath { + root_id: root_id.clone(), + path: path.clone(), + message: err.to_string(), + } + }) +} + +async fn resolve_plugin_root( + selected_root: &SelectedCapabilityRoot, + plugin_root: AbsolutePathBuf, + file_system: &dyn ExecutorFileSystem, +) -> Result, ExecutorPluginProviderError> { + let root_id = &selected_root.id; + let CapabilityRootLocation::Environment { + environment_id, + path, + } = &selected_root.location; + let root_uri = PathUri::from_abs_path(&plugin_root).map_err(|err| { + ExecutorPluginProviderError::InvalidRootPath { + root_id: root_id.clone(), + path: path.clone(), + message: err.to_string(), + } + })?; + let root_metadata = file_system + .get_metadata(&root_uri, /*sandbox*/ None) + .await + .map_err(|source| ExecutorPluginProviderError::InspectRoot { + root_id: root_id.clone(), + path: plugin_root.clone(), + source, + })?; + if !root_metadata.is_directory { + return Err(ExecutorPluginProviderError::RootNotDirectory { + root_id: root_id.clone(), + path: plugin_root, + }); + } + + let mut manifest_path = None; + for relative_path in DISCOVERABLE_PLUGIN_MANIFEST_PATHS { + let candidate = plugin_root.join(relative_path); + let candidate_uri = PathUri::from_abs_path(&candidate).map_err(|err| { + ExecutorPluginProviderError::InvalidRootPath { + root_id: root_id.clone(), + path: candidate.as_path().to_string_lossy().into_owned(), + message: err.to_string(), + } + })?; + match file_system + .get_metadata(&candidate_uri, /*sandbox*/ None) + .await + { + Ok(metadata) if metadata.is_file => { + manifest_path = Some((candidate, candidate_uri)); + break; + } + Ok(_) => {} + Err(err) if err.kind() == io::ErrorKind::NotFound => {} + Err(source) => { + return Err(ExecutorPluginProviderError::InspectManifest { + root_id: root_id.clone(), + path: candidate, + source, + }); + } + } + } + let Some((manifest_path, manifest_uri)) = manifest_path else { + return Ok(None); + }; + let contents = file_system + .read_file_text(&manifest_uri, /*sandbox*/ None) + .await + .map_err(|source| ExecutorPluginProviderError::ReadManifest { + root_id: root_id.clone(), + path: manifest_path.clone(), + source, + })?; + let manifest = + parse_plugin_manifest(&plugin_root, &manifest_path, &contents).map_err(|source| { + ExecutorPluginProviderError::ParseManifest { + root_id: root_id.clone(), + path: manifest_path.clone(), + source, + } + })?; + + let plugin = ResolvedPlugin::from_environment( + root_id.clone(), + environment_id.clone(), + plugin_root, + manifest_path, + manifest, + ) + .map_err(|source| ExecutorPluginProviderError::ConstructDescriptor { + root_id: root_id.clone(), + source, + })?; + + Ok(Some(plugin)) +} + +#[cfg(test)] +#[path = "provider_tests.rs"] +mod tests; diff --git a/codex-rs/core-plugins/src/provider_tests.rs b/codex-rs/core-plugins/src/provider_tests.rs new file mode 100644 index 000000000..e2c22121b --- /dev/null +++ b/codex-rs/core-plugins/src/provider_tests.rs @@ -0,0 +1,332 @@ +use super::ExecutorPluginProvider; +use super::ExecutorPluginProviderError; +use super::resolve_plugin_root; +use crate::manifest::parse_plugin_manifest; +use codex_exec_server::CopyOptions; +use codex_exec_server::CreateDirectoryOptions; +use codex_exec_server::EnvironmentManager; +use codex_exec_server::ExecutorFileSystem; +use codex_exec_server::ExecutorFileSystemFuture; +use codex_exec_server::FileMetadata; +use codex_exec_server::FileSystemResult; +use codex_exec_server::FileSystemSandboxContext; +use codex_exec_server::LOCAL_ENVIRONMENT_ID; +use codex_exec_server::ReadDirectoryEntry; +use codex_exec_server::RemoveOptions; +use codex_plugin::PluginProvider; +use codex_plugin::ResolvedPlugin; +use codex_protocol::capabilities::CapabilityRootLocation; +use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_path_uri::PathUri; +use pretty_assertions::assert_eq; +use std::fs; +use std::io; +use std::path::Path; +use std::sync::Arc; +use std::sync::Mutex; +use tempfile::tempdir; + +const MANIFEST_CONTENTS: &str = r#"{ + "name": "demo-plugin", + "version": " 1.2.3 ", + "description": "Demo plugin", + "skills": "./skills", + "mcpServers": "./.mcp.json", + "apps": "./.app.json", + "interface": { + "displayName": "Demo Plugin", + "composerIcon": "./assets/icon.svg" + } +}"#; + +#[derive(Debug, PartialEq, Eq)] +enum FileSystemCall { + Metadata(AbsolutePathBuf), + Read(AbsolutePathBuf), +} + +struct SyntheticPluginFileSystem { + plugin_root: AbsolutePathBuf, + manifest_path: AbsolutePathBuf, + calls: Mutex>, +} + +impl SyntheticPluginFileSystem { + fn unsupported() -> FileSystemResult { + Err(io::Error::new( + io::ErrorKind::Unsupported, + "operation is not used by plugin resolution", + )) + } +} + +impl ExecutorFileSystem for SyntheticPluginFileSystem { + fn canonicalize<'a>( + &'a self, + _path: &'a PathUri, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, PathUri> { + Box::pin(async { Self::unsupported() }) + } + + fn read_file<'a>( + &'a self, + path: &'a PathUri, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, Vec> { + Box::pin(async move { + let path = path.to_abs_path()?; + self.calls + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(FileSystemCall::Read(path.clone())); + if path == self.manifest_path { + Ok(MANIFEST_CONTENTS.as_bytes().to_vec()) + } else { + Err(io::Error::new(io::ErrorKind::NotFound, "not found")) + } + }) + } + + fn write_file<'a>( + &'a self, + _path: &'a PathUri, + _contents: Vec, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, ()> { + Box::pin(async { Self::unsupported() }) + } + + fn create_directory<'a>( + &'a self, + _path: &'a PathUri, + _options: CreateDirectoryOptions, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, ()> { + Box::pin(async { Self::unsupported() }) + } + + fn get_metadata<'a>( + &'a self, + path: &'a PathUri, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, FileMetadata> { + Box::pin(async move { + let path = path.to_abs_path()?; + self.calls + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .push(FileSystemCall::Metadata(path.clone())); + let (is_directory, is_file) = if path == self.plugin_root { + (true, false) + } else if path == self.manifest_path { + (false, true) + } else { + return Err(io::Error::new(io::ErrorKind::NotFound, "not found")); + }; + Ok(FileMetadata { + is_directory, + is_file, + is_symlink: false, + created_at_ms: 0, + modified_at_ms: 0, + }) + }) + } + + fn read_directory<'a>( + &'a self, + _path: &'a PathUri, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, Vec> { + Box::pin(async { Self::unsupported() }) + } + + fn remove<'a>( + &'a self, + _path: &'a PathUri, + _options: RemoveOptions, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, ()> { + Box::pin(async { Self::unsupported() }) + } + + fn copy<'a>( + &'a self, + _source_path: &'a PathUri, + _destination_path: &'a PathUri, + _options: CopyOptions, + _sandbox: Option<&'a FileSystemSandboxContext>, + ) -> ExecutorFileSystemFuture<'a, ()> { + Box::pin(async { Self::unsupported() }) + } +} + +fn write_manifest(plugin_root: &Path, relative_path: &str, contents: &str) { + let manifest_path = plugin_root.join(relative_path); + fs::create_dir_all(manifest_path.parent().expect("manifest parent")) + .expect("create manifest parent"); + fs::write(manifest_path, contents).expect("write manifest"); +} + +fn selected_root(id: &str, environment_id: &str, path: &Path) -> SelectedCapabilityRoot { + SelectedCapabilityRoot { + id: id.to_string(), + location: CapabilityRootLocation::Environment { + environment_id: environment_id.to_string(), + path: path.to_string_lossy().into_owned(), + }, + } +} + +#[tokio::test] +async fn plugin_root_resolution_uses_supplied_executor_file_system() { + let temp_dir = tempdir().expect("tempdir"); + let plugin_root = temp_dir.path().join("executor-only-plugin"); + assert!(!plugin_root.exists()); + let plugin_root = + AbsolutePathBuf::from_absolute_path_checked(plugin_root).expect("absolute plugin root"); + let manifest_path = plugin_root.join(".codex-plugin/plugin.json"); + let parsed_manifest = parse_plugin_manifest( + plugin_root.as_path(), + manifest_path.as_path(), + MANIFEST_CONTENTS, + ) + .expect("parse manifest"); + let file_system = SyntheticPluginFileSystem { + plugin_root: plugin_root.clone(), + manifest_path: manifest_path.clone(), + calls: Mutex::new(Vec::new()), + }; + let resolved = resolve_plugin_root( + &selected_root("selected-demo", "executor-test", plugin_root.as_path()), + plugin_root.clone(), + &file_system, + ) + .await + .expect("resolve executor plugin"); + + assert_eq!( + resolved, + Some( + ResolvedPlugin::from_environment( + "selected-demo".to_string(), + "executor-test".to_string(), + plugin_root.clone(), + manifest_path.clone(), + parsed_manifest, + ) + .expect("valid expected descriptor") + ) + ); + assert_eq!( + *file_system + .calls + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner), + vec![ + FileSystemCall::Metadata(plugin_root), + FileSystemCall::Metadata(manifest_path.clone()), + FileSystemCall::Read(manifest_path), + ] + ); +} + +#[tokio::test] +async fn standalone_capability_root_is_not_a_plugin() { + let temp_dir = tempdir().expect("tempdir"); + let standalone_root = temp_dir.path().join("standalone-skill"); + fs::create_dir_all(&standalone_root).expect("create standalone root"); + let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); + + let resolved = provider + .resolve(&selected_root( + "standalone", + LOCAL_ENVIRONMENT_ID, + &standalone_root, + )) + .await + .expect("resolve standalone root"); + + assert_eq!(resolved, None); +} + +#[tokio::test] +async fn unavailable_environment_does_not_fall_back_to_host_filesystem() { + let temp_dir = tempdir().expect("tempdir"); + let plugin_root = temp_dir.path().join("host-plugin"); + write_manifest(&plugin_root, ".codex-plugin/plugin.json", MANIFEST_CONTENTS); + let provider = + ExecutorPluginProvider::new(Arc::new(EnvironmentManager::without_environments())); + + let err = provider + .resolve(&selected_root("host-plugin", "missing", &plugin_root)) + .await + .expect_err("missing environment should fail"); + + assert_eq!( + err.to_string(), + "selected capability root `host-plugin` references unavailable environment `missing`" + ); +} + +#[tokio::test] +async fn malformed_preferred_manifest_does_not_fall_through_to_alternate() { + let temp_dir = tempdir().expect("tempdir"); + let plugin_root = temp_dir.path().join("demo-plugin"); + write_manifest(&plugin_root, ".codex-plugin/plugin.json", "{not-json"); + write_manifest( + &plugin_root, + ".claude-plugin/plugin.json", + MANIFEST_CONTENTS, + ); + let expected_path = + AbsolutePathBuf::from_absolute_path_checked(plugin_root.join(".codex-plugin/plugin.json")) + .expect("absolute manifest path"); + let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); + + let err = provider + .resolve(&selected_root( + "selected-demo", + LOCAL_ENVIRONMENT_ID, + &plugin_root, + )) + .await + .expect_err("malformed preferred manifest should fail"); + + let ExecutorPluginProviderError::ParseManifest { + root_id, + path, + source: _, + } = err + else { + panic!("expected parse error"); + }; + assert_eq!( + (root_id, path), + ("selected-demo".to_string(), expected_path) + ); +} + +#[tokio::test] +async fn executor_root_must_be_an_explicit_absolute_path() { + let provider = ExecutorPluginProvider::new(Arc::new(EnvironmentManager::default_for_tests())); + let selected_root = SelectedCapabilityRoot { + id: "selected-demo".to_string(), + location: CapabilityRootLocation::Environment { + environment_id: LOCAL_ENVIRONMENT_ID.to_string(), + path: "~/plugins/demo".to_string(), + }, + }; + + let err = provider + .resolve(&selected_root) + .await + .expect_err("home-relative executor path should fail"); + + assert_eq!( + err.to_string(), + "selected capability root `selected-demo` has invalid path `~/plugins/demo`: executor path must be absolute" + ); +} diff --git a/codex-rs/plugin/Cargo.toml b/codex-rs/plugin/Cargo.toml index a431a543d..77116dfa3 100644 --- a/codex-rs/plugin/Cargo.toml +++ b/codex-rs/plugin/Cargo.toml @@ -14,6 +14,10 @@ workspace = true [dependencies] codex-config = { workspace = true } +codex-protocol = { workspace = true } codex-utils-absolute-path = { workspace = true } codex-utils-plugins = { workspace = true } thiserror = { workspace = true } + +[dev-dependencies] +pretty_assertions = { workspace = true } diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs index 92a2ace21..7bd4ac648 100644 --- a/codex-rs/plugin/src/lib.rs +++ b/codex-rs/plugin/src/lib.rs @@ -1,10 +1,12 @@ -//! Shared plugin identifiers and telemetry-facing summaries. +//! Shared plugin package models, source providers, identifiers, and telemetry summaries. pub use codex_utils_plugins::mention_syntax; pub use codex_utils_plugins::plugin_namespace_for_skill_path; mod load_outcome; +pub mod manifest; mod plugin_id; +mod provider; use codex_config::HookEventsToml; use codex_utils_absolute_path::AbsolutePathBuf; @@ -15,6 +17,11 @@ pub use load_outcome::prompt_safe_plugin_description; pub use plugin_id::PluginId; pub use plugin_id::PluginIdError; pub use plugin_id::validate_plugin_segment; +pub use provider::PluginProvider; +pub use provider::PluginResourceLocator; +pub use provider::ResolvedPlugin; +pub use provider::ResolvedPluginError; +pub use provider::ResolvedPluginLocation; #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct AppConnectorId(pub String); diff --git a/codex-rs/plugin/src/manifest.rs b/codex-rs/plugin/src/manifest.rs new file mode 100644 index 000000000..178a29af8 --- /dev/null +++ b/codex-rs/plugin/src/manifest.rs @@ -0,0 +1,157 @@ +use codex_config::HooksFile; + +/// Parsed plugin metadata parameterized by its resource locator representation. +/// +/// Host loading uses absolute paths, while resolved packages replace them with +/// authority-bound locators before exposing the manifest to consumers. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginManifest { + pub name: String, + pub version: Option, + pub description: Option, + pub keywords: Vec, + pub paths: PluginManifestPaths, + pub interface: Option>, +} + +/// Component resources declared by a plugin manifest. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginManifestPaths { + pub skills: Option, + pub mcp_servers: Option, + pub apps: Option, + pub hooks: Option>, +} + +/// Hook declarations embedded in or referenced by a plugin manifest. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum PluginManifestHooks { + Paths(Vec), + Inline(Vec), +} + +/// Optional model- and UI-facing plugin metadata. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginManifestInterface { + pub display_name: Option, + pub short_description: Option, + pub long_description: Option, + pub developer_name: Option, + pub category: Option, + pub capabilities: Vec, + pub website_url: Option, + pub privacy_policy_url: Option, + pub terms_of_service_url: Option, + pub default_prompt: Option>, + pub brand_color: Option, + pub composer_icon: Option, + pub logo: Option, + pub screenshots: Vec, +} + +impl Default for PluginManifestInterface { + fn default() -> Self { + Self { + display_name: None, + short_description: None, + long_description: None, + developer_name: None, + category: None, + capabilities: Vec::new(), + website_url: None, + privacy_policy_url: None, + terms_of_service_url: None, + default_prompt: None, + brand_color: None, + composer_icon: None, + logo: None, + screenshots: Vec::new(), + } + } +} + +impl PluginManifest { + pub(crate) fn try_map_resources( + self, + mut map: impl FnMut(Resource) -> Result, + ) -> Result, Error> { + let PluginManifest { + name, + version, + description, + keywords, + paths, + interface, + } = self; + let PluginManifestPaths { + skills, + mcp_servers, + apps, + hooks, + } = paths; + let hooks = match hooks { + Some(PluginManifestHooks::Paths(paths)) => Some(PluginManifestHooks::Paths( + paths + .into_iter() + .map(&mut map) + .collect::, _>>()?, + )), + Some(PluginManifestHooks::Inline(hooks)) => Some(PluginManifestHooks::Inline(hooks)), + None => None, + }; + let interface = match interface { + Some(interface) => { + let PluginManifestInterface { + display_name, + short_description, + long_description, + developer_name, + category, + capabilities, + website_url, + privacy_policy_url, + terms_of_service_url, + default_prompt, + brand_color, + composer_icon, + logo, + screenshots, + } = interface; + Some(PluginManifestInterface { + display_name, + short_description, + long_description, + developer_name, + category, + capabilities, + website_url, + privacy_policy_url, + terms_of_service_url, + default_prompt, + brand_color, + composer_icon: composer_icon.map(&mut map).transpose()?, + logo: logo.map(&mut map).transpose()?, + screenshots: screenshots + .into_iter() + .map(&mut map) + .collect::, _>>()?, + }) + } + None => None, + }; + + Ok(PluginManifest { + name, + version, + description, + keywords, + paths: PluginManifestPaths { + skills: skills.map(&mut map).transpose()?, + mcp_servers: mcp_servers.map(&mut map).transpose()?, + apps: apps.map(&mut map).transpose()?, + hooks, + }, + interface, + }) + } +} diff --git a/codex-rs/plugin/src/provider.rs b/codex-rs/plugin/src/provider.rs new file mode 100644 index 000000000..d2d8cc7c8 --- /dev/null +++ b/codex-rs/plugin/src/provider.rs @@ -0,0 +1,128 @@ +use crate::manifest::PluginManifest; +use codex_protocol::capabilities::SelectedCapabilityRoot; +use codex_utils_absolute_path::AbsolutePathBuf; +use std::error::Error as StdError; +use std::future::Future; +use thiserror::Error; + +/// A plugin resource paired with the environment that owns its filesystem. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum PluginResourceLocator { + Environment { + /// Environment whose filesystem owns the resource. + environment_id: String, + /// Absolute resource path within that filesystem. + path: AbsolutePathBuf, + }, +} + +/// Authority-bound location of a resolved plugin package. +#[derive(Clone, Debug, PartialEq, Eq)] +pub enum ResolvedPluginLocation { + Environment { + /// Environment whose filesystem owns the package. + environment_id: String, + /// Absolute package root within that filesystem. + root: AbsolutePathBuf, + }, +} + +/// An inert plugin descriptor whose resources retain their source authority. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct ResolvedPlugin { + selected_root_id: String, + location: ResolvedPluginLocation, + manifest_path: PluginResourceLocator, + manifest: PluginManifest, +} + +/// Failure to construct a resolved plugin with internally consistent resources. +#[derive(Clone, Debug, Error, PartialEq, Eq)] +pub enum ResolvedPluginError { + #[error("plugin resource path `{path}` is outside package root `{root}`")] + ResourceOutsideRoot { + root: AbsolutePathBuf, + path: AbsolutePathBuf, + }, +} + +impl ResolvedPlugin { + /// Creates an environment-owned descriptor from a validated plugin manifest. + pub fn from_environment( + selected_root_id: String, + environment_id: String, + root: AbsolutePathBuf, + manifest_path: AbsolutePathBuf, + manifest: PluginManifest, + ) -> Result { + let manifest_path = environment_resource(&environment_id, &root, manifest_path)?; + let manifest = manifest + .try_map_resources(|path| environment_resource(&environment_id, &root, path))?; + Ok(Self { + selected_root_id, + location: ResolvedPluginLocation::Environment { + environment_id, + root, + }, + manifest_path, + manifest, + }) + } + + /// Returns the opaque ID supplied for the selected capability root. + pub fn selected_root_id(&self) -> &str { + &self.selected_root_id + } + + /// Returns the authority-bound package location. + pub fn location(&self) -> &ResolvedPluginLocation { + &self.location + } + + /// Returns the manifest resource used to resolve this package. + pub fn manifest_path(&self) -> &PluginResourceLocator { + &self.manifest_path + } + + /// Returns package metadata whose resource fields retain their source authority. + pub fn manifest(&self) -> &PluginManifest { + &self.manifest + } +} + +fn environment_resource( + environment_id: &str, + root: &AbsolutePathBuf, + path: AbsolutePathBuf, +) -> Result { + if !path.as_path().starts_with(root.as_path()) { + return Err(ResolvedPluginError::ResourceOutsideRoot { + root: root.clone(), + path, + }); + } + Ok(PluginResourceLocator::Environment { + environment_id: environment_id.to_string(), + path, + }) +} + +/// Resolves source-owned package roots into inert plugin descriptors. +/// +/// Implementations must perform all filesystem access through the authority +/// named by the selected root. `None` means the root contains no plugin +/// manifest and may be handled as another standalone capability. +pub trait PluginProvider: Send + Sync { + /// Source-specific resolution failure. + type Error: StdError + Send + Sync + 'static; + + /// Resolves one selected root without activating any of its components. + fn resolve( + &self, + root: &SelectedCapabilityRoot, + ) -> impl Future, Self::Error>> + Send; +} + +#[cfg(test)] +#[path = "provider_tests.rs"] +mod tests; diff --git a/codex-rs/plugin/src/provider_tests.rs b/codex-rs/plugin/src/provider_tests.rs new file mode 100644 index 000000000..44993a640 --- /dev/null +++ b/codex-rs/plugin/src/provider_tests.rs @@ -0,0 +1,126 @@ +use super::PluginResourceLocator; +use super::ResolvedPlugin; +use super::ResolvedPluginError; +use crate::manifest::PluginManifest; +use crate::manifest::PluginManifestHooks; +use crate::manifest::PluginManifestInterface; +use crate::manifest::PluginManifestPaths; +use codex_utils_absolute_path::AbsolutePathBuf; +use pretty_assertions::assert_eq; + +fn absolute(path: impl AsRef) -> AbsolutePathBuf { + AbsolutePathBuf::from_absolute_path_checked(path.as_ref()).expect("absolute test path") +} + +fn resource(environment_id: &str, path: AbsolutePathBuf) -> PluginResourceLocator { + PluginResourceLocator::Environment { + environment_id: environment_id.to_string(), + path, + } +} + +#[test] +fn environment_descriptor_binds_every_manifest_resource() { + let root = absolute(std::env::current_dir().expect("cwd").join("plugin-root")); + let manifest_path = root.join(".codex-plugin/plugin.json"); + let skills = root.join("skills"); + let mcp_servers = root.join(".mcp.json"); + let apps = root.join(".app.json"); + let hooks = root.join("hooks/hooks.json"); + let composer_icon = root.join("assets/composer.svg"); + let logo = root.join("assets/logo.svg"); + let screenshot = root.join("assets/screenshot.png"); + let manifest = PluginManifest { + name: "demo".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: PluginManifestPaths { + skills: Some(skills.clone()), + mcp_servers: Some(mcp_servers.clone()), + apps: Some(apps.clone()), + hooks: Some(PluginManifestHooks::Paths(vec![hooks.clone()])), + }, + interface: Some(PluginManifestInterface { + composer_icon: Some(composer_icon.clone()), + logo: Some(logo.clone()), + screenshots: vec![screenshot.clone()], + ..PluginManifestInterface::default() + }), + }; + + let plugin = ResolvedPlugin::from_environment( + "selected-demo".to_string(), + "executor-1".to_string(), + root, + manifest_path.clone(), + manifest, + ) + .expect("valid descriptor"); + + assert_eq!( + plugin.manifest_path(), + &resource("executor-1", manifest_path) + ); + assert_eq!( + plugin.manifest(), + &PluginManifest { + name: "demo".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: PluginManifestPaths { + skills: Some(resource("executor-1", skills)), + mcp_servers: Some(resource("executor-1", mcp_servers)), + apps: Some(resource("executor-1", apps)), + hooks: Some(PluginManifestHooks::Paths(vec![resource( + "executor-1", + hooks, + )])), + }, + interface: Some(PluginManifestInterface { + composer_icon: Some(resource("executor-1", composer_icon)), + logo: Some(resource("executor-1", logo)), + screenshots: vec![resource("executor-1", screenshot)], + ..PluginManifestInterface::default() + }), + } + ); +} + +#[test] +fn environment_descriptor_rejects_resources_outside_package_root() { + let cwd = std::env::current_dir().expect("cwd"); + let root = absolute(cwd.join("plugin-root")); + let outside = absolute(cwd.join("outside/.mcp.json")); + let manifest = PluginManifest { + name: "demo".to_string(), + version: None, + description: None, + keywords: Vec::new(), + paths: PluginManifestPaths { + skills: None, + mcp_servers: Some(outside.clone()), + apps: None, + hooks: None, + }, + interface: None, + }; + + let err = ResolvedPlugin::from_environment( + "selected-demo".to_string(), + "executor-1".to_string(), + root.clone(), + root.join(".codex-plugin/plugin.json"), + manifest, + ) + .expect_err("outside resource should fail"); + + assert_eq!( + err, + ResolvedPluginError::ResourceOutsideRoot { + root, + path: outside, + } + ); +} diff --git a/codex-rs/utils/plugins/src/lib.rs b/codex-rs/utils/plugins/src/lib.rs index 38bf68040..fe178e679 100644 --- a/codex-rs/utils/plugins/src/lib.rs +++ b/codex-rs/utils/plugins/src/lib.rs @@ -7,6 +7,7 @@ pub mod mcp_connector; pub mod mention_syntax; pub mod plugin_namespace; +pub use plugin_namespace::DISCOVERABLE_PLUGIN_MANIFEST_PATHS; pub use plugin_namespace::find_plugin_manifest_path; pub use plugin_namespace::plugin_namespace_for_skill_path; diff --git a/codex-rs/utils/plugins/src/plugin_namespace.rs b/codex-rs/utils/plugins/src/plugin_namespace.rs index d0d8bf685..32ed608b3 100644 --- a/codex-rs/utils/plugins/src/plugin_namespace.rs +++ b/codex-rs/utils/plugins/src/plugin_namespace.rs @@ -6,7 +6,8 @@ use codex_utils_path_uri::PathUri; use std::path::Path; use std::path::PathBuf; -const DISCOVERABLE_PLUGIN_MANIFEST_PATHS: &[&str] = +/// Ordered plugin manifest paths recognized beneath a plugin root. +pub const DISCOVERABLE_PLUGIN_MANIFEST_PATHS: &[&str] = &[".codex-plugin/plugin.json", ".claude-plugin/plugin.json"]; pub fn find_plugin_manifest_path(plugin_root: &Path) -> Option {