mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
[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)*
This commit is contained in:
committed by
GitHub
Unverified
parent
d0fa2d81d8
commit
192481d1a1
@@ -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<ElicitationReviewerHandle>,
|
||||
@@ -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();
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
|
||||
@@ -144,6 +144,7 @@ impl AsyncManagedClient {
|
||||
tool_plugin_provenance: Arc<ToolPluginProvenance>,
|
||||
runtime_environment: McpRuntimeEnvironment,
|
||||
runtime_auth_provider: Option<SharedAuthProvider>,
|
||||
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<anyhow::Error> for StartupOutcomeError {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn elicitation_capability_for_server(
|
||||
_server_name: &str,
|
||||
) -> Option<ElicitationCapability> {
|
||||
// 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<RmcpClient>,
|
||||
@@ -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<Event>,
|
||||
elicitation_requests: ElicitationRequestManager,
|
||||
codex_apps_tools_cache_context: Option<CodexAppsToolsCacheContext>,
|
||||
client_elicitation_capability: ElicitationCapability,
|
||||
}
|
||||
|
||||
async fn make_rmcp_client(
|
||||
|
||||
@@ -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()?;
|
||||
|
||||
@@ -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(),
|
||||
}
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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()),
|
||||
|
||||
Reference in New Issue
Block a user