diff --git a/codex-rs/app-server/tests/suite/v2/selected_capability_stack.rs b/codex-rs/app-server/tests/suite/v2/selected_capability_stack.rs index 6a70116aef..22153f1af7 100644 --- a/codex-rs/app-server/tests/suite/v2/selected_capability_stack.rs +++ b/codex-rs/app-server/tests/suite/v2/selected_capability_stack.rs @@ -28,6 +28,7 @@ use codex_exec_server::LOCAL_ENVIRONMENT_ID; use codex_protocol::config_types::CollaborationMode; use codex_protocol::config_types::ModeKind; use codex_protocol::config_types::Settings; +use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_path_uri::PathUri; use core_test_support::process::wait_for_pid_file; @@ -235,7 +236,9 @@ async fn selected_capability_stack_tracks_environment_availability_and_resume() for request in &requests[1..4] { assert_selected_skill_is_injected(request, /*expected_count*/ 1); assert_selected_plugin_tools(request); + assert_plugin_guidance_count(request, /*expected_count*/ 1); } + assert_plugin_guidance_count(&requests[4], /*expected_count*/ 1); assert_selected_skill_is_injected(&requests[5], /*expected_count*/ 2); assert_selected_plugin_tools(&requests[5]); let output = requests[2].function_call_output(MCP_CALL_ID); @@ -398,7 +401,9 @@ async fn selected_capabilities_become_available_between_samples_in_one_turn() -> assert_eq!(3, requests.len()); assert_selected_skill_catalog_available(&requests[1]); assert_selected_plugin_tools(&requests[1]); + assert_plugin_guidance_count(&requests[1], /*expected_count*/ 1); assert_selected_plugin_tools(&requests[2]); + assert_plugin_guidance_count(&requests[2], /*expected_count*/ 1); let output = requests[2].function_call_output(MCP_CALL_ID); let output = output["output"] .as_str() @@ -537,6 +542,7 @@ fn assert_selected_capabilities_absent(request: &ResponsesRequest) { .all(|text| !text.contains(SKILL_DESCRIPTION)) ); assert_selected_plugin_tools_absent(request); + assert_plugin_guidance_count(request, /*expected_count*/ 0); } fn assert_selected_plugin_tools_absent(request: &ResponsesRequest) { @@ -555,6 +561,17 @@ fn assert_selected_plugin_tools_absent(request: &ResponsesRequest) { ); } +fn assert_plugin_guidance_count(request: &ResponsesRequest, expected_count: usize) { + assert_eq!( + expected_count, + request + .message_input_texts("developer") + .into_iter() + .filter(|text| text.starts_with(PLUGINS_INSTRUCTIONS_OPEN_TAG)) + .count() + ); +} + fn assert_selected_skill_is_injected(request: &ResponsesRequest, expected_count: usize) { assert_selected_skill_catalog_available(request); diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index e29cb3875c..54fbb97f69 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -52,6 +52,7 @@ use codex_config::types::TuiNotificationSettings; use codex_config::types::TuiPetAnchor; use codex_config::types::UriBasedFileOpener; use codex_config::types::WindowsSandboxModeToml; +use codex_core_plugins::PluginLoadOutcome; use codex_core_plugins::PluginsConfigInput; use codex_exec_server::ExecutorFileSystem; use codex_exec_server::LOCAL_FS; @@ -1528,6 +1529,14 @@ impl Config { ) -> McpConfig { let plugins_input = self.plugins_config_input(); let loaded_plugins = plugins_manager.plugins_for_config(&plugins_input).await; + self.to_mcp_config_with_loaded_plugins(&loaded_plugins, additional_plugin_registrations) + } + + pub(crate) fn to_mcp_config_with_loaded_plugins( + &self, + loaded_plugins: &PluginLoadOutcome, + additional_plugin_registrations: impl IntoIterator, + ) -> McpConfig { let mut catalog = ResolvedMcpCatalog::builder(); for (plugin_order, plugin) in loaded_plugins .plugins() diff --git a/codex-rs/core/src/context/available_plugins_instructions.rs b/codex-rs/core/src/context/available_plugins_instructions.rs index 683d939aa0..a31d61d7fd 100644 --- a/codex-rs/core/src/context/available_plugins_instructions.rs +++ b/codex-rs/core/src/context/available_plugins_instructions.rs @@ -1,25 +1,10 @@ -use codex_plugin::PluginCapabilitySummary; use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_CLOSE_TAG; use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG; use super::ContextualUserFragment; #[derive(Debug, Clone, PartialEq)] -pub(crate) struct AvailablePluginsInstructions { - plugins: Vec, -} - -impl AvailablePluginsInstructions { - pub(crate) fn from_plugins(plugins: &[PluginCapabilitySummary]) -> Option { - if plugins.is_empty() { - return None; - } - - Some(Self { - plugins: plugins.to_vec(), - }) - } -} +pub(crate) struct AvailablePluginsInstructions; impl ContextualUserFragment for AvailablePluginsInstructions { fn role(&self) -> &'static str { diff --git a/codex-rs/core/src/context/world_state/mod.rs b/codex-rs/core/src/context/world_state/mod.rs index ea82ddd11f..5dc22030ba 100644 --- a/codex-rs/core/src/context/world_state/mod.rs +++ b/codex-rs/core/src/context/world_state/mod.rs @@ -1,5 +1,6 @@ mod agents_md; mod environment; +mod plugins_instructions; use crate::context::ContextualUserFragment; use codex_extension_api::PreviousWorldStateSection; @@ -17,6 +18,7 @@ use std::fmt; pub(crate) use agents_md::AgentsMdState; pub(crate) use environment::EnvironmentsState; +pub(crate) use plugins_instructions::PluginsInstructionsState; trait ErasedWorldStateSection: Send + Sync { fn snapshot(&self) -> Option; @@ -62,11 +64,11 @@ impl ErasedWorldStateSection for S { } fn has_retained_fragment_matcher(&self) -> bool { - false + S::has_retained_fragment_matcher() } - fn matches_retained_fragment(&self, _role: &str, _text: &str) -> bool { - false + fn matches_retained_fragment(&self, role: &str, text: &str) -> bool { + S::matches_retained_fragment(role, text) } fn render_diff( @@ -183,6 +185,16 @@ pub(crate) trait WorldStateSection: Send + Sync + 'static { false } + /// Whether retained history must still contain this section's rendered fragment. + fn has_retained_fragment_matcher() -> bool { + false + } + + /// Recognizes this section's rendered fragment in retained model history. + fn matches_retained_fragment(_role: &str, _text: &str) -> bool { + false + } + fn render_diff( &self, previous: PreviousSectionState<'_, Self::Snapshot>, diff --git a/codex-rs/core/src/context/world_state/plugins_instructions.rs b/codex-rs/core/src/context/world_state/plugins_instructions.rs new file mode 100644 index 0000000000..303307317d --- /dev/null +++ b/codex-rs/core/src/context/world_state/plugins_instructions.rs @@ -0,0 +1,55 @@ +use super::PreviousSectionState; +use super::WorldStateSection; +use crate::context::AvailablePluginsInstructions; +use crate::context::ContextualUserFragment; + +/// Whether generic plugin usage guidance should be visible to the model. +#[derive(Clone, Copy, Debug, Default)] +pub(crate) struct PluginsInstructionsState { + available: bool, +} + +impl PluginsInstructionsState { + pub(crate) fn new(available: bool) -> Self { + Self { available } + } +} + +impl WorldStateSection for PluginsInstructionsState { + const ID: &'static str = "plugins_instructions"; + type Snapshot = bool; + + fn snapshot(&self) -> Self::Snapshot { + self.available + } + + fn matches_legacy_fragment(role: &str, text: &str) -> bool { + role == "developer" && AvailablePluginsInstructions::matches_text(text) + } + + fn has_retained_fragment_matcher() -> bool { + true + } + + fn matches_retained_fragment(role: &str, text: &str) -> bool { + Self::matches_legacy_fragment(role, text) + } + + fn render_diff( + &self, + previous: PreviousSectionState<'_, Self::Snapshot>, + ) -> Option> { + if !self.available + || matches!(previous, PreviousSectionState::Known(previous) if *previous) + || matches!(previous, PreviousSectionState::Unknown) + { + return None; + } + + Some(Box::new(AvailablePluginsInstructions)) + } +} + +#[cfg(test)] +#[path = "plugins_instructions_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/context/world_state/plugins_instructions_tests.rs b/codex-rs/core/src/context/world_state/plugins_instructions_tests.rs new file mode 100644 index 0000000000..a688ff93ca --- /dev/null +++ b/codex-rs/core/src/context/world_state/plugins_instructions_tests.rs @@ -0,0 +1,72 @@ +use super::*; +use crate::context::ContextualUserFragment; +use codex_protocol::models::ResponseItem; +use pretty_assertions::assert_eq; + +fn render( + state: PluginsInstructionsState, + previous: PreviousSectionState<'_, bool>, +) -> Vec { + state + .render_diff(previous) + .into_iter() + .map(|fragment| fragment.render()) + .collect() +} + +#[test] +fn renders_only_when_plugins_become_available() { + let unavailable = PluginsInstructionsState::new(/*available*/ false); + let available = PluginsInstructionsState::new(/*available*/ true); + let false_snapshot = false; + let true_snapshot = true; + + assert_eq!( + render(unavailable, PreviousSectionState::Absent), + Vec::::new() + ); + assert_eq!(render(available, PreviousSectionState::Absent).len(), 1); + assert_eq!( + render(available, PreviousSectionState::Known(&false_snapshot)).len(), + 1 + ); + assert_eq!( + render(available, PreviousSectionState::Known(&true_snapshot)), + Vec::::new() + ); + assert_eq!( + render(unavailable, PreviousSectionState::Known(&true_snapshot)), + Vec::::new() + ); +} + +#[test] +fn legacy_guidance_is_not_injected_again() { + let mut world_state = super::super::WorldState::default(); + world_state.add_section(PluginsInstructionsState::new(/*available*/ true)); + let legacy: ResponseItem = ContextualUserFragment::into(AvailablePluginsInstructions); + + assert!( + world_state + .render_history_diff(/*previous*/ None, &[legacy]) + .is_empty() + ); +} + +#[test] +fn persisted_guidance_is_restored_only_when_missing_from_history() { + let mut world_state = super::super::WorldState::default(); + world_state.add_section(PluginsInstructionsState::new(/*available*/ true)); + let snapshot = world_state.snapshot(); + let retained: ResponseItem = ContextualUserFragment::into(AvailablePluginsInstructions); + + assert_eq!( + world_state.render_history_diff(Some(&snapshot), &[]).len(), + 1 + ); + assert!( + world_state + .render_history_diff(Some(&snapshot), &[retained]) + .is_empty() + ); +} diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index cbe0209e21..df5abd4708 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -22,6 +22,7 @@ use codex_protocol::openai_models::InputModality; use codex_protocol::openai_models::default_input_modalities; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::InterAgentCommunication; +use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::SandboxPolicy; use codex_protocol::protocol::TurnContextItem; use codex_utils_absolute_path::AbsolutePathBuf; @@ -1021,6 +1022,9 @@ fn drop_last_n_user_turns_trims_context_updates_above_rolled_back_turn() { user_input_text_msg("turn 1 user"), assistant_msg("turn 1 assistant"), developer_msg("Generated images are saved to /tmp as /tmp/image-1.png by default."), + developer_msg(&format!( + "{PLUGINS_INSTRUCTIONS_OPEN_TAG}\nROLLED_BACK_PLUGIN_INSTRUCTIONS" + )), developer_msg("ROLLED_BACK_DEV_INSTRUCTIONS"), developer_msg("ROLLED_BACK_MULTI_AGENT_MODE"), user_input_text_msg( diff --git a/codex-rs/core/src/event_mapping.rs b/codex-rs/core/src/event_mapping.rs index 392203c079..5e24b78d05 100644 --- a/codex-rs/core/src/event_mapping.rs +++ b/codex-rs/core/src/event_mapping.rs @@ -18,6 +18,7 @@ use codex_protocol::protocol::COLLABORATION_MODE_OPEN_TAG; use codex_protocol::protocol::CONTEXT_WINDOW_GUIDANCE_OPEN_TAG; use codex_protocol::protocol::CONTEXT_WINDOW_OPEN_TAG; use codex_protocol::protocol::MULTI_AGENT_MODE_OPEN_TAG; +use codex_protocol::protocol::PLUGINS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::protocol::REALTIME_CONVERSATION_OPEN_TAG; use codex_protocol::protocol::SKILLS_INSTRUCTIONS_OPEN_TAG; use codex_protocol::user_input::UserInput; @@ -33,6 +34,7 @@ const CONTEXTUAL_DEVELOPER_PREFIXES: &[&str] = &[ "", COLLABORATION_MODE_OPEN_TAG, MULTI_AGENT_MODE_OPEN_TAG, + PLUGINS_INSTRUCTIONS_OPEN_TAG, REALTIME_CONVERSATION_OPEN_TAG, SKILLS_INSTRUCTIONS_OPEN_TAG, "", diff --git a/codex-rs/core/src/mcp.rs b/codex-rs/core/src/mcp.rs index 45ca0d563d..835c47ff51 100644 --- a/codex-rs/core/src/mcp.rs +++ b/codex-rs/core/src/mcp.rs @@ -25,6 +25,12 @@ use codex_plugin::AppConnectorId; const LEGACY_CODEX_APPS_REGISTRATION_ID: &str = "legacy_codex_apps"; +/// MCP configuration and capability availability derived from the same inputs. +pub(crate) struct McpRuntimeProjection { + pub(crate) config: McpConfig, + pub(crate) plugins_available: bool, +} + enum OrderedMcpOverlay { Set { contributor_id: &'static str, @@ -75,6 +81,7 @@ impl McpManager { pub async fn runtime_config(&self, config: &Config) -> McpConfig { self.runtime_config_with_context(McpServerContributionContext::global(config)) .await + .config } pub(crate) async fn runtime_config_for_step( @@ -83,7 +90,7 @@ impl McpManager { thread_init: &ExtensionDataInit, thread_store: &ExtensionData, available_environment_ids: &[String], - ) -> McpConfig { + ) -> McpRuntimeProjection { self.runtime_config_with_context(McpServerContributionContext::for_step( config, thread_init, @@ -96,8 +103,9 @@ impl McpManager { async fn runtime_config_with_context( &self, context: McpServerContributionContext<'_, Config>, - ) -> McpConfig { + ) -> McpRuntimeProjection { let config = context.config(); + let mut selected_plugin_available = false; let mut selected_plugin_connector_sources = Vec::new(); let mut selected_plugin_registrations = Vec::new(); let mut overlays = Vec::new(); @@ -129,17 +137,22 @@ impl McpManager { *config, ), ), - McpServerContribution::SelectedPluginConnectors { + McpServerContribution::SelectedPluginPackage { plugin_id, plugin_display_name, connector_ids, - } => selected_plugin_connector_sources.push( - PluginConnectorSource::from_connector_ids( - plugin_id, - plugin_display_name, - connector_ids.into_iter().map(AppConnectorId), - ), - ), + } => { + selected_plugin_available = true; + if !connector_ids.is_empty() { + selected_plugin_connector_sources.push( + PluginConnectorSource::from_connector_ids( + plugin_id, + plugin_display_name, + connector_ids.into_iter().map(AppConnectorId), + ), + ); + } + } McpServerContribution::Remove { name } => { overlays.push(OrderedMcpOverlay::Remove { contributor_id: contributor.id(), @@ -152,12 +165,14 @@ impl McpManager { } } - let mut mcp_config = config - .to_mcp_config_with_plugin_registrations( - self.plugins_manager.as_ref(), - selected_plugin_registrations, - ) + let loaded_plugins = self + .plugins_manager + .plugins_for_config(&config.plugins_config_input()) .await; + let plugins_available = + selected_plugin_available || !loaded_plugins.capability_summaries().is_empty(); + let mut mcp_config = config + .to_mcp_config_with_loaded_plugins(&loaded_plugins, selected_plugin_registrations); let mut catalog = mcp_config.mcp_server_catalog.to_builder(); if mcp_config.apps_enabled { catalog.register(McpServerRegistration::from_compatibility( @@ -211,7 +226,10 @@ impl McpManager { .merged_with(&ConnectorSnapshot::from_plugin_sources( selected_plugin_connector_sources, )); - mcp_config + McpRuntimeProjection { + config: mcp_config, + plugins_available, + } } /// Returns config- and plugin-backed servers without runtime contributions. diff --git a/codex-rs/core/src/plugins/render.rs b/codex-rs/core/src/plugins/render.rs index fc197dbbea..4e7575bd62 100644 --- a/codex-rs/core/src/plugins/render.rs +++ b/codex-rs/core/src/plugins/render.rs @@ -6,7 +6,7 @@ use crate::plugins::PluginCapabilitySummary; #[cfg(test)] pub(crate) fn render_plugins_section(plugins: &[PluginCapabilitySummary]) -> Option { - AvailablePluginsInstructions::from_plugins(plugins).map(|instructions| instructions.render()) + (!plugins.is_empty()).then(|| AvailablePluginsInstructions.render()) } pub(crate) fn render_explicit_plugin_instructions( diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 25d95e30ea..4cfb0a36de 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -1,4 +1,5 @@ use super::*; +use crate::mcp::McpRuntimeProjection; use codex_exec_server::ResolvedSelectedCapabilityRoot; use codex_mcp::ElicitationReviewRequest; use codex_mcp::ElicitationReviewer; @@ -92,6 +93,7 @@ impl Session { &available_environment_ids, ) .await + .config } pub(crate) async fn runtime_mcp_servers( @@ -123,7 +125,7 @@ impl Session { if current.available_environment_ids() == available_environment_ids { return current; } - let mcp_config = self + let mcp_projection = self .services .mcp_manager .runtime_config_for_step( @@ -133,6 +135,7 @@ impl Session { &available_environment_ids, ) .await; + let mcp_config = &mcp_projection.config; let changed_environment_is_used_by_mcp = mcp_config .mcp_server_catalog .configured_servers() @@ -156,6 +159,7 @@ impl Session { // replacing the live manager and restarting its processes. let runtime = Arc::new(McpRuntimeSnapshot::new( Arc::new(current.config().clone()), + mcp_projection.plugins_available, current.manager_arc(), current.runtime_context().clone(), available_environment_ids, @@ -165,7 +169,7 @@ impl Session { } self.refresh_mcp_servers_inner( turn_context, - mcp_config, + mcp_projection, environments, &available_environment_ids, Some(self.mcp_elicitation_reviewer()), @@ -309,12 +313,16 @@ impl Session { async fn refresh_mcp_servers_inner( &self, turn_context: &TurnContext, - mcp_config: McpConfig, + mcp_projection: McpRuntimeProjection, environments: &TurnEnvironmentSnapshot, available_environment_ids: &[String], elicitation_reviewer: Option, ) -> Arc { let auth = self.services.auth_manager.auth().await; + let McpRuntimeProjection { + config: mcp_config, + plugins_available, + } = mcp_projection; let mcp_config = Arc::new(mcp_config); let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(&mcp_config); let mcp_servers = effective_mcp_servers(&mcp_config, auth.as_ref()); @@ -376,6 +384,7 @@ impl Session { .set_elicitations_auto_deny(current_runtime.manager().elicitations_auto_deny()); self.services.publish_mcp_runtime( mcp_config, + plugins_available, mcp_runtime_context, available_environment_ids.to_vec(), refreshed_manager, @@ -448,7 +457,7 @@ impl Session { .latest_mcp_runtime() .available_environment_ids() .to_vec(); - let mut mcp_config = self + let mut mcp_projection = self .services .mcp_manager .runtime_config_for_step( @@ -458,12 +467,13 @@ impl Session { &available_environment_ids, ) .await; - mcp_config.mcp_server_catalog = mcp_config + mcp_projection.config.mcp_server_catalog = mcp_projection + .config .mcp_server_catalog .with_materialized_servers(mcp_servers); self.refresh_mcp_servers_inner( turn_context, - mcp_config, + mcp_projection, &turn_context.environments, &available_environment_ids, elicitation_reviewer, @@ -515,7 +525,7 @@ impl Session { .latest_mcp_runtime() .available_environment_ids() .to_vec(); - let mcp_config = self + let mcp_projection = self .services .mcp_manager .runtime_config_for_step( @@ -527,7 +537,7 @@ impl Session { .await; self.refresh_mcp_servers_inner( turn_context, - mcp_config, + mcp_projection, &turn_context.environments, &available_environment_ids, elicitation_reviewer, diff --git a/codex-rs/core/src/session/mcp_runtime.rs b/codex-rs/core/src/session/mcp_runtime.rs index 34cf07cc35..5e7aeffcee 100644 --- a/codex-rs/core/src/session/mcp_runtime.rs +++ b/codex-rs/core/src/session/mcp_runtime.rs @@ -5,9 +5,10 @@ use codex_mcp::McpConfig; use codex_mcp::McpConnectionManager; use codex_mcp::McpRuntimeContext; -/// MCP config, exact environment bindings, and manager used by one model request. +/// MCP config, plugin availability, exact environment bindings, and manager for one request. pub struct McpRuntimeSnapshot { config: Arc, + plugins_available: bool, manager: Arc, runtime_context: McpRuntimeContext, available_environment_ids: Vec, @@ -16,12 +17,14 @@ pub struct McpRuntimeSnapshot { impl McpRuntimeSnapshot { pub(crate) fn new( config: Arc, + plugins_available: bool, manager: Arc, runtime_context: McpRuntimeContext, available_environment_ids: Vec, ) -> Self { Self { config, + plugins_available, manager, runtime_context, available_environment_ids, @@ -32,6 +35,10 @@ impl McpRuntimeSnapshot { self.config.as_ref() } + pub(crate) fn plugins_available(&self) -> bool { + self.plugins_available + } + pub fn manager(&self) -> &McpConnectionManager { self.manager.as_ref() } @@ -86,6 +93,7 @@ impl McpRuntimeSnapshot { ); Arc::new(Self::new( Arc::new(mcp_config), + /*plugins_available*/ false, Arc::new(manager), runtime_context, Vec::new(), diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 978768ba00..560dcf5985 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -24,7 +24,6 @@ use crate::config::resolve_tool_suggest_config_from_layer_stack; use crate::connectors; use crate::context::ApprovedCommandPrefixSaved; use crate::context::AppsInstructions; -use crate::context::AvailablePluginsInstructions; use crate::context::AvailableSkillsInstructions; use crate::context::CollaborationModeInstructions; use crate::context::ContextualUserFragment; @@ -3306,11 +3305,6 @@ impl Session { { contextual_user_sections.push(recommended_plugins.render()); } - if let Some(plugin_instructions) = - AvailablePluginsInstructions::from_plugins(loaded_plugins.capability_summaries()) - { - developer_sections.push(plugin_instructions.render()); - } let context_contributors = self.services.extensions.context_contributors().to_vec(); for contributor in &context_contributors { for fragment in contributor diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 5e7046b148..48a9ee447e 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -679,7 +679,7 @@ impl Session { let mcp_runtime_context_for_auth = mcp_runtime_context.clone(); let auth_and_mcp_fut = async move { let auth = auth_manager_clone.auth().await; - let mcp_config = mcp_manager_for_mcp + let mcp_projection = mcp_manager_for_mcp .runtime_config_for_step( &config_for_mcp, mcp_thread_init_for_startup, @@ -687,8 +687,9 @@ impl Session { /*available_environment_ids*/ &[], ) .await; - let mcp_servers = codex_mcp::effective_mcp_servers(&mcp_config, auth.as_ref()); - let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(&mcp_config); + let mcp_config = &mcp_projection.config; + let mcp_servers = codex_mcp::effective_mcp_servers(mcp_config, auth.as_ref()); + let tool_plugin_provenance = codex_mcp::tool_plugin_provenance(mcp_config); let auth_statuses = compute_auth_statuses( mcp_servers.iter(), config_for_mcp.mcp_oauth_credentials_store_mode, @@ -699,7 +700,7 @@ impl Session { .await; ( auth, - mcp_config, + mcp_projection, mcp_servers, auth_statuses, tool_plugin_provenance, @@ -714,7 +715,7 @@ impl Session { let ( thread_persistence_result, state_db_ctx, - (auth, mcp_config, mcp_servers, auth_statuses, tool_plugin_provenance), + (auth, mcp_projection, mcp_servers, auth_statuses, tool_plugin_provenance), ) = tokio::join!(thread_persistence_fut, state_db_fut, auth_and_mcp_fut); let mut live_thread_init = @@ -1208,7 +1209,10 @@ impl Session { sess.services.mcp_manager.codex_apps_tools_cache(), codex_apps_tools_cache_key(auth), config.prefix_mcp_tool_names(), - mcp_config.client_elicitation_capability.clone(), + mcp_projection + .config + .client_elicitation_capability + .clone(), sess.services .supports_openai_form_elicitation .load(std::sync::atomic::Ordering::Relaxed), @@ -1224,7 +1228,8 @@ impl Session { .await; sess.services .install_mcp_connection_manager( - Arc::new(mcp_config), + Arc::new(mcp_projection.config), + mcp_projection.plugins_available, mcp_runtime_context, /*available_environment_ids*/ Vec::new(), mcp_connection_manager, diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ece9d446b5..9ae29a0917 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -7729,6 +7729,91 @@ async fn refresh_mcp_servers_keeps_the_previous_runtime_alive() { ); } +#[tokio::test] +async fn plugin_availability_change_reuses_the_mcp_manager() { + struct ReadyPluginContributor; + + impl codex_extension_api::McpServerContributor for ReadyPluginContributor { + fn id(&self) -> &'static str { + "ready_plugin_test" + } + + fn contribute<'a>( + &'a self, + context: codex_extension_api::McpServerContributionContext<'a, Config>, + ) -> codex_extension_api::ExtensionFuture<'a, Vec> + { + Box::pin(async move { + let available = context + .available_environment_ids() + .is_some_and(|ids| ids.iter().any(|id| id == "executor")); + available + .then( + || codex_extension_api::McpServerContribution::SelectedPluginPackage { + plugin_id: "skill-only".to_string(), + plugin_display_name: "Skill Only".to_string(), + connector_ids: Vec::new(), + }, + ) + .into_iter() + .collect() + }) + } + } + + let (mut session, turn_context) = make_session_and_context().await; + let mut registry = codex_extension_api::ExtensionRegistryBuilder::new(); + registry.mcp_server_contributor(Arc::new(ReadyPluginContributor)); + let registry = Arc::new(registry.build()); + session.services.extensions = Arc::clone(®istry); + session.services.mcp_manager = Arc::new(McpManager::new_with_extensions( + Arc::clone(&session.services.plugins_manager), + registry, + )); + let session = Arc::new(session); + session + .refresh_mcp_servers_now( + &turn_context, + &turn_context.config, + /*elicitation_reviewer*/ None, + ) + .await; + let old_runtime = session.services.latest_mcp_runtime(); + let old_manager = old_runtime.manager_arc(); + + let selected_root = codex_protocol::capabilities::SelectedCapabilityRoot { + id: "skill-only".to_string(), + location: codex_protocol::capabilities::CapabilityRootLocation::Environment { + environment_id: "executor".to_string(), + path: PathUri::from_host_native_path(turn_context.config.cwd.as_path()) + .expect("selected capability root URI"), + }, + }; + let environment = Arc::clone( + &turn_context + .environments + .primary() + .expect("ready test environment") + .environment, + ); + let captured_environments = HashMap::from([("executor".to_string(), Some(environment))]); + let resolved_roots = session + .services + .turn_environments + .environment_manager() + .resolve_selected_capability_roots(&[selected_root], &captured_environments) + .await; + + let new_runtime = session + .mcp_runtime_for_step(&turn_context, &turn_context.environments, &resolved_roots) + .await; + + assert!(!old_runtime.plugins_available()); + assert!(new_runtime.plugins_available()); + assert!(!Arc::ptr_eq(&old_runtime, &new_runtime)); + assert!(Arc::ptr_eq(&old_manager, &new_runtime.manager_arc())); +} + #[tokio::test] async fn built_tools_uses_the_step_mcp_runtime() -> anyhow::Result<()> { let (session, turn_context) = make_session_and_context().await; diff --git a/codex-rs/core/src/session/world_state.rs b/codex-rs/core/src/session/world_state.rs index 10d6a5d0d1..a78116d457 100644 --- a/codex-rs/core/src/session/world_state.rs +++ b/codex-rs/core/src/session/world_state.rs @@ -2,6 +2,7 @@ use super::session::Session; use super::step_context::StepContext; use crate::context::world_state::AgentsMdState; use crate::context::world_state::EnvironmentsState; +use crate::context::world_state::PluginsInstructionsState; use crate::context::world_state::WorldState; use codex_extension_api::WorldStateContributionInput; @@ -35,6 +36,9 @@ impl Session { .with_subagents(environment_subagents), ); } + world_state.add_section(PluginsInstructionsState::new( + step_context.mcp.plugins_available(), + )); let environments = step_context.environments.to_selections(); let ready_selected_capability_roots = step_context .selected_capability_roots diff --git a/codex-rs/core/src/state/service.rs b/codex-rs/core/src/state/service.rs index a611b94887..c4b67a629b 100644 --- a/codex-rs/core/src/state/service.rs +++ b/codex-rs/core/src/state/service.rs @@ -107,18 +107,25 @@ impl SessionServices { pub(crate) async fn install_mcp_connection_manager( &self, config: Arc, + plugins_available: bool, runtime_context: McpRuntimeContext, available_environment_ids: Vec, manager: McpConnectionManager, ) -> Result<()> { - let runtime = - self.publish_mcp_runtime(config, runtime_context, available_environment_ids, manager); + let runtime = self.publish_mcp_runtime( + config, + plugins_available, + runtime_context, + available_environment_ids, + manager, + ); runtime.manager().validate_required_servers().await } pub(crate) fn publish_mcp_runtime( &self, config: Arc, + plugins_available: bool, runtime_context: McpRuntimeContext, available_environment_ids: Vec, manager: McpConnectionManager, @@ -129,6 +136,7 @@ impl SessionServices { self.mcp_connection_manager.store(Arc::clone(&manager)); let runtime = Arc::new(McpRuntimeSnapshot::new( config, + plugins_available, manager, runtime_context, available_environment_ids, diff --git a/codex-rs/core/src/thread_manager_tests.rs b/codex-rs/core/src/thread_manager_tests.rs index 4f0da0c9c7..3c1d0146e7 100644 --- a/codex-rs/core/src/thread_manager_tests.rs +++ b/codex-rs/core/src/thread_manager_tests.rs @@ -699,11 +699,11 @@ async fn start_thread_seeds_extension_data_for_mcp_and_lifecycle_contributors() .collect::>() }; assert_eq!( - selected_servers(&first_resolved), + selected_servers(&first_resolved.config), std::collections::BTreeMap::from([("selected-a".to_string(), "env-a".to_string())]) ); assert_eq!( - selected_servers(&second_resolved), + selected_servers(&second_resolved.config), std::collections::BTreeMap::from([("selected-b".to_string(), "env-b".to_string())]) ); } diff --git a/codex-rs/ext/extension-api/src/contributors/mcp.rs b/codex-rs/ext/extension-api/src/contributors/mcp.rs index 499657f556..3fe648a1e9 100644 --- a/codex-rs/ext/extension-api/src/contributors/mcp.rs +++ b/codex-rs/ext/extension-api/src/contributors/mcp.rs @@ -93,8 +93,8 @@ pub enum McpServerContribution { selection_order: usize, config: Box, }, - /// Adds connector IDs declared by a plugin selected for this thread. - SelectedPluginConnectors { + /// Records a plugin selected for this thread and any connector IDs it declares. + SelectedPluginPackage { plugin_id: String, plugin_display_name: String, connector_ids: Vec, diff --git a/codex-rs/ext/mcp/src/executor_plugin.rs b/codex-rs/ext/mcp/src/executor_plugin.rs index 32637b4f47..98cfcd3615 100644 --- a/codex-rs/ext/mcp/src/executor_plugin.rs +++ b/codex-rs/ext/mcp/src/executor_plugin.rs @@ -186,13 +186,12 @@ impl McpServerContributor for SelectedExecutorPluginMcpContributor { config: Box::new(config), } })); - if !plugin.connector_ids.is_empty() { - contributions.push(McpServerContribution::SelectedPluginConnectors { - plugin_id: plugin.plugin_id, - plugin_display_name: plugin.plugin_display_name, - connector_ids: plugin.connector_ids, - }); - } + // Keep the package visible even when it contributes only skills. + contributions.push(McpServerContribution::SelectedPluginPackage { + plugin_id: plugin.plugin_id, + plugin_display_name: plugin.plugin_display_name, + connector_ids: plugin.connector_ids, + }); } contributions diff --git a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs index 65a4248a32..da5c1fb6ad 100644 --- a/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs +++ b/codex-rs/ext/mcp/tests/executor_plugin_mcp.rs @@ -25,6 +25,13 @@ struct ContributionSummary { enabled: bool, } +#[derive(Debug, PartialEq, Eq)] +struct PackageSummary { + plugin_id: String, + plugin_display_name: String, + connector_ids: Vec, +} + #[tokio::test] async fn selected_plugin_servers_use_managed_requirements_for_the_selected_root_id() -> TestResult { let codex_home = tempfile::tempdir()?; @@ -92,10 +99,87 @@ command = "expected-command" Ok(()) } +#[tokio::test] +async fn selected_plugin_package_is_contributed_without_servers_or_connectors() -> TestResult { + let codex_home = tempfile::tempdir()?; + let plugin_root = tempfile::tempdir()?; + std::fs::create_dir_all(plugin_root.path().join(".codex-plugin"))?; + std::fs::create_dir_all(plugin_root.path().join("skills/deploy"))?; + std::fs::write( + plugin_root.path().join(".codex-plugin/plugin.json"), + r#"{"name":"skill-only","interface":{"displayName":"Skill Only"}}"#, + )?; + std::fs::write( + plugin_root.path().join("skills/deploy/SKILL.md"), + "---\nname: deploy\ndescription: Deploy the project.\n---\n", + )?; + let config = ConfigBuilder::default() + .codex_home(codex_home.path().to_path_buf()) + .fallback_cwd(Some(codex_home.path().to_path_buf())) + .build() + .await?; + + let contributions = raw_selected_plugin_contributions(&config, plugin_root.path()).await?; + let package = contributions.into_iter().find_map(|contribution| { + let McpServerContribution::SelectedPluginPackage { + plugin_id, + plugin_display_name, + connector_ids, + } = contribution + else { + return None; + }; + Some(PackageSummary { + plugin_id, + plugin_display_name, + connector_ids, + }) + }); + + assert_eq!( + package, + Some(PackageSummary { + plugin_id: "selected-root".to_string(), + plugin_display_name: "Skill Only".to_string(), + connector_ids: Vec::new(), + }) + ); + Ok(()) +} + async fn selected_plugin_contributions( config: &Config, plugin_root: &std::path::Path, ) -> Result, Box> { + Ok(raw_selected_plugin_contributions(config, plugin_root) + .await? + .into_iter() + .filter_map(|contribution| match contribution { + McpServerContribution::SelectedPlugin { + name, + plugin_id, + plugin_display_name, + selection_order, + config, + } => Some(ContributionSummary { + name, + plugin_id, + plugin_display_name, + selection_order, + enabled: config.enabled, + }), + McpServerContribution::SelectedPluginPackage { .. } => None, + McpServerContribution::Set { .. } | McpServerContribution::Remove { .. } => { + panic!("expected selected plugin contribution") + } + }) + .collect()) +} + +async fn raw_selected_plugin_contributions( + config: &Config, + plugin_root: &std::path::Path, +) -> Result, Box> { let mut builder = ExtensionRegistryBuilder::new(); codex_mcp_extension::install_executor_plugins( &mut builder, @@ -120,27 +204,5 @@ async fn selected_plugin_contributions( &thread_store, &available_environment_ids, )) - .await - .into_iter() - .map(|contribution| match contribution { - McpServerContribution::SelectedPlugin { - name, - plugin_id, - plugin_display_name, - selection_order, - config, - } => ContributionSummary { - name, - plugin_id, - plugin_display_name, - selection_order, - enabled: config.enabled, - }, - McpServerContribution::Set { .. } - | McpServerContribution::SelectedPluginConnectors { .. } - | McpServerContribution::Remove { .. } => { - panic!("expected selected plugin contribution") - } - }) - .collect()) + .await) }