From 8b2c84ddccafe40dc0dc09f9f52bcbdc9dc45d66 Mon Sep 17 00:00:00 2001 From: jif Date: Mon, 13 Jul 2026 11:56:04 +0000 Subject: [PATCH] Apply MCP startup timeouts during client creation (#32781) ## What changed - Apply each server's `startup_timeout_sec` (or the default) while creating the MCP client, so the deadline also covers transport setup. - Launch local stdio servers on a blocking task so synchronous command resolution and process creation do not prevent the deadline from firing. - Recognize the new client-startup timeout error and show the existing `startup_timeout_sec` configuration hint. ## Testing - Extend the timeout error display test to cover the client-startup timeout. GitOrigin-RevId: 1967c62f943d55f6aa18792d4488e52c22f1e717 --- codex-rs/codex-mcp/src/connection_manager.rs | 1 + .../codex-mcp/src/connection_manager_tests.rs | 17 +++++++----- codex-rs/codex-mcp/src/rmcp_client.rs | 26 +++++++++++++------ .../rmcp-client/src/stdio_server_launcher.rs | 9 ++++++- 4 files changed, 38 insertions(+), 15 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index ef5cb45599..b8e58e7ae9 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -1043,6 +1043,7 @@ fn is_mcp_client_startup_timeout_error(error: &StartupOutcomeError) -> bool { StartupOutcomeError::Failed { error, .. } => { error.contains("request timed out") || error.contains("timed out handshaking with MCP server") + || error.contains("MCP client startup timed out") } _ => false, } diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index ff11ef596f..1d330a29b1 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -1706,12 +1706,17 @@ fn mcp_init_error_display_reports_generic_errors() { #[test] fn mcp_init_error_display_includes_startup_timeout_hint() { let server_name = "slow"; - let err: StartupOutcomeError = anyhow::anyhow!("request timed out").into(); + for error in [ + "request timed out", + "MCP client startup timed out after 30s", + ] { + let err: StartupOutcomeError = anyhow::anyhow!(error).into(); - let display = mcp_init_error_display(server_name, /*entry*/ None, &err); + let display = mcp_init_error_display(server_name, /*entry*/ None, &err); - assert_eq!( - "MCP client for `slow` timed out after 30 seconds. Add or adjust `startup_timeout_sec` in your config.toml:\n[mcp_servers.slow]\nstartup_timeout_sec = XX", - display - ); + assert_eq!( + "MCP client for `slow` timed out after 30 seconds. Add or adjust `startup_timeout_sec` in your config.toml:\n[mcp_servers.slow]\nstartup_timeout_sec = XX", + display + ); + } } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 52c4a5d462..aefcac67ea 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -302,6 +302,10 @@ impl ManagedClientStartup { .configured_config() .map(ToolFilter::from_config) .unwrap_or_default(); + let startup_timeout = server + .configured_config() + .and_then(|config| config.startup_timeout_sec) + .unwrap_or(DEFAULT_STARTUP_TIMEOUT); let cancel_token_for_fut = cancel_token; async move { let refresh_start = is_codex_apps_mcp_server.then(Instant::now); @@ -310,7 +314,8 @@ impl ManagedClientStartup { return Err(error.into()); } - let client = Arc::new( + let client = match tokio::time::timeout( + startup_timeout, make_rmcp_client( &server_name, server.clone(), @@ -318,18 +323,23 @@ impl ManagedClientStartup { keyring_backend_kind, runtime_context, runtime_auth_provider, - ) - .await?, - ); + ), + ) + .await + { + Ok(result) => Arc::new(result?), + Err(_) => { + return Err(StartupOutcomeError::from(anyhow!( + "MCP client startup timed out after {startup_timeout:?}" + ))); + } + }; start_server_task( server_name, client, StartServerTaskParams { is_codex_apps_mcp_server, - startup_timeout: server - .configured_config() - .and_then(|config| config.startup_timeout_sec) - .or(Some(DEFAULT_STARTUP_TIMEOUT)), + startup_timeout: Some(startup_timeout), tool_timeout: server .configured_config() .and_then(|config| config.tool_timeout_sec) diff --git a/codex-rs/rmcp-client/src/stdio_server_launcher.rs b/codex-rs/rmcp-client/src/stdio_server_launcher.rs index 7d03e1437e..a863754862 100644 --- a/codex-rs/rmcp-client/src/stdio_server_launcher.rs +++ b/codex-rs/rmcp-client/src/stdio_server_launcher.rs @@ -191,7 +191,14 @@ impl StdioServerLauncher for LocalStdioServerLauncher { command: StdioServerCommand, ) -> BoxFuture<'static, io::Result> { let fallback_cwd = self.fallback_cwd.clone(); - async move { Self::launch_server(command, fallback_cwd) }.boxed() + async move { + // Keep synchronous program resolution and process creation from blocking the + // caller's startup deadline. + tokio::task::spawn_blocking(move || Self::launch_server(command, fallback_cwd)) + .await + .map_err(io::Error::other)? + } + .boxed() } }