From 8570091e14dc794b256402e27a7773e8de132f3e Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Thu, 10 Sep 2026 21:39:35 +0000 Subject: [PATCH] Honor thread-level plugin exclusions across runtime capabilities (#44655) ## What changed - Apply `disabled_plugin_ids` to plugin skills, recommendations, hooks, and MCP servers without modifying shared plugin state. - Activate selection changes when the next task starts, keeping pending settings from changing the active runtime. - Hide app tools contributed only by disabled plugins from model tool catalogs and search. Preserve connectors shared with enabled plugins and existing direct Apps RPC behavior. - Include plugin identity in MCP approval keys and use that identity when persisting plugin tool approvals. ## Testing Add regression coverage for disabling and re-enabling plugin skills and tools, deferred activation of settings, shared connector ownership, selected plugin servers, executor hooks, and plugin-specific approval keys and persistence. GitOrigin-RevId: 1153ff6aa7ab6953d50e838da05203904ecd2956 --- codex-rs/codex-mcp/src/connection_manager.rs | 24 +-- .../src/connection_manager/tool_catalog.rs | 11 +- .../codex-mcp/src/connection_manager_tests.rs | 8 +- codex-rs/codex-mcp/src/lib.rs | 4 +- codex-rs/codex-mcp/src/mcp/mod.rs | 45 +++--- codex-rs/codex-mcp/src/mcp/mod_tests.rs | 12 +- codex-rs/codex-mcp/src/rmcp_client.rs | 21 +-- codex-rs/connectors/src/snapshot.rs | 56 ++++--- codex-rs/connectors/src/snapshot_tests.rs | 50 +++++- codex-rs/core/src/connectors.rs | 14 +- codex-rs/core/src/hook_runtime.rs | 8 + codex-rs/core/src/mcp.rs | 36 +++-- codex-rs/core/src/mcp_tool_call.rs | 27 ++-- codex-rs/core/src/mcp_tool_call_tests.rs | 46 +++--- codex-rs/core/src/session/mcp.rs | 12 +- codex-rs/core/src/session/mcp_runtime.rs | 10 +- codex-rs/core/src/session/mod.rs | 15 +- codex-rs/core/src/session/plugin_selection.rs | 32 ++++ codex-rs/core/src/session/session.rs | 5 + codex-rs/core/src/session/turn.rs | 6 +- codex-rs/core/src/session/turn_context.rs | 14 +- codex-rs/core/src/state/session.rs | 3 + codex-rs/core/src/tasks/mod.rs | 1 + codex-rs/core/src/thread_manager_tests.rs | 61 +++++++- codex-rs/core/tests/suite/hooks_executor.rs | 52 ++++++ codex-rs/core/tests/suite/plugins.rs | 148 ++++++++++++++++++ codex-rs/plugin/src/load_outcome.rs | 15 +- 27 files changed, 565 insertions(+), 171 deletions(-) create mode 100644 codex-rs/core/src/session/plugin_selection.rs diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index ffa238bb87..12a29288e6 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -36,7 +36,7 @@ use crate::elicitation::ElicitationRequestManager; use crate::elicitation::ElicitationRequestRouter; use crate::event_stream::EventStreamConnectionSettings; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; -use crate::mcp::ToolPluginProvenance; +use crate::mcp::ToolPluginContext; use crate::pagination::MAX_CODEX_APPS_TOOL_CATALOG_ITEMS; use crate::pagination::MAX_MCP_CATALOG_ITEMS; use crate::rmcp_client::AsyncManagedClient; @@ -170,14 +170,14 @@ struct McpServerView { impl McpServerView { async fn listed_tools( &self, - tool_plugin_provenance: &ToolPluginProvenance, + tool_plugin_context: &ToolPluginContext, ) -> Result, StartupOutcomeError> { let tools = self.connection.client.listed_tools().await?; let tools = filter_tools(tools, &self.tool_filter); Ok(if self.connection.client.is_codex_apps_mcp_server { - prepare_codex_apps_tools_for_model(tools, tool_plugin_provenance) + prepare_codex_apps_tools_for_model(tools, tool_plugin_context) } else { - prepare_regular_mcp_tools_for_model(tools, tool_plugin_provenance) + prepare_regular_mcp_tools_for_model(tools, tool_plugin_context) }) } } @@ -189,7 +189,7 @@ pub(crate) struct McpConnectionSet { disabled_servers: Vec, required_servers: Vec, optional_startup_deadline: OnceLock, - tool_plugin_provenance: Arc, + tool_plugin_context: Arc, prefix_mcp_tool_names: bool, non_prefixed_mcp_tool_servers: Vec, elicitation_requests: ElicitationRequestManager, @@ -234,7 +234,7 @@ impl McpConnectionSet { let default_protocol_mode = config.protocol_mode; let host_owned_apps_protocol_mode = config.host_owned_apps_protocol_mode; let client_elicitation_capability = config.client_elicitation_capability.clone(); - let tool_plugin_provenance = crate::mcp::tool_plugin_provenance(&config); + let tool_plugin_context = crate::mcp::tool_plugin_context(&config); let auth = auth.as_ref(); let mut servers = HashMap::new(); let mut event_stream_connection = None; @@ -272,7 +272,7 @@ impl McpConnectionSet { elicitation_router, ) }; - let tool_plugin_provenance = Arc::new(tool_plugin_provenance); + let tool_plugin_context = Arc::new(tool_plugin_context); let startup_submit_id = submit_id; let static_chatgpt_auth_provider = auth .filter(|auth| auth.uses_codex_backend()) @@ -601,7 +601,7 @@ impl McpConnectionSet { catalog_item_limit, ); let defer_startup = allow_deferred_startup - && !tool_plugin_provenance.is_selected_plugin_mcp_server(&server_name) + && !tool_plugin_context.is_selected_plugin_mcp_server(&server_name) && async_managed_client .tool_catalog_cache_context .as_ref() @@ -755,7 +755,7 @@ impl McpConnectionSet { disabled_servers, required_servers, optional_startup_deadline: OnceLock::new(), - tool_plugin_provenance, + tool_plugin_context, prefix_mcp_tool_names, non_prefixed_mcp_tool_servers, elicitation_requests: elicitation_requests.clone(), @@ -813,7 +813,7 @@ impl McpConnectionSet { disabled_servers: Vec::new(), required_servers: Vec::new(), optional_startup_deadline: OnceLock::new(), - tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), + tool_plugin_context: Arc::new(ToolPluginContext::default()), prefix_mcp_tool_names, non_prefixed_mcp_tool_servers: Vec::new(), elicitation_requests: ElicitationRequestManager::default(), @@ -915,12 +915,12 @@ impl McpConnectionSet { } pub fn plugin_id_for_mcp_server_name(&self, server_name: &str) -> Option<&str> { - self.tool_plugin_provenance + self.tool_plugin_context .plugin_id_for_mcp_server_name(server_name) } pub fn is_selected_plugin_mcp_server(&self, server_name: &str) -> bool { - self.tool_plugin_provenance + self.tool_plugin_context .is_selected_plugin_mcp_server(server_name) } diff --git a/codex-rs/codex-mcp/src/connection_manager/tool_catalog.rs b/codex-rs/codex-mcp/src/connection_manager/tool_catalog.rs index 2e08cf004d..8b750ba326 100644 --- a/codex-rs/codex-mcp/src/connection_manager/tool_catalog.rs +++ b/codex-rs/codex-mcp/src/connection_manager/tool_catalog.rs @@ -149,7 +149,7 @@ impl McpConnectionSet { .startup_complete .load(Ordering::Acquire); let server_tools = view - .listed_tools(&self.tool_plugin_provenance) + .listed_tools(&self.tool_plugin_context) .instrument(trace_span!( "list_tools_for_server", server_name = %server_name, @@ -304,11 +304,11 @@ impl McpConnectionSet { }; let server_tools = filter_tools(server_tools, &view.tool_filter); let server_tools = if server_name == CODEX_APPS_MCP_SERVER_NAME { - prepare_codex_apps_tools_for_model(server_tools, &self.tool_plugin_provenance) + prepare_codex_apps_tools_for_model(server_tools, &self.tool_plugin_context) } else { crate::rmcp_client::prepare_regular_mcp_tools_for_model( server_tools, - &self.tool_plugin_provenance, + &self.tool_plugin_context, ) }; let server_tools = server_tools @@ -427,6 +427,9 @@ impl McpConnectionSet { .filter(|tool| { server_has_permission && view.tool_filter.allows(&tool.tool.name) + && self + .tool_plugin_context + .allows_connector_id(tool.connector_id.as_deref()) && tool_is_model_visible(tool) }) .map(|tool| tool.tool.name.to_string()) @@ -452,7 +455,7 @@ impl McpConnectionSet { let (_, tools) = self.refresh_codex_apps_tool_catalog().await?; let tools = prepare_codex_apps_tools_for_model( filter_tools(tools, &view.tool_filter), - &self.tool_plugin_provenance, + &self.tool_plugin_context, ) .into_iter() .map(|tool| Self::with_server_metadata(tool, &view.metadata)); diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 502f379026..875d8be2e4 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -108,7 +108,7 @@ impl McpConnectionSet { disabled_servers: Vec::new(), required_servers: Vec::new(), optional_startup_deadline: OnceLock::new(), - tool_plugin_provenance: Arc::new(ToolPluginProvenance::default()), + tool_plugin_context: Arc::new(ToolPluginContext::default()), prefix_mcp_tool_names, non_prefixed_mcp_tool_servers: Vec::new(), elicitation_requests: ElicitationRequestManager::new( @@ -501,7 +501,7 @@ async fn prepared_call_timeout_includes_trusted_access_lookup() { config .server_permission_profiles .insert("docs".to_string(), PermissionProfile::default()); - manager.tool_plugin_provenance = Arc::new(crate::tool_plugin_provenance(&config)); + manager.tool_plugin_context = Arc::new(crate::tool_plugin_context(&config)); let auth = CodexAuth::create_dummy_chatgpt_auth_for_testing(); manager.trusted_access = Some(TrustedAccessContext::new( auth.clone(), @@ -2816,7 +2816,7 @@ async fn capture_binding_skips_pending_optional_servers_after_configured_shared_ )); plugin_config.mcp_server_catalog = catalog.build(); plugin_config.optional_mcp_startup_grace = Duration::from_millis(250); - manager.tool_plugin_provenance = Arc::new(crate::tool_plugin_provenance(&plugin_config)); + manager.tool_plugin_context = Arc::new(crate::tool_plugin_context(&plugin_config)); for server_name in ["pending-one", "pending-two", "pending-selected"] { manager.insert_test_client( server_name.to_string(), @@ -2840,7 +2840,7 @@ async fn capture_binding_skips_pending_optional_servers_after_configured_shared_ &permission_profile, /*prefix_mcp_tool_names*/ true, ); - required_manager.tool_plugin_provenance = Arc::clone(&manager.tool_plugin_provenance); + required_manager.tool_plugin_context = Arc::clone(&manager.tool_plugin_context); required_manager.insert_test_client( "pending-selected", manager.test_client("pending-selected").clone(), diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index af66f91394..f1d85c45e6 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -48,7 +48,7 @@ pub use catalog::ResolvedMcpServer; pub use mcp::CODEX_APPS_MCP_SERVER_NAME; pub use mcp::DEFAULT_OPTIONAL_MCP_STARTUP_GRACE; pub use mcp::McpConfig; -pub use mcp::ToolPluginProvenance; +pub use mcp::ToolPluginContext; pub use server::EffectiveMcpServer; pub use auth_elicitation::CodexAppsAuthElicitation; @@ -68,7 +68,7 @@ pub use mcp::effective_mcp_servers; pub use mcp::effective_mcp_servers_from_configured; pub use mcp::host_owned_codex_apps_enabled; pub use mcp::hosted_plugin_runtime_mcp_server_config; -pub use mcp::tool_plugin_provenance; +pub use mcp::tool_plugin_context; pub use plugin_config::PluginMcpConfigParseOutcome; pub use plugin_config::PluginMcpServerParseError; pub use plugin_config::parse_agent_plugin_mcp_config; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 33b47cadde..c9cd8ceb6f 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -240,15 +240,21 @@ impl McpConfig { } } +/// Plugin attribution and selection data derived from the current MCP configuration. #[derive(Debug, Clone, Default, PartialEq, Eq)] -pub struct ToolPluginProvenance { +pub struct ToolPluginContext { plugin_display_names_by_connector_id: HashMap>, + disabled_connector_ids: HashSet, plugin_display_names_by_mcp_server_name: HashMap>, plugin_ids_by_mcp_server_name: HashMap, selected_plugin_mcp_server_names: HashSet, } -impl ToolPluginProvenance { +impl ToolPluginContext { + pub(crate) fn allows_connector_id(&self, connector_id: Option<&str>) -> bool { + connector_id.is_none_or(|id| !self.disabled_connector_ids.contains(id)) + } + pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] { self.plugin_display_names_by_connector_id .get(connector_id) @@ -274,9 +280,12 @@ impl ToolPluginProvenance { } fn from_config(config: &McpConfig) -> Self { - let mut tool_plugin_provenance = Self::default(); + let mut tool_plugin_context = Self { + disabled_connector_ids: config.connector_snapshot.disabled_connector_ids().clone(), + ..Self::default() + }; for connector_id in config.connector_snapshot.connector_ids() { - tool_plugin_provenance + tool_plugin_context .plugin_display_names_by_connector_id .insert( connector_id.0.clone(), @@ -291,30 +300,28 @@ impl ToolPluginProvenance { .mcp_server_catalog .plugin_attributions_by_server_name() { - tool_plugin_provenance + tool_plugin_context .plugin_display_names_by_mcp_server_name .insert( server_name.clone(), vec![attribution.display_name().to_string()], ); - tool_plugin_provenance + tool_plugin_context .plugin_ids_by_mcp_server_name .insert(server_name, attribution.plugin_id().to_string()); } - tool_plugin_provenance - .selected_plugin_mcp_server_names - .extend( - config - .mcp_server_catalog - .selected_plugin_server_names() - .map(str::to_string), - ); + tool_plugin_context.selected_plugin_mcp_server_names.extend( + config + .mcp_server_catalog + .selected_plugin_server_names() + .map(str::to_string), + ); - for plugin_names in tool_plugin_provenance + for plugin_names in tool_plugin_context .plugin_display_names_by_connector_id .values_mut() .chain( - tool_plugin_provenance + tool_plugin_context .plugin_display_names_by_mcp_server_name .values_mut(), ) @@ -322,7 +329,7 @@ impl ToolPluginProvenance { plugin_names.sort_unstable(); plugin_names.dedup(); } - tool_plugin_provenance + tool_plugin_context } } @@ -409,8 +416,8 @@ pub fn effective_mcp_servers_from_configured( servers } -pub fn tool_plugin_provenance(config: &McpConfig) -> ToolPluginProvenance { - ToolPluginProvenance::from_config(config) +pub fn tool_plugin_context(config: &McpConfig) -> ToolPluginContext { + ToolPluginContext::from_config(config) } pub async fn read_mcp_resource( diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index e6e5313717..6272a57ce7 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -253,7 +253,7 @@ fn mcp_prompt_auto_approval_rejects_auto_mode_in_default_permission_mode() { } #[test] -fn tool_plugin_provenance_collects_app_and_mcp_sources() { +fn tool_plugin_context_collects_app_and_mcp_sources() { let mut config = test_mcp_config(PathBuf::new()); let mut catalog = ResolvedMcpCatalog::builder(); catalog.register(McpServerRegistration::from_plugin( @@ -289,11 +289,12 @@ fn tool_plugin_provenance_collects_app_and_mcp_sources() { ..PluginCapabilitySummary::default() }, ]); - let provenance = tool_plugin_provenance(&config); + let provenance = tool_plugin_context(&config); assert_eq!( provenance, - ToolPluginProvenance { + ToolPluginContext { + disabled_connector_ids: HashSet::new(), plugin_display_names_by_connector_id: HashMap::from([ ( "connector_example".to_string(), @@ -351,11 +352,12 @@ fn selected_mcp_attribution_does_not_join_an_unrelated_local_summary() { }, ]); - let provenance = tool_plugin_provenance(&config); + let provenance = tool_plugin_context(&config); assert_eq!( provenance, - ToolPluginProvenance { + ToolPluginContext { + disabled_connector_ids: HashSet::new(), plugin_display_names_by_connector_id: HashMap::new(), plugin_display_names_by_mcp_server_name: HashMap::from([( "github".to_string(), diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 60f204e034..393218883c 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -29,7 +29,7 @@ use crate::codex_apps::prepare_openai_file_params_for_model; use crate::elicitation::ElicitationRequestManager; use crate::executor_environment_http_client::ExecutorEnvironmentHttpClient; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; -use crate::mcp::ToolPluginProvenance; +use crate::mcp::ToolPluginContext; use crate::openai_docs_source_attribution::maybe_with_openai_docs_source_attribution; use crate::pagination::collect_paginated_with_limit; use crate::runtime::McpRuntimeContext; @@ -688,20 +688,21 @@ pub(crate) async fn list_tools_for_client_uncached( Ok(tools) } -/// Presents declared Codex Apps file parameters to the model as local-path inputs and adds plugin -/// names to each tool. Plugin membership is resolved by connector ID, falling back to the MCP -/// server when absent. +/// Filters disabled connectors, presents declared Codex Apps file parameters to the model as +/// local-path inputs, and adds plugin names to each tool. Plugin membership is resolved by +/// connector ID, falling back to the MCP server when absent. pub(crate) fn prepare_codex_apps_tools_for_model( mut tools: Vec, - tool_plugin_provenance: &ToolPluginProvenance, + tool_plugin_context: &ToolPluginContext, ) -> Vec { + tools.retain(|tool| tool_plugin_context.allows_connector_id(tool.connector_id.as_deref())); for tool in &mut tools { prepare_openai_file_params_for_model(tool); let plugin_names = match tool.connector_id.as_deref() { Some(connector_id) => { - tool_plugin_provenance.plugin_display_names_for_connector_id(connector_id) + tool_plugin_context.plugin_display_names_for_connector_id(connector_id) } - None => tool_plugin_provenance + None => tool_plugin_context .plugin_display_names_for_mcp_server_name(tool.server_name.as_str()), }; add_plugin_provenance_to_tool(tool, plugin_names); @@ -747,11 +748,11 @@ fn add_plugin_provenance_to_tool(tool: &mut ToolInfo, plugin_names: &[String]) { /// Adds server-scoped plugin names to regular MCP tools without changing their input schemas. pub(crate) fn prepare_regular_mcp_tools_for_model( mut tools: Vec, - tool_plugin_provenance: &ToolPluginProvenance, + tool_plugin_context: &ToolPluginContext, ) -> Vec { for tool in &mut tools { - let plugin_names = tool_plugin_provenance - .plugin_display_names_for_mcp_server_name(tool.server_name.as_str()); + let plugin_names = + tool_plugin_context.plugin_display_names_for_mcp_server_name(tool.server_name.as_str()); add_plugin_provenance_to_tool(tool, plugin_names); } tools diff --git a/codex-rs/connectors/src/snapshot.rs b/codex-rs/connectors/src/snapshot.rs index a07b1ef589..7011a74d3c 100644 --- a/codex-rs/connectors/src/snapshot.rs +++ b/codex-rs/connectors/src/snapshot.rs @@ -59,28 +59,43 @@ impl PluginConnectorSource { } } -/// Immutable connector declarations and their plugin provenance. +impl From<&PluginCapabilitySummary> for PluginConnectorSource { + fn from(summary: &PluginCapabilitySummary) -> Self { + Self::from_connector_ids( + summary.config_name.clone(), + summary.display_name.clone(), + summary.app_connector_ids.clone(), + ) + } +} + +/// Immutable connector selection and provenance after applying disabled plugins. #[derive(Clone, Debug, Default, PartialEq, Eq)] pub struct ConnectorSnapshot { - sources: Vec, connector_ids: Vec, plugin_display_names_by_connector_id: HashMap>, + disabled_connector_ids: HashSet, } impl ConnectorSnapshot { - /// Builds a connector snapshot from package-scoped declarations. - pub fn from_plugin_sources(sources: impl IntoIterator) -> Self { - let sources = sources - .into_iter() - .filter(|source| !source.connector_ids().is_empty()) - .collect::>(); + /// Builds the final selection from all plugin sources, preserving contribution order. + /// Pass host and selected sources together so any enabled shared owner keeps a connector. + pub fn from_plugin_sources( + sources: impl IntoIterator, + disabled_plugin_ids: &[String], + ) -> Self { let mut connector_ids = Vec::new(); let mut seen_connector_ids = HashSet::new(); let mut plugin_display_names_by_connector_id: HashMap> = HashMap::new(); + let mut disabled_connector_ids = HashSet::new(); - for source in &sources { + for source in sources { + if disabled_plugin_ids.contains(&source.plugin_id) { + disabled_connector_ids.extend(source.connector_ids.into_iter().map(|id| id.0)); + continue; + } for connector_id in source.connector_ids() { - if seen_connector_ids.insert(connector_id.clone()) { + if seen_connector_ids.insert(connector_id.0.clone()) { connector_ids.push(connector_id.clone()); } plugin_display_names_by_connector_id @@ -93,23 +108,18 @@ impl ConnectorSnapshot { plugin_names.sort_unstable(); plugin_names.dedup(); } + disabled_connector_ids.retain(|id| !seen_connector_ids.contains(id)); Self { - sources, connector_ids, plugin_display_names_by_connector_id, + disabled_connector_ids, } } /// Adapts the current host plugin summaries to the connector-owned snapshot. pub fn from_plugin_capability_summaries(summaries: &[PluginCapabilitySummary]) -> Self { - Self::from_plugin_sources(summaries.iter().map(|summary| { - PluginConnectorSource::from_connector_ids( - summary.config_name.clone(), - summary.display_name.clone(), - summary.app_connector_ids.clone(), - ) - })) + Self::from_plugin_sources(summaries.iter().map(PluginConnectorSource::from), &[]) } /// Returns the connector IDs in source contribution order. @@ -117,6 +127,11 @@ impl ConnectorSnapshot { &self.connector_ids } + /// Connector tools excluded because no enabled plugin still contributes them. + pub fn disabled_connector_ids(&self) -> &HashSet { + &self.disabled_connector_ids + } + /// Returns the package display names associated with one connector. pub fn plugin_display_names_for_connector_id(&self, connector_id: &str) -> &[String] { self.plugin_display_names_by_connector_id @@ -124,11 +139,6 @@ impl ConnectorSnapshot { .map(Vec::as_slice) .unwrap_or_default() } - - /// Combines two snapshots while preserving source order and provenance. - pub fn merged_with(&self, other: &Self) -> Self { - Self::from_plugin_sources(self.sources.iter().chain(&other.sources).cloned()) - } } #[cfg(test)] diff --git a/codex-rs/connectors/src/snapshot_tests.rs b/codex-rs/connectors/src/snapshot_tests.rs index 3087bc73cd..e4f0bc2714 100644 --- a/codex-rs/connectors/src/snapshot_tests.rs +++ b/codex-rs/connectors/src/snapshot_tests.rs @@ -6,19 +6,17 @@ use super::PluginConnectorSource; #[test] fn snapshot_merges_sources_in_order_and_dedupes_provenance() { - let host_source = source("host", "Zulu", &["calendar", "calendar"]); - let host = ConnectorSnapshot::from_plugin_sources([ + let host = [ source("skills", "Skills only", &[]), - host_source.clone(), - ]); - let selected = ConnectorSnapshot::from_plugin_sources([ + source("host", "Zulu", &["calendar", "calendar"]), + ]; + let selected = [ source("selected-a", "Alpha", &["drive", "calendar"]), source("selected-b", "Alpha", &["calendar"]), - ]); + ]; - let merged = host.merged_with(&selected); + let merged = ConnectorSnapshot::from_plugin_sources(host.into_iter().chain(selected), &[]); - assert_eq!(host.sources, vec![host_source]); assert_eq!( merged.connector_ids(), &[ @@ -36,6 +34,42 @@ fn snapshot_merges_sources_in_order_and_dedupes_provenance() { ); } +#[test] +fn disabled_plugins_preserve_shared_connectors() { + let sources = [ + source("alpha", "Alpha", &["exclusive", "shared"]), + source("beta", "Beta", &["other", "shared"]), + ]; + let filtered = ConnectorSnapshot::from_plugin_sources(sources.clone(), &["alpha".to_string()]); + + let expected = ConnectorSnapshot { + disabled_connector_ids: std::collections::HashSet::from(["exclusive".to_string()]), + ..ConnectorSnapshot::from_plugin_sources( + [source("beta", "Beta", &["other", "shared"])], + &[], + ) + }; + assert_eq!(filtered, expected); + assert_eq!( + ConnectorSnapshot::from_plugin_sources( + sources.iter().rev().cloned(), + &["alpha".to_string()], + ), + expected + ); + assert_eq!( + ConnectorSnapshot::from_plugin_sources(sources, &["alpha".to_string(), "beta".to_string()],), + ConnectorSnapshot { + disabled_connector_ids: std::collections::HashSet::from([ + "exclusive".to_string(), + "other".to_string(), + "shared".to_string(), + ]), + ..ConnectorSnapshot::default() + } + ); +} + fn source(id: &str, display_name: &str, connector_ids: &[&str]) -> PluginConnectorSource { PluginConnectorSource::from_connector_ids( id, diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 5929ae7d5a..4156fcfc39 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -37,9 +37,9 @@ use codex_mcp::McpRuntimeContext; use codex_mcp::McpRuntimeInput; use codex_mcp::McpStartupPolicy; use codex_mcp::ToolInfo; -use codex_mcp::ToolPluginProvenance; +use codex_mcp::ToolPluginContext; use codex_mcp::effective_mcp_servers; -use codex_mcp::tool_plugin_provenance; +use codex_mcp::tool_plugin_context; use codex_protocol::mcp::ClientMcpExtensions; const CONNECTORS_READY_TIMEOUT_ON_EMPTY_TOOLS: Duration = Duration::from_secs(30); @@ -224,10 +224,10 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( } let cache_key = accessible_connectors_cache_key(config, auth.as_ref()); let mcp_config = mcp_manager.runtime_config(config).await; - let tool_plugin_provenance = tool_plugin_provenance(&mcp_config); + let tool_plugin_context = tool_plugin_context(&mcp_config); if !force_refetch && let Some(cached_connectors) = read_cached_accessible_connectors(&cache_key) { - let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_provenance); + let cached_connectors = with_app_plugin_sources(cached_connectors, &tool_plugin_context); return Ok(AccessibleConnectorsStatus { connectors: cached_connectors, codex_apps_ready: true, @@ -334,7 +334,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_mcp_manager( write_cached_accessible_connectors(cache_key, &accessible_connectors); } let accessible_connectors = - with_app_plugin_sources(accessible_connectors, &tool_plugin_provenance); + with_app_plugin_sources(accessible_connectors, &tool_plugin_context); mcp_runtime.shutdown().await; Ok(AccessibleConnectorsStatus { connectors: accessible_connectors, @@ -501,10 +501,10 @@ fn accessible_connectors_for_app_list_from_mcp_tools(mcp_tools: &[ToolInfo]) -> pub fn with_app_plugin_sources( mut connectors: Vec, - tool_plugin_provenance: &ToolPluginProvenance, + tool_plugin_context: &ToolPluginContext, ) -> Vec { for connector in &mut connectors { - connector.plugin_display_names = tool_plugin_provenance + connector.plugin_display_names = tool_plugin_context .plugin_display_names_for_connector_id(connector.id.as_str()) .to_vec(); } diff --git a/codex-rs/core/src/hook_runtime.rs b/codex-rs/core/src/hook_runtime.rs index bdd5663def..4046cc8826 100644 --- a/codex-rs/core/src/hook_runtime.rs +++ b/codex-rs/core/src/hook_runtime.rs @@ -346,6 +346,14 @@ fn executor_hook_sources_for_step(step_context: &StepContext) -> Vec { pub(crate) session_source: &'a SessionSource, pub(crate) originator: &'a str, pub(crate) environments: McpEnvironmentScope<'a>, + pub(crate) disabled_plugin_ids: &'a [String], } enum OrderedMcpOverlay { @@ -119,6 +120,7 @@ impl McpManager { /*originator*/ None, McpEnvironmentScope::HostOnly, + &[], ) .await .config @@ -146,6 +148,7 @@ impl McpManager { .with_session_source(identity.session_source), Some(identity.originator), identity.environments, + identity.disabled_plugin_ids, ) .await } @@ -155,6 +158,7 @@ impl McpManager { context: McpServerContributionContext<'_, Config>, originator: Option<&str>, environment_scope: McpEnvironmentScope<'_>, + disabled_plugin_ids: &[String], ) -> McpRuntimeProjection { let config = context.config(); let mut selected_plugin_available = false; @@ -203,6 +207,8 @@ impl McpManager { ), ))); } + McpServerContribution::SelectedPlugin { ref plugin_id, .. } + if disabled_plugin_ids.contains(plugin_id) => {} McpServerContribution::SelectedPlugin { name, plugin_id, @@ -228,11 +234,15 @@ impl McpManager { plugin_display_name, connector_ids, } => { - selected_plugin_available = true; - selected_plugins.push(SelectedPluginIdentity { - selected_root_id, - plugin_id: plugin_id.clone(), - }); + if disabled_plugin_ids.contains(&plugin_id) { + disabled_plugin_roots.push(selected_root_id); + } else { + selected_plugin_available = true; + selected_plugins.push(SelectedPluginIdentity { + selected_root_id, + plugin_id: plugin_id.clone(), + }); + } if !connector_ids.is_empty() { selected_plugin_connector_sources.push( PluginConnectorSource::from_connector_ids( @@ -259,6 +269,15 @@ impl McpManager { .plugins_manager .plugins_for_config(&config.plugins_config_input()) .await; + let host_plugin_connector_sources = loaded_plugins + .capability_summaries() + .iter() + .map(PluginConnectorSource::from); + let connector_snapshot = ConnectorSnapshot::from_plugin_sources( + host_plugin_connector_sources.chain(selected_plugin_connector_sources), + disabled_plugin_ids, + ); + let loaded_plugins = loaded_plugins.without_plugins(disabled_plugin_ids); let plugins_available = selected_plugin_available || !loaded_plugins.capability_summaries().is_empty(); let mut mcp_config = config @@ -333,12 +352,7 @@ impl McpManager { ); } mcp_config.mcp_server_catalog = catalog; - mcp_config.connector_snapshot = - mcp_config - .connector_snapshot - .merged_with(&ConnectorSnapshot::from_plugin_sources( - selected_plugin_connector_sources, - )); + mcp_config.connector_snapshot = connector_snapshot; McpRuntimeProjection { config: mcp_config, plugins_available, diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index d4f8211d59..8b57655124 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1374,6 +1374,7 @@ const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; #[derive(Clone, Debug, PartialEq, Eq, Serialize)] struct McpToolApprovalKey { server: String, + plugin_id: Option, connector_id: Option, link_id: Option, tool_name: String, @@ -1642,6 +1643,7 @@ fn session_mcp_tool_approval_key( Some(McpToolApprovalKey { server: invocation.server.clone(), + plugin_id: metadata.and_then(|metadata| metadata.plugin_id.clone()), connector_id, link_id: metadata.and_then(|metadata| metadata.link_id.clone()), tool_name: invocation.tool.clone(), @@ -2154,7 +2156,13 @@ async fn maybe_persist_mcp_tool_approval( }; persist_codex_app_tool_approval(&turn_context.config, &connector_id, &tool_name).await } else { - persist_non_app_mcp_tool_approval(sess, &turn_context.config, &key.server, &tool_name).await + persist_non_app_mcp_tool_approval( + &turn_context.config, + &key.server, + &tool_name, + key.plugin_id.as_deref(), + ) + .await }; if let Err(err) = persist_result { @@ -2207,33 +2215,22 @@ async fn persist_custom_mcp_tool_approval( } async fn persist_non_app_mcp_tool_approval( - sess: &Session, config: &Config, server: &str, tool_name: &str, + plugin_id: Option<&str>, ) -> anyhow::Result<()> { if let Some(config_edits_builder) = custom_mcp_tool_approval_config_builder(config, server)? { return persist_custom_mcp_tool_approval_with(config_edits_builder, server, tool_name) .await; } - let plugin_config_name = sess - .services - .plugins_manager - .plugins_for_config(&config.plugins_config_input()) - .await - .plugins() - .iter() - .filter(|plugin| plugin.is_active()) - .find(|plugin| plugin.mcp_servers.contains_key(server)) - .map(|plugin| plugin.config_name.clone()); - - if let Some(plugin_config_name) = plugin_config_name { + if let Some(plugin_id) = plugin_id { return ConfigEditsBuilder::for_config(config) .with_edits([ConfigEdit::SetPath { segments: vec![ "plugins".to_string(), - plugin_config_name, + plugin_id.to_string(), "mcp_servers".to_string(), server.to_string(), "tools".to_string(), diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 3b44bafecd..980ed317c3 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -910,6 +910,7 @@ fn custom_servers_support_session_and_persistent_approval() { }; let expected = McpToolApprovalKey { server: "custom_server".to_string(), + plugin_id: None, connector_id: None, link_id: None, tool_name: "run_action".to_string(), @@ -925,8 +926,24 @@ fn custom_servers_support_session_and_persistent_approval() { /*metadata*/ None, AppToolApproval::Auto ), - Some(expected) + Some(expected.clone()) ); + let mut metadata = approval_metadata( + /*connector_id*/ None, /*connector_name*/ None, + /*connector_description*/ None, /*tool_title*/ None, + /*tool_description*/ None, + ); + for plugin_id in ["alpha@test", "beta@test"] { + metadata.plugin_id = Some(plugin_id.to_string()); + let plugin_key = McpToolApprovalKey { + plugin_id: Some(plugin_id.to_string()), + ..expected.clone() + }; + assert_eq!( + session_mcp_tool_approval_key(&invocation, Some(&metadata), AppToolApproval::Auto), + Some(plugin_key) + ); + } } #[test] @@ -946,6 +963,7 @@ fn codex_apps_connectors_support_persistent_approval() { metadata.link_id = Some("link_a".to_string()); let expected = McpToolApprovalKey { server: CODEX_APPS_MCP_SERVER_NAME.to_string(), + plugin_id: None, connector_id: Some("calendar".to_string()), link_id: Some("link_a".to_string()), tool_name: "calendar/list_events".to_string(), @@ -2341,6 +2359,7 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config() { std::fs::create_dir_all(&codex_home).expect("create codex home"); let key = McpToolApprovalKey { server: CODEX_APPS_MCP_SERVER_NAME.to_string(), + plugin_id: None, connector_id: Some("calendar".to_string()), link_id: None, tool_name: "calendar/list_events".to_string(), @@ -2392,6 +2411,7 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve turn_context.config = Arc::new(config); let key = McpToolApprovalKey { server: "docs".to_string(), + plugin_id: None, connector_id: None, link_id: None, tool_name: "search".to_string(), @@ -2426,29 +2446,12 @@ async fn maybe_persist_mcp_tool_approval_reloads_session_config_for_custom_serve #[tokio::test] async fn maybe_persist_mcp_tool_approval_writes_plugin_mcp_policy() { - let (session, mut turn_context) = make_session_and_context().await; + let (session, turn_context) = make_session_and_context().await; let codex_home = session.codex_home().await; - write_sample_plugin_mcp(codex_home.as_path()); - std::fs::write( - codex_home.join(CONFIG_TOML_FILE), - r#" -[features] -plugins = true - -[plugins."sample@test"] -enabled = true -"#, - ) - .expect("seed config"); - let config = ConfigBuilder::default() - .codex_home(codex_home.to_path_buf()) - .build() - .await - .expect("load config"); - turn_context.config = Arc::new(config); - session.services.plugins_manager.clear_cache(); + std::fs::create_dir_all(&codex_home).expect("create codex home"); let key = McpToolApprovalKey { server: "sample".to_string(), + plugin_id: Some("sample@test".to_string()), connector_id: None, link_id: None, tool_name: "search".to_string(), @@ -2506,6 +2509,7 @@ async fn maybe_persist_mcp_tool_approval_writes_project_config_for_project_serve turn_context.config = Arc::new(config); let key = McpToolApprovalKey { server: "docs".to_string(), + plugin_id: None, connector_id: None, link_id: None, tool_name: "search".to_string(), diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index adc4740b98..12bfee4acb 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -98,11 +98,12 @@ impl Session { config: &Config, ) -> (McpConfig, McpRuntimeContext) { let originator = self.originator().await; - let (session_source, host_fallback_cwd) = { + let (session_source, host_fallback_cwd, disabled_plugin_ids) = { let state = self.state.lock().await; ( state.session_configuration.session_source.clone(), state.session_configuration.cwd().clone(), + state.active_disabled_plugin_ids.clone(), ) }; let environments = self.services.turn_environments.snapshot().await; @@ -128,6 +129,7 @@ impl Session { McpThreadIdentity { session_source: &session_source, originator: &originator, + disabled_plugin_ids: &disabled_plugin_ids, environments: McpEnvironmentScope::Live(&self.services.turn_environments), }, &ready_selected_capability_roots, @@ -233,6 +235,7 @@ impl Session { McpThreadIdentity { session_source: &desired.session_source, originator: &desired.originator, + disabled_plugin_ids: &desired.disabled_plugin_ids, environments: McpEnvironmentScope::Live(&self.services.turn_environments), }, &ready_selected_capability_roots, @@ -299,6 +302,7 @@ impl Session { McpThreadIdentity { session_source: &desired.session_source, originator: &desired.originator, + disabled_plugin_ids: &desired.disabled_plugin_ids, environments: McpEnvironmentScope::Live(&self.services.turn_environments), }, &ready_selected_capability_roots, @@ -662,12 +666,13 @@ impl Session { return; }; let auth = self.services.auth_manager.auth().await; - { + let disabled_plugin_ids = { let mut state = self.state.lock().await; let mut config = (*state.session_configuration.original_config_do_not_use).clone(); config.mcp_servers = refresh_config.mcp_servers.clone(); state.session_configuration.original_config_do_not_use = Arc::new(config); - } + state.active_disabled_plugin_ids.clone() + }; let ready_selected_capability_roots = self .services .mcp_runtime @@ -690,6 +695,7 @@ impl Session { McpThreadIdentity { session_source: &turn_context.session_source, originator: &turn_context.originator, + disabled_plugin_ids: &disabled_plugin_ids, environments: McpEnvironmentScope::Live(&self.services.turn_environments), }, &ready_selected_capability_roots, diff --git a/codex-rs/core/src/session/mcp_runtime.rs b/codex-rs/core/src/session/mcp_runtime.rs index e14034de7b..b85bbc2ffe 100644 --- a/codex-rs/core/src/session/mcp_runtime.rs +++ b/codex-rs/core/src/session/mcp_runtime.rs @@ -28,6 +28,7 @@ pub(super) struct McpDesiredState { pub(super) session_source: SessionSource, pub(super) environments: TurnEnvironmentSnapshot, pub(super) local_process_cwd: PathBuf, + pub(super) disabled_plugin_ids: Vec, } impl Session { @@ -75,9 +76,12 @@ impl Session { &self, auth: Option, ) -> McpDesiredState { - let session_configuration = { + let (session_configuration, disabled_plugin_ids) = { let state = self.state.lock().await; - state.session_configuration.clone() + ( + state.session_configuration.clone(), + state.active_disabled_plugin_ids.clone(), + ) }; let environments = self.services.turn_environments.snapshot().await; let cwd = environments @@ -98,6 +102,7 @@ impl Session { session_source: session_configuration.session_source.clone(), environments, local_process_cwd, + disabled_plugin_ids, } } @@ -124,6 +129,7 @@ impl Session { session_source: session_configuration.session_source.clone(), environments: resolved_environments.clone(), local_process_cwd, + disabled_plugin_ids: session_configuration.disabled_plugin_ids.clone(), }; self.publish_mcp_runtime( &desired, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index cb51fdfb19..4824e1c79d 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -235,6 +235,7 @@ mod mcp_prewarm; mod mcp_refresh; mod mcp_runtime; pub(crate) mod multi_agents; +mod plugin_selection; mod realtime_history; mod retained_context; mod review; @@ -2006,11 +2007,13 @@ impl Session { } pub(crate) async fn refresh_hooks(&self, config: Arc) { + let disabled_plugin_ids = self.state.lock().await.active_disabled_plugin_ids.clone(); let environments = self.services.turn_environments.snapshot().await; let hooks_config = build_hooks_config( config.as_ref(), self.services.plugins_manager.as_ref(), environments.single_local_environment(), + &disabled_plugin_ids, ) .await; @@ -2020,7 +2023,8 @@ impl Session { if Arc::ptr_eq( &state.session_configuration.original_config_do_not_use, &config, - ) { + ) && state.active_disabled_plugin_ids == disabled_plugin_ids + { let hooks = self.hooks().reconfigured(hooks_config); self.services.hooks.store(Arc::new(hooks)); } @@ -4112,7 +4116,8 @@ impl Session { .services .plugins_manager .plugins_for_config(&turn_context.config.plugins_config_input()) - .await; + .await + .without_plugins(&turn_context.disabled_plugin_ids); let recommended_plugin_candidates = if turn_context .config .features @@ -4874,6 +4879,7 @@ async fn build_hooks_config( config: &Config, plugins_manager: &PluginsManager, environment: Option<&TurnEnvironment>, + disabled_plugin_ids: &[String], ) -> HooksConfig { let (hook_shell_program, hook_shell_argv) = environment .and_then(|environment| environment.shell.as_ref()) @@ -4885,7 +4891,10 @@ async fn build_hooks_config( }) .unwrap_or_default(); let plugins_input = config.plugins_config_input(); - let plugin_outcome = plugins_manager.plugins_for_config(&plugins_input).await; + let plugin_outcome = plugins_manager + .plugins_for_config(&plugins_input) + .await + .without_plugins(disabled_plugin_ids); let plugin_hook_sources = plugin_outcome.effective_plugin_hook_sources(); let plugin_hook_load_warnings = plugin_outcome.effective_plugin_hook_warnings(); HooksConfig { diff --git a/codex-rs/core/src/session/plugin_selection.rs b/codex-rs/core/src/session/plugin_selection.rs new file mode 100644 index 0000000000..0a9580db0c --- /dev/null +++ b/codex-rs/core/src/session/plugin_selection.rs @@ -0,0 +1,32 @@ +//! Activates an admitted task's plugin selection without changing pending settings. + +use std::sync::Arc; + +use super::Session; +use super::turn_context::TurnContext; + +impl Session { + pub(crate) async fn activate_plugin_selection(&self, turn_context: &TurnContext) { + let config = { + let mut state = self.state.lock().await; + if state.active_disabled_plugin_ids != turn_context.disabled_plugin_ids { + state.active_disabled_plugin_ids = turn_context.disabled_plugin_ids.clone(); + self.mark_mcp_runtime_dirty(); + } + Arc::clone(&state.session_configuration.original_config_do_not_use) + }; + let plugin_outcome = self + .services + .plugins_manager + .plugins_for_config(&config.plugins_config_input()) + .await + .without_plugins(&turn_context.disabled_plugin_ids); + // Cache changes from another process do not notify this session's hook runtime. + if !self.hooks().matches_plugin_hooks( + plugin_outcome.iter_effective_plugin_hook_sources(), + plugin_outcome.iter_effective_plugin_hook_warnings(), + ) { + self.refresh_hooks(config).await; + } + } +} diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index d949d77ed7..5302eae816 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -996,6 +996,7 @@ impl Session { let thread_extension_data_for_mcp = &thread_extension_data; let mcp_originator = session_configuration.originator.clone(); let mcp_session_source = session_configuration.session_source.clone(); + let mcp_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone(); let mcp_runtime_cwd = environment_selections .first() .and_then(|environment| environment.cwd.to_abs_path().ok()) @@ -1025,6 +1026,7 @@ impl Session { McpThreadIdentity { session_source: &mcp_session_source, originator: &mcp_originator, + disabled_plugin_ids: &mcp_disabled_plugin_ids, environments: McpEnvironmentScope::Initial(environment_selections), }, /*ready_selected_capability_roots*/ &[], @@ -1340,6 +1342,7 @@ impl Session { ), ); state.base_instructions_provenance = base_instructions_provenance.clone(); + state.active_disabled_plugin_ids = session_configuration.disabled_plugin_ids.clone(); let managed_network_requirements_configured = config .config_layer_stack .requirements_toml() @@ -1419,6 +1422,7 @@ impl Session { &config, plugins_manager.as_ref(), resolved_environments.single_local_environment(), + &session_configuration.disabled_plugin_ids, ) .await; let (hooks, async_hook_results) = Hooks::new( @@ -1672,6 +1676,7 @@ impl Session { McpThreadIdentity { session_source: &session_configuration.session_source, originator: &session_configuration.originator, + disabled_plugin_ids: &session_configuration.disabled_plugin_ids, environments: McpEnvironmentScope::Live( &sess.services.turn_environments, ), diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 5d5fa3a0aa..8b1089860c 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -878,7 +878,8 @@ async fn required_mcp_servers_for_input( .services .plugins_manager .plugins_for_config(&turn_context.config.plugins_config_input()) - .await; + .await + .without_plugins(&turn_context.disabled_plugin_ids); let current_config = sess.services.mcp_runtime.current_config(); let mentioned_plugins = collect_explicit_plugin_mentions(user_input, loaded_plugins.capability_summaries()); @@ -1661,7 +1662,8 @@ pub(crate) async fn prepare_tool_recommendations( .plugins_manager .plugins_for_config(&turn_context.config.plugins_config_input()) .instrument(trace_span!("built_tools.load_plugins")) - .await; + .await + .without_plugins(&turn_context.disabled_plugin_ids); let tool_suggest_is_enabled = tool_suggest_enabled(turn_context); let auth = if tool_suggest_is_enabled { sess.services.auth_manager.auth().await diff --git a/codex-rs/core/src/session/turn_context.rs b/codex-rs/core/src/session/turn_context.rs index ae596b734b..fdf3487abc 100644 --- a/codex-rs/core/src/session/turn_context.rs +++ b/codex-rs/core/src/session/turn_context.rs @@ -1039,18 +1039,8 @@ impl Session { .services .plugins_manager .plugins_for_config(&plugins_input) - .await; - // Cache changes from another process do not notify this session's hook runtime. - if !self.hooks().matches_plugin_hooks( - plugin_outcome.iter_effective_plugin_hook_sources(), - plugin_outcome.iter_effective_plugin_hook_warnings(), - ) { - // Keep the refresh state out of the enclosing turn-construction future. - Box::pin(self.refresh_hooks(Arc::clone( - &session_configuration.original_config_do_not_use, - ))) - .await; - } + .await + .without_plugins(&session_configuration.disabled_plugin_ids); let trusted_plugin_roots = TrustedPluginRoots::from_plugin_load_outcome( &plugin_outcome, per_turn_config.codex_home.as_path(), diff --git a/codex-rs/core/src/state/session.rs b/codex-rs/core/src/state/session.rs index fa14b1da0f..d059d0502b 100644 --- a/codex-rs/core/src/state/session.rs +++ b/codex-rs/core/src/state/session.rs @@ -68,6 +68,8 @@ impl ReasoningEffortPin { /// Persistent, session-scoped state previously stored directly on `Session`. pub(crate) struct SessionState { pub(crate) session_configuration: SessionConfiguration, + /// Plugin selection of the last admitted task; settings updates take effect on the next task. + pub(crate) active_disabled_plugin_ids: Vec, /// Persisted origin of the session base instructions, when known. pub(crate) base_instructions_provenance: Option, pub(crate) history: ContextManager, @@ -112,6 +114,7 @@ impl SessionState { history: ContextManager, ) -> Self { Self { + active_disabled_plugin_ids: Vec::new(), session_configuration, base_instructions_provenance: None, history, diff --git a/codex-rs/core/src/tasks/mod.rs b/codex-rs/core/src/tasks/mod.rs index 8ac1030269..f3033d488b 100644 --- a/codex-rs/core/src/tasks/mod.rs +++ b/codex-rs/core/src/tasks/mod.rs @@ -285,6 +285,7 @@ impl Session { input: Vec, task: T, ) { + self.activate_plugin_selection(&turn_context).await; // Inherited or recovered roots are applied before task start. Otherwise this // task owns its turn, including background work. Later mail cannot change it. turn_context diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index ec516141b0..f4a41b8482 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -1458,13 +1458,21 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() server.environment_id = environment_id.clone(); server.enabled = false; let plugin_id = selected_root.id; - vec![codex_extension_api::McpServerContribution::SelectedPlugin { - name: plugin_id.clone(), - plugin_display_name: plugin_id.clone(), - plugin_id, - selection_order: 0, - config: Box::new(server), - }] + vec![ + codex_extension_api::McpServerContribution::SelectedPluginPackage { + selected_root_id: plugin_id.clone(), + plugin_id: plugin_id.clone(), + plugin_display_name: plugin_id.clone(), + connector_ids: vec![], + }, + codex_extension_api::McpServerContribution::SelectedPlugin { + name: plugin_id.clone(), + plugin_display_name: plugin_id.clone(), + plugin_id, + selection_order: 0, + config: Box::new(server), + }, + ] }) } } @@ -1477,6 +1485,10 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() .features .enable(Feature::Apps) .expect("test config should allow apps"); + config + .features + .enable(Feature::Plugins) + .expect("enable plugins"); std::fs::create_dir_all(&config.codex_home).expect("create codex home"); let lifecycle_observed = Arc::new(std::sync::Mutex::new(Vec::new())); @@ -1550,6 +1562,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() McpThreadIdentity { session_source: &SessionSource::Exec, originator: &first_originator, + disabled_plugin_ids: &[], environments: McpEnvironmentScope::Live(&first_session.services.turn_environments), }, /*ready_selected_capability_roots*/ &[], @@ -1568,6 +1581,7 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() McpThreadIdentity { session_source: &second_session_source, originator: &second_originator, + disabled_plugin_ids: &[], environments: McpEnvironmentScope::Live(&second_session.services.turn_environments), }, /*ready_selected_capability_roots*/ &[], @@ -1628,6 +1642,39 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() .get("originator"), Some(&"codex_work_desktop".to_string()) ); + for disabled_plugin_ids in [vec!["selected-a".to_string()], vec![]] { + let projection = first_session + .services + .mcp_manager + .runtime_config_for_step( + &config, + &first_session.services.mcp_thread_init, + &first_session.services.thread_extension_data, + McpThreadIdentity { + session_source: &SessionSource::Exec, + originator: &first_originator, + disabled_plugin_ids: &disabled_plugin_ids, + environments: McpEnvironmentScope::Live( + &first_session.services.turn_environments, + ), + }, + /*ready_selected_capability_roots*/ &[], + /*executor_capability_discovery*/ None, + ) + .await; + assert_eq!( + projection.selected_plugins.disabled_plugin_roots, + disabled_plugin_ids + ); + assert_eq!( + selected_servers(&projection.config).contains_key("selected-a"), + disabled_plugin_ids.is_empty() + ); + assert_eq!( + projection.selected_plugins.plugins.len(), + usize::from(disabled_plugin_ids.is_empty()) + ); + } } #[tokio::test] diff --git a/codex-rs/core/tests/suite/hooks_executor.rs b/codex-rs/core/tests/suite/hooks_executor.rs index 11e728f717..8dd2ef42c3 100644 --- a/codex-rs/core/tests/suite/hooks_executor.rs +++ b/codex-rs/core/tests/suite/hooks_executor.rs @@ -61,6 +61,58 @@ use wiremock::matchers::path; use super::rmcp_client::remote_aware_environment_id; +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn thread_plugin_selection_disables_executor_hooks_without_disabling_their_server() +-> Result<()> { + skip_if_no_network!(Ok(())); + let fixture = executor_hook_fixture( + ["disabled", "enabled", "disabled-again"] + .map(completed_turn_response) + .to_vec(), + ) + .await?; + fixture.attach().await?; + for (disabled_plugin_ids, expected_calls) in [ + (vec!["computer-use@openai-bundled".to_string()], 0), + (Vec::new(), 1), + (vec!["computer-use@openai-bundled".to_string()], 1), + ] { + let enabled = disabled_plugin_ids.is_empty(); + submit_thread_settings( + &fixture.test.codex, + ThreadSettingsOverrides { + disabled_plugin_ids: Some(disabled_plugin_ids), + ..Default::default() + }, + ) + .await?; + fixture.test.submit_text_turn("finish this turn").await?; + if enabled { + fixture.wait_for_hook_call().await?; + } + assert_eq!(fixture.calls().await?.len(), expected_calls); + } + fixture + .test + .codex + .call_mcp_tool( + "node_repl", + "js", + Some(json!({"code": "1 + 1"})), + /*meta*/ None, + ) + .await?; + assert_eq!( + fixture + .calls() + .await? + .last() + .context("standalone tool call")?["params"]["name"], + "js" + ); + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn executor_stop_hook_runs_after_attachment() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/core/tests/suite/plugins.rs b/codex-rs/core/tests/suite/plugins.rs index d549c62a49..f93f0f6b3e 100644 --- a/codex-rs/core/tests/suite/plugins.rs +++ b/codex-rs/core/tests/suite/plugins.rs @@ -34,6 +34,7 @@ use codex_skills_extension::SkillsExtensionConfig; use codex_skills_extension::install; use core_test_support::apps_test_server::AppsTestServer; use core_test_support::apps_test_server::SEARCH_CALENDAR_CREATE_TOOL; +use core_test_support::apps_test_server::recorded_apps_tool_calls; use core_test_support::responses::ResponseMock; use core_test_support::responses::ResponsesRequest; use core_test_support::responses::ev_assistant_message; @@ -51,6 +52,7 @@ use core_test_support::skip_if_no_network; use core_test_support::skip_if_remote; use core_test_support::skip_if_target_windows; use core_test_support::stdio_server_bin; +use core_test_support::submit_thread_settings; use core_test_support::test_codex::TestCodex; use core_test_support::test_codex::local_selections; use core_test_support::test_codex::test_codex; @@ -496,6 +498,152 @@ async fn persisted_remote_plugin_command_attribution_flows_through_turn_context( Ok(()) } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn thread_disabled_plugins_filter_skills_and_tools_without_changing_shared_plugins() +-> Result<()> { + skip_if_no_network!(Ok(())); + let server = start_mock_server().await; + let apps = AppsTestServer::mount_with_connector_name(&server, "Google Calendar").await?; + let home = Arc::new(TempDir::new()?); + let skill_path = dunce::canonicalize(write_plugin_skill_plugin(home.as_ref()))?; + std::fs::write( + &skill_path, + "---\ndescription: inspect sample data\n---\nTHREAD_PLUGIN_SKILL_BODY\n", + )?; + write_plugin_mcp_plugin(home.as_ref(), &stdio_server_bin()?); + write_plugin_app_plugin_with_name(home.as_ref(), "sample_app"); + let mut builder = test_codex() + .with_home(home) + .with_extensions(skills_extensions()) + .with_auth(CodexAuth::create_dummy_chatgpt_auth_for_testing()) + .with_config(move |config| { + config.features.enable(Feature::Apps).unwrap(); + config.chatgpt_base_url = apps.chatgpt_base_url; + }); + let test = builder.build_with_remote_and_local_env(&server).await?; + let manager = test.thread_manager.plugins_manager(); + let plugins_input = test.config.plugins_config_input(); + let shared_plugins = manager.plugins_for_config(&plugins_input).await; + let tool_calls = [ + ( + "sample", + "echo", + Some(serde_json::json!({"message": "plugin check"})), + ), + (CODEX_APPS_MCP_SERVER_NAME, "calendar_list_events", None), + ]; + + for (phase, enabled, injection_count) in [(0, true, 1), (1, false, 1), (2, true, 2)] { + if !enabled { + submit_thread_settings( + &test.codex, + ThreadSettingsOverrides { + disabled_plugin_ids: Some(vec![SAMPLE_PLUGIN_CONFIG_NAME.to_string()]), + ..Default::default() + }, + ) + .await?; + // Saving pending settings must not change the admitted runtime. + for (server_name, tool, arguments) in &tool_calls { + test.codex + .call_mcp_tool(server_name, tool, arguments.clone(), /*meta*/ None) + .await?; + } + } + let app_call_id = format!("plugin-app-{phase}"); + let mcp_call_id = format!("plugin-mcp-{phase}"); + let mock = mount_sse_sequence( + &server, + vec![ + sse(vec![ + ev_response_created("search"), + ev_tool_search_call( + &app_call_id, + &serde_json::json!({"query": "create calendar event"}), + ), + ev_tool_search_call(&mcp_call_id, &serde_json::json!({"query": "echo"})), + ev_completed("search"), + ]), + sse(vec![ev_response_created("done"), ev_completed("done")]), + ], + ) + .await; + test.codex + .start_or_steer_turn( + TurnInputRequest::user_input(vec![ + UserInput::Skill { + name: "sample:sample-search".into(), + path: skill_path.clone(), + }, + UserInput::Mention { + name: "sample".into(), + path: format!("plugin://{SAMPLE_PLUGIN_CONFIG_NAME}"), + }, + ]) + .with_thread_settings(ThreadSettingsOverrides { + disabled_plugin_ids: enabled.then(Vec::new), + ..Default::default() + }), + ) + .await?; + wait_for_event(&test.codex, |event| { + matches!(event, EventMsg::TurnComplete(_)) + }) + .await; + let requests = mock.requests(); + // Previously injected instructions remain in history; disabled turns must add none. + for (role, marker) in [ + ("user", "THREAD_PLUGIN_SKILL_BODY"), + ("developer", "Skills from this plugin"), + ] { + assert_eq!( + requests[0] + .message_input_texts(role) + .iter() + .filter(|text| text.contains(marker)) + .count(), + injection_count + ); + } + for (call_id, namespace, tool) in [ + ( + &app_call_id, + SAMPLE_PLUGIN_APP_NAMESPACE, + SEARCH_CALENDAR_CREATE_TOOL, + ), + (&mcp_call_id, SAMPLE_PLUGIN_MCP_NAMESPACE, "echo"), + ] { + assert_eq!( + namespace_child_tool(&requests[1].tool_search_output(call_id), namespace, tool) + .is_some(), + enabled + ); + } + let calls_before = recorded_apps_tool_calls(&server).await.len(); + // Direct Apps RPC calls keep their existing behavior outside model tool filtering. + for (server_name, tool, arguments) in &tool_calls { + let result = test + .codex + .call_mcp_tool(server_name, tool, arguments.clone(), /*meta*/ None) + .await; + assert_eq!( + result.is_ok(), + enabled || *server_name == CODEX_APPS_MCP_SERVER_NAME, + "unexpected {server_name}/{tool} result: {result:?}" + ); + } + assert_eq!( + recorded_apps_tool_calls(&server).await.len() - calls_before, + 1 + ); + assert_eq!( + manager.plugins_for_config(&plugins_input).await, + shared_plugins + ); + } + Ok(()) +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn agent_plugin_skills_use_shared_catalog_and_direct_child_discovery() -> Result<()> { skip_if_no_network!(Ok(())); diff --git a/codex-rs/plugin/src/load_outcome.rs b/codex-rs/plugin/src/load_outcome.rs index eda5871346..2d08aa1bdc 100644 --- a/codex-rs/plugin/src/load_outcome.rs +++ b/codex-rs/plugin/src/load_outcome.rs @@ -95,7 +95,7 @@ pub fn prompt_safe_plugin_description(description: Option<&str>) -> Option { plugins: Vec>, @@ -120,6 +120,19 @@ impl PluginLoadOutcome { } } + /// Marks matching canonical plugin IDs inactive while retaining their loaded metadata. + pub fn without_plugins(mut self, disabled_plugin_ids: &[String]) -> Self { + if disabled_plugin_ids.is_empty() { + return self; + } + for plugin in &mut self.plugins { + if disabled_plugin_ids.contains(&plugin.config_name) { + plugin.enabled = false; + } + } + Self::from_plugins(self.plugins) + } + pub fn effective_plugin_skill_roots(&self) -> Vec { let mut skill_roots = Vec::new(); let mut seen_paths = HashSet::new();