From 192481d1a148eede2df387112c9476c18ff06675 Mon Sep 17 00:00:00 2001 From: Matthew Zeng Date: Mon, 11 May 2026 12:23:55 -0700 Subject: [PATCH] [elicitation] Advertise new url elicitation capability when auth_elicitation is enabled. (#22188) ## Why We've added support for auth elicitation behind the auth_elicitation flag, but servers need to explicitly check the capability before it decides to send elicitations in order to be backward compatible. This PR adds the capability advertising conditioned on the flag. ## What changed - Build `client_elicitation_capability` from the `AuthElicitation` feature state. - Thread that capability through MCP config, session startup, and `McpConnectionManager` so RMCP initialization advertises the correct elicitation support. - Advertise both `form` and `url` elicitation when the feature is enabled, and preserve the empty default capability when it is disabled. - Add coverage for the feature-derived config shape and the advertised initialization payload. ## Testing - `cargo test -p codex-mcp` - `cargo test -p codex-core to_mcp_config_preserves_auth_elicitation_feature_from_config` - `cargo test -p codex-core` *(currently fails outside this change in `tools::handlers::multi_agents::tests::tool_handlers_cascade_close_and_resume_and_keep_explicitly_closed_subtrees_closed` with a stack overflow after unrelated tests have started running)* --- codex-rs/codex-mcp/src/connection_manager.rs | 3 ++ .../codex-mcp/src/connection_manager_tests.rs | 31 +++++++++++------ codex-rs/codex-mcp/src/mcp/mod.rs | 5 +++ codex-rs/codex-mcp/src/mcp/mod_tests.rs | 1 + codex-rs/codex-mcp/src/rmcp_client.rs | 15 +++------ codex-rs/core/src/config/config_tests.rs | 33 +++++++++++++++++++ codex-rs/core/src/config/mod.rs | 13 ++++++++ codex-rs/core/src/connectors.rs | 1 + codex-rs/core/src/mcp_tool_call_tests.rs | 1 + codex-rs/core/src/session/mcp.rs | 1 + codex-rs/core/src/session/mod.rs | 3 ++ codex-rs/core/src/session/session.rs | 9 +++++ 12 files changed, 96 insertions(+), 20 deletions(-) diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index e82aa0ba5..1b09ac5da 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -55,6 +55,7 @@ use codex_protocol::protocol::McpStartupFailure; use codex_protocol::protocol::McpStartupStatus; use codex_protocol::protocol::McpStartupUpdateEvent; use codex_rmcp_client::ElicitationResponse; +use rmcp::model::ElicitationCapability; use rmcp::model::ListResourceTemplatesResult; use rmcp::model::ListResourcesResult; use rmcp::model::PaginatedRequestParams; @@ -178,6 +179,7 @@ impl McpConnectionManager { codex_home: PathBuf, codex_apps_tools_cache_key: CodexAppsToolsCacheKey, host_owned_codex_apps_enabled: bool, + client_elicitation_capability: ElicitationCapability, tool_plugin_provenance: ToolPluginProvenance, auth: Option<&CodexAuth>, elicitation_reviewer: Option, @@ -247,6 +249,7 @@ impl McpConnectionManager { Arc::clone(&tool_plugin_provenance), runtime_environment.clone(), runtime_auth_provider, + client_elicitation_capability.clone(), ); clients.insert(server_name.clone(), async_managed_client.clone()); let tx_event = tx_event.clone(); diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 61be90ed7..b2cf5f94f 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -10,7 +10,6 @@ use crate::elicitation::elicitation_is_rejected_by_policy; use crate::rmcp_client::AsyncManagedClient; use crate::rmcp_client::ManagedClient; use crate::rmcp_client::StartupOutcomeError; -use crate::rmcp_client::elicitation_capability_for_server; use crate::tools::ToolFilter; use crate::tools::ToolInfo; use crate::tools::filter_tools; @@ -844,15 +843,27 @@ async fn list_all_tools_uses_startup_snapshot_when_client_startup_fails() { } #[test] -fn elicitation_capability_uses_2025_06_18_shape_for_all_servers() { - for server_name in [CODEX_APPS_MCP_SERVER_NAME, "custom_mcp"] { - let capability = elicitation_capability_for_server(server_name); - assert_eq!(capability, Some(ElicitationCapability::default())); - assert_eq!( - serde_json::to_value(capability).expect("serialize elicitation capability"), - serde_json::json!({}) - ); - } +fn elicitation_capability_uses_2025_06_18_shape_for_form_only_support() { + let capability = Some(ElicitationCapability::default()); + assert_eq!( + serde_json::to_value(capability).expect("serialize elicitation capability"), + serde_json::json!({}) + ); +} + +#[test] +fn elicitation_capability_advertises_url_support_when_enabled() { + let capability = Some(ElicitationCapability { + form: Some(rmcp::model::FormElicitationCapability::default()), + url: Some(rmcp::model::UrlElicitationCapability::default()), + }); + assert_eq!( + serde_json::to_value(capability).expect("serialize elicitation capability"), + serde_json::json!({ + "form": {}, + "url": {}, + }) + ); } #[test] diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index b71a21377..b8fb9a080 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -31,6 +31,7 @@ use codex_protocol::mcp::Tool; use codex_protocol::models::PermissionProfile; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::McpAuthStatus; +use rmcp::model::ElicitationCapability; use rmcp::model::ReadResourceRequestParams; use rmcp::model::ReadResourceResult; use serde_json::Value; @@ -129,6 +130,8 @@ pub struct McpConfig { /// ChatGPT auth is checked separately at runtime before the host-owned apps /// MCP server is added. pub apps_enabled: bool, + /// Client-side elicitation capabilities advertised during MCP initialization. + pub client_elicitation_capability: ElicitationCapability, /// Config-backed MCP servers keyed by server name. /// /// Runtime-only additions are merged later by [`effective_mcp_servers`]. @@ -272,6 +275,7 @@ pub async fn read_mcp_resource( config.codex_home.clone(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, + config.client_elicitation_capability.clone(), tool_plugin_provenance(config), auth, /*elicitation_reviewer*/ None, @@ -340,6 +344,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( config.codex_home.clone(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, + config.client_elicitation_capability.clone(), tool_plugin_provenance, auth, /*elicitation_reviewer*/ None, diff --git a/codex-rs/codex-mcp/src/mcp/mod_tests.rs b/codex-rs/codex-mcp/src/mcp/mod_tests.rs index 5d09a7005..0d0afc6c4 100644 --- a/codex-rs/codex-mcp/src/mcp/mod_tests.rs +++ b/codex-rs/codex-mcp/src/mcp/mod_tests.rs @@ -27,6 +27,7 @@ fn test_mcp_config(codex_home: PathBuf) -> McpConfig { codex_linux_sandbox_exe: None, use_legacy_landlock: false, apps_enabled: false, + client_elicitation_capability: ElicitationCapability::default(), configured_mcp_servers: HashMap::new(), plugin_capability_summaries: Vec::new(), } diff --git a/codex-rs/codex-mcp/src/rmcp_client.rs b/codex-rs/codex-mcp/src/rmcp_client.rs index 164255523..bca85f00d 100644 --- a/codex-rs/codex-mcp/src/rmcp_client.rs +++ b/codex-rs/codex-mcp/src/rmcp_client.rs @@ -144,6 +144,7 @@ impl AsyncManagedClient { tool_plugin_provenance: Arc, runtime_environment: McpRuntimeEnvironment, runtime_auth_provider: Option, + client_elicitation_capability: ElicitationCapability, ) -> Self { let tool_filter = server .configured_config() @@ -190,6 +191,7 @@ impl AsyncManagedClient { tx_event, elicitation_requests, codex_apps_tools_cache_context, + client_elicitation_capability, }, ) .await @@ -326,14 +328,6 @@ impl From for StartupOutcomeError { } } -pub(crate) fn elicitation_capability_for_server( - _server_name: &str, -) -> Option { - // https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities - // indicates this should be an empty object. - Some(ElicitationCapability::default()) -} - pub(crate) async fn list_tools_for_client_uncached( server_name: &str, client: &Arc, @@ -472,8 +466,8 @@ async fn start_server_task( tx_event, elicitation_requests, codex_apps_tools_cache_context, + client_elicitation_capability, } = params; - let elicitation = elicitation_capability_for_server(&server_name); let params = InitializeRequestParams { meta: None, capabilities: ClientCapabilities { @@ -481,7 +475,7 @@ async fn start_server_task( extensions: None, roots: None, sampling: None, - elicitation, + elicitation: Some(client_elicitation_capability), tasks: None, }, client_info: Implementation { @@ -557,6 +551,7 @@ struct StartServerTaskParams { tx_event: Sender, elicitation_requests: ElicitationRequestManager, codex_apps_tools_cache_context: Option, + client_elicitation_capability: ElicitationCapability, } async fn make_rmcp_client( diff --git a/codex-rs/core/src/config/config_tests.rs b/codex-rs/core/src/config/config_tests.rs index e7f374a65..e5cc8c62c 100644 --- a/codex-rs/core/src/config/config_tests.rs +++ b/codex-rs/core/src/config/config_tests.rs @@ -89,6 +89,9 @@ use core_test_support::PathExt; use core_test_support::TempDirExt; use core_test_support::test_absolute_path; use pretty_assertions::assert_eq; +use rmcp::model::ElicitationCapability; +use rmcp::model::FormElicitationCapability; +use rmcp::model::UrlElicitationCapability; use std::collections::BTreeMap; use std::collections::HashMap; @@ -4192,6 +4195,36 @@ async fn to_mcp_config_preserves_apps_feature_from_config() -> std::io::Result<( Ok(()) } +#[tokio::test] +async fn to_mcp_config_preserves_auth_elicitation_feature_from_config() -> std::io::Result<()> { + let codex_home = TempDir::new()?; + let mut config = Config::load_from_base_config_with_overrides( + ConfigToml::default(), + ConfigOverrides::default(), + codex_home.abs(), + ) + .await?; + let plugins_manager = PluginsManager::new(codex_home.path().to_path_buf()); + + let mcp_config = config.to_mcp_config(&plugins_manager).await; + assert_eq!( + mcp_config.client_elicitation_capability, + ElicitationCapability::default() + ); + + let _ = config.features.enable(Feature::AuthElicitation); + let mcp_config = config.to_mcp_config(&plugins_manager).await; + assert_eq!( + mcp_config.client_elicitation_capability, + ElicitationCapability { + form: Some(FormElicitationCapability::default()), + url: Some(UrlElicitationCapability::default()), + } + ); + + Ok(()) +} + #[tokio::test] async fn load_global_mcp_servers_rejects_inline_bearer_token() -> anyhow::Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/core/src/config/mod.rs b/codex-rs/core/src/config/mod.rs index c68381abd..f965ed509 100644 --- a/codex-rs/core/src/config/mod.rs +++ b/codex-rs/core/src/config/mod.rs @@ -98,6 +98,9 @@ use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::SandboxPolicy; use codex_utils_absolute_path::AbsolutePathBuf; use codex_utils_absolute_path::AbsolutePathBufGuard; +use rmcp::model::ElicitationCapability; +use rmcp::model::FormElicitationCapability; +use rmcp::model::UrlElicitationCapability; use serde::Deserialize; use serde::Serialize; use std::collections::BTreeMap; @@ -1122,6 +1125,16 @@ impl Config { codex_linux_sandbox_exe: self.codex_linux_sandbox_exe.clone(), use_legacy_landlock: self.features.use_legacy_landlock(), apps_enabled: self.features.enabled(Feature::Apps), + client_elicitation_capability: if self.features.enabled(Feature::AuthElicitation) { + ElicitationCapability { + form: Some(FormElicitationCapability::default()), + url: Some(UrlElicitationCapability::default()), + } + } else { + // https://modelcontextprotocol.io/specification/2025-06-18/client/elicitation#capabilities + // indicates this should be an empty object. + ElicitationCapability::default() + }, configured_mcp_servers, plugin_capability_summaries: loaded_plugins.capability_summaries().to_vec(), } diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index 944e67b5b..f4ee42c82 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -277,6 +277,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), host_owned_codex_apps_enabled, + mcp_config.client_elicitation_capability, ToolPluginProvenance::default(), auth.as_ref(), /*elicitation_reviewer*/ None, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index a556e228a..326e0cfe3 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1122,6 +1122,7 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: turn_context.config.codex_home.to_path_buf(), codex_mcp::codex_apps_tools_cache_key(auth.as_ref()), /*host_owned_codex_apps_enabled*/ true, + rmcp::model::ElicitationCapability::default(), codex_mcp::ToolPluginProvenance::default(), auth.as_ref(), /*elicitation_reviewer*/ None, diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index a7d7a965a..102a9f211 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -329,6 +329,7 @@ impl Session { config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth.as_ref()), host_owned_codex_apps_enabled, + mcp_config.client_elicitation_capability, tool_plugin_provenance, auth.as_ref(), elicitation_reviewer, diff --git a/codex-rs/core/src/session/mod.rs b/codex-rs/core/src/session/mod.rs index af36fda48..5f7f3fd16 100644 --- a/codex-rs/core/src/session/mod.rs +++ b/codex-rs/core/src/session/mod.rs @@ -143,12 +143,15 @@ use codex_utils_output_truncation::TruncationPolicy; use futures::future::BoxFuture; use futures::future::Shared; use futures::prelude::*; +use rmcp::model::ElicitationCapability; +use rmcp::model::FormElicitationCapability; use rmcp::model::ListResourceTemplatesResult; use rmcp::model::ListResourcesResult; use rmcp::model::PaginatedRequestParams; use rmcp::model::ReadResourceRequestParams; use rmcp::model::ReadResourceResult; use rmcp::model::RequestId; +use rmcp::model::UrlElicitationCapability; use serde_json::Value; use tokio::sync::Mutex; use tokio::sync::RwLock; diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 8c9ea1a12..32aaf8fb0 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -961,6 +961,14 @@ impl Session { let host_owned_codex_apps_enabled = config .features .apps_enabled_for_auth(auth.as_ref().is_some_and(|auth| auth.uses_codex_backend())); + let client_elicitation_capability = if config.features.enabled(Feature::AuthElicitation) { + ElicitationCapability { + form: Some(FormElicitationCapability::default()), + url: Some(UrlElicitationCapability::default()), + } + } else { + ElicitationCapability::default() + }; { let mut cancel_guard = sess.services.mcp_startup_cancellation_token.lock().await; cancel_guard.cancel(); @@ -1003,6 +1011,7 @@ impl Session { config.codex_home.to_path_buf(), codex_apps_tools_cache_key(auth), host_owned_codex_apps_enabled, + client_elicitation_capability, tool_plugin_provenance, auth, Some(sess.mcp_elicitation_reviewer()),