diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 04603152b..f8f64ac9e 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -700,6 +700,14 @@ impl Session { for event in events { sess.send_event_raw(event).await; } + + // Construct sandbox_state before initialize() so it can be sent to each + // MCP server immediately after it becomes ready (avoiding blocking). + let sandbox_state = SandboxState { + sandbox_policy: session_configuration.sandbox_policy.clone(), + codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + sandbox_cwd: session_configuration.cwd.clone(), + }; sess.services .mcp_connection_manager .write() @@ -710,25 +718,10 @@ impl Session { auth_statuses.clone(), tx_event.clone(), sess.services.mcp_startup_cancellation_token.clone(), + sandbox_state, ) .await; - let sandbox_state = SandboxState { - sandbox_policy: session_configuration.sandbox_policy.clone(), - codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), - sandbox_cwd: session_configuration.cwd.clone(), - }; - if let Err(e) = sess - .services - .mcp_connection_manager - .read() - .await - .notify_sandbox_state_change(&sandbox_state) - .await - { - tracing::error!("Failed to notify sandbox state change: {e}"); - } - // record_initial_history can emit events. We record only after the SessionConfiguredEvent is emitted. sess.record_initial_history(initial_history).await; @@ -840,14 +833,34 @@ impl Session { sub_id: String, updates: SessionSettingsUpdate, ) -> Arc { - let session_configuration = { + let (session_configuration, sandbox_policy_changed) = { let mut state = self.state.lock().await; let session_configuration = state.session_configuration.clone().apply(&updates); + let sandbox_policy_changed = + state.session_configuration.sandbox_policy != session_configuration.sandbox_policy; state.session_configuration = session_configuration.clone(); - session_configuration + (session_configuration, sandbox_policy_changed) }; - let per_turn_config = Self::build_per_turn_config(&session_configuration); + + if sandbox_policy_changed { + let sandbox_state = SandboxState { + sandbox_policy: per_turn_config.sandbox_policy.clone(), + codex_linux_sandbox_exe: per_turn_config.codex_linux_sandbox_exe.clone(), + sandbox_cwd: per_turn_config.cwd.clone(), + }; + if let Err(e) = self + .services + .mcp_connection_manager + .read() + .await + .notify_sandbox_state_change(&sandbox_state) + .await + { + warn!("Failed to notify sandbox state change to MCP servers: {e:#}"); + } + } + let model_family = self .services .models_manager diff --git a/codex-rs/core/src/mcp/mod.rs b/codex-rs/core/src/mcp/mod.rs index ed5f2ea69..677483646 100644 --- a/codex-rs/core/src/mcp/mod.rs +++ b/codex-rs/core/src/mcp/mod.rs @@ -1,14 +1,18 @@ pub mod auth; use std::collections::HashMap; +use std::env; +use std::path::PathBuf; use async_channel::unbounded; use codex_protocol::protocol::McpListToolsResponseEvent; +use codex_protocol::protocol::SandboxPolicy; use mcp_types::Tool as McpTool; use tokio_util::sync::CancellationToken; use crate::config::Config; use crate::mcp::auth::compute_auth_statuses; use crate::mcp_connection_manager::McpConnectionManager; +use crate::mcp_connection_manager::SandboxState; const MCP_TOOL_NAME_PREFIX: &str = "mcp"; const MCP_TOOL_NAME_DELIMITER: &str = "__"; @@ -34,6 +38,13 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent drop(rx_event); let cancel_token = CancellationToken::new(); + // Use ReadOnly sandbox policy for MCP snapshot collection (safest default) + let sandbox_state = SandboxState { + sandbox_policy: SandboxPolicy::ReadOnly, + codex_linux_sandbox_exe: config.codex_linux_sandbox_exe.clone(), + sandbox_cwd: env::current_dir().unwrap_or_else(|_| PathBuf::from("/")), + }; + mcp_connection_manager .initialize( config.mcp_servers.clone(), @@ -41,6 +52,7 @@ pub async fn collect_mcp_snapshot(config: &Config) -> McpListToolsResponseEvent auth_status_entries.clone(), tx_event, cancel_token.clone(), + sandbox_state, ) .await; diff --git a/codex-rs/core/src/mcp_connection_manager.rs b/codex-rs/core/src/mcp_connection_manager.rs index 11a90f77a..4eddb1cd8 100644 --- a/codex-rs/core/src/mcp_connection_manager.rs +++ b/codex-rs/core/src/mcp_connection_manager.rs @@ -182,6 +182,21 @@ struct ManagedClient { server_supports_sandbox_state_capability: bool, } +impl ManagedClient { + async fn notify_sandbox_state_change(&self, sandbox_state: &SandboxState) -> Result<()> { + if !self.server_supports_sandbox_state_capability { + return Ok(()); + } + + self.client + .send_custom_notification( + MCP_SANDBOX_STATE_NOTIFICATION, + Some(serde_json::to_value(sandbox_state)?), + ) + .await + } +} + #[derive(Clone)] struct AsyncManagedClient { client: Shared>>, @@ -231,17 +246,7 @@ impl AsyncManagedClient { async fn notify_sandbox_state_change(&self, sandbox_state: &SandboxState) -> Result<()> { let managed = self.client().await?; - if !managed.server_supports_sandbox_state_capability { - return Ok(()); - } - - managed - .client - .send_custom_notification( - MCP_SANDBOX_STATE_NOTIFICATION, - Some(serde_json::to_value(sandbox_state)?), - ) - .await + managed.notify_sandbox_state_change(sandbox_state).await } } @@ -274,6 +279,7 @@ impl McpConnectionManager { auth_entries: HashMap, tx_event: Sender, cancel_token: CancellationToken, + initial_sandbox_state: SandboxState, ) { if cancel_token.is_cancelled() { return; @@ -302,13 +308,25 @@ impl McpConnectionManager { clients.insert(server_name.clone(), async_managed_client.clone()); let tx_event = tx_event.clone(); let auth_entry = auth_entries.get(&server_name).cloned(); + let sandbox_state = initial_sandbox_state.clone(); join_set.spawn(async move { let outcome = async_managed_client.client().await; if cancel_token.is_cancelled() { return (server_name, Err(StartupOutcomeError::Cancelled)); } let status = match &outcome { - Ok(_) => McpStartupStatus::Ready, + Ok(_) => { + // Send sandbox state notification immediately after Ready + if let Err(e) = async_managed_client + .notify_sandbox_state_change(&sandbox_state) + .await + { + warn!( + "Failed to notify sandbox state to MCP server {server_name}: {e:#}", + ); + } + McpStartupStatus::Ready + } Err(error) => { let error_str = mcp_init_error_display( server_name.as_str(), diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap new file mode 100644 index 000000000..c6866c1b5 --- /dev/null +++ b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__mcp_startup_header_booting.snap @@ -0,0 +1,11 @@ +--- +source: tui/src/chatwidget/tests.rs +expression: terminal.backend() +--- +" " +"• Booting MCP server: alpha (0s • esc to interrupt) " +" " +" " +"› Ask Codex to do anything " +" " +" 100% context left · ? for shortcuts " diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 65f6708c9..face118d6 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -27,6 +27,8 @@ use codex_core::protocol::ExecCommandSource; use codex_core::protocol::ExecPolicyAmendment; use codex_core::protocol::ExitedReviewModeEvent; use codex_core::protocol::FileChange; +use codex_core::protocol::McpStartupStatus; +use codex_core::protocol::McpStartupUpdateEvent; use codex_core::protocol::Op; use codex_core::protocol::PatchApplyBeginEvent; use codex_core::protocol::PatchApplyEndEvent; @@ -2417,6 +2419,28 @@ fn status_widget_active_snapshot() { assert_snapshot!("status_widget_active", terminal.backend()); } +#[test] +fn mcp_startup_header_booting_snapshot() { + let (mut chat, _rx, _op_rx) = make_chatwidget_manual(None); + chat.show_welcome_banner = false; + + chat.handle_codex_event(Event { + id: "mcp-1".into(), + msg: EventMsg::McpStartupUpdate(McpStartupUpdateEvent { + server: "alpha".into(), + status: McpStartupStatus::Starting, + }), + }); + + let height = chat.desired_height(80); + let mut terminal = ratatui::Terminal::new(ratatui::backend::TestBackend::new(80, height)) + .expect("create terminal"); + terminal + .draw(|f| chat.render(f.area(), f.buffer_mut())) + .expect("draw chat widget"); + assert_snapshot!("mcp_startup_header_booting", terminal.backend()); +} + #[test] fn background_event_updates_status_header() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(None);