From 665b6558fb25de11dcb38fc8df1c763bc69a6bf3 Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Wed, 25 Feb 2026 20:34:22 -0800 Subject: [PATCH 1/3] feat: load from plugins --- .../app-server/src/codex_message_processor.rs | 9 +- codex-rs/cli/src/mcp_cmd.rs | 27 +- codex-rs/core/config.schema.json | 24 + codex-rs/core/src/codex.rs | 53 +- codex-rs/core/src/codex_delegate.rs | 1 + codex-rs/core/src/config/mod.rs | 5 + codex-rs/core/src/config/types.rs | 8 + codex-rs/core/src/file_watcher.rs | 4 +- codex-rs/core/src/lib.rs | 1 + codex-rs/core/src/mcp/mod.rs | 113 +++- codex-rs/core/src/mcp/skill_dependencies.rs | 6 +- codex-rs/core/src/plugins.rs | 572 ++++++++++++++++++ codex-rs/core/src/skills/injection.rs | 11 +- codex-rs/core/src/skills/loader.rs | 225 +++++-- codex-rs/core/src/skills/manager.rs | 61 +- codex-rs/core/src/skills/mod.rs | 1 - codex-rs/core/src/state/service.rs | 2 + codex-rs/core/src/thread_manager.rs | 41 +- codex-rs/core/tests/suite/client.rs | 87 +++ codex-rs/tui/src/chatwidget.rs | 5 +- codex-rs/tui/src/history_cell.rs | 6 +- 21 files changed, 1158 insertions(+), 104 deletions(-) create mode 100644 codex-rs/core/src/plugins.rs diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 1f6e1ac0f1f..280ea5e0713 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -223,6 +223,7 @@ use codex_core::find_thread_names_by_ids; use codex_core::find_thread_path_by_id_str; use codex_core::git_info::git_diff_to_remote; use codex_core::mcp::collect_mcp_snapshot; +use codex_core::mcp::configured_mcp_servers; use codex_core::mcp::group_tools_by_server; use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig; use codex_core::parse_cursor; @@ -4070,7 +4071,9 @@ impl CodexMessageProcessor { } }; - let mcp_servers = match serde_json::to_value(config.mcp_servers.get()) { + let configured_servers = + configured_mcp_servers(&config, self.thread_manager.plugins_manager().as_ref()); + let mcp_servers = match serde_json::to_value(configured_servers) { Ok(value) => value, Err(err) => { let error = JSONRPCErrorError { @@ -4131,7 +4134,9 @@ impl CodexMessageProcessor { timeout_secs, } = params; - let Some(server) = config.mcp_servers.get().get(&name) else { + let configured_servers = + configured_mcp_servers(&config, self.thread_manager.plugins_manager().as_ref()); + let Some(server) = configured_servers.get(&name) else { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, message: format!("No MCP server named '{name}' found."), diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 2da868cada2..fa06dc000db 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -14,6 +14,8 @@ use codex_core::config::types::McpServerTransportConfig; use codex_core::mcp::auth::McpOAuthLoginSupport; use codex_core::mcp::auth::compute_auth_statuses; use codex_core::mcp::auth::oauth_login_support; +use codex_core::mcp::effective_mcp_servers; +use codex_core::plugins::PluginsManager; use codex_protocol::protocol::McpAuthStatus; use codex_rmcp_client::delete_oauth_tokens; use codex_rmcp_client::perform_oauth_login; @@ -329,10 +331,12 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); let LoginArgs { name, scopes } = login_args; - let Some(server) = config.mcp_servers.get().get(&name) else { + let Some(server) = mcp_servers.get(&name) else { bail!("No MCP server named '{name}' found."); }; @@ -374,12 +378,12 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); let LogoutArgs { name } = logout_args; - let server = config - .mcp_servers - .get() + let server = mcp_servers .get(&name) .ok_or_else(|| anyhow!("No MCP server named '{name}' found in configuration."))?; @@ -404,14 +408,13 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); - let mut entries: Vec<_> = config.mcp_servers.iter().collect(); + let mut entries: Vec<_> = mcp_servers.iter().collect(); entries.sort_by(|(a, _), (b, _)| a.cmp(b)); - let auth_statuses = compute_auth_statuses( - config.mcp_servers.iter(), - config.mcp_oauth_credentials_store_mode, - ) - .await; + let auth_statuses = + compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode).await; if list_args.json { let json_entries: Vec<_> = entries @@ -654,8 +657,10 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); - let Some(server) = config.mcp_servers.get().get(&get_args.name) else { + let Some(server) = mcp_servers.get(&get_args.name) else { bail!("No MCP server named '{name}' found.", name = get_args.name); }; diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index d207d5bd8e9..03e269819c9 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -1069,6 +1069,22 @@ ], "type": "string" }, + "PluginConfig": { + "additionalProperties": false, + "properties": { + "enabled": { + "default": true, + "type": "boolean" + }, + "path": { + "$ref": "#/definitions/AbsolutePathBuf" + } + }, + "required": [ + "path" + ], + "type": "object" + }, "ProjectConfig": { "additionalProperties": false, "properties": { @@ -1990,6 +2006,14 @@ "plan_mode_reasoning_effort": { "$ref": "#/definitions/ReasoningEffort" }, + "plugins": { + "additionalProperties": { + "$ref": "#/definitions/PluginConfig" + }, + "default": {}, + "description": "User-level plugin config entries keyed by plugin name.", + "type": "object" + }, "profile": { "description": "Profile to use from the `profiles` map.", "type": "string" diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index c511739f3a1..56f76909ce5 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -184,6 +184,7 @@ use crate::mentions::build_skill_name_counts; use crate::mentions::collect_explicit_app_ids; use crate::mentions::collect_tool_mentions_from_messages; use crate::network_policy_decision::execpolicy_network_rule_amendment; +use crate::plugins::PluginsManager; use crate::project_doc::get_user_instructions; use crate::protocol::AgentMessageContentDeltaEvent; use crate::protocol::AgentReasoningSectionBreakEvent; @@ -317,6 +318,7 @@ impl Codex { auth_manager: Arc, models_manager: Arc, skills_manager: Arc, + plugins_manager: Arc, file_watcher: Arc, conversation_history: InitialHistory, session_source: SessionSource, @@ -328,6 +330,7 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); + let loaded_plugins = plugins_manager.plugins_for_config(&config); let loaded_skills = skills_manager.skills_for_config(&config); for err in &loaded_skills.errors { @@ -337,6 +340,19 @@ impl Codex { err.message ); } + for plugin in loaded_plugins + .plugins + .iter() + .filter(|plugin| plugin.error.is_some()) + { + if let Some(error) = plugin.error.as_deref() { + warn!( + plugin = plugin.config_name, + path = %plugin.root.display(), + "failed to load plugin: {error}" + ); + } + } if let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) = session_source && depth >= config.agent_max_depth @@ -465,6 +481,7 @@ impl Codex { conversation_history, session_source_clone, skills_manager, + plugins_manager, file_watcher, agent_control, ) @@ -1106,6 +1123,7 @@ impl Session { initial_history: InitialHistory, session_source: SessionSource, skills_manager: Arc, + plugins_manager: Arc, file_watcher: Arc, agent_control: AgentControl, ) -> anyhow::Result> { @@ -1197,9 +1215,11 @@ impl Session { }; let auth_manager_clone = Arc::clone(&auth_manager); let config_for_mcp = Arc::clone(&config); + let plugins_manager_for_mcp = Arc::clone(&plugins_manager); let auth_and_mcp_fut = async move { let auth = auth_manager_clone.auth().await; - let mcp_servers = effective_mcp_servers(&config_for_mcp, auth.as_ref()); + let mcp_servers = + effective_mcp_servers(&config_for_mcp, auth.as_ref(), &plugins_manager_for_mcp); let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config_for_mcp.mcp_oauth_credentials_store_mode, @@ -1449,6 +1469,7 @@ impl Session { tool_approvals: Mutex::new(ApprovalStore::default()), execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, + plugins_manager, file_watcher, agent_control, network_proxy, @@ -2365,6 +2386,8 @@ impl Session { .config_layer_stack .with_user_config(&config_toml_path, user_config); state.session_configuration.original_config_do_not_use = Arc::new(config); + self.services.skills_manager.clear_cache(); + self.services.plugins_manager.clear_cache(); } pub(crate) async fn new_default_turn_with_sub_id(&self, sub_id: String) -> Arc { @@ -4194,7 +4217,11 @@ mod handlers { pub async fn list_mcp_tools(sess: &Session, config: &Arc, sub_id: String) { let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; let auth = sess.services.auth_manager.auth().await; - let mcp_servers = effective_mcp_servers(config, auth.as_ref()); + let mcp_servers = effective_mcp_servers( + config, + auth.as_ref(), + sess.services.plugins_manager.as_ref(), + ); let snapshot = collect_mcp_snapshot_from_manager( &mcp_connection_manager, compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode) @@ -8384,6 +8411,11 @@ mod tests { let (tx_event, _rx_event) = async_channel::unbounded(); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); + let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let skills_manager = Arc::new(SkillsManager::new( + config.codex_home.clone(), + Arc::clone(&plugins_manager), + )); let result = Session::new( session_configuration, Arc::clone(&config), @@ -8394,7 +8426,8 @@ mod tests { agent_status_tx, InitialHistory::New, SessionSource::Exec, - Arc::new(SkillsManager::new(config.codex_home.clone())), + skills_manager, + plugins_manager, Arc::new(FileWatcher::noop()), AgentControl::default(), ) @@ -8476,7 +8509,11 @@ mod tests { ); let state = SessionState::new(session_configuration.clone()); - let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); + let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let skills_manager = Arc::new(SkillsManager::new( + config.codex_home.clone(), + Arc::clone(&plugins_manager), + )); let network_approval = Arc::new(NetworkApprovalService::default()); let file_watcher = Arc::new(FileWatcher::noop()); @@ -8510,6 +8547,7 @@ mod tests { tool_approvals: Mutex::new(ApprovalStore::default()), execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, + plugins_manager, file_watcher, agent_control, network_proxy: None, @@ -8636,7 +8674,11 @@ mod tests { ); let state = SessionState::new(session_configuration.clone()); - let skills_manager = Arc::new(SkillsManager::new(config.codex_home.clone())); + let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let skills_manager = Arc::new(SkillsManager::new( + config.codex_home.clone(), + Arc::clone(&plugins_manager), + )); let network_approval = Arc::new(NetworkApprovalService::default()); let file_watcher = Arc::new(FileWatcher::noop()); @@ -8670,6 +8712,7 @@ mod tests { tool_approvals: Mutex::new(ApprovalStore::default()), execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, + plugins_manager, file_watcher, agent_control, network_proxy: None, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index 0777f8e3b28..dde9435e151 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -53,6 +53,7 @@ pub(crate) async fn run_codex_thread_interactive( auth_manager, models_manager, Arc::clone(&parent_session.services.skills_manager), + Arc::clone(&parent_session.services.plugins_manager), Arc::clone(&parent_session.services.file_watcher), initial_history.unwrap_or(InitialHistory::New), SessionSource::SubAgent(SubAgentSource::Review), diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index b3661fe790b..9a79ef855b3 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -15,6 +15,7 @@ use crate::config::types::Notifications; use crate::config::types::OtelConfig; use crate::config::types::OtelConfigToml; use crate::config::types::OtelExporterKind; +use crate::config::types::PluginConfig; use crate::config::types::SandboxWorkspaceWrite; use crate::config::types::ShellEnvironmentPolicy; use crate::config::types::ShellEnvironmentPolicyToml; @@ -1207,6 +1208,10 @@ pub struct ConfigToml { /// User-level skill config entries keyed by SKILL.md path. pub skills: Option, + /// User-level plugin config entries keyed by plugin name. + #[serde(default)] + pub plugins: HashMap, + /// Centralized feature flags (new). Prefer this over individual toggles. #[serde(default)] // Injects known feature keys into the schema and forbids unknown keys. diff --git a/codex-rs/core/src/config/types.rs b/codex-rs/core/src/config/types.rs index ec0bf15320b..5fe5d331a45 100644 --- a/codex-rs/core/src/config/types.rs +++ b/codex-rs/core/src/config/types.rs @@ -745,6 +745,14 @@ pub struct SkillConfig { pub enabled: bool, } +#[derive(Serialize, Deserialize, Debug, Clone, PartialEq, Eq, JsonSchema)] +#[schemars(deny_unknown_fields)] +pub struct PluginConfig { + pub path: AbsolutePathBuf, + #[serde(default = "default_enabled")] + pub enabled: bool, +} + #[derive(Serialize, Deserialize, Debug, Clone, Default, PartialEq, Eq, JsonSchema)] #[schemars(deny_unknown_fields)] pub struct SkillsConfig { diff --git a/codex-rs/core/src/file_watcher.rs b/codex-rs/core/src/file_watcher.rs index ab0b175db11..c804e11f38b 100644 --- a/codex-rs/core/src/file_watcher.rs +++ b/codex-rs/core/src/file_watcher.rs @@ -23,7 +23,7 @@ use tokio::time::sleep_until; use tracing::warn; use crate::config::Config; -use crate::skills::loader::skill_roots_from_layer_stack_with_agents; +use crate::skills::loader::skill_roots; #[derive(Debug, Clone, PartialEq, Eq)] pub enum FileWatcherEvent { @@ -145,7 +145,7 @@ impl FileWatcher { pub(crate) fn register_config(self: &Arc, config: &Config) -> WatchRegistration { let deduped_roots: HashSet = - skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd) + skill_roots(&config.config_layer_stack, &config.cwd, Vec::new()) .into_iter() .map(|root| root.path) .collect(); diff --git a/codex-rs/core/src/lib.rs b/codex-rs/core/src/lib.rs index 7edb94b793e..a7bcb58f35d 100644 --- a/codex-rs/core/src/lib.rs +++ b/codex-rs/core/src/lib.rs @@ -57,6 +57,7 @@ mod message_history; mod model_provider_info; pub mod path_utils; pub mod personality_migration; +pub mod plugins; mod sandbox_tags; pub mod sandboxing; mod session_prefix; diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index d744010469b..09fee1ce806 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -25,6 +25,7 @@ use crate::mcp::auth::compute_auth_statuses; use crate::mcp_connection_manager::McpConnectionManager; use crate::mcp_connection_manager::SandboxState; use crate::mcp_connection_manager::codex_apps_tools_cache_key; +use crate::plugins::PluginsManager; const MCP_TOOL_NAME_PREFIX: &str = "mcp"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; @@ -160,12 +161,26 @@ pub(crate) fn with_codex_apps_mcp( servers } -pub(crate) fn effective_mcp_servers( +pub fn configured_mcp_servers( + config: &Config, + plugins_manager: &PluginsManager, +) -> HashMap { + let loaded_plugins = plugins_manager.plugins_for_config(config); + let mut servers = config.mcp_servers.get().clone(); + for (name, plugin_server) in loaded_plugins.effective_mcp_servers() { + servers.entry(name).or_insert(plugin_server); + } + servers +} + +pub fn effective_mcp_servers( config: &Config, auth: Option<&CodexAuth>, + plugins_manager: &PluginsManager, ) -> HashMap { + let servers = configured_mcp_servers(config, plugins_manager); with_codex_apps_mcp( - config.mcp_servers.get().clone(), + servers, config.features.enabled(Feature::Apps), auth, config, @@ -179,7 +194,8 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent config.cli_auth_credentials_store_mode, ); let auth = auth_manager.auth().await; - let mcp_servers = effective_mcp_servers(config, auth.as_ref()); + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let mcp_servers = effective_mcp_servers(config, auth.as_ref(), &plugins_manager); if mcp_servers.is_empty() { return McpListToolsResponseEvent { tools: HashMap::new(), @@ -366,7 +382,16 @@ pub(crate) async fn collect_mcp_snapshot_from_manager( #[cfg(test)] mod tests { use super::*; + use crate::config::CONFIG_TOML_FILE; + use crate::config::ConfigBuilder; use pretty_assertions::assert_eq; + use std::fs; + use std::path::Path; + + fn write_file(path: &Path, contents: &str) { + fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap(); + fs::write(path, contents).unwrap(); + } fn make_tool(name: &str) -> Tool { Tool { @@ -542,4 +567,86 @@ mod tests { let expected_url = format!("{OPENAI_CONNECTORS_MCP_BASE_URL}{OPENAI_CONNECTORS_MCP_PATH}"); assert_eq!(url, &expected_url); } + + #[tokio::test] + async fn effective_mcp_servers_include_plugins_without_overriding_user_config() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let plugin_root = codex_home.path().join("plugin-sample"); + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "sample": { + "type": "http", + "url": "https://plugin.example/mcp" + }, + "docs": { + "type": "http", + "url": "https://docs.example/mcp" + } + } +}"#, + ); + write_file( + &codex_home.path().join(CONFIG_TOML_FILE), + &format!( + "[plugins.sample]\npath = \"{}\"\nenabled = true\n", + plugin_root.display() + ), + ); + + let mut config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .build() + .await + .expect("config should load"); + + let mut configured_servers = config.mcp_servers.get().clone(); + configured_servers.insert( + "sample".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://user.example/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + }, + ); + config + .mcp_servers + .set(configured_servers) + .expect("test config should accept MCP servers"); + + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let effective = effective_mcp_servers(&config, None, &plugins_manager); + + let sample = effective.get("sample").expect("user server should exist"); + let docs = effective.get("docs").expect("plugin server should exist"); + + match &sample.transport { + McpServerTransportConfig::StreamableHttp { url, .. } => { + assert_eq!(url, "https://user.example/mcp"); + } + other => panic!("expected streamable http transport, got {other:?}"), + } + match &docs.transport { + McpServerTransportConfig::StreamableHttp { url, .. } => { + assert_eq!(url, "https://docs.example/mcp"); + } + other => panic!("expected streamable http transport, got {other:?}"), + } + } } diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index 8d5fc3c1a35..e62020c8a6e 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -254,7 +254,11 @@ pub(crate) async fn maybe_install_mcp_dependencies( // Refresh from the effective merged MCP map (global + repo + managed) and // overlay the updated global servers so we don't drop repo-scoped servers. let auth = sess.services.auth_manager.auth().await; - let mut refresh_servers = effective_mcp_servers(config, auth.as_ref()); + let mut refresh_servers = effective_mcp_servers( + config, + auth.as_ref(), + sess.services.plugins_manager.as_ref(), + ); for (name, server_config) in &servers { refresh_servers .entry(name.clone()) diff --git a/codex-rs/core/src/plugins.rs b/codex-rs/core/src/plugins.rs new file mode 100644 index 00000000000..231171f91cf --- /dev/null +++ b/codex-rs/core/src/plugins.rs @@ -0,0 +1,572 @@ +use crate::config::Config; +use crate::config::types::McpServerConfig; +use crate::config::types::PluginConfig; +use crate::config_loader::CloudRequirementsLoader; +use crate::config_loader::ConfigLayerStack; +use crate::config_loader::LoaderOverrides; +use crate::config_loader::load_config_layers_state; +use codex_utils_absolute_path::AbsolutePathBuf; +use serde::Deserialize; +use serde_json::Map as JsonMap; +use serde_json::Value as JsonValue; +use std::collections::HashMap; +use std::fs; +use std::path::Path; +use std::path::PathBuf; +use std::sync::RwLock; +use tracing::warn; + +const PLUGIN_MANIFEST_PATH: &str = ".codex-plugin/plugin.json"; +const DEFAULT_SKILLS_DIR_NAME: &str = "skills"; +const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; + +#[derive(Debug, Clone, PartialEq)] +pub struct LoadedPlugin { + pub config_name: String, + pub manifest_name: Option, + pub root: PathBuf, + pub enabled: bool, + pub skill_roots: Vec, + pub mcp_servers: HashMap, + pub error: Option, +} + +impl LoadedPlugin { + fn is_active(&self) -> bool { + self.enabled && self.error.is_none() + } +} + +#[derive(Debug, Clone, Default, PartialEq)] +pub struct PluginLoadOutcome { + pub plugins: Vec, +} + +impl PluginLoadOutcome { + 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 struct PluginsManager { + codex_home: PathBuf, + cache_by_cwd: RwLock>, +} + +impl PluginsManager { + pub fn new(codex_home: PathBuf) -> Self { + Self { + codex_home, + cache_by_cwd: RwLock::new(HashMap::new()), + } + } + + pub fn plugins_for_config(&self, config: &Config) -> PluginLoadOutcome { + let cwd = &config.cwd; + if let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + return outcome; + } + + let outcome = load_plugins_from_layer_stack(&config.config_layer_stack); + let mut cache = match self.cache_by_cwd.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.insert(cwd.to_path_buf(), outcome.clone()); + outcome + } + + pub async fn plugins_for_cwd(&self, cwd: &Path, force_reload: bool) -> PluginLoadOutcome { + if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + return outcome; + } + + let cwd_abs = match AbsolutePathBuf::try_from(cwd) { + Ok(cwd_abs) => cwd_abs, + Err(err) => { + warn!("failed to resolve cwd for plugin loading: {err}"); + return PluginLoadOutcome::default(); + } + }; + + let config_layer_stack = match load_config_layers_state( + &self.codex_home, + Some(cwd_abs), + &[], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await + { + Ok(config_layer_stack) => config_layer_stack, + Err(err) => { + warn!("failed to load config layers for plugin loading: {err}"); + return PluginLoadOutcome::default(); + } + }; + + let outcome = load_plugins_from_layer_stack(&config_layer_stack); + let mut cache = match self.cache_by_cwd.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.insert(cwd.to_path_buf(), outcome.clone()); + outcome + } + + pub fn clear_cache(&self) { + let mut cache = match self.cache_by_cwd.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + cache.clear(); + } + + fn cached_outcome_for_cwd(&self, cwd: &Path) -> Option { + match self.cache_by_cwd.read() { + Ok(cache) => cache.get(cwd).cloned(), + Err(err) => err.into_inner().get(cwd).cloned(), + } + } +} + +#[derive(Debug, Default, Deserialize)] +struct PluginManifest { + name: String, +} + +#[derive(Debug, Default, Deserialize)] +#[serde(rename_all = "camelCase")] +struct PluginMcpFile { + #[serde(default)] + mcp_servers: HashMap, +} + +pub fn load_plugins_from_layer_stack(config_layer_stack: &ConfigLayerStack) -> PluginLoadOutcome { + let mut configured_plugins: Vec<_> = configured_plugins_from_stack(config_layer_stack) + .into_iter() + .collect(); + configured_plugins.sort_unstable_by(|(a, _), (b, _)| a.cmp(b)); + + let mut plugins = Vec::with_capacity(configured_plugins.len()); + let mut seen_mcp_server_names = HashMap::::new(); + for (configured_name, plugin) in configured_plugins { + let loaded_plugin = load_plugin(configured_name.clone(), &plugin); + for name in loaded_plugin.mcp_servers.keys() { + if let Some(previous_plugin) = + seen_mcp_server_names.insert(name.clone(), configured_name.clone()) + { + warn!( + plugin = configured_name, + previous_plugin, + server = name, + "skipping duplicate plugin MCP server name" + ); + } + } + plugins.push(loaded_plugin); + } + + PluginLoadOutcome { 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(plugin_manifest_name(&manifest, ancestor)); + } + } + + None +} + +fn configured_plugins_from_stack( + config_layer_stack: &ConfigLayerStack, +) -> HashMap { + let Some(user_layer) = config_layer_stack.get_user_layer() else { + return HashMap::new(); + }; + let Some(plugins_value) = user_layer.config.get("plugins") else { + return HashMap::new(); + }; + match plugins_value.clone().try_into() { + Ok(plugins) => plugins, + Err(err) => { + warn!("invalid plugins config: {err}"); + HashMap::new() + } + } +} + +fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin { + let plugin_root = plugin.path.to_path_buf(); + let mut loaded_plugin = LoadedPlugin { + config_name, + manifest_name: None, + root: plugin_root.clone(), + enabled: plugin.enabled, + skill_roots: Vec::new(), + mcp_servers: HashMap::new(), + error: None, + }; + + if !plugin.enabled { + return loaded_plugin; + } + + if !plugin_root.is_dir() { + loaded_plugin.error = Some("path does not exist or is not a directory".to_string()); + return loaded_plugin; + } + + let Some(manifest) = load_plugin_manifest(&plugin_root) else { + loaded_plugin.error = Some("missing or invalid .codex-plugin/plugin.json".to_string()); + return loaded_plugin; + }; + + loaded_plugin.manifest_name = Some(plugin_manifest_name(&manifest, &plugin_root)); + loaded_plugin.skill_roots = default_skill_roots(&plugin_root); + let mut mcp_servers = HashMap::new(); + for mcp_config_path in default_mcp_config_paths(&plugin_root) { + let plugin_mcp = load_mcp_servers_from_file(&plugin_root, &mcp_config_path); + for (name, config) in plugin_mcp.mcp_servers { + if mcp_servers.insert(name.clone(), config).is_some() { + warn!( + plugin = %plugin_root.display(), + path = %mcp_config_path.display(), + server = name, + "plugin MCP file overwrote an earlier server definition" + ); + } + } + } + loaded_plugin.mcp_servers = mcp_servers; + loaded_plugin +} + +fn load_plugin_manifest(plugin_root: &Path) -> Option { + let manifest_path = plugin_root.join(PLUGIN_MANIFEST_PATH); + if !manifest_path.is_file() { + return None; + } + let contents = fs::read_to_string(&manifest_path).ok()?; + match serde_json::from_str(&contents) { + Ok(manifest) => Some(manifest), + Err(err) => { + warn!( + path = %manifest_path.display(), + "failed to parse plugin manifest: {err}" + ); + None + } + } +} + +fn plugin_manifest_name(manifest: &PluginManifest, plugin_root: &Path) -> String { + plugin_root + .file_name() + .and_then(|name| name.to_str()) + .filter(|_| manifest.name.trim().is_empty()) + .unwrap_or(&manifest.name) + .to_string() +} + +fn default_skill_roots(plugin_root: &Path) -> Vec { + let skills_dir = plugin_root.join(DEFAULT_SKILLS_DIR_NAME); + if skills_dir.is_dir() { + vec![skills_dir] + } else { + Vec::new() + } +} + +fn default_mcp_config_paths(plugin_root: &Path) -> Vec { + let mut paths = Vec::new(); + let default_path = plugin_root.join(DEFAULT_MCP_CONFIG_FILE); + if default_path.is_file() { + paths.push(default_path); + } + paths.sort_unstable(); + paths.dedup(); + paths +} + +fn load_mcp_servers_from_file(plugin_root: &Path, mcp_config_path: &Path) -> PluginMcpDiscovery { + let Ok(contents) = fs::read_to_string(mcp_config_path) else { + return PluginMcpDiscovery::default(); + }; + let parsed = match serde_json::from_str::(&contents) { + Ok(parsed) => parsed, + Err(err) => { + warn!( + path = %mcp_config_path.display(), + "failed to parse plugin MCP config: {err}" + ); + return PluginMcpDiscovery::default(); + } + }; + normalize_plugin_mcp_servers( + plugin_root, + parsed.mcp_servers, + mcp_config_path.to_string_lossy().as_ref(), + ) +} + +fn normalize_plugin_mcp_servers( + plugin_root: &Path, + plugin_mcp_servers: HashMap, + source: &str, +) -> PluginMcpDiscovery { + let mut mcp_servers = HashMap::new(); + + for (name, config_value) in plugin_mcp_servers { + let normalized = normalize_plugin_mcp_server_value(plugin_root, config_value); + match serde_json::from_value::(JsonValue::Object(normalized)) { + Ok(config) => { + mcp_servers.insert(name, config); + } + Err(err) => { + warn!( + plugin = %plugin_root.display(), + server = name, + "failed to parse plugin MCP server from {source}: {err}" + ); + } + } + } + + PluginMcpDiscovery { mcp_servers } +} + +fn normalize_plugin_mcp_server_value( + plugin_root: &Path, + value: JsonValue, +) -> JsonMap { + let mut object = match value { + JsonValue::Object(object) => object, + _ => return JsonMap::new(), + }; + + if let Some(JsonValue::String(transport_type)) = object.remove("type") { + match transport_type.as_str() { + "http" | "streamable_http" | "streamable-http" => {} + "stdio" => {} + other => { + warn!( + plugin = %plugin_root.display(), + transport = other, + "plugin MCP server uses an unknown transport type" + ); + } + } + } + + if let Some(JsonValue::Object(oauth)) = object.remove("oauth") + && oauth.contains_key("callbackPort") + { + warn!( + plugin = %plugin_root.display(), + "plugin MCP server OAuth callbackPort is ignored; Codex uses global MCP OAuth callback settings" + ); + } + + if let Some(JsonValue::String(cwd)) = object.get("cwd") + && !Path::new(cwd).is_absolute() + { + object.insert( + "cwd".to_string(), + JsonValue::String(plugin_root.join(cwd).display().to_string()), + ); + } + + object +} + +#[derive(Debug, Default)] +struct PluginMcpDiscovery { + mcp_servers: HashMap, +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::config::CONFIG_TOML_FILE; + use crate::config::ConfigBuilder; + use crate::config::types::McpServerTransportConfig; + use pretty_assertions::assert_eq; + use tempfile::TempDir; + + fn write_file(path: &Path, contents: &str) { + fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap(); + fs::write(path, contents).unwrap(); + } + + async fn load_plugins_from_config(config_toml: &str, codex_home: &Path) -> PluginLoadOutcome { + write_file(&codex_home.join(CONFIG_TOML_FILE), config_toml); + let config = ConfigBuilder::default() + .codex_home(codex_home.to_path_buf()) + .build() + .await + .expect("config should load"); + load_plugins_from_layer_stack(&config.config_layer_stack) + } + + #[tokio::test] + async fn load_plugins_loads_default_skills_and_mcp_servers() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home.path().join("plugin-sample"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join("skills/sample-search/SKILL.md"), + "---\nname: sample-search\ndescription: search sample data\n---\n", + ); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "sample": { + "type": "http", + "url": "https://sample.example/mcp", + "oauth": { + "clientId": "client-id", + "callbackPort": 3118 + } + } + } +}"#, + ); + + let outcome = load_plugins_from_config( + &format!( + "[plugins.sample]\npath = \"{}\"\nenabled = true\n", + plugin_root.display() + ), + codex_home.path(), + ) + .await; + + assert_eq!( + outcome.plugins, + vec![LoadedPlugin { + config_name: "sample".to_string(), + manifest_name: Some("sample".to_string()), + root: plugin_root.clone(), + enabled: true, + skill_roots: vec![plugin_root.join("skills")], + mcp_servers: HashMap::from([( + "sample".to_string(), + McpServerConfig { + transport: McpServerTransportConfig::StreamableHttp { + url: "https://sample.example/mcp".to_string(), + bearer_token_env_var: None, + http_headers: None, + env_http_headers: None, + }, + enabled: true, + required: false, + disabled_reason: None, + startup_timeout_sec: None, + tool_timeout_sec: None, + enabled_tools: None, + disabled_tools: None, + scopes: None, + }, + )]), + error: None, + }] + ); + assert_eq!( + outcome.effective_skill_roots(), + vec![plugin_root.join("skills")] + ); + assert_eq!(outcome.effective_mcp_servers().len(), 1); + } + + #[tokio::test] + async fn load_plugins_preserves_disabled_plugins_without_effective_contributions() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home.path().join("plugin-sample"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join(".mcp.json"), + r#"{ + "mcpServers": { + "sample": { + "type": "http", + "url": "https://sample.example/mcp" + } + } +}"#, + ); + + let outcome = load_plugins_from_config( + &format!( + "[plugins.sample]\npath = \"{}\"\nenabled = false\n", + plugin_root.display() + ), + codex_home.path(), + ) + .await; + + assert_eq!( + outcome.plugins, + vec![LoadedPlugin { + config_name: "sample".to_string(), + manifest_name: None, + root: plugin_root, + enabled: false, + skill_roots: Vec::new(), + mcp_servers: HashMap::new(), + error: None, + }] + ); + assert!(outcome.effective_skill_roots().is_empty()); + assert!(outcome.effective_mcp_servers().is_empty()); + } + + #[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()) + ); + } +} diff --git a/codex-rs/core/src/skills/injection.rs b/codex-rs/core/src/skills/injection.rs index e7856897a2b..9bb3a74781e 100644 --- a/codex-rs/core/src/skills/injection.rs +++ b/codex-rs/core/src/skills/injection.rs @@ -464,7 +464,7 @@ fn text_mentions_skill(text: &str, skill_name: &str) -> bool { } fn is_mention_name_char(byte: u8) -> bool { - matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-') + matches!(byte, b'a'..=b'z' | b'A'..=b'Z' | b'0'..=b'9' | b'_' | b'-' | b':') } #[cfg(test)] @@ -586,6 +586,15 @@ mod tests { ); } + #[test] + fn extract_tool_mentions_keeps_plugin_skill_namespaces() { + assert_mentions( + "use $slack:search and $alpha", + &["alpha", "slack:search"], + &[], + ); + } + #[test] fn collect_explicit_skill_mentions_text_respects_skill_order() { let alpha = make_skill("alpha-skill", "/tmp/alpha"); diff --git a/codex-rs/core/src/skills/loader.rs b/codex-rs/core/src/skills/loader.rs index a1aa3c630ee..a368915c3d1 100644 --- a/codex-rs/core/src/skills/loader.rs +++ b/codex-rs/core/src/skills/loader.rs @@ -1,10 +1,10 @@ -use crate::config::Config; use crate::config::Permissions; use crate::config_loader::ConfigLayerStack; use crate::config_loader::ConfigLayerStackOrdering; use crate::config_loader::default_project_root_markers; use crate::config_loader::merge_toml_values; use crate::config_loader::project_root_markers_from_config; +use crate::plugins::plugin_namespace_for_skill_path; use crate::skills::model::SkillDependencies; use crate::skills::model::SkillError; use crate::skills::model::SkillInterface; @@ -31,10 +31,15 @@ use std::path::PathBuf; use toml::Value as TomlValue; use tracing::error; +#[cfg(test)] +use crate::config::Config; + #[derive(Debug, Deserialize)] struct SkillFrontmatter { - name: String, - description: String, + #[serde(default)] + name: Option, + #[serde(default)] + description: Option, #[serde(default)] metadata: SkillFrontmatterMetadata, } @@ -145,20 +150,6 @@ impl fmt::Display for SkillParseError { impl Error for SkillParseError {} -pub fn load_skills(config: &Config) -> SkillLoadOutcome { - load_skills_with_home_dir(config, home_dir().as_deref()) -} - -fn load_skills_with_home_dir(config: &Config, home_dir: Option<&Path>) -> SkillLoadOutcome { - let mut roots = skill_roots_from_layer_stack_inner(&config.config_layer_stack, home_dir); - roots.extend(repo_agents_skill_roots( - &config.config_layer_stack, - &config.cwd, - )); - dedupe_skill_roots_by_path(&mut roots); - load_skills_from_roots(roots) -} - pub(crate) struct SkillRoot { pub(crate) path: PathBuf, pub(crate) scope: SkillScope, @@ -198,6 +189,35 @@ where outcome } +pub(crate) fn skill_roots( + config_layer_stack: &ConfigLayerStack, + cwd: &Path, + plugin_skill_roots: Vec, +) -> Vec { + skill_roots_with_home_dir( + config_layer_stack, + cwd, + home_dir().as_deref(), + plugin_skill_roots, + ) +} + +fn skill_roots_with_home_dir( + config_layer_stack: &ConfigLayerStack, + cwd: &Path, + home_dir: Option<&Path>, + plugin_skill_roots: Vec, +) -> Vec { + let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir); + roots.extend(plugin_skill_roots.into_iter().map(|path| SkillRoot { + path, + scope: SkillScope::User, + })); + roots.extend(repo_agents_skill_roots(config_layer_stack, cwd)); + dedupe_skill_roots_by_path(&mut roots); + roots +} + fn skill_roots_from_layer_stack_inner( config_layer_stack: &ConfigLayerStack, home_dir: Option<&Path>, @@ -259,34 +279,6 @@ fn skill_roots_from_layer_stack_inner( roots } -#[cfg(test)] -fn skill_roots(config: &Config) -> Vec { - skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd) -} - -#[cfg(test)] -pub(crate) fn skill_roots_from_layer_stack( - config_layer_stack: &ConfigLayerStack, - home_dir: Option<&Path>, -) -> Vec { - skill_roots_from_layer_stack_inner(config_layer_stack, home_dir) -} - -pub(crate) fn skill_roots_from_layer_stack_with_agents( - config_layer_stack: &ConfigLayerStack, - cwd: &Path, -) -> Vec { - let mut roots = skill_roots_from_layer_stack_inner(config_layer_stack, home_dir().as_deref()); - roots.extend(repo_agents_skill_roots(config_layer_stack, cwd)); - dedupe_skill_roots_by_path(&mut roots); - roots -} - -fn dedupe_skill_roots_by_path(roots: &mut Vec) { - let mut seen: HashSet = HashSet::new(); - roots.retain(|root| seen.insert(root.path.clone())); -} - fn repo_agents_skill_roots(config_layer_stack: &ConfigLayerStack, cwd: &Path) -> Vec { let project_root_markers = project_root_markers_from_stack(config_layer_stack); let project_root = find_project_root(cwd, &project_root_markers); @@ -360,6 +352,11 @@ fn dirs_between_project_root_and_cwd(cwd: &Path, project_root: &Path) -> Vec) { + let mut seen: HashSet = HashSet::new(); + roots.retain(|root| seen.insert(root.path.clone())); +} + fn discover_skills_under_root(root: &Path, scope: SkillScope, outcome: &mut SkillLoadOutcome) { let Ok(root) = canonicalize_path(root) else { return; @@ -507,8 +504,18 @@ fn parse_skill_file(path: &Path, scope: SkillScope) -> Result Result String { + path.parent() + .and_then(Path::file_name) + .and_then(|name| name.to_str()) + .map(sanitize_single_line) + .filter(|value| !value.is_empty()) + .unwrap_or_else(|| "skill".to_string()) +} + +fn namespaced_skill_name(path: &Path, base_name: &str) -> String { + plugin_namespace_for_skill_path(path) + .map(|namespace| format!("{namespace}:{base_name}")) + .unwrap_or_else(|| base_name.to_string()) +} + fn load_skill_metadata(skill_path: &Path) -> LoadedSkillMetadata { // Fail open: optional metadata should not block loading SKILL.md. let Some(skill_dir) = skill_path.parent() else { @@ -824,6 +846,13 @@ fn extract_frontmatter(contents: &str) -> Option { Some(frontmatter_lines.join("\n")) } +#[cfg(test)] +pub(crate) fn skill_roots_from_layer_stack( + config_layer_stack: &ConfigLayerStack, + home_dir: Option<&Path>, +) -> Vec { + skill_roots_with_home_dir(config_layer_stack, Path::new("."), home_dir, Vec::new()) +} #[cfg(test)] mod tests { @@ -893,7 +922,12 @@ mod tests { fn load_skills_for_test(config: &Config) -> SkillLoadOutcome { // Keep unit tests hermetic by never scanning the real `$HOME/.agents/skills`. - super::load_skills_with_home_dir(config, None) + super::load_skills_from_roots(super::skill_roots_with_home_dir( + &config.config_layer_stack, + &config.cwd, + None, + Vec::new(), + )) } fn mark_as_git_repo(dir: &Path) { @@ -1101,6 +1135,15 @@ mod tests { path } + fn write_raw_skill_at(root: &Path, dir: &str, frontmatter: &str) -> PathBuf { + let skill_dir = root.join(dir); + fs::create_dir_all(&skill_dir).unwrap(); + let path = skill_dir.join(SKILLS_FILENAME); + let content = format!("---\n{frontmatter}\n---\n\n# Body\n"); + fs::write(&path, content).unwrap(); + path + } + fn write_skill_metadata_at(skill_dir: &Path, contents: &str) -> PathBuf { let path = skill_dir .join(SKILLS_METADATA_DIR) @@ -2047,6 +2090,83 @@ permissions: ); } + #[tokio::test] + async fn falls_back_to_directory_name_when_skill_name_is_missing() { + let codex_home = tempfile::tempdir().expect("tempdir"); + let skill_path = write_raw_skill_at( + &codex_home.path().join("skills"), + "directory-derived", + "description: fallback name", + ); + let cfg = make_config(&codex_home).await; + + let outcome = load_skills_for_test(&cfg); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "directory-derived".to_string(), + description: "fallback name".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + permission_profile: None, + permissions: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + + #[tokio::test] + async fn namespaces_plugin_skills_using_plugin_name() { + let root = tempfile::tempdir().expect("tempdir"); + let plugin_root = root.path().join("plugins/sample"); + let skill_path = write_raw_skill_at( + &plugin_root.join("skills"), + "sample-search", + "description: search sample data", + ); + fs::create_dir_all(plugin_root.join(".codex-plugin")).unwrap(); + fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .unwrap(); + + let outcome = load_skills_from_roots([SkillRoot { + path: plugin_root.join("skills"), + scope: SkillScope::User, + }]); + + assert!( + outcome.errors.is_empty(), + "unexpected errors: {:?}", + outcome.errors + ); + assert_eq!( + outcome.skills, + vec![SkillMetadata { + name: "sample:sample-search".to_string(), + description: "search sample data".to_string(), + short_description: None, + interface: None, + dependencies: None, + policy: None, + permission_profile: None, + permissions: None, + path_to_skills_md: normalized(&skill_path), + scope: SkillScope::User, + }] + ); + } + #[tokio::test] async fn loads_short_description_from_metadata() { let codex_home = tempfile::tempdir().expect("tempdir"); @@ -2649,10 +2769,11 @@ permissions: let codex_home = tempfile::tempdir().expect("tempdir"); let cfg = make_config(&codex_home).await; - let scopes: Vec = skill_roots(&cfg) - .into_iter() - .map(|root| root.scope) - .collect(); + let scopes: Vec = + super::skill_roots(&cfg.config_layer_stack, &cfg.cwd, Vec::new()) + .into_iter() + .map(|root| root.scope) + .collect(); let mut expected = vec![SkillScope::User, SkillScope::System]; if home_dir().is_some() { expected.insert(1, SkillScope::User); diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index 7d2c6b89fd6..b11a839f831 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -16,26 +16,29 @@ use crate::config::types::SkillsConfig; use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; +use crate::plugins::PluginsManager; use crate::skills::SkillLoadOutcome; use crate::skills::build_implicit_skill_path_indexes; use crate::skills::loader::SkillRoot; use crate::skills::loader::load_skills_from_roots; -use crate::skills::loader::skill_roots_from_layer_stack_with_agents; +use crate::skills::loader::skill_roots; use crate::skills::system::install_system_skills; pub struct SkillsManager { codex_home: PathBuf, + plugins_manager: Arc, cache_by_cwd: RwLock>, } impl SkillsManager { - pub fn new(codex_home: PathBuf) -> Self { + pub fn new(codex_home: PathBuf, plugins_manager: Arc) -> Self { if let Err(err) = install_system_skills(&codex_home) { tracing::error!("failed to install system skills: {err}"); } Self { codex_home, + plugins_manager, cache_by_cwd: RwLock::new(HashMap::new()), } } @@ -48,14 +51,14 @@ impl SkillsManager { return outcome; } - let roots = - skill_roots_from_layer_stack_with_agents(&config.config_layer_stack, &config.cwd); - let mut outcome = load_skills_from_roots(roots); - outcome.disabled_paths = disabled_paths_from_stack(&config.config_layer_stack); - let (by_scripts_dir, by_doc_path) = - build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()); - outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); - outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path); + let loaded_plugins = self.plugins_manager.plugins_for_config(config); + let roots = skill_roots( + &config.config_layer_stack, + &config.cwd, + loaded_plugins.effective_skill_roots(), + ); + let outcome = + finalize_skill_outcome(load_skills_from_roots(roots), &config.config_layer_stack); let mut cache = match self.cache_by_cwd.write() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -119,7 +122,15 @@ impl SkillsManager { } }; - let mut roots = skill_roots_from_layer_stack_with_agents(&config_layer_stack, cwd); + let loaded_plugins = self + .plugins_manager + .plugins_for_cwd(cwd, force_reload) + .await; + let mut roots = skill_roots( + &config_layer_stack, + cwd, + loaded_plugins.effective_skill_roots(), + ); roots.extend( normalized_extra_user_roots .iter() @@ -136,11 +147,7 @@ impl SkillsManager { .skills .retain(|skill| skill.scope != SkillScope::System); } - outcome.disabled_paths = disabled_paths_from_stack(&config_layer_stack); - let (by_scripts_dir, by_doc_path) = - build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()); - outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); - outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path); + let outcome = finalize_skill_outcome(outcome, &config_layer_stack); let mut cache = match self.cache_by_cwd.write() { Ok(cache) => cache, Err(err) => err.into_inner(), @@ -201,6 +208,18 @@ fn disabled_paths_from_stack( disabled } +fn finalize_skill_outcome( + mut outcome: SkillLoadOutcome, + config_layer_stack: &crate::config_loader::ConfigLayerStack, +) -> SkillLoadOutcome { + outcome.disabled_paths = disabled_paths_from_stack(config_layer_stack); + let (by_scripts_dir, by_doc_path) = + build_implicit_skill_path_indexes(outcome.allowed_skills_for_implicit_invocation()); + outcome.implicit_skills_by_scripts_dir = Arc::new(by_scripts_dir); + outcome.implicit_skills_by_doc_path = Arc::new(by_doc_path); + outcome +} + fn normalize_override_path(path: &Path) -> PathBuf { dunce::canonicalize(path).unwrap_or_else(|_| path.to_path_buf()) } @@ -220,6 +239,7 @@ mod tests { use super::*; use crate::config::ConfigBuilder; use crate::config::ConfigOverrides; + use crate::plugins::PluginsManager; use pretty_assertions::assert_eq; use std::fs; use std::path::PathBuf; @@ -247,7 +267,8 @@ mod tests { .await .expect("defaults for test should always succeed"); - let skills_manager = SkillsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf())); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager); write_user_skill(&codex_home, "a", "skill-a", "from a"); let outcome1 = skills_manager.skills_for_config(&cfg); @@ -280,7 +301,8 @@ mod tests { .await .expect("defaults for test should always succeed"); - let skills_manager = SkillsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf())); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager); let _ = skills_manager.skills_for_config(&config); write_user_skill(&extra_root, "x", "extra-skill", "from extra root"); @@ -323,7 +345,8 @@ mod tests { .await .expect("defaults for test should always succeed"); - let skills_manager = SkillsManager::new(codex_home.path().to_path_buf()); + let plugins_manager = Arc::new(PluginsManager::new(codex_home.path().to_path_buf())); + let skills_manager = SkillsManager::new(codex_home.path().to_path_buf(), plugins_manager); let _ = skills_manager.skills_for_config(&config); write_user_skill(&extra_root_a, "x", "extra-skill-a", "from extra root a"); diff --git a/codex-rs/core/src/skills/mod.rs b/codex-rs/core/src/skills/mod.rs index 868a1d7def8..2dc7e11c33d 100644 --- a/codex-rs/core/src/skills/mod.rs +++ b/codex-rs/core/src/skills/mod.rs @@ -16,7 +16,6 @@ pub(crate) use injection::build_skill_injections; pub(crate) use injection::collect_explicit_skill_mentions; pub(crate) use invocation_utils::build_implicit_skill_path_indexes; pub(crate) use invocation_utils::maybe_emit_implicit_skill_invocation; -pub use loader::load_skills; pub use manager::SkillsManager; pub use model::SkillError; pub use model::SkillLoadOutcome; diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index f501551f951..426aa5b99a7 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -11,6 +11,7 @@ use crate::exec_policy::ExecPolicyManager; use crate::file_watcher::FileWatcher; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; +use crate::plugins::PluginsManager; use crate::skills::SkillsManager; use crate::state_db::StateDbHandle; use crate::tools::network_approval::NetworkApprovalService; @@ -48,6 +49,7 @@ pub(crate) struct SessionServices { #[cfg_attr(not(unix), allow(dead_code))] pub(crate) execve_session_approvals: RwLock>, pub(crate) skills_manager: Arc, + pub(crate) plugins_manager: Arc, pub(crate) file_watcher: Arc, pub(crate) agent_control: AgentControl, pub(crate) network_proxy: Option, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index 0a56cab1d66..c24f20e1d29 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -13,6 +13,7 @@ use crate::file_watcher::FileWatcher; use crate::file_watcher::FileWatcherEvent; use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig; use crate::models_manager::manager::ModelsManager; +use crate::plugins::PluginsManager; use crate::protocol::Event; use crate::protocol::EventMsg; use crate::protocol::SessionConfiguredEvent; @@ -68,7 +69,11 @@ impl Drop for TempCodexHomeGuard { } } -fn build_file_watcher(codex_home: PathBuf, skills_manager: Arc) -> Arc { +fn build_file_watcher( + codex_home: PathBuf, + skills_manager: Arc, + plugins_manager: Arc, +) -> Arc { if should_use_test_thread_manager_behavior() && let Ok(handle) = Handle::try_current() && handle.runtime_flavor() == RuntimeFlavor::CurrentThread @@ -89,12 +94,14 @@ fn build_file_watcher(codex_home: PathBuf, skills_manager: Arc) - let mut rx = file_watcher.subscribe(); let skills_manager = Arc::clone(&skills_manager); + let plugins_manager = Arc::clone(&plugins_manager); if let Ok(handle) = Handle::try_current() { handle.spawn(async move { loop { match rx.recv().await { Ok(FileWatcherEvent::SkillsChanged { .. }) => { skills_manager.clear_cache(); + plugins_manager.clear_cache(); } Err(broadcast::error::RecvError::Closed) => break, Err(broadcast::error::RecvError::Lagged(_)) => continue, @@ -132,6 +139,7 @@ pub(crate) struct ThreadManagerState { auth_manager: Arc, models_manager: Arc, skills_manager: Arc, + plugins_manager: Arc, file_watcher: Arc, session_source: SessionSource, // Captures submitted ops for testing purpose when test mode is enabled. @@ -147,8 +155,16 @@ impl ThreadManager { collaboration_modes_config: CollaborationModesConfig, ) -> Self { let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); - let skills_manager = Arc::new(SkillsManager::new(codex_home.clone())); - let file_watcher = build_file_watcher(codex_home.clone(), Arc::clone(&skills_manager)); + let plugins_manager = Arc::new(PluginsManager::new(codex_home.clone())); + let skills_manager = Arc::new(SkillsManager::new( + codex_home.clone(), + Arc::clone(&plugins_manager), + )); + let file_watcher = build_file_watcher( + codex_home.clone(), + Arc::clone(&skills_manager), + Arc::clone(&plugins_manager), + ); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -160,6 +176,7 @@ impl ThreadManager { collaboration_modes_config, )), skills_manager, + plugins_manager, file_watcher, auth_manager, session_source, @@ -199,8 +216,16 @@ impl ThreadManager { set_thread_manager_test_mode_for_tests(true); let auth_manager = AuthManager::from_auth_for_testing(auth); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); - let skills_manager = Arc::new(SkillsManager::new(codex_home.clone())); - let file_watcher = build_file_watcher(codex_home.clone(), Arc::clone(&skills_manager)); + let plugins_manager = Arc::new(PluginsManager::new(codex_home.clone())); + let skills_manager = Arc::new(SkillsManager::new( + codex_home.clone(), + Arc::clone(&plugins_manager), + )); + let file_watcher = build_file_watcher( + codex_home.clone(), + Arc::clone(&skills_manager), + Arc::clone(&plugins_manager), + ); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -211,6 +236,7 @@ impl ThreadManager { provider, )), skills_manager, + plugins_manager, file_watcher, auth_manager, session_source: SessionSource::Exec, @@ -229,6 +255,10 @@ impl ThreadManager { self.state.skills_manager.clone() } + pub fn plugins_manager(&self) -> Arc { + self.state.plugins_manager.clone() + } + pub fn subscribe_file_watcher(&self) -> broadcast::Receiver { self.state.file_watcher.subscribe() } @@ -565,6 +595,7 @@ impl ThreadManagerState { auth_manager, Arc::clone(&self.models_manager), Arc::clone(&self.skills_manager), + Arc::clone(&self.plugins_manager), Arc::clone(&self.file_watcher), initial_history, session_source, diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index 1de0522cb55..b562bf5b156 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -956,6 +956,93 @@ async fn skills_append_to_instructions() { let _codex_home_guard = codex_home; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plugin_skills_append_to_instructions() { + skip_if_no_network!(); + let server = MockServer::start().await; + + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new().unwrap()); + let plugin_root = codex_home.path().join("plugins/sample"); + let skill_dir = plugin_root.join("skills/sample-search"); + std::fs::create_dir_all(skill_dir.as_path()).expect("create plugin skill dir"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write plugin manifest"); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\ndescription: inspect sample data\n---\n\n# body\n", + ) + .expect("write plugin skill"); + std::fs::write( + codex_home.path().join("config.toml"), + format!( + "[plugins.sample]\nenabled = true\npath = \"{}\"\n", + plugin_root.display() + ), + ) + .expect("write config"); + + let codex_home_path = codex_home.path().to_path_buf(); + let mut builder = test_codex() + .with_home(codex_home.clone()) + .with_auth(CodexAuth::from_api_key("Test API Key")) + .with_config(move |config| { + config.cwd = codex_home_path; + }); + let codex = builder + .build(&server) + .await + .expect("create new conversation") + .codex; + + codex + .submit(Op::UserInput { + items: vec![UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await + .unwrap(); + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let request_body = request.body_json(); + + assert_message_role(&request_body["input"][0], "developer"); + + assert_message_role(&request_body["input"][1], "user"); + let instructions_text = request_body["input"][1]["content"][0]["text"] + .as_str() + .expect("instructions text"); + assert!( + instructions_text.contains("## Skills"), + "expected skills section present" + ); + assert!( + instructions_text.contains("sample:sample-search: inspect sample data"), + "expected namespaced plugin skill summary" + ); + let expected_path = normalize_path(skill_dir.join("SKILL.md")).unwrap(); + let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); + assert!( + instructions_text.contains(&expected_path_str), + "expected path {expected_path_str} in instructions" + ); + let _codex_home_guard = codex_home; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_configured_effort_in_request() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 4031cecaf63..39fbfbfb564 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -63,7 +63,9 @@ use codex_core::find_thread_name_by_id; use codex_core::git_info::current_branch_name; use codex_core::git_info::get_git_repo_root; use codex_core::git_info::local_git_branches; +use codex_core::mcp::effective_mcp_servers; use codex_core::models_manager::manager::ModelsManager; +use codex_core::plugins::PluginsManager; use codex_core::project_doc::DEFAULT_PROJECT_DOC_FILENAME; use codex_core::skills::model::SkillMetadata; use codex_core::terminal::TerminalName; @@ -7160,7 +7162,8 @@ impl ChatWidget { } pub(crate) fn add_mcp_output(&mut self) { - if self.config.mcp_servers.is_empty() { + let plugins_manager = PluginsManager::new(self.config.codex_home.clone()); + if effective_mcp_servers(&self.config, None, &plugins_manager).is_empty() { self.add_to_history(history_cell::empty_mcp_output()); } else { self.submit_op(Op::ListMcpTools); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 624d47d9154..7ff113baeff 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -39,6 +39,8 @@ use crate::wrapping::adaptive_wrap_lines; use base64::Engine; use codex_core::config::Config; use codex_core::config::types::McpServerTransportConfig; +use codex_core::mcp::effective_mcp_servers; +use codex_core::plugins::PluginsManager; use codex_core::web_search::web_search_detail; use codex_otel::RuntimeMetricsSummary; use codex_protocol::account::PlanType; @@ -1710,7 +1712,9 @@ pub(crate) fn new_mcp_tools_output( lines.push("".into()); } - let mut servers: Vec<_> = config.mcp_servers.iter().collect(); + let plugins_manager = PluginsManager::new(config.codex_home.clone()); + let effective_servers = effective_mcp_servers(config, None, &plugins_manager); + let mut servers: Vec<_> = effective_servers.iter().collect(); servers.sort_by(|(a, _), (b, _)| a.cmp(b)); for (server, cfg) in servers { From b68896a502d34026fea9d860f053062f241739ab Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Thu, 26 Feb 2026 21:02:20 -0800 Subject: [PATCH 2/3] refactor + tests + feature flag --- .../app-server/src/codex_message_processor.rs | 13 +- codex-rs/cli/src/mcp_cmd.rs | 19 +- codex-rs/core/config.schema.json | 6 + codex-rs/core/src/codex.rs | 42 ++-- codex-rs/core/src/codex_delegate.rs | 1 + codex-rs/core/src/features.rs | 8 + codex-rs/core/src/file_watcher.rs | 18 +- codex-rs/core/src/mcp/mod.rs | 63 +++++- codex-rs/core/src/mcp/skill_dependencies.rs | 10 +- codex-rs/core/src/plugins.rs | 197 +++++++++++++----- codex-rs/core/src/skills/manager.rs | 21 +- codex-rs/core/src/state/service.rs | 2 + codex-rs/core/src/thread_manager.rs | 35 ++-- codex-rs/core/tests/suite/client.rs | 87 -------- codex-rs/core/tests/suite/mod.rs | 1 + codex-rs/core/tests/suite/plugins.rs | 182 ++++++++++++++++ codex-rs/tui/src/chatwidget.rs | 8 +- codex-rs/tui/src/history_cell.rs | 7 +- 18 files changed, 491 insertions(+), 229 deletions(-) create mode 100644 codex-rs/core/tests/suite/plugins.rs diff --git a/codex-rs/app-server/src/codex_message_processor.rs b/codex-rs/app-server/src/codex_message_processor.rs index 280ea5e0713..7a62f045c56 100644 --- a/codex-rs/app-server/src/codex_message_processor.rs +++ b/codex-rs/app-server/src/codex_message_processor.rs @@ -223,7 +223,6 @@ use codex_core::find_thread_names_by_ids; use codex_core::find_thread_path_by_id_str; use codex_core::git_info::git_diff_to_remote; use codex_core::mcp::collect_mcp_snapshot; -use codex_core::mcp::configured_mcp_servers; use codex_core::mcp::group_tools_by_server; use codex_core::models_manager::collaboration_mode_presets::CollaborationModesConfig; use codex_core::parse_cursor; @@ -4071,8 +4070,10 @@ impl CodexMessageProcessor { } }; - let configured_servers = - configured_mcp_servers(&config, self.thread_manager.plugins_manager().as_ref()); + let configured_servers = self + .thread_manager + .mcp_manager() + .configured_servers(&config); let mcp_servers = match serde_json::to_value(configured_servers) { Ok(value) => value, Err(err) => { @@ -4134,8 +4135,10 @@ impl CodexMessageProcessor { timeout_secs, } = params; - let configured_servers = - configured_mcp_servers(&config, self.thread_manager.plugins_manager().as_ref()); + let configured_servers = self + .thread_manager + .mcp_manager() + .configured_servers(&config); let Some(server) = configured_servers.get(&name) else { let error = JSONRPCErrorError { code: INVALID_REQUEST_ERROR_CODE, diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index fa06dc000db..00a04693f96 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::sync::Arc; use anyhow::Context; use anyhow::Result; @@ -11,10 +12,10 @@ use codex_core::config::find_codex_home; use codex_core::config::load_global_mcp_servers; use codex_core::config::types::McpServerConfig; use codex_core::config::types::McpServerTransportConfig; +use codex_core::mcp::McpManager; use codex_core::mcp::auth::McpOAuthLoginSupport; use codex_core::mcp::auth::compute_auth_statuses; use codex_core::mcp::auth::oauth_login_support; -use codex_core::mcp::effective_mcp_servers; use codex_core::plugins::PluginsManager; use codex_protocol::protocol::McpAuthStatus; use codex_rmcp_client::delete_oauth_tokens; @@ -331,8 +332,8 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let mcp_servers = mcp_manager.effective_servers(&config, None); let LoginArgs { name, scopes } = login_args; @@ -378,8 +379,8 @@ async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutAr let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let mcp_servers = mcp_manager.effective_servers(&config, None); let LogoutArgs { name } = logout_args; @@ -408,8 +409,8 @@ async fn run_list(config_overrides: &CliConfigOverrides, list_args: ListArgs) -> let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let mcp_servers = mcp_manager.effective_servers(&config, None); let mut entries: Vec<_> = mcp_servers.iter().collect(); entries.sort_by(|(a, _), (b, _)| a.cmp(b)); @@ -657,8 +658,8 @@ async fn run_get(config_overrides: &CliConfigOverrides, get_args: GetArgs) -> Re let config = Config::load_with_cli_overrides(overrides) .await .context("failed to load configuration")?; - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let mcp_servers = effective_mcp_servers(&config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let mcp_servers = mcp_manager.effective_servers(&config, None); let Some(server) = mcp_servers.get(&get_args.name) else { bail!("No MCP server named '{name}' found.", name = get_args.name); diff --git a/codex-rs/core/config.schema.json b/codex-rs/core/config.schema.json index 03e269819c9..ee87866e0f3 100644 --- a/codex-rs/core/config.schema.json +++ b/codex-rs/core/config.schema.json @@ -370,6 +370,9 @@ "personality": { "type": "boolean" }, + "plugins": { + "type": "boolean" + }, "powershell_utf8": { "type": "boolean" }, @@ -1709,6 +1712,9 @@ "personality": { "type": "boolean" }, + "plugins": { + "type": "boolean" + }, "powershell_utf8": { "type": "boolean" }, diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 56f76909ce5..8a3f0a36eee 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -169,8 +169,8 @@ use crate::file_watcher::FileWatcherEvent; use crate::git_info::get_git_repo_root; use crate::instructions::UserInstructions; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; +use crate::mcp::McpManager; use crate::mcp::auth::compute_auth_statuses; -use crate::mcp::effective_mcp_servers; use crate::mcp::maybe_prompt_and_install_mcp_dependencies; use crate::mcp::with_codex_apps_mcp; use crate::mcp_connection_manager::McpConnectionManager; @@ -319,6 +319,7 @@ impl Codex { models_manager: Arc, skills_manager: Arc, plugins_manager: Arc, + mcp_manager: Arc, file_watcher: Arc, conversation_history: InitialHistory, session_source: SessionSource, @@ -330,7 +331,7 @@ impl Codex { let (tx_sub, rx_sub) = async_channel::bounded(SUBMISSION_CHANNEL_CAPACITY); let (tx_event, rx_event) = async_channel::unbounded(); - let loaded_plugins = plugins_manager.plugins_for_config(&config); + plugins_manager.plugins_for_config(&config); let loaded_skills = skills_manager.skills_for_config(&config); for err in &loaded_skills.errors { @@ -340,19 +341,6 @@ impl Codex { err.message ); } - for plugin in loaded_plugins - .plugins - .iter() - .filter(|plugin| plugin.error.is_some()) - { - if let Some(error) = plugin.error.as_deref() { - warn!( - plugin = plugin.config_name, - path = %plugin.root.display(), - "failed to load plugin: {error}" - ); - } - } if let SessionSource::SubAgent(SubAgentSource::ThreadSpawn { depth, .. }) = session_source && depth >= config.agent_max_depth @@ -482,6 +470,7 @@ impl Codex { session_source_clone, skills_manager, plugins_manager, + mcp_manager, file_watcher, agent_control, ) @@ -1124,6 +1113,7 @@ impl Session { session_source: SessionSource, skills_manager: Arc, plugins_manager: Arc, + mcp_manager: Arc, file_watcher: Arc, agent_control: AgentControl, ) -> anyhow::Result> { @@ -1215,11 +1205,10 @@ impl Session { }; let auth_manager_clone = Arc::clone(&auth_manager); let config_for_mcp = Arc::clone(&config); - let plugins_manager_for_mcp = Arc::clone(&plugins_manager); + let mcp_manager_for_mcp = Arc::clone(&mcp_manager); let auth_and_mcp_fut = async move { let auth = auth_manager_clone.auth().await; - let mcp_servers = - effective_mcp_servers(&config_for_mcp, auth.as_ref(), &plugins_manager_for_mcp); + let mcp_servers = mcp_manager_for_mcp.effective_servers(&config_for_mcp, auth.as_ref()); let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config_for_mcp.mcp_oauth_credentials_store_mode, @@ -1470,6 +1459,7 @@ impl Session { execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, plugins_manager, + mcp_manager, file_watcher, agent_control, network_proxy, @@ -3887,7 +3877,6 @@ mod handlers { use crate::mcp::auth::compute_auth_statuses; use crate::mcp::collect_mcp_snapshot_from_manager; - use crate::mcp::effective_mcp_servers; use crate::review_prompts::resolve_review_request; use crate::rollout::session_index; use crate::tasks::CompactTask; @@ -4217,11 +4206,10 @@ mod handlers { pub async fn list_mcp_tools(sess: &Session, config: &Arc, sub_id: String) { let mcp_connection_manager = sess.services.mcp_connection_manager.read().await; let auth = sess.services.auth_manager.auth().await; - let mcp_servers = effective_mcp_servers( - config, - auth.as_ref(), - sess.services.plugins_manager.as_ref(), - ); + let mcp_servers = sess + .services + .mcp_manager + .effective_servers(config, auth.as_ref()); let snapshot = collect_mcp_snapshot_from_manager( &mcp_connection_manager, compute_auth_statuses(mcp_servers.iter(), config.mcp_oauth_credentials_store_mode) @@ -8412,6 +8400,7 @@ mod tests { let (tx_event, _rx_event) = async_channel::unbounded(); let (agent_status_tx, _agent_status_rx) = watch::channel(AgentStatus::PendingInit); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new( config.codex_home.clone(), Arc::clone(&plugins_manager), @@ -8428,6 +8417,7 @@ mod tests { SessionSource::Exec, skills_manager, plugins_manager, + mcp_manager, Arc::new(FileWatcher::noop()), AgentControl::default(), ) @@ -8510,6 +8500,7 @@ mod tests { let state = SessionState::new(session_configuration.clone()); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new( config.codex_home.clone(), Arc::clone(&plugins_manager), @@ -8548,6 +8539,7 @@ mod tests { execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, plugins_manager, + mcp_manager, file_watcher, agent_control, network_proxy: None, @@ -8675,6 +8667,7 @@ mod tests { let state = SessionState::new(session_configuration.clone()); let plugins_manager = Arc::new(PluginsManager::new(config.codex_home.clone())); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new( config.codex_home.clone(), Arc::clone(&plugins_manager), @@ -8713,6 +8706,7 @@ mod tests { execve_session_approvals: RwLock::new(HashMap::new()), skills_manager, plugins_manager, + mcp_manager, file_watcher, agent_control, network_proxy: None, diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index dde9435e151..252c5a11c01 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -54,6 +54,7 @@ pub(crate) async fn run_codex_thread_interactive( models_manager, Arc::clone(&parent_session.services.skills_manager), Arc::clone(&parent_session.services.plugins_manager), + Arc::clone(&parent_session.services.mcp_manager), Arc::clone(&parent_session.services.file_watcher), initial_history.unwrap_or(InitialHistory::New), SessionSource::SubAgent(SubAgentSource::Review), diff --git a/codex-rs/core/src/features.rs b/codex-rs/core/src/features.rs index 8f8178d201a..cd8af6aa180 100644 --- a/codex-rs/core/src/features.rs +++ b/codex-rs/core/src/features.rs @@ -127,6 +127,8 @@ pub enum Feature { Collab, /// Enable apps. Apps, + /// Enable plugins. + Plugins, /// Route apps MCP calls through the configured gateway. AppsMcpGateway, /// Allow prompting and installing missing MCP dependencies. @@ -610,6 +612,12 @@ pub const FEATURES: &[FeatureSpec] = &[ }, default_enabled: false, }, + FeatureSpec { + id: Feature::Plugins, + key: "plugins", + stage: Stage::UnderDevelopment, + default_enabled: false, + }, FeatureSpec { id: Feature::AppsMcpGateway, key: "apps_mcp_gateway", diff --git a/codex-rs/core/src/file_watcher.rs b/codex-rs/core/src/file_watcher.rs index c804e11f38b..f44a4315685 100644 --- a/codex-rs/core/src/file_watcher.rs +++ b/codex-rs/core/src/file_watcher.rs @@ -23,7 +23,7 @@ use tokio::time::sleep_until; use tracing::warn; use crate::config::Config; -use crate::skills::loader::skill_roots; +use crate::skills::SkillsManager; #[derive(Debug, Clone, PartialEq, Eq)] pub enum FileWatcherEvent { @@ -143,12 +143,16 @@ impl FileWatcher { self.tx.subscribe() } - pub(crate) fn register_config(self: &Arc, config: &Config) -> WatchRegistration { - let deduped_roots: HashSet = - skill_roots(&config.config_layer_stack, &config.cwd, Vec::new()) - .into_iter() - .map(|root| root.path) - .collect(); + pub(crate) fn register_config( + self: &Arc, + config: &Config, + skills_manager: &SkillsManager, + ) -> WatchRegistration { + let deduped_roots: HashSet = skills_manager + .skill_roots_for_config(config) + .into_iter() + .map(|root| root.path) + .collect(); let mut registered_roots: Vec = deduped_roots.into_iter().collect(); registered_roots.sort_unstable_by(|a, b| a.as_os_str().cmp(b.as_os_str())); for root in ®istered_roots { diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index 09fee1ce806..4edb45c4625 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -5,6 +5,7 @@ pub(crate) use skill_dependencies::maybe_prompt_and_install_mcp_dependencies; use std::collections::HashMap; use std::env; use std::path::PathBuf; +use std::sync::Arc; use std::time::Duration; use async_channel::unbounded; @@ -161,7 +162,29 @@ pub(crate) fn with_codex_apps_mcp( servers } -pub fn configured_mcp_servers( +pub struct McpManager { + plugins_manager: Arc, +} + +impl McpManager { + pub fn new(plugins_manager: Arc) -> Self { + Self { plugins_manager } + } + + pub fn configured_servers(&self, config: &Config) -> HashMap { + configured_mcp_servers(config, self.plugins_manager.as_ref()) + } + + pub fn effective_servers( + &self, + config: &Config, + auth: Option<&CodexAuth>, + ) -> HashMap { + effective_mcp_servers(config, auth, self.plugins_manager.as_ref()) + } +} + +fn configured_mcp_servers( config: &Config, plugins_manager: &PluginsManager, ) -> HashMap { @@ -173,7 +196,7 @@ pub fn configured_mcp_servers( servers } -pub fn effective_mcp_servers( +fn effective_mcp_servers( config: &Config, auth: Option<&CodexAuth>, plugins_manager: &PluginsManager, @@ -194,8 +217,8 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent config.cli_auth_credentials_store_mode, ); let auth = auth_manager.auth().await; - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let mcp_servers = effective_mcp_servers(config, auth.as_ref(), &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let mcp_servers = mcp_manager.effective_servers(config, auth.as_ref()); if mcp_servers.is_empty() { return McpListToolsResponseEvent { tools: HashMap::new(), @@ -387,12 +410,34 @@ mod tests { use pretty_assertions::assert_eq; use std::fs; use std::path::Path; + use toml::Value; fn write_file(path: &Path, contents: &str) { fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap(); fs::write(path, contents).unwrap(); } + fn plugin_config_toml(plugin_root: &Path) -> String { + let mut root = toml::map::Map::new(); + + let mut features = toml::map::Map::new(); + features.insert("plugins".to_string(), Value::Boolean(true)); + root.insert("features".to_string(), Value::Table(features)); + + let mut plugin = toml::map::Map::new(); + plugin.insert( + "path".to_string(), + Value::String(plugin_root.display().to_string()), + ); + plugin.insert("enabled".to_string(), Value::Boolean(true)); + + let mut plugins = toml::map::Map::new(); + plugins.insert("sample".to_string(), Value::Table(plugin)); + root.insert("plugins".to_string(), Value::Table(plugins)); + + toml::to_string(&Value::Table(root)).expect("plugin test config should serialize") + } + fn make_tool(name: &str) -> Tool { Tool { name: name.to_string(), @@ -593,10 +638,7 @@ mod tests { ); write_file( &codex_home.path().join(CONFIG_TOML_FILE), - &format!( - "[plugins.sample]\npath = \"{}\"\nenabled = true\n", - plugin_root.display() - ), + &plugin_config_toml(&plugin_root), ); let mut config = ConfigBuilder::default() @@ -623,6 +665,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, ); config @@ -630,8 +673,8 @@ mod tests { .set(configured_servers) .expect("test config should accept MCP servers"); - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let effective = effective_mcp_servers(&config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let effective = mcp_manager.effective_servers(&config, None); let sample = effective.get("sample").expect("user server should exist"); let docs = effective.get("docs").expect("plugin server should exist"); diff --git a/codex-rs/core/src/mcp/skill_dependencies.rs b/codex-rs/core/src/mcp/skill_dependencies.rs index e62020c8a6e..fea5bdb5c58 100644 --- a/codex-rs/core/src/mcp/skill_dependencies.rs +++ b/codex-rs/core/src/mcp/skill_dependencies.rs @@ -13,7 +13,6 @@ use tracing::warn; use super::auth::McpOAuthLoginSupport; use super::auth::oauth_login_support; -use super::effective_mcp_servers; use crate::codex::Session; use crate::codex::TurnContext; use crate::config::Config; @@ -254,11 +253,10 @@ pub(crate) async fn maybe_install_mcp_dependencies( // Refresh from the effective merged MCP map (global + repo + managed) and // overlay the updated global servers so we don't drop repo-scoped servers. let auth = sess.services.auth_manager.auth().await; - let mut refresh_servers = effective_mcp_servers( - config, - auth.as_ref(), - sess.services.plugins_manager.as_ref(), - ); + let mut refresh_servers = sess + .services + .mcp_manager + .effective_servers(config, auth.as_ref()); for (name, server_config) in &servers { refresh_servers .entry(name.clone()) diff --git a/codex-rs/core/src/plugins.rs b/codex-rs/core/src/plugins.rs index 231171f91cf..634531bd55a 100644 --- a/codex-rs/core/src/plugins.rs +++ b/codex-rs/core/src/plugins.rs @@ -1,11 +1,15 @@ use crate::config::Config; +use crate::config::ConfigToml; +use crate::config::profile::ConfigProfile; use crate::config::types::McpServerConfig; use crate::config::types::PluginConfig; use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerStack; use crate::config_loader::LoaderOverrides; use crate::config_loader::load_config_layers_state; -use codex_utils_absolute_path::AbsolutePathBuf; +use crate::features::Feature; +use crate::features::FeatureOverrides; +use crate::features::Features; use serde::Deserialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; @@ -70,86 +74,138 @@ impl PluginLoadOutcome { pub struct PluginsManager { codex_home: PathBuf, - cache_by_cwd: RwLock>, + cache: RwLock>, } impl PluginsManager { pub fn new(codex_home: PathBuf) -> Self { Self { codex_home, - cache_by_cwd: RwLock::new(HashMap::new()), + cache: RwLock::new(None), } } pub fn plugins_for_config(&self, config: &Config) -> PluginLoadOutcome { - let cwd = &config.cwd; - if let Some(outcome) = self.cached_outcome_for_cwd(cwd) { + if !config.features.enabled(Feature::Plugins) { + let mut cache = match self.cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + *cache = Some(PluginLoadOutcome::default()); + return PluginLoadOutcome::default(); + } + + if let Some(outcome) = self.cached_outcome() { return outcome; } let outcome = load_plugins_from_layer_stack(&config.config_layer_stack); - let mut cache = match self.cache_by_cwd.write() { + for plugin in outcome + .plugins + .iter() + .filter(|plugin| plugin.error.is_some()) + { + if let Some(error) = plugin.error.as_deref() { + warn!( + plugin = plugin.config_name, + path = %plugin.root.display(), + "failed to load plugin: {error}" + ); + } + } + let mut cache = match self.cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - cache.insert(cwd.to_path_buf(), outcome.clone()); + *cache = Some(outcome.clone()); outcome } - pub async fn plugins_for_cwd(&self, cwd: &Path, force_reload: bool) -> PluginLoadOutcome { - if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { - return outcome; - } - - let cwd_abs = match AbsolutePathBuf::try_from(cwd) { - Ok(cwd_abs) => cwd_abs, - Err(err) => { - warn!("failed to resolve cwd for plugin loading: {err}"); - return PluginLoadOutcome::default(); - } - }; - - let config_layer_stack = match load_config_layers_state( - &self.codex_home, - Some(cwd_abs), - &[], - LoaderOverrides::default(), - CloudRequirementsLoader::default(), - ) - .await - { + pub async fn plugins(&self, force_reload: bool) -> PluginLoadOutcome { + let config_layer_stack = match load_global_plugin_config_layers(&self.codex_home).await { Ok(config_layer_stack) => config_layer_stack, Err(err) => { - warn!("failed to load config layers for plugin loading: {err}"); + warn!("failed to load global config layers for plugin loading: {err}"); return PluginLoadOutcome::default(); } }; + if !plugins_feature_enabled_from_stack(&config_layer_stack) { + let mut cache = match self.cache.write() { + Ok(cache) => cache, + Err(err) => err.into_inner(), + }; + *cache = Some(PluginLoadOutcome::default()); + return PluginLoadOutcome::default(); + } + + if !force_reload && let Some(outcome) = self.cached_outcome() { + return outcome; + } + let outcome = load_plugins_from_layer_stack(&config_layer_stack); - let mut cache = match self.cache_by_cwd.write() { + for plugin in outcome + .plugins + .iter() + .filter(|plugin| plugin.error.is_some()) + { + if let Some(error) = plugin.error.as_deref() { + warn!( + plugin = plugin.config_name, + path = %plugin.root.display(), + "failed to load plugin: {error}" + ); + } + } + let mut cache = match self.cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - cache.insert(cwd.to_path_buf(), outcome.clone()); + *cache = Some(outcome.clone()); outcome } pub fn clear_cache(&self) { - let mut cache = match self.cache_by_cwd.write() { + let mut cache = match self.cache.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - cache.clear(); + *cache = None; } - fn cached_outcome_for_cwd(&self, cwd: &Path) -> Option { - match self.cache_by_cwd.read() { - Ok(cache) => cache.get(cwd).cloned(), - Err(err) => err.into_inner().get(cwd).cloned(), + fn cached_outcome(&self) -> Option { + match self.cache.read() { + Ok(cache) => cache.clone(), + Err(err) => err.into_inner().clone(), } } } +fn plugins_feature_enabled_from_stack(config_layer_stack: &ConfigLayerStack) -> bool { + let effective_config = config_layer_stack.effective_config(); + let Ok(config_toml) = effective_config.try_into::() else { + warn!("failed to deserialize config when checking plugin feature flag"); + return false; + }; + let config_profile = config_toml + .get_config_profile(config_toml.profile.clone()) + .unwrap_or_else(|_| ConfigProfile::default()); + let features = + Features::from_config(&config_toml, &config_profile, FeatureOverrides::default()); + features.enabled(Feature::Plugins) +} + +async fn load_global_plugin_config_layers(codex_home: &Path) -> std::io::Result { + load_config_layers_state( + codex_home, + None, + &[], + LoaderOverrides::default(), + CloudRequirementsLoader::default(), + ) + .await +} + #[derive(Debug, Default, Deserialize)] struct PluginManifest { name: String, @@ -415,12 +471,41 @@ mod tests { use crate::config::types::McpServerTransportConfig; use pretty_assertions::assert_eq; use tempfile::TempDir; + use toml::Value; fn write_file(path: &Path, contents: &str) { fs::create_dir_all(path.parent().expect("file should have a parent")).unwrap(); fs::write(path, contents).unwrap(); } + fn plugin_config_toml( + plugin_root: &Path, + enabled: bool, + plugins_feature_enabled: bool, + ) -> String { + let mut root = toml::map::Map::new(); + + let mut features = toml::map::Map::new(); + features.insert( + "plugins".to_string(), + Value::Boolean(plugins_feature_enabled), + ); + root.insert("features".to_string(), Value::Table(features)); + + let mut plugin = toml::map::Map::new(); + plugin.insert( + "path".to_string(), + Value::String(plugin_root.display().to_string()), + ); + plugin.insert("enabled".to_string(), Value::Boolean(enabled)); + + let mut plugins = toml::map::Map::new(); + plugins.insert("sample".to_string(), Value::Table(plugin)); + root.insert("plugins".to_string(), Value::Table(plugins)); + + toml::to_string(&Value::Table(root)).expect("plugin test config should serialize") + } + async fn load_plugins_from_config(config_toml: &str, codex_home: &Path) -> PluginLoadOutcome { write_file(&codex_home.join(CONFIG_TOML_FILE), config_toml); let config = ConfigBuilder::default() @@ -428,7 +513,7 @@ mod tests { .build() .await .expect("config should load"); - load_plugins_from_layer_stack(&config.config_layer_stack) + PluginsManager::new(codex_home.to_path_buf()).plugins_for_config(&config) } #[tokio::test] @@ -461,10 +546,7 @@ mod tests { ); let outcome = load_plugins_from_config( - &format!( - "[plugins.sample]\npath = \"{}\"\nenabled = true\n", - plugin_root.display() - ), + &plugin_config_toml(&plugin_root, true, true), codex_home.path(), ) .await; @@ -494,6 +576,7 @@ mod tests { enabled_tools: None, disabled_tools: None, scopes: None, + oauth_resource: None, }, )]), error: None, @@ -528,10 +611,7 @@ mod tests { ); let outcome = load_plugins_from_config( - &format!( - "[plugins.sample]\npath = \"{}\"\nenabled = false\n", - plugin_root.display() - ), + &plugin_config_toml(&plugin_root, false, true), codex_home.path(), ) .await; @@ -569,4 +649,27 @@ mod tests { Some("sample".to_string()) ); } + + #[tokio::test] + async fn load_plugins_returns_empty_when_feature_disabled() { + let codex_home = TempDir::new().unwrap(); + let plugin_root = codex_home.path().join("plugin-sample"); + + write_file( + &plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ); + write_file( + &plugin_root.join("skills/sample-search/SKILL.md"), + "---\nname: sample-search\ndescription: search sample data\n---\n", + ); + + let outcome = load_plugins_from_config( + &plugin_config_toml(&plugin_root, true, false), + codex_home.path(), + ) + .await; + + assert_eq!(outcome, PluginLoadOutcome::default()); + } } diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index b11a839f831..13a7586571f 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -51,12 +51,7 @@ impl SkillsManager { return outcome; } - let loaded_plugins = self.plugins_manager.plugins_for_config(config); - let roots = skill_roots( - &config.config_layer_stack, - &config.cwd, - loaded_plugins.effective_skill_roots(), - ); + let roots = self.skill_roots_for_config(config); let outcome = finalize_skill_outcome(load_skills_from_roots(roots), &config.config_layer_stack); let mut cache = match self.cache_by_cwd.write() { @@ -67,6 +62,15 @@ impl SkillsManager { outcome } + pub(crate) fn skill_roots_for_config(&self, config: &Config) -> Vec { + let loaded_plugins = self.plugins_manager.plugins_for_config(config); + skill_roots( + &config.config_layer_stack, + &config.cwd, + loaded_plugins.effective_skill_roots(), + ) + } + pub async fn skills_for_cwd(&self, cwd: &Path, force_reload: bool) -> SkillLoadOutcome { if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { return outcome; @@ -122,10 +126,7 @@ impl SkillsManager { } }; - let loaded_plugins = self - .plugins_manager - .plugins_for_cwd(cwd, force_reload) - .await; + let loaded_plugins = self.plugins_manager.plugins(force_reload).await; let mut roots = skill_roots( &config_layer_stack, cwd, diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index 426aa5b99a7..e2792638e8e 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -9,6 +9,7 @@ use crate::client::ModelClient; use crate::config::StartedNetworkProxy; use crate::exec_policy::ExecPolicyManager; use crate::file_watcher::FileWatcher; +use crate::mcp::McpManager; use crate::mcp_connection_manager::McpConnectionManager; use crate::models_manager::manager::ModelsManager; use crate::plugins::PluginsManager; @@ -50,6 +51,7 @@ pub(crate) struct SessionServices { pub(crate) execve_session_approvals: RwLock>, pub(crate) skills_manager: Arc, pub(crate) plugins_manager: Arc, + pub(crate) mcp_manager: Arc, pub(crate) file_watcher: Arc, pub(crate) agent_control: AgentControl, pub(crate) network_proxy: Option, diff --git a/codex-rs/core/src/thread_manager.rs b/codex-rs/core/src/thread_manager.rs index c24f20e1d29..805d1f2ef2c 100644 --- a/codex-rs/core/src/thread_manager.rs +++ b/codex-rs/core/src/thread_manager.rs @@ -11,6 +11,7 @@ use crate::error::CodexErr; use crate::error::Result as CodexResult; use crate::file_watcher::FileWatcher; use crate::file_watcher::FileWatcherEvent; +use crate::mcp::McpManager; use crate::models_manager::collaboration_mode_presets::CollaborationModesConfig; use crate::models_manager::manager::ModelsManager; use crate::plugins::PluginsManager; @@ -69,11 +70,7 @@ impl Drop for TempCodexHomeGuard { } } -fn build_file_watcher( - codex_home: PathBuf, - skills_manager: Arc, - plugins_manager: Arc, -) -> Arc { +fn build_file_watcher(codex_home: PathBuf, skills_manager: Arc) -> Arc { if should_use_test_thread_manager_behavior() && let Ok(handle) = Handle::try_current() && handle.runtime_flavor() == RuntimeFlavor::CurrentThread @@ -94,14 +91,12 @@ fn build_file_watcher( let mut rx = file_watcher.subscribe(); let skills_manager = Arc::clone(&skills_manager); - let plugins_manager = Arc::clone(&plugins_manager); if let Ok(handle) = Handle::try_current() { handle.spawn(async move { loop { match rx.recv().await { Ok(FileWatcherEvent::SkillsChanged { .. }) => { skills_manager.clear_cache(); - plugins_manager.clear_cache(); } Err(broadcast::error::RecvError::Closed) => break, Err(broadcast::error::RecvError::Lagged(_)) => continue, @@ -140,6 +135,7 @@ pub(crate) struct ThreadManagerState { models_manager: Arc, skills_manager: Arc, plugins_manager: Arc, + mcp_manager: Arc, file_watcher: Arc, session_source: SessionSource, // Captures submitted ops for testing purpose when test mode is enabled. @@ -156,15 +152,12 @@ impl ThreadManager { ) -> Self { let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); let plugins_manager = Arc::new(PluginsManager::new(codex_home.clone())); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new( codex_home.clone(), Arc::clone(&plugins_manager), )); - let file_watcher = build_file_watcher( - codex_home.clone(), - Arc::clone(&skills_manager), - Arc::clone(&plugins_manager), - ); + let file_watcher = build_file_watcher(codex_home.clone(), Arc::clone(&skills_manager)); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -177,6 +170,7 @@ impl ThreadManager { )), skills_manager, plugins_manager, + mcp_manager, file_watcher, auth_manager, session_source, @@ -217,15 +211,12 @@ impl ThreadManager { let auth_manager = AuthManager::from_auth_for_testing(auth); let (thread_created_tx, _) = broadcast::channel(THREAD_CREATED_CHANNEL_CAPACITY); let plugins_manager = Arc::new(PluginsManager::new(codex_home.clone())); + let mcp_manager = Arc::new(McpManager::new(Arc::clone(&plugins_manager))); let skills_manager = Arc::new(SkillsManager::new( codex_home.clone(), Arc::clone(&plugins_manager), )); - let file_watcher = build_file_watcher( - codex_home.clone(), - Arc::clone(&skills_manager), - Arc::clone(&plugins_manager), - ); + let file_watcher = build_file_watcher(codex_home.clone(), Arc::clone(&skills_manager)); Self { state: Arc::new(ThreadManagerState { threads: Arc::new(RwLock::new(HashMap::new())), @@ -237,6 +228,7 @@ impl ThreadManager { )), skills_manager, plugins_manager, + mcp_manager, file_watcher, auth_manager, session_source: SessionSource::Exec, @@ -259,6 +251,10 @@ impl ThreadManager { self.state.plugins_manager.clone() } + pub fn mcp_manager(&self) -> Arc { + self.state.mcp_manager.clone() + } + pub fn subscribe_file_watcher(&self) -> broadcast::Receiver { self.state.file_watcher.subscribe() } @@ -587,7 +583,9 @@ impl ThreadManagerState { persist_extended_history: bool, metrics_service_name: Option, ) -> CodexResult { - let watch_registration = self.file_watcher.register_config(&config); + let watch_registration = self + .file_watcher + .register_config(&config, self.skills_manager.as_ref()); let CodexSpawnOk { codex, thread_id, .. } = Codex::spawn( @@ -596,6 +594,7 @@ impl ThreadManagerState { Arc::clone(&self.models_manager), Arc::clone(&self.skills_manager), Arc::clone(&self.plugins_manager), + Arc::clone(&self.mcp_manager), Arc::clone(&self.file_watcher), initial_history, session_source, diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index b562bf5b156..1de0522cb55 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -956,93 +956,6 @@ async fn skills_append_to_instructions() { let _codex_home_guard = codex_home; } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn plugin_skills_append_to_instructions() { - skip_if_no_network!(); - let server = MockServer::start().await; - - let resp_mock = mount_sse_once( - &server, - sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), - ) - .await; - - let codex_home = Arc::new(TempDir::new().unwrap()); - let plugin_root = codex_home.path().join("plugins/sample"); - let skill_dir = plugin_root.join("skills/sample-search"); - std::fs::create_dir_all(skill_dir.as_path()).expect("create plugin skill dir"); - std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); - std::fs::write( - plugin_root.join(".codex-plugin/plugin.json"), - r#"{"name":"sample"}"#, - ) - .expect("write plugin manifest"); - std::fs::write( - skill_dir.join("SKILL.md"), - "---\ndescription: inspect sample data\n---\n\n# body\n", - ) - .expect("write plugin skill"); - std::fs::write( - codex_home.path().join("config.toml"), - format!( - "[plugins.sample]\nenabled = true\npath = \"{}\"\n", - plugin_root.display() - ), - ) - .expect("write config"); - - let codex_home_path = codex_home.path().to_path_buf(); - let mut builder = test_codex() - .with_home(codex_home.clone()) - .with_auth(CodexAuth::from_api_key("Test API Key")) - .with_config(move |config| { - config.cwd = codex_home_path; - }); - let codex = builder - .build(&server) - .await - .expect("create new conversation") - .codex; - - codex - .submit(Op::UserInput { - items: vec![UserInput::Text { - text: "hello".into(), - text_elements: Vec::new(), - }], - final_output_json_schema: None, - }) - .await - .unwrap(); - - wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; - - let request = resp_mock.single_request(); - let request_body = request.body_json(); - - assert_message_role(&request_body["input"][0], "developer"); - - assert_message_role(&request_body["input"][1], "user"); - let instructions_text = request_body["input"][1]["content"][0]["text"] - .as_str() - .expect("instructions text"); - assert!( - instructions_text.contains("## Skills"), - "expected skills section present" - ); - assert!( - instructions_text.contains("sample:sample-search: inspect sample data"), - "expected namespaced plugin skill summary" - ); - let expected_path = normalize_path(skill_dir.join("SKILL.md")).unwrap(); - let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); - assert!( - instructions_text.contains(&expected_path_str), - "expected path {expected_path_str} in instructions" - ); - let _codex_home_guard = codex_home; -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn includes_configured_effort_in_request() -> anyhow::Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index e23fd9ddbbf..e9867caa76e 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -95,6 +95,7 @@ mod pending_input; mod permissions_messages; mod personality; mod personality_migration; +mod plugins; mod prompt_caching; mod quota_exceeded; mod read_file; diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs new file mode 100644 index 00000000000..ed2ba54a3a0 --- /dev/null +++ b/codex-rs/core/tests/suite/plugins.rs @@ -0,0 +1,182 @@ +#![cfg(not(target_os = "windows"))] +#![allow(clippy::unwrap_used, clippy::expect_used)] + +use std::sync::Arc; +use std::time::Duration; +use std::time::Instant; + +use anyhow::Result; +use codex_core::CodexAuth; +use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_once; +use core_test_support::responses::sse; +use core_test_support::responses::start_mock_server; +use core_test_support::skip_if_no_network; +use core_test_support::stdio_server_bin; +use core_test_support::test_codex::test_codex; +use core_test_support::wait_for_event; +use core_test_support::wait_for_event_with_timeout; +use dunce::canonicalize as normalize_path; +use tempfile::TempDir; +use wiremock::MockServer; + +fn write_plugin_skill_plugin(home: &TempDir) -> std::path::PathBuf { + let plugin_root = home.path().join("plugins/sample"); + let skill_dir = plugin_root.join("skills/sample-search"); + std::fs::create_dir_all(skill_dir.as_path()).expect("create plugin skill dir"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write plugin manifest"); + std::fs::write( + skill_dir.join("SKILL.md"), + "---\ndescription: inspect sample data\n---\n\n# body\n", + ) + .expect("write plugin skill"); + std::fs::write( + home.path().join("config.toml"), + format!( + "[features]\nplugins = true\n\n[plugins.sample]\nenabled = true\npath = \"{}\"\n", + plugin_root.display() + ), + ) + .expect("write config"); + skill_dir.join("SKILL.md") +} + +fn write_plugin_mcp_plugin(home: &TempDir, command: &str) { + let plugin_root = home.path().join("plugins/sample"); + std::fs::create_dir_all(plugin_root.join(".codex-plugin")).expect("create plugin manifest dir"); + std::fs::write( + plugin_root.join(".codex-plugin/plugin.json"), + r#"{"name":"sample"}"#, + ) + .expect("write plugin manifest"); + std::fs::write( + plugin_root.join(".mcp.json"), + format!( + r#"{{ + "mcpServers": {{ + "sample": {{ + "command": "{command}" + }} + }} +}}"# + ), + ) + .expect("write plugin mcp config"); + std::fs::write( + home.path().join("config.toml"), + format!( + "[features]\nplugins = true\n\n[plugins.sample]\nenabled = true\npath = \"{}\"\n", + plugin_root.display() + ), + ) + .expect("write config"); +} + +async fn build_plugin_test_codex( + server: &MockServer, + codex_home: Arc, +) -> Result> { + let mut builder = test_codex() + .with_home(codex_home) + .with_auth(CodexAuth::from_api_key("Test API Key")); + Ok(builder + .build(server) + .await + .expect("create new conversation") + .codex) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn plugin_skills_append_to_instructions() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = MockServer::start().await; + + let resp_mock = mount_sse_once( + &server, + sse(vec![ev_response_created("resp1"), ev_completed("resp1")]), + ) + .await; + + let codex_home = Arc::new(TempDir::new()?); + let skill_path = write_plugin_skill_plugin(codex_home.as_ref()); + let codex = build_plugin_test_codex(&server, Arc::clone(&codex_home)).await?; + + codex + .submit(Op::UserInput { + items: vec![codex_protocol::user_input::UserInput::Text { + text: "hello".into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + }) + .await?; + + wait_for_event(&codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await; + + let request = resp_mock.single_request(); + let request_body = request.body_json(); + let instructions_text = request_body["input"][1]["content"][0]["text"] + .as_str() + .expect("instructions text"); + assert!( + instructions_text.contains("## Skills"), + "expected skills section present" + ); + assert!( + instructions_text.contains("sample:sample-search: inspect sample data"), + "expected namespaced plugin skill summary" + ); + let expected_path = normalize_path(skill_path)?; + let expected_path_str = expected_path.to_string_lossy().replace('\\', "/"); + assert!( + instructions_text.contains(&expected_path_str), + "expected path {expected_path_str} in instructions" + ); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn plugin_mcp_tools_are_listed() -> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let codex_home = Arc::new(TempDir::new()?); + let rmcp_test_server_bin = stdio_server_bin()?; + write_plugin_mcp_plugin(codex_home.as_ref(), &rmcp_test_server_bin); + let codex = build_plugin_test_codex(&server, codex_home).await?; + + let tools_ready_deadline = Instant::now() + Duration::from_secs(30); + loop { + codex.submit(Op::ListMcpTools).await?; + let list_event = wait_for_event_with_timeout( + &codex, + |ev| matches!(ev, EventMsg::McpListToolsResponse(_)), + Duration::from_secs(10), + ) + .await; + let EventMsg::McpListToolsResponse(tool_list) = list_event else { + unreachable!("event guard guarantees McpListToolsResponse"); + }; + if tool_list.tools.contains_key("mcp__sample__echo") + && tool_list.tools.contains_key("mcp__sample__image") + { + break; + } + + let available_tools: Vec<&str> = tool_list.tools.keys().map(String::as_str).collect(); + if Instant::now() >= tools_ready_deadline { + panic!("timed out waiting for plugin MCP tools; discovered tools: {available_tools:?}"); + } + tokio::time::sleep(Duration::from_millis(200)).await; + } + + Ok(()) +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 39fbfbfb564..bd3df3b95a0 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -63,7 +63,7 @@ use codex_core::find_thread_name_by_id; use codex_core::git_info::current_branch_name; use codex_core::git_info::get_git_repo_root; use codex_core::git_info::local_git_branches; -use codex_core::mcp::effective_mcp_servers; +use codex_core::mcp::McpManager; use codex_core::models_manager::manager::ModelsManager; use codex_core::plugins::PluginsManager; use codex_core::project_doc::DEFAULT_PROJECT_DOC_FILENAME; @@ -7162,8 +7162,10 @@ impl ChatWidget { } pub(crate) fn add_mcp_output(&mut self) { - let plugins_manager = PluginsManager::new(self.config.codex_home.clone()); - if effective_mcp_servers(&self.config, None, &plugins_manager).is_empty() { + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( + self.config.codex_home.clone(), + ))); + if mcp_manager.effective_servers(&self.config, None).is_empty() { self.add_to_history(history_cell::empty_mcp_output()); } else { self.submit_op(Op::ListMcpTools); diff --git a/codex-rs/tui/src/history_cell.rs b/codex-rs/tui/src/history_cell.rs index 7ff113baeff..abb1cf1737a 100644 --- a/codex-rs/tui/src/history_cell.rs +++ b/codex-rs/tui/src/history_cell.rs @@ -39,7 +39,7 @@ use crate::wrapping::adaptive_wrap_lines; use base64::Engine; use codex_core::config::Config; use codex_core::config::types::McpServerTransportConfig; -use codex_core::mcp::effective_mcp_servers; +use codex_core::mcp::McpManager; use codex_core::plugins::PluginsManager; use codex_core::web_search::web_search_detail; use codex_otel::RuntimeMetricsSummary; @@ -75,6 +75,7 @@ use std::collections::HashMap; use std::io::Cursor; use std::path::Path; use std::path::PathBuf; +use std::sync::Arc; use std::time::Duration; use std::time::Instant; use tracing::error; @@ -1712,8 +1713,8 @@ pub(crate) fn new_mcp_tools_output( lines.push("".into()); } - let plugins_manager = PluginsManager::new(config.codex_home.clone()); - let effective_servers = effective_mcp_servers(config, None, &plugins_manager); + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new(config.codex_home.clone()))); + let effective_servers = mcp_manager.effective_servers(config, None); let mut servers: Vec<_> = effective_servers.iter().collect(); servers.sort_by(|(a, _), (b, _)| a.cmp(b)); From 8da5d5c8dd53b2f136f783c5fddd000f6737a110 Mon Sep 17 00:00:00 2001 From: Xin Lin Date: Fri, 27 Feb 2026 20:31:28 -0800 Subject: [PATCH 3/3] per-cwd plugin --- codex-rs/core/src/plugins.rs | 150 ++++++++++------------------ codex-rs/core/src/skills/manager.rs | 4 +- 2 files changed, 54 insertions(+), 100 deletions(-) diff --git a/codex-rs/core/src/plugins.rs b/codex-rs/core/src/plugins.rs index 634531bd55a..890d15a323f 100644 --- a/codex-rs/core/src/plugins.rs +++ b/codex-rs/core/src/plugins.rs @@ -3,13 +3,11 @@ use crate::config::ConfigToml; use crate::config::profile::ConfigProfile; use crate::config::types::McpServerConfig; use crate::config::types::PluginConfig; -use crate::config_loader::CloudRequirementsLoader; use crate::config_loader::ConfigLayerStack; -use crate::config_loader::LoaderOverrides; -use crate::config_loader::load_config_layers_state; use crate::features::Feature; use crate::features::FeatureOverrides; use crate::features::Features; +use codex_utils_absolute_path::AbsolutePathBuf; use serde::Deserialize; use serde_json::Map as JsonMap; use serde_json::Value as JsonValue; @@ -28,7 +26,7 @@ const DEFAULT_MCP_CONFIG_FILE: &str = ".mcp.json"; pub struct LoadedPlugin { pub config_name: String, pub manifest_name: Option, - pub root: PathBuf, + pub root: AbsolutePathBuf, pub enabled: bool, pub skill_roots: Vec, pub mcp_servers: HashMap, @@ -73,110 +71,61 @@ impl PluginLoadOutcome { } pub struct PluginsManager { - codex_home: PathBuf, - cache: RwLock>, + cache_by_cwd: RwLock>, } impl PluginsManager { - pub fn new(codex_home: PathBuf) -> Self { + pub fn new(_codex_home: PathBuf) -> Self { Self { - codex_home, - cache: RwLock::new(None), + cache_by_cwd: RwLock::new(HashMap::new()), } } pub fn plugins_for_config(&self, config: &Config) -> PluginLoadOutcome { - if !config.features.enabled(Feature::Plugins) { - let mut cache = match self.cache.write() { - Ok(cache) => cache, - Err(err) => err.into_inner(), - }; - *cache = Some(PluginLoadOutcome::default()); - return PluginLoadOutcome::default(); - } - - if let Some(outcome) = self.cached_outcome() { - return outcome; - } - - let outcome = load_plugins_from_layer_stack(&config.config_layer_stack); - for plugin in outcome - .plugins - .iter() - .filter(|plugin| plugin.error.is_some()) - { - if let Some(error) = plugin.error.as_deref() { - warn!( - plugin = plugin.config_name, - path = %plugin.root.display(), - "failed to load plugin: {error}" - ); - } - } - let mut cache = match self.cache.write() { - Ok(cache) => cache, - Err(err) => err.into_inner(), - }; - *cache = Some(outcome.clone()); - outcome + self.plugins_for_layer_stack(&config.cwd, &config.config_layer_stack, false) } - pub async fn plugins(&self, force_reload: bool) -> PluginLoadOutcome { - let config_layer_stack = match load_global_plugin_config_layers(&self.codex_home).await { - Ok(config_layer_stack) => config_layer_stack, - Err(err) => { - warn!("failed to load global config layers for plugin loading: {err}"); - return PluginLoadOutcome::default(); - } - }; - - if !plugins_feature_enabled_from_stack(&config_layer_stack) { - let mut cache = match self.cache.write() { + pub fn plugins_for_layer_stack( + &self, + cwd: &Path, + config_layer_stack: &ConfigLayerStack, + force_reload: bool, + ) -> PluginLoadOutcome { + if !plugins_feature_enabled_from_stack(config_layer_stack) { + let mut cache = match self.cache_by_cwd.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - *cache = Some(PluginLoadOutcome::default()); + cache.insert(cwd.to_path_buf(), PluginLoadOutcome::default()); return PluginLoadOutcome::default(); } - if !force_reload && let Some(outcome) = self.cached_outcome() { + if !force_reload && let Some(outcome) = self.cached_outcome_for_cwd(cwd) { return outcome; } - let outcome = load_plugins_from_layer_stack(&config_layer_stack); - for plugin in outcome - .plugins - .iter() - .filter(|plugin| plugin.error.is_some()) - { - if let Some(error) = plugin.error.as_deref() { - warn!( - plugin = plugin.config_name, - path = %plugin.root.display(), - "failed to load plugin: {error}" - ); - } - } - let mut cache = match self.cache.write() { + let outcome = load_plugins_from_layer_stack(config_layer_stack); + log_plugin_load_errors(&outcome); + let mut cache = match self.cache_by_cwd.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - *cache = Some(outcome.clone()); + cache.insert(cwd.to_path_buf(), outcome.clone()); outcome } pub fn clear_cache(&self) { - let mut cache = match self.cache.write() { + let mut cache_by_cwd = match self.cache_by_cwd.write() { Ok(cache) => cache, Err(err) => err.into_inner(), }; - *cache = None; + cache_by_cwd.clear(); } - fn cached_outcome(&self) -> Option { - match self.cache.read() { - Ok(cache) => cache.clone(), - Err(err) => err.into_inner().clone(), + fn cached_outcome_for_cwd(&self, cwd: &Path) -> Option { + match self.cache_by_cwd.read() { + Ok(cache) => cache.get(cwd).cloned(), + Err(err) => err.into_inner().get(cwd).cloned(), } } } @@ -195,15 +144,20 @@ fn plugins_feature_enabled_from_stack(config_layer_stack: &ConfigLayerStack) -> features.enabled(Feature::Plugins) } -async fn load_global_plugin_config_layers(codex_home: &Path) -> std::io::Result { - load_config_layers_state( - codex_home, - None, - &[], - LoaderOverrides::default(), - CloudRequirementsLoader::default(), - ) - .await +fn log_plugin_load_errors(outcome: &PluginLoadOutcome) { + for plugin in outcome + .plugins + .iter() + .filter(|plugin| plugin.error.is_some()) + { + if let Some(error) = plugin.error.as_deref() { + warn!( + plugin = plugin.config_name, + path = %plugin.root.display(), + "failed to load plugin: {error}" + ); + } + } } #[derive(Debug, Default, Deserialize)] @@ -259,10 +213,8 @@ pub(crate) fn plugin_namespace_for_skill_path(path: &Path) -> Option { fn configured_plugins_from_stack( config_layer_stack: &ConfigLayerStack, ) -> HashMap { - let Some(user_layer) = config_layer_stack.get_user_layer() else { - return HashMap::new(); - }; - let Some(plugins_value) = user_layer.config.get("plugins") else { + let effective_config = config_layer_stack.effective_config(); + let Some(plugins_value) = effective_config.get("plugins") else { return HashMap::new(); }; match plugins_value.clone().try_into() { @@ -275,7 +227,7 @@ fn configured_plugins_from_stack( } fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin { - let plugin_root = plugin.path.to_path_buf(); + let plugin_root = plugin.path.clone(); let mut loaded_plugin = LoadedPlugin { config_name, manifest_name: None, @@ -290,21 +242,21 @@ fn load_plugin(config_name: String, plugin: &PluginConfig) -> LoadedPlugin { return loaded_plugin; } - if !plugin_root.is_dir() { + if !plugin_root.as_path().is_dir() { loaded_plugin.error = Some("path does not exist or is not a directory".to_string()); return loaded_plugin; } - let Some(manifest) = load_plugin_manifest(&plugin_root) else { + let Some(manifest) = load_plugin_manifest(plugin_root.as_path()) else { loaded_plugin.error = Some("missing or invalid .codex-plugin/plugin.json".to_string()); return loaded_plugin; }; - loaded_plugin.manifest_name = Some(plugin_manifest_name(&manifest, &plugin_root)); - loaded_plugin.skill_roots = default_skill_roots(&plugin_root); + loaded_plugin.manifest_name = Some(plugin_manifest_name(&manifest, plugin_root.as_path())); + loaded_plugin.skill_roots = default_skill_roots(plugin_root.as_path()); let mut mcp_servers = HashMap::new(); - for mcp_config_path in default_mcp_config_paths(&plugin_root) { - let plugin_mcp = load_mcp_servers_from_file(&plugin_root, &mcp_config_path); + for mcp_config_path in default_mcp_config_paths(plugin_root.as_path()) { + let plugin_mcp = load_mcp_servers_from_file(plugin_root.as_path(), &mcp_config_path); for (name, config) in plugin_mcp.mcp_servers { if mcp_servers.insert(name.clone(), config).is_some() { warn!( @@ -556,7 +508,7 @@ mod tests { vec![LoadedPlugin { config_name: "sample".to_string(), manifest_name: Some("sample".to_string()), - root: plugin_root.clone(), + root: AbsolutePathBuf::try_from(plugin_root.clone()).unwrap(), enabled: true, skill_roots: vec![plugin_root.join("skills")], mcp_servers: HashMap::from([( @@ -621,7 +573,7 @@ mod tests { vec![LoadedPlugin { config_name: "sample".to_string(), manifest_name: None, - root: plugin_root, + root: AbsolutePathBuf::try_from(plugin_root).unwrap(), enabled: false, skill_roots: Vec::new(), mcp_servers: HashMap::new(), diff --git a/codex-rs/core/src/skills/manager.rs b/codex-rs/core/src/skills/manager.rs index 13a7586571f..c7db60cb515 100644 --- a/codex-rs/core/src/skills/manager.rs +++ b/codex-rs/core/src/skills/manager.rs @@ -126,7 +126,9 @@ impl SkillsManager { } }; - let loaded_plugins = self.plugins_manager.plugins(force_reload).await; + let loaded_plugins = + self.plugins_manager + .plugins_for_layer_stack(cwd, &config_layer_stack, force_reload); let mut roots = skill_roots( &config_layer_stack, cwd,