From 2bb1027e37fd2ff7b5a72de8b5f5d8ce8cca2cba Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Wed, 25 Mar 2026 11:07:31 -0700 Subject: [PATCH] Extract codex-plugin crate (#15747) ## Summary - extract plugin identifiers and load-outcome types into codex-plugin - update codex-core to consume the new plugin crate ## Testing - CI --------- Co-authored-by: Codex --- codex-rs/Cargo.lock | 12 ++ codex-rs/Cargo.toml | 2 + codex-rs/core/Cargo.toml | 1 + codex-rs/core/src/plugins/manager.rs | 216 +++------------------ codex-rs/core/src/plugins/manager_tests.rs | 54 ++---- codex-rs/core/src/plugins/marketplace.rs | 4 +- codex-rs/core/src/plugins/mod.rs | 20 +- codex-rs/core/src/plugins/store.rs | 66 +------ codex-rs/core/src/plugins/store_tests.rs | 1 + codex-rs/plugin/BUILD.bazel | 15 ++ codex-rs/plugin/Cargo.toml | 18 ++ codex-rs/plugin/src/lib.rs | 55 ++++++ codex-rs/plugin/src/load_outcome.rs | 163 ++++++++++++++++ codex-rs/plugin/src/plugin_id.rs | 64 ++++++ 14 files changed, 394 insertions(+), 297 deletions(-) create mode 100644 codex-rs/plugin/BUILD.bazel create mode 100644 codex-rs/plugin/Cargo.toml create mode 100644 codex-rs/plugin/src/lib.rs create mode 100644 codex-rs/plugin/src/load_outcome.rs create mode 100644 codex-rs/plugin/src/plugin_id.rs diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index d9f7512fc..b9760de41 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -1887,6 +1887,7 @@ dependencies = [ "codex-login", "codex-network-proxy", "codex-otel", + "codex-plugin", "codex-protocol", "codex-rmcp-client", "codex-rollout", @@ -2394,6 +2395,17 @@ dependencies = [ "zip", ] +[[package]] +name = "codex-plugin" +version = "0.0.0" +dependencies = [ + "codex-utils-absolute-path", + "codex-utils-plugins", + "serde", + "serde_json", + "thiserror 2.0.18", +] + [[package]] name = "codex-process-hardening" version = "0.0.0" diff --git a/codex-rs/Cargo.toml b/codex-rs/Cargo.toml index 571e03790..4b23f4865 100644 --- a/codex-rs/Cargo.toml +++ b/codex-rs/Cargo.toml @@ -80,6 +80,7 @@ members = [ "codex-experimental-api-macros", "test-macros", "package-manager", + "plugin", "artifacts", ] resolver = "2" @@ -134,6 +135,7 @@ codex-mcp-server = { path = "mcp-server" } codex-network-proxy = { path = "network-proxy" } codex-ollama = { path = "ollama" } codex-otel = { path = "otel" } +codex-plugin = { path = "plugin" } codex-process-hardening = { path = "process-hardening" } codex-protocol = { path = "protocol" } codex-rollout = { path = "rollout" } diff --git a/codex-rs/core/Cargo.toml b/codex-rs/core/Cargo.toml index ebff813ab..9e01e6662 100644 --- a/codex-rs/core/Cargo.toml +++ b/codex-rs/core/Cargo.toml @@ -46,6 +46,7 @@ codex-instructions = { workspace = true } codex-network-proxy = { workspace = true } codex-otel = { workspace = true } codex-artifacts = { workspace = true } +codex-plugin = { workspace = true } codex-protocol = { workspace = true } codex-rollout = { workspace = true } codex-rmcp-client = { workspace = true } diff --git a/codex-rs/core/src/plugins/manager.rs b/codex-rs/core/src/plugins/manager.rs index d2d2e1b1c..261cdf88d 100644 --- a/codex-rs/core/src/plugins/manager.rs +++ b/codex-rs/core/src/plugins/manager.rs @@ -1,3 +1,5 @@ +use super::LoadedPlugin; +use super::PluginLoadOutcome; use super::PluginManifestPaths; use super::curated_plugins_repo_path; use super::load_plugin_manifest; @@ -20,14 +22,11 @@ use super::remote::fetch_remote_featured_plugin_ids; use super::remote::fetch_remote_plugin_status; use super::remote::uninstall_remote_plugin; use super::startup_sync::start_startup_remote_plugin_sync_once; -use super::store::PluginId; -use super::store::PluginIdError; use super::store::PluginInstallResult as StorePluginInstallResult; use super::store::PluginStore; use super::store::PluginStoreError; use super::sync_openai_plugins_repo; use crate::AuthManager; -use crate::analytics_client::AnalyticsEventsClient; use crate::auth::CodexAuth; use crate::config::Config; use crate::config::ConfigService; @@ -46,6 +45,12 @@ use crate::skills::loader::load_skills_from_roots; use codex_app_server_protocol::ConfigValueWriteParams; use codex_app_server_protocol::MergeStrategy; use codex_features::Feature; +use codex_plugin::AppConnectorId; +use codex_plugin::PluginCapabilitySummary; +use codex_plugin::PluginId; +use codex_plugin::PluginIdError; +use codex_plugin::PluginTelemetryMetadata; +use codex_plugin::prompt_safe_plugin_description; use codex_protocol::protocol::Product; use codex_protocol::protocol::SkillScope; use codex_utils_absolute_path::AbsolutePathBuf; @@ -68,12 +73,13 @@ use toml_edit::value; use tracing::info; use tracing::warn; +use crate::AnalyticsEventsClient; + const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; const DEFAULT_APP_CONFIG_FILE: &str = ".app.json"; pub const OPENAI_CURATED_MARKETPLACE_NAME: &str = "openai-curated"; static CURATED_REPO_SYNC_STARTED: AtomicBool = AtomicBool::new(false); -const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; const FEATURED_PLUGIN_IDS_CACHE_TTL: std::time::Duration = std::time::Duration::from_secs(60 * 60 * 3); @@ -114,9 +120,6 @@ fn featured_plugin_ids_cache_key( } } -#[derive(Debug, Clone, PartialEq, Eq, Hash)] -pub struct AppConnectorId(pub String); - #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallRequest { pub plugin_name: String, @@ -185,89 +188,6 @@ pub struct ConfiguredMarketplaceListOutcome { pub errors: Vec, } -#[derive(Debug, Clone, PartialEq)] -pub struct LoadedPlugin { - pub config_name: String, - pub manifest_name: Option, - pub manifest_description: Option, - pub root: AbsolutePathBuf, - pub enabled: bool, - pub skill_roots: Vec, - pub disabled_skill_paths: HashSet, - pub has_enabled_skills: bool, - pub mcp_servers: HashMap, - pub apps: Vec, - pub error: Option, -} - -impl LoadedPlugin { - fn is_active(&self) -> bool { - self.enabled && self.error.is_none() - } -} - -#[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct PluginCapabilitySummary { - pub config_name: String, - pub display_name: String, - pub description: Option, - pub has_skills: bool, - pub mcp_server_names: Vec, - pub app_connector_ids: Vec, -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginTelemetryMetadata { - pub plugin_id: PluginId, - pub capability_summary: Option, -} - -impl PluginTelemetryMetadata { - pub fn from_plugin_id(plugin_id: &PluginId) -> Self { - Self { - plugin_id: plugin_id.clone(), - capability_summary: None, - } - } -} - -impl PluginCapabilitySummary { - fn from_plugin(plugin: &LoadedPlugin) -> Option { - if !plugin.is_active() { - return None; - } - - let mut mcp_server_names: Vec = plugin.mcp_servers.keys().cloned().collect(); - mcp_server_names.sort_unstable(); - - let summary = Self { - config_name: plugin.config_name.clone(), - display_name: plugin - .manifest_name - .clone() - .unwrap_or_else(|| plugin.config_name.clone()), - description: prompt_safe_plugin_description(plugin.manifest_description.as_deref()), - has_skills: plugin.has_enabled_skills, - mcp_server_names, - app_connector_ids: plugin.apps.clone(), - }; - - (summary.has_skills - || !summary.mcp_server_names.is_empty() - || !summary.app_connector_ids.is_empty()) - .then_some(summary) - } - - pub fn telemetry_metadata(&self) -> Option { - PluginId::parse(&self.config_name) - .ok() - .map(|plugin_id| PluginTelemetryMetadata { - plugin_id, - capability_summary: Some(self.clone()), - }) - } -} - impl From for PluginCapabilitySummary { fn from(value: PluginDetail) -> Self { let has_skills = value.skills.iter().any(|skill| { @@ -286,95 +206,6 @@ impl From for PluginCapabilitySummary { } } -fn prompt_safe_plugin_description(description: Option<&str>) -> Option { - let description = description? - .split_whitespace() - .collect::>() - .join(" "); - if description.is_empty() { - return None; - } - - Some( - description - .chars() - .take(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN) - .collect(), - ) -} - -#[derive(Debug, Clone, PartialEq)] -pub struct PluginLoadOutcome { - plugins: Vec, - capability_summaries: Vec, -} - -impl Default for PluginLoadOutcome { - fn default() -> Self { - Self::from_plugins(Vec::new()) - } -} - -impl PluginLoadOutcome { - fn from_plugins(plugins: Vec) -> Self { - let capability_summaries = plugins - .iter() - .filter_map(PluginCapabilitySummary::from_plugin) - .collect::>(); - Self { - plugins, - capability_summaries, - } - } - - pub fn effective_skill_roots(&self) -> Vec { - let mut skill_roots: Vec = self - .plugins - .iter() - .filter(|plugin| plugin.is_active()) - .flat_map(|plugin| plugin.skill_roots.iter().cloned()) - .collect(); - skill_roots.sort_unstable(); - skill_roots.dedup(); - skill_roots - } - - pub fn effective_mcp_servers(&self) -> HashMap { - let mut mcp_servers = HashMap::new(); - for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { - for (name, config) in &plugin.mcp_servers { - mcp_servers - .entry(name.clone()) - .or_insert_with(|| config.clone()); - } - } - mcp_servers - } - - pub fn effective_apps(&self) -> Vec { - let mut apps = Vec::new(); - let mut seen_connector_ids = std::collections::HashSet::new(); - - for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { - for connector_id in &plugin.apps { - if seen_connector_ids.insert(connector_id.clone()) { - apps.push(connector_id.clone()); - } - } - } - - apps - } - - pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] { - &self.capability_summaries - } - - pub fn plugins(&self) -> &[LoadedPlugin] { - &self.plugins - } -} - #[derive(Debug, Clone, Default, PartialEq, Eq)] pub struct RemotePluginSyncResult { /// Plugin ids newly installed into the local plugin cache. @@ -575,6 +406,19 @@ impl PluginsManager { *cached_enabled_outcome = None; } + /// Resolve plugin skill roots for a config layer stack without touching the plugins cache. + pub fn effective_skill_roots_for_layer_stack( + &self, + config_layer_stack: &ConfigLayerStack, + plugins_feature_enabled: bool, + ) -> Vec { + if !plugins_feature_enabled { + return Vec::new(); + } + load_plugins_from_layer_stack(config_layer_stack, &self.store, self.restriction_product) + .effective_skill_roots() + } + fn cached_enabled_outcome(&self) -> Option { match self.cached_enabled_outcome.read() { Ok(cache) => cache.clone(), @@ -1174,7 +1018,7 @@ impl PluginsManager { } }) .collect::>(); - configured_curated_plugin_ids.sort_unstable_by_key(super::store::PluginId::as_key); + configured_curated_plugin_ids.sort_unstable_by_key(PluginId::as_key); self.start_curated_repo_sync(configured_curated_plugin_ids); start_startup_remote_plugin_sync_once( Arc::clone(self), @@ -1341,7 +1185,7 @@ impl PluginUninstallError { fn log_plugin_load_errors(outcome: &PluginLoadOutcome) { for plugin in outcome - .plugins + .plugins() .iter() .filter(|plugin| plugin.error.is_some()) { @@ -1413,16 +1257,6 @@ pub(crate) fn load_plugins_from_layer_stack( PluginLoadOutcome::from_plugins(plugins) } -pub(crate) fn plugin_namespace_for_skill_path(path: &Path) -> Option { - for ancestor in path.ancestors() { - if let Some(manifest) = load_plugin_manifest(ancestor) { - return Some(manifest.name); - } - } - - None -} - fn refresh_curated_plugin_cache( codex_home: &Path, plugin_version: &str, diff --git a/codex-rs/core/src/plugins/manager_tests.rs b/codex-rs/core/src/plugins/manager_tests.rs index 3d61c3937..884beee66 100644 --- a/codex-rs/core/src/plugins/manager_tests.rs +++ b/codex-rs/core/src/plugins/manager_tests.rs @@ -7,7 +7,9 @@ use crate::config_loader::ConfigLayerEntry; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigRequirements; use crate::config_loader::ConfigRequirementsToml; +use crate::plugins::LoadedPlugin; use crate::plugins::MarketplacePluginInstallPolicy; +use crate::plugins::PluginLoadOutcome; use crate::plugins::test_support::TEST_CURATED_PLUGIN_SHA; use crate::plugins::test_support::write_curated_plugin_sha_with as write_curated_plugin_sha; use crate::plugins::test_support::write_file; @@ -26,6 +28,8 @@ use wiremock::matchers::method; use wiremock::matchers::path; use wiremock::matchers::query_param; +const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; + fn write_plugin(root: &Path, dir_name: &str, manifest_name: &str) { let plugin_root = root.join(dir_name); fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); @@ -130,7 +134,7 @@ fn load_plugins_loads_default_skills_and_mcp_servers() { let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()); assert_eq!( - outcome.plugins, + outcome.plugins(), vec![LoadedPlugin { config_name: "sample@test".to_string(), manifest_name: Some("sample".to_string()), @@ -220,10 +224,10 @@ enabled = true let skill_path = dunce::canonicalize(skill_path).expect("skill path should canonicalize"); assert_eq!( - outcome.plugins[0].disabled_skill_paths, + outcome.plugins()[0].disabled_skill_paths, HashSet::from([skill_path]) ); - assert!(!outcome.plugins[0].has_enabled_skills); + assert!(!outcome.plugins()[0].has_enabled_skills); assert!(outcome.capability_summaries().is_empty()); } @@ -256,8 +260,8 @@ enabled = true "#; let outcome = load_plugins_from_config(config_toml, codex_home.path()); - assert!(outcome.plugins[0].disabled_skill_paths.is_empty()); - assert!(outcome.plugins[0].has_enabled_skills); + assert!(outcome.plugins()[0].disabled_skill_paths.is_empty()); + assert!(outcome.plugins()[0].has_enabled_skills); assert_eq!( outcome.capability_summaries(), &[PluginCapabilitySummary { @@ -338,7 +342,7 @@ fn capability_summary_sanitizes_plugin_descriptions_to_one_line() { let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()); assert_eq!( - outcome.plugins[0].manifest_description.as_deref(), + outcome.plugins()[0].manifest_description.as_deref(), Some("Plugin that\n includes the sample\tserver") ); assert_eq!( @@ -373,7 +377,7 @@ fn capability_summary_truncates_overlong_plugin_descriptions() { let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()); assert_eq!( - outcome.plugins[0].manifest_description.as_deref(), + outcome.plugins()[0].manifest_description.as_deref(), Some(too_long.as_str()) ); assert_eq!( @@ -453,14 +457,14 @@ fn load_plugins_uses_manifest_configured_component_paths() { let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()); assert_eq!( - outcome.plugins[0].skill_roots, + outcome.plugins()[0].skill_roots, vec![ plugin_root.join("custom-skills"), plugin_root.join("skills") ] ); assert_eq!( - outcome.plugins[0].mcp_servers, + outcome.plugins()[0].mcp_servers, HashMap::from([( "custom".to_string(), McpServerConfig { @@ -483,7 +487,7 @@ fn load_plugins_uses_manifest_configured_component_paths() { )]) ); assert_eq!( - outcome.plugins[0].apps, + outcome.plugins()[0].apps, vec![AppConnectorId("connector_custom".to_string())] ); } @@ -559,11 +563,11 @@ fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { let outcome = load_plugins_from_config(&plugin_config_toml(true, true), codex_home.path()); assert_eq!( - outcome.plugins[0].skill_roots, + outcome.plugins()[0].skill_roots, vec![plugin_root.join("skills")] ); assert_eq!( - outcome.plugins[0].mcp_servers, + outcome.plugins()[0].mcp_servers, HashMap::from([( "default".to_string(), McpServerConfig { @@ -586,7 +590,7 @@ fn load_plugins_ignores_manifest_component_paths_without_dot_slash() { )]) ); assert_eq!( - outcome.plugins[0].apps, + outcome.plugins()[0].apps, vec![AppConnectorId("connector_default".to_string())] ); } @@ -618,7 +622,7 @@ fn load_plugins_preserves_disabled_plugins_without_effective_contributions() { let outcome = load_plugins_from_config(&plugin_config_toml(false, true), codex_home.path()); assert_eq!( - outcome.plugins, + outcome.plugins(), vec![LoadedPlugin { config_name: "sample@test".to_string(), manifest_name: None, @@ -805,24 +809,6 @@ fn capability_index_filters_inactive_and_zero_capability_plugins() { ); } -#[test] -fn plugin_namespace_for_skill_path_uses_manifest_name() { - let codex_home = TempDir::new().unwrap(); - let plugin_root = codex_home.path().join("plugins/sample"); - let skill_path = plugin_root.join("skills/search/SKILL.md"); - - write_file( - &plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"sample"}"#, - ); - write_file(&skill_path, "---\ndescription: search\n---\n"); - - assert_eq!( - plugin_namespace_for_skill_path(&skill_path), - Some("sample".to_string()) - ); -} - #[test] fn load_plugins_returns_empty_when_feature_disabled() { let codex_home = TempDir::new().unwrap(); @@ -880,9 +866,9 @@ fn load_plugins_rejects_invalid_plugin_keys() { codex_home.path(), ); - assert_eq!(outcome.plugins.len(), 1); + assert_eq!(outcome.plugins().len(), 1); assert_eq!( - outcome.plugins[0].error.as_deref(), + outcome.plugins()[0].error.as_deref(), Some("invalid plugin key `sample`; expected @") ); assert!(outcome.effective_skill_roots().is_empty()); diff --git a/codex-rs/core/src/plugins/marketplace.rs b/codex-rs/core/src/plugins/marketplace.rs index 5563f0c3a..ee775262a 100644 --- a/codex-rs/core/src/plugins/marketplace.rs +++ b/codex-rs/core/src/plugins/marketplace.rs @@ -1,10 +1,10 @@ use super::PluginManifestInterface; use super::load_plugin_manifest; -use super::store::PluginId; -use super::store::PluginIdError; use codex_app_server_protocol::PluginAuthPolicy; use codex_app_server_protocol::PluginInstallPolicy; use codex_git_utils::get_git_repo_root; +use codex_plugin::PluginId; +use codex_plugin::PluginIdError; use codex_protocol::protocol::Product; use codex_utils_absolute_path::AbsolutePathBuf; use dirs::home_dir; diff --git a/codex-rs/core/src/plugins/mod.rs b/codex-rs/core/src/plugins/mod.rs index 34403f4d1..c16ebfcc5 100644 --- a/codex-rs/core/src/plugins/mod.rs +++ b/codex-rs/core/src/plugins/mod.rs @@ -1,3 +1,5 @@ +use crate::config::types::McpServerConfig; + mod discoverable; mod injection; mod manager; @@ -11,31 +13,36 @@ mod store; pub(crate) mod test_support; mod toggles; +pub use codex_plugin::AppConnectorId; +pub use codex_plugin::EffectiveSkillRoots; +pub use codex_plugin::PluginCapabilitySummary; +pub use codex_plugin::PluginId; +pub use codex_plugin::PluginIdError; +pub use codex_plugin::PluginTelemetryMetadata; + +pub type LoadedPlugin = codex_plugin::LoadedPlugin; +pub type PluginLoadOutcome = codex_plugin::PluginLoadOutcome; + +pub(crate) use codex_plugin::plugin_namespace_for_skill_path; pub(crate) use discoverable::list_tool_suggest_discoverable_plugins; pub(crate) use injection::build_plugin_injections; -pub use manager::AppConnectorId; pub use manager::ConfiguredMarketplace; pub use manager::ConfiguredMarketplaceListOutcome; pub use manager::ConfiguredMarketplacePlugin; -pub use manager::LoadedPlugin; pub use manager::OPENAI_CURATED_MARKETPLACE_NAME; -pub use manager::PluginCapabilitySummary; pub use manager::PluginDetail; pub use manager::PluginInstallError; pub use manager::PluginInstallOutcome; pub use manager::PluginInstallRequest; -pub use manager::PluginLoadOutcome; pub use manager::PluginReadOutcome; pub use manager::PluginReadRequest; pub use manager::PluginRemoteSyncError; -pub use manager::PluginTelemetryMetadata; pub use manager::PluginUninstallError; pub use manager::PluginsManager; pub use manager::RemotePluginSyncResult; pub use manager::installed_plugin_telemetry_metadata; pub use manager::load_plugin_apps; pub use manager::load_plugin_mcp_servers; -pub(crate) use manager::plugin_namespace_for_skill_path; pub use manager::plugin_telemetry_metadata_from_root; pub use manifest::PluginManifestInterface; pub(crate) use manifest::PluginManifestPaths; @@ -53,5 +60,4 @@ pub(crate) use render::render_plugins_section; pub(crate) use startup_sync::curated_plugins_repo_path; pub(crate) use startup_sync::read_curated_plugins_sha; pub(crate) use startup_sync::sync_openai_plugins_repo; -pub use store::PluginId; pub use toggles::collect_plugin_enabled_candidates; diff --git a/codex-rs/core/src/plugins/store.rs b/codex-rs/core/src/plugins/store.rs index 97b0ccefe..faa7fc081 100644 --- a/codex-rs/core/src/plugins/store.rs +++ b/codex-rs/core/src/plugins/store.rs @@ -1,6 +1,8 @@ use super::load_plugin_manifest; -use super::manifest::PLUGIN_MANIFEST_PATH; +use codex_plugin::PluginId; +use codex_plugin::validate_plugin_segment; use codex_utils_absolute_path::AbsolutePathBuf; +use codex_utils_plugins::PLUGIN_MANIFEST_PATH; use std::fs; use std::io; use std::path::Path; @@ -9,53 +11,6 @@ use std::path::PathBuf; pub(crate) const DEFAULT_PLUGIN_VERSION: &str = "local"; pub(crate) const PLUGINS_CACHE_DIR: &str = "plugins/cache"; -#[derive(Debug, thiserror::Error)] -pub enum PluginIdError { - #[error("{0}")] - Invalid(String), -} - -#[derive(Debug, Clone, PartialEq, Eq)] -pub struct PluginId { - pub plugin_name: String, - pub marketplace_name: String, -} - -impl PluginId { - pub fn new(plugin_name: String, marketplace_name: String) -> Result { - validate_plugin_segment(&plugin_name, "plugin name").map_err(PluginIdError::Invalid)?; - validate_plugin_segment(&marketplace_name, "marketplace name") - .map_err(PluginIdError::Invalid)?; - Ok(Self { - plugin_name, - marketplace_name, - }) - } - - pub fn parse(plugin_key: &str) -> Result { - let Some((plugin_name, marketplace_name)) = plugin_key.rsplit_once('@') else { - return Err(PluginIdError::Invalid(format!( - "invalid plugin key `{plugin_key}`; expected @" - ))); - }; - if plugin_name.is_empty() || marketplace_name.is_empty() { - return Err(PluginIdError::Invalid(format!( - "invalid plugin key `{plugin_key}`; expected @" - ))); - } - - Self::new(plugin_name.to_string(), marketplace_name.to_string()).map_err(|err| match err { - PluginIdError::Invalid(message) => { - PluginIdError::Invalid(format!("{message} in `{plugin_key}`")) - } - }) - } - - pub fn as_key(&self) -> String { - format!("{}@{}", self.plugin_name, self.marketplace_name) - } -} - #[derive(Debug, Clone, PartialEq, Eq)] pub struct PluginInstallResult { pub plugin_id: PluginId, @@ -221,21 +176,6 @@ fn plugin_name_for_source(source_path: &Path) -> Result Result<(), String> { - if segment.is_empty() { - return Err(format!("invalid {kind}: must not be empty")); - } - if !segment - .chars() - .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_') - { - return Err(format!( - "invalid {kind}: only ASCII letters, digits, `_`, and `-` are allowed" - )); - } - Ok(()) -} - fn remove_existing_target(path: &Path) -> Result<(), PluginStoreError> { if !path.exists() { return Ok(()); diff --git a/codex-rs/core/src/plugins/store_tests.rs b/codex-rs/core/src/plugins/store_tests.rs index 61c346025..e75e71c56 100644 --- a/codex-rs/core/src/plugins/store_tests.rs +++ b/codex-rs/core/src/plugins/store_tests.rs @@ -1,4 +1,5 @@ use super::*; +use codex_plugin::PluginId; use pretty_assertions::assert_eq; use tempfile::tempdir; diff --git a/codex-rs/plugin/BUILD.bazel b/codex-rs/plugin/BUILD.bazel new file mode 100644 index 000000000..292606ff0 --- /dev/null +++ b/codex-rs/plugin/BUILD.bazel @@ -0,0 +1,15 @@ +load("//:defs.bzl", "codex_rust_crate") + +codex_rust_crate( + name = "plugin", + crate_name = "codex_plugin", + compile_data = glob( + include = ["**"], + exclude = [ + "**/* *", + "BUILD.bazel", + "Cargo.toml", + ], + allow_empty = True, + ), +) diff --git a/codex-rs/plugin/Cargo.toml b/codex-rs/plugin/Cargo.toml new file mode 100644 index 000000000..b72d74682 --- /dev/null +++ b/codex-rs/plugin/Cargo.toml @@ -0,0 +1,18 @@ +[package] +edition.workspace = true +license.workspace = true +name = "codex-plugin" +version.workspace = true + +[lib] +doctest = false +name = "codex_plugin" +path = "src/lib.rs" + +[lints] +workspace = true + +[dependencies] +codex-utils-absolute-path = { workspace = true } +codex-utils-plugins = { workspace = true } +thiserror = { workspace = true } diff --git a/codex-rs/plugin/src/lib.rs b/codex-rs/plugin/src/lib.rs new file mode 100644 index 000000000..7b56cd3fa --- /dev/null +++ b/codex-rs/plugin/src/lib.rs @@ -0,0 +1,55 @@ +//! Shared plugin identifiers and telemetry-facing summaries. + +pub use codex_utils_plugins::PLUGIN_MANIFEST_PATH; +pub use codex_utils_plugins::mention_syntax; +pub use codex_utils_plugins::plugin_namespace_for_skill_path; + +mod load_outcome; +mod plugin_id; + +pub use load_outcome::EffectiveSkillRoots; +pub use load_outcome::LoadedPlugin; +pub use load_outcome::PluginLoadOutcome; +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; + +#[derive(Debug, Clone, PartialEq, Eq, Hash)] +pub struct AppConnectorId(pub String); + +#[derive(Debug, Clone, Default, PartialEq, Eq)] +pub struct PluginCapabilitySummary { + pub config_name: String, + pub display_name: String, + pub description: Option, + pub has_skills: bool, + pub mcp_server_names: Vec, + pub app_connector_ids: Vec, +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginTelemetryMetadata { + pub plugin_id: PluginId, + pub capability_summary: Option, +} + +impl PluginTelemetryMetadata { + pub fn from_plugin_id(plugin_id: &PluginId) -> Self { + Self { + plugin_id: plugin_id.clone(), + capability_summary: None, + } + } +} + +impl PluginCapabilitySummary { + pub fn telemetry_metadata(&self) -> Option { + PluginId::parse(&self.config_name) + .ok() + .map(|plugin_id| PluginTelemetryMetadata { + plugin_id, + capability_summary: Some(self.clone()), + }) + } +} diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs new file mode 100644 index 000000000..9faebaebe --- /dev/null +++ b/codex-rs/plugin/src/load_outcome.rs @@ -0,0 +1,163 @@ +use std::collections::HashMap; +use std::collections::HashSet; +use std::path::PathBuf; + +use codex_utils_absolute_path::AbsolutePathBuf; + +use crate::AppConnectorId; +use crate::PluginCapabilitySummary; + +const MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN: usize = 1024; + +/// A plugin that was loaded from disk, including merged MCP server definitions. +#[derive(Debug, Clone, PartialEq)] +pub struct LoadedPlugin { + pub config_name: String, + pub manifest_name: Option, + pub manifest_description: Option, + pub root: AbsolutePathBuf, + pub enabled: bool, + pub skill_roots: Vec, + pub disabled_skill_paths: HashSet, + pub has_enabled_skills: bool, + pub mcp_servers: HashMap, + pub apps: Vec, + pub error: Option, +} + +impl LoadedPlugin { + pub fn is_active(&self) -> bool { + self.enabled && self.error.is_none() + } +} + +fn plugin_capability_summary_from_loaded( + plugin: &LoadedPlugin, +) -> Option { + if !plugin.is_active() { + return None; + } + + let mut mcp_server_names: Vec = plugin.mcp_servers.keys().cloned().collect(); + mcp_server_names.sort_unstable(); + + let summary = PluginCapabilitySummary { + config_name: plugin.config_name.clone(), + display_name: plugin + .manifest_name + .clone() + .unwrap_or_else(|| plugin.config_name.clone()), + description: prompt_safe_plugin_description(plugin.manifest_description.as_deref()), + has_skills: plugin.has_enabled_skills, + mcp_server_names, + app_connector_ids: plugin.apps.clone(), + }; + + (summary.has_skills + || !summary.mcp_server_names.is_empty() + || !summary.app_connector_ids.is_empty()) + .then_some(summary) +} + +/// Normalizes plugin descriptions for inclusion in model-facing capability summaries. +pub fn prompt_safe_plugin_description(description: Option<&str>) -> Option { + let description = description? + .split_whitespace() + .collect::>() + .join(" "); + if description.is_empty() { + return None; + } + + Some( + description + .chars() + .take(MAX_CAPABILITY_SUMMARY_DESCRIPTION_LEN) + .collect(), + ) +} + +/// Outcome of loading configured plugins (skills roots, MCP, apps, errors). +#[derive(Debug, Clone, PartialEq)] +pub struct PluginLoadOutcome { + plugins: Vec>, + capability_summaries: Vec, +} + +impl Default for PluginLoadOutcome { + fn default() -> Self { + Self::from_plugins(Vec::new()) + } +} + +impl PluginLoadOutcome { + pub fn from_plugins(plugins: Vec>) -> Self { + let capability_summaries = plugins + .iter() + .filter_map(plugin_capability_summary_from_loaded) + .collect::>(); + Self { + plugins, + capability_summaries, + } + } + + pub fn effective_skill_roots(&self) -> Vec { + let mut skill_roots: Vec = self + .plugins + .iter() + .filter(|plugin| plugin.is_active()) + .flat_map(|plugin| plugin.skill_roots.iter().cloned()) + .collect(); + skill_roots.sort_unstable(); + skill_roots.dedup(); + skill_roots + } + + pub fn effective_mcp_servers(&self) -> HashMap { + let mut mcp_servers = HashMap::new(); + for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { + for (name, config) in &plugin.mcp_servers { + mcp_servers + .entry(name.clone()) + .or_insert_with(|| config.clone()); + } + } + mcp_servers + } + + pub fn effective_apps(&self) -> Vec { + let mut apps = Vec::new(); + let mut seen_connector_ids = HashSet::new(); + + for plugin in self.plugins.iter().filter(|plugin| plugin.is_active()) { + for connector_id in &plugin.apps { + if seen_connector_ids.insert(connector_id.clone()) { + apps.push(connector_id.clone()); + } + } + } + + apps + } + + pub fn capability_summaries(&self) -> &[PluginCapabilitySummary] { + &self.capability_summaries + } + + pub fn plugins(&self) -> &[LoadedPlugin] { + &self.plugins + } +} + +/// Implemented by [`PluginLoadOutcome`] so callers (e.g. skills) can depend on `codex-plugin` +/// without naming the MCP config type parameter. +pub trait EffectiveSkillRoots { + fn effective_skill_roots(&self) -> Vec; +} + +impl EffectiveSkillRoots for PluginLoadOutcome { + fn effective_skill_roots(&self) -> Vec { + PluginLoadOutcome::effective_skill_roots(self) + } +} diff --git a/codex-rs/plugin/src/plugin_id.rs b/codex-rs/plugin/src/plugin_id.rs new file mode 100644 index 000000000..075116322 --- /dev/null +++ b/codex-rs/plugin/src/plugin_id.rs @@ -0,0 +1,64 @@ +//! Stable plugin identifier parsing and validation shared with the plugin cache. + +#[derive(Debug, thiserror::Error)] +pub enum PluginIdError { + #[error("{0}")] + Invalid(String), +} + +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct PluginId { + pub plugin_name: String, + pub marketplace_name: String, +} + +impl PluginId { + pub fn new(plugin_name: String, marketplace_name: String) -> Result { + validate_plugin_segment(&plugin_name, "plugin name").map_err(PluginIdError::Invalid)?; + validate_plugin_segment(&marketplace_name, "marketplace name") + .map_err(PluginIdError::Invalid)?; + Ok(Self { + plugin_name, + marketplace_name, + }) + } + + pub fn parse(plugin_key: &str) -> Result { + let Some((plugin_name, marketplace_name)) = plugin_key.rsplit_once('@') else { + return Err(PluginIdError::Invalid(format!( + "invalid plugin key `{plugin_key}`; expected @" + ))); + }; + if plugin_name.is_empty() || marketplace_name.is_empty() { + return Err(PluginIdError::Invalid(format!( + "invalid plugin key `{plugin_key}`; expected @" + ))); + } + + Self::new(plugin_name.to_string(), marketplace_name.to_string()).map_err(|err| match err { + PluginIdError::Invalid(message) => { + PluginIdError::Invalid(format!("{message} in `{plugin_key}`")) + } + }) + } + + pub fn as_key(&self) -> String { + format!("{}@{}", self.plugin_name, self.marketplace_name) + } +} + +/// Validates a single path segment used in plugin IDs and cache layout. +pub fn validate_plugin_segment(segment: &str, kind: &str) -> Result<(), String> { + if segment.is_empty() { + return Err(format!("invalid {kind}: must not be empty")); + } + if !segment + .chars() + .all(|ch| ch.is_ascii_alphanumeric() || ch == '-' || ch == '_') + { + return Err(format!( + "invalid {kind}: only ASCII letters, digits, `_`, and `-` are allowed" + )); + } + Ok(()) +}