diff --git a/codex-rs/app-server/src/mcp_refresh.rs b/codex-rs/app-server/src/mcp_refresh.rs index cdd4670092..68197e49d8 100644 --- a/codex-rs/app-server/src/mcp_refresh.rs +++ b/codex-rs/app-server/src/mcp_refresh.rs @@ -2,8 +2,6 @@ use crate::config_manager::ConfigManager; use codex_core::CodexThread; use codex_core::ThreadManager; use codex_core::config::Config; -use codex_protocol::protocol::McpServerRefreshConfig; -use codex_protocol::protocol::Op; use std::io; use std::sync::Arc; @@ -20,18 +18,11 @@ pub(crate) async fn reload_mcp_config( .get_thread(thread_id) .await .map_err(|err| io::Error::other(format!("failed to load thread {thread_id}: {err}")))?; - let config = build_refresh_config(thread.as_ref(), config_manager).await?; - refreshes.push((thread_id, thread, config)); + let config = load_refresh_config(thread.as_ref(), config_manager).await?; + refreshes.push((thread, config)); } - for (thread_id, thread, config) in refreshes { - thread - .submit(Op::ReloadMcpConfig { config }) - .await - .map_err(|err| { - io::Error::other(format!( - "failed to queue MCP config reload for thread {thread_id}: {err}" - )) - })?; + for (thread, config) in refreshes { + thread.refresh_mcp_config(config).await; } Ok(()) } @@ -46,22 +37,6 @@ async fn load_refresh_config( .await } -async fn build_refresh_config( - thread: &CodexThread, - config_manager: &ConfigManager, -) -> io::Result { - let config = load_refresh_config(thread, config_manager).await?; - Ok(McpServerRefreshConfig { - mcp_servers: serde_json::to_value(config.mcp_servers.get()).map_err(io::Error::other)?, - mcp_oauth_credentials_store_mode: serde_json::to_value( - config.mcp_oauth_credentials_store_mode, - ) - .map_err(io::Error::other)?, - auth_keyring_backend_kind: serde_json::to_value(config.auth_keyring_backend_kind()) - .map_err(io::Error::other)?, - }) -} - #[cfg(test)] mod tests { use super::*; @@ -153,12 +128,13 @@ mod tests { let thread = good_thread.expect("good test thread should exist"); let original_model = thread.config().await.model.clone(); - let refresh_config = build_refresh_config(thread.as_ref(), &config_manager).await?; - let keyring_backend_kind = serde_json::from_value::( - refresh_config.auth_keyring_backend_kind, - )?; + let refresh_config = load_refresh_config(thread.as_ref(), &config_manager).await?; + thread.refresh_mcp_config(refresh_config).await; - assert_eq!(keyring_backend_kind, AuthKeyringBackendKind::Secrets); + assert_eq!( + thread.config().await.auth_keyring_backend_kind(), + AuthKeyringBackendKind::Secrets + ); assert_eq!(thread.config().await.model, original_model); Ok(()) } @@ -194,9 +170,8 @@ enabled = false "#, )?; - let refresh_config = build_refresh_config(thread.as_ref(), &config_manager).await?; - let mut actual = - serde_json::from_value::>(refresh_config.mcp_servers)?; + let refresh_config = load_refresh_config(thread.as_ref(), &config_manager).await?; + let mut actual = refresh_config.mcp_servers.get().clone(); actual.remove(codex_mcp::CODEX_APPS_MCP_SERVER_NAME); let expected = serde_json::from_value::>(json!({ "global": { @@ -214,7 +189,7 @@ enabled = false } #[tokio::test] - async fn strict_refresh_does_not_mutate_thread_config_out_of_band() -> anyhow::Result<()> { + async fn strict_refresh_installs_refreshed_thread_mcp_config() -> anyhow::Result<()> { let (temp_dir, thread_manager, config_manager, _loader) = refresh_test_state().await?; let mut good_thread = None; for thread_id in thread_manager.list_thread_ids().await { @@ -238,9 +213,13 @@ enabled = false reload_mcp_config(&thread_manager, &config_manager).await?; - assert_eq!( - thread.config().await.mcp_servers.get(), - &HashMap::::new() + assert!( + thread + .config() + .await + .mcp_servers + .get() + .contains_key("refreshed") ); Ok(()) } diff --git a/codex-rs/app-server/src/request_processors/account_processor.rs b/codex-rs/app-server/src/request_processors/account_processor.rs index 94ffa0a5d9..ec9a0c72bb 100644 --- a/codex-rs/app-server/src/request_processors/account_processor.rs +++ b/codex-rs/app-server/src/request_processors/account_processor.rs @@ -225,6 +225,10 @@ impl AccountRequestProcessor { .await { Ok(config) => { + Self::spawn_effective_plugins_changed_task( + Arc::clone(thread_manager), + config_manager.clone(), + ); let refresh_thread_manager = Arc::clone(thread_manager); let refresh_config_manager = config_manager.clone(); thread_manager diff --git a/codex-rs/core/src/codex_thread.rs b/codex-rs/core/src/codex_thread.rs index 12c76d181b..cfd5517f9b 100644 --- a/codex-rs/core/src/codex_thread.rs +++ b/codex-rs/core/src/codex_thread.rs @@ -627,6 +627,11 @@ impl CodexThread { self.session.refresh_runtime_config(next_config).await; } + /// Refresh MCP configuration and managed requirements without reloading unrelated settings. + pub async fn refresh_mcp_config(&self, next_config: crate::config::Config) { + self.session.refresh_mcp_config(next_config).await; + } + pub async fn environment_selections(&self) -> Vec { self.session.thread_environment_selections().await } diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index a8a654ee78..d7ac662013 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -785,10 +785,6 @@ pub(super) async fn submission_loop( refresh_mcp_servers(&sess); false } - Op::ReloadMcpConfig { config } => { - sess.refresh_mcp_config(config).await; - false - } Op::ReloadUserConfig => { reload_user_config(&sess).await; false diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index 2b6d8f04f8..48ec8a9020 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -194,9 +194,7 @@ use crate::thread_rollout_truncation::initial_history_has_prior_user_turns; use codex_config::CONFIG_TOML_FILE; use codex_config::ConfigLayerSource; use codex_config::ConfigLayerStackOrdering; -use codex_config::types::AuthKeyringBackendKind; use codex_config::types::McpServerConfig; -use codex_config::types::OAuthCredentialsStoreMode; use codex_model_provider_info::ModelProviderInfo; use codex_protocol::error::CodexErr; use codex_protocol::error::CodexErrorDetails; @@ -365,7 +363,6 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_protocol::protocol::ExecApprovalRequestEvent; use codex_protocol::protocol::InitialHistory; -use codex_protocol::protocol::McpServerRefreshConfig; use codex_protocol::protocol::ModelRerouteEvent; use codex_protocol::protocol::ModelRerouteReason; use codex_protocol::protocol::ModelVerification; @@ -1663,47 +1660,17 @@ impl Session { } } - pub(crate) async fn refresh_mcp_config(&self, next_config: McpServerRefreshConfig) { - let McpServerRefreshConfig { - mcp_servers, - mcp_oauth_credentials_store_mode, - auth_keyring_backend_kind, - } = next_config; - let mcp_servers = - match serde_json::from_value::>(mcp_servers) { - Ok(servers) => servers, - Err(err) => { - warn!("failed to parse MCP server refresh config: {err}"); - return; - } - }; - let store_mode = match serde_json::from_value::( - mcp_oauth_credentials_store_mode, - ) { - Ok(mode) => mode, - Err(err) => { - warn!("failed to parse MCP OAuth refresh config: {err}"); - return; - } - }; - let keyring_backend_kind = - match serde_json::from_value::(auth_keyring_backend_kind) { - Ok(kind) => kind, - Err(err) => { - warn!("failed to parse MCP auth keyring backend refresh config: {err}"); - return; - } - }; + pub(crate) async fn refresh_mcp_config(&self, next_config: Config) { let mut state = self.state.lock().await; let mut config = (*state.session_configuration.original_config_do_not_use).clone(); - if let Err(err) = config.mcp_servers.set(mcp_servers) { - warn!("failed to apply MCP server refresh config: {err}"); - return; - } - config.mcp_oauth_credentials_store_mode = store_mode; + config.config_layer_stack = next_config + .config_layer_stack + .with_user_layer_from(&config.config_layer_stack); + config.mcp_servers = next_config.mcp_servers; + config.mcp_oauth_credentials_store_mode = next_config.mcp_oauth_credentials_store_mode; if let Err(err) = config.features.set_enabled( Feature::SecretAuthStorage, - matches!(keyring_backend_kind, AuthKeyringBackendKind::Secrets), + next_config.features.enabled(Feature::SecretAuthStorage), ) { warn!("failed to refresh MCP auth storage config: {err}"); } diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ad1bb4aec1..3d74d96cbb 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -1729,6 +1729,89 @@ disabled_tools = [ ); } +#[tokio::test] +async fn refresh_mcp_config_replaces_managed_server_and_plugin_requirements() { + let (session, _turn_context) = make_session_and_context().await; + let server = serde_json::from_value::(json!({ + "url": "https://example.com/mcp", + "enabled": true + })) + .expect("valid test MCP server"); + let requirement = serde_json::from_value::(json!({ + "identity": { "url": "https://example.com/mcp" } + })) + .expect("valid managed MCP requirement"); + let plugin_requirements = std::collections::BTreeMap::from([( + "example-plugin".to_string(), + codex_config::PluginRequirementsToml { + mcp_servers: Some(std::collections::BTreeMap::from([( + "beta".to_string(), + requirement, + )])), + }, + )]); + + let mut next_config = session.get_config().await.as_ref().clone(); + next_config.mcp_servers = codex_config::Constrained::normalized( + HashMap::from([("beta".to_string(), server.clone())]), + |mut servers: HashMap| { + servers.retain(|name, _| name == "beta"); + servers + }, + ) + .expect("valid refreshed MCP constraints"); + let mut requirements = next_config.config_layer_stack.requirements().clone(); + requirements.plugins = Some(Sourced::new( + plugin_requirements.clone(), + RequirementSource::LegacyManagedConfigTomlFromMdm, + )); + let mut requirements_toml = next_config.config_layer_stack.requirements_toml().clone(); + requirements_toml.plugins = Some(plugin_requirements.clone()); + let layers = next_config + .config_layer_stack + .get_layers( + ConfigLayerStackOrdering::LowestPrecedenceFirst, + /*include_disabled*/ true, + ) + .into_iter() + .cloned() + .collect(); + next_config.config_layer_stack = ConfigLayerStack::new(layers, requirements, requirements_toml) + .expect("managed MCP and plugin requirements"); + + session.refresh_mcp_config(next_config).await; + + let config = session.get_config().await; + let mut managed_servers = config.mcp_servers.clone(); + managed_servers + .set(HashMap::from([ + ("alpha".to_string(), server.clone()), + ("beta".to_string(), server.clone()), + ])) + .expect("apply refreshed managed MCP constraints"); + assert_eq!( + managed_servers.get(), + &HashMap::from([("beta".to_string(), server.clone())]) + ); + assert_eq!( + config + .config_layer_stack + .requirements() + .plugins + .as_ref() + .map(|requirements| &requirements.value), + Some(&plugin_requirements) + ); + + let mut plugin_servers = HashMap::from([ + ("alpha".to_string(), server.clone()), + ("beta".to_string(), server), + ]); + config.apply_plugin_mcp_server_requirements("example-plugin", &mut plugin_servers); + assert!(!plugin_servers["alpha"].enabled); + assert!(plugin_servers["beta"].enabled); +} + #[test] fn collect_explicit_app_ids_from_skill_items_includes_linked_mentions() { let connectors = vec![make_connector("calendar", "Calendar")]; diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index de3dace63a..ad5a646021 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -194,14 +194,6 @@ pub struct W3cTraceContext { pub tracestate: Option, } -/// Resolved MCP inputs to apply through a thread's submission queue. -#[derive(Debug, Clone, PartialEq)] -pub struct McpServerRefreshConfig { - pub mcp_servers: Value, - pub mcp_oauth_credentials_store_mode: Value, - pub auth_keyring_backend_kind: Value, -} - #[derive(Debug, Clone, PartialEq)] pub struct ConversationStartParams { /// Whether Codex response handoffs are managed through explicit client append calls. @@ -643,9 +635,6 @@ pub enum Op { /// Request MCP servers to reinitialize and refresh cached tool lists. RefreshMcpServers, - /// Replace the thread's resolved MCP configuration before its next turn. - ReloadMcpConfig { config: McpServerRefreshConfig }, - /// Reload user config layer overrides for the active session. /// /// This updates runtime config-derived behavior (for example app @@ -886,7 +875,6 @@ impl Op { Self::RequestPermissionsResponse { .. } => "request_permissions_response", Self::DynamicToolResponse { .. } => "dynamic_tool_response", Self::RefreshMcpServers => "refresh_mcp_servers", - Self::ReloadMcpConfig { .. } => "reload_mcp_config", Self::ReloadUserConfig => "reload_user_config", Self::Compact => "compact", Self::SetThreadMemoryMode { .. } => "set_thread_memory_mode",