diff --git a/codex-rs/codex-mcp/src/connection_manager.rs b/codex-rs/codex-mcp/src/connection_manager.rs index 58b5566e2330..f93ac5e089fd 100644 --- a/codex-rs/codex-mcp/src/connection_manager.rs +++ b/codex-rs/codex-mcp/src/connection_manager.rs @@ -17,6 +17,7 @@ use crate::codex_apps::CodexAppsToolsCacheContext; use crate::codex_apps::CodexAppsToolsCacheKey; use crate::codex_apps::write_cached_codex_apps_tools_if_needed; use crate::elicitation::ElicitationRequestManager; +use crate::elicitation::ElicitationReviewerHandle; use crate::mcp::CODEX_APPS_MCP_SERVER_NAME; use crate::mcp::ToolPluginProvenance; use crate::rmcp_client::AsyncManagedClient; @@ -87,6 +88,7 @@ impl McpConnectionManager { elicitation_requests: ElicitationRequestManager::new( approval_policy.value(), permission_profile.get().clone(), + /*reviewer*/ None, ), startup_cancellation_token: CancellationToken::new(), } @@ -157,13 +159,17 @@ impl McpConnectionManager { host_owned_codex_apps_enabled: bool, tool_plugin_provenance: ToolPluginProvenance, auth: Option<&CodexAuth>, + elicitation_reviewer: Option, ) -> (Self, CancellationToken) { let cancel_token = CancellationToken::new(); let mut clients = HashMap::new(); let mut server_origins = HashMap::new(); let mut join_set = JoinSet::new(); - let elicitation_requests = - ElicitationRequestManager::new(approval_policy.value(), initial_permission_profile); + let elicitation_requests = ElicitationRequestManager::new( + approval_policy.value(), + initial_permission_profile, + elicitation_reviewer, + ); let tool_plugin_provenance = Arc::new(tool_plugin_provenance); let startup_submit_id = submit_id.clone(); let codex_apps_auth_provider = auth diff --git a/codex-rs/codex-mcp/src/connection_manager_tests.rs b/codex-rs/codex-mcp/src/connection_manager_tests.rs index 69ff10e02389..f2dc85cde043 100644 --- a/codex-rs/codex-mcp/src/connection_manager_tests.rs +++ b/codex-rs/codex-mcp/src/connection_manager_tests.rs @@ -203,8 +203,11 @@ fn elicitation_granular_policy_respects_never_and_config() { #[tokio::test] async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() { - let manager = - ElicitationRequestManager::new(AskForApproval::Never, PermissionProfile::Disabled); + let manager = ElicitationRequestManager::new( + AskForApproval::Never, + PermissionProfile::Disabled, + /*reviewer*/ None, + ); let (tx_event, _rx_event) = async_channel::bounded(1); let sender = manager.make_sender("server".to_string(), tx_event); @@ -233,8 +236,11 @@ async fn disabled_permissions_auto_accept_elicitation_with_empty_form_schema() { #[tokio::test] async fn disabled_permissions_do_not_auto_accept_elicitation_with_requested_fields() { - let manager = - ElicitationRequestManager::new(AskForApproval::Never, PermissionProfile::Disabled); + let manager = ElicitationRequestManager::new( + AskForApproval::Never, + PermissionProfile::Disabled, + /*reviewer*/ None, + ); let (tx_event, _rx_event) = async_channel::bounded(1); let sender = manager.make_sender("server".to_string(), tx_event); diff --git a/codex-rs/codex-mcp/src/elicitation.rs b/codex-rs/codex-mcp/src/elicitation.rs index 43c9deb246af..a51cd7c62353 100644 --- a/codex-rs/codex-mcp/src/elicitation.rs +++ b/codex-rs/codex-mcp/src/elicitation.rs @@ -24,6 +24,7 @@ use codex_protocol::protocol::Event; use codex_protocol::protocol::EventMsg; use codex_rmcp_client::ElicitationResponse; use codex_rmcp_client::SendElicitation; +use futures::future::BoxFuture; use futures::future::FutureExt; use rmcp::model::CreateElicitationRequestParams; use rmcp::model::ElicitationAction; @@ -31,24 +32,43 @@ use rmcp::model::RequestId; use tokio::sync::Mutex; use tokio::sync::oneshot; +#[derive(Debug, Clone)] +pub struct ElicitationReviewRequest { + pub server_name: String, + pub request_id: RequestId, + pub elicitation: CreateElicitationRequestParams, +} + +pub trait ElicitationReviewer: Send + Sync { + fn review( + &self, + request: ElicitationReviewRequest, + ) -> BoxFuture<'static, Result>>; +} + +pub type ElicitationReviewerHandle = Arc; + #[derive(Clone)] pub(crate) struct ElicitationRequestManager { requests: Arc>, pub(crate) approval_policy: Arc>, pub(crate) permission_profile: Arc>, auto_deny: Arc>, + reviewer: Option, } impl ElicitationRequestManager { pub(crate) fn new( approval_policy: AskForApproval, permission_profile: PermissionProfile, + reviewer: Option, ) -> Self { Self { requests: Arc::new(Mutex::new(HashMap::new())), approval_policy: Arc::new(StdMutex::new(approval_policy)), permission_profile: Arc::new(StdMutex::new(permission_profile)), auto_deny: Arc::new(StdMutex::new(false)), + reviewer, } } @@ -89,6 +109,7 @@ impl ElicitationRequestManager { let approval_policy = self.approval_policy.clone(); let permission_profile = self.permission_profile.clone(); let auto_deny = self.auto_deny.clone(); + let reviewer = self.reviewer.clone(); Box::new(move |id, elicitation| { let elicitation_requests = elicitation_requests.clone(); let tx_event = tx_event.clone(); @@ -96,6 +117,7 @@ impl ElicitationRequestManager { let approval_policy = approval_policy.clone(); let permission_profile = permission_profile.clone(); let auto_deny = auto_deny.clone(); + let reviewer = reviewer.clone(); async move { let auto_deny = auto_deny .lock() @@ -138,6 +160,17 @@ impl ElicitationRequestManager { }); } + if let Some(reviewer) = reviewer.as_ref() { + let request = ElicitationReviewRequest { + server_name: server_name.clone(), + request_id: id.clone(), + elicitation: elicitation.clone(), + }; + if let Some(response) = reviewer.review(request).await? { + return Ok(response); + } + } + let request = match elicitation { CreateElicitationRequestParams::FormElicitationParams { meta, diff --git a/codex-rs/codex-mcp/src/lib.rs b/codex-rs/codex-mcp/src/lib.rs index 2080a5c31920..83dc65dd0698 100644 --- a/codex-rs/codex-mcp/src/lib.rs +++ b/codex-rs/codex-mcp/src/lib.rs @@ -1,4 +1,7 @@ pub use connection_manager::McpConnectionManager; +pub use elicitation::ElicitationReviewRequest; +pub use elicitation::ElicitationReviewer; +pub use elicitation::ElicitationReviewerHandle; pub use rmcp_client::MCP_SANDBOX_STATE_META_CAPABILITY; pub use runtime::McpRuntimeEnvironment; pub use runtime::SandboxState; diff --git a/codex-rs/codex-mcp/src/mcp/mod.rs b/codex-rs/codex-mcp/src/mcp/mod.rs index 03a5d1a068c4..ae667c698a16 100644 --- a/codex-rs/codex-mcp/src/mcp/mod.rs +++ b/codex-rs/codex-mcp/src/mcp/mod.rs @@ -264,6 +264,7 @@ pub async fn read_mcp_resource( host_owned_codex_apps_enabled, tool_plugin_provenance(config), auth, + /*elicitation_reviewer*/ None, ) .await; @@ -331,6 +332,7 @@ pub async fn collect_mcp_server_status_snapshot_with_detail( host_owned_codex_apps_enabled, tool_plugin_provenance, auth, + /*elicitation_reviewer*/ None, ) .await; diff --git a/codex-rs/core/src/connectors.rs b/codex-rs/core/src/connectors.rs index e2154db17eb0..0bd53a50eed0 100644 --- a/codex-rs/core/src/connectors.rs +++ b/codex-rs/core/src/connectors.rs @@ -283,6 +283,7 @@ pub async fn list_accessible_connectors_from_mcp_tools_with_environment_manager( host_owned_codex_apps_enabled, ToolPluginProvenance::default(), auth.as_ref(), + /*elicitation_reviewer*/ None, ) .await; diff --git a/codex-rs/core/src/mcp_skill_dependencies.rs b/codex-rs/core/src/mcp_skill_dependencies.rs index c24a6f3a4884..aefc783cb609 100644 --- a/codex-rs/core/src/mcp_skill_dependencies.rs +++ b/codex-rs/core/src/mcp_skill_dependencies.rs @@ -19,6 +19,7 @@ use crate::SkillMetadata; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use crate::skills::model::SkillToolDependency; +use codex_mcp::ElicitationReviewerHandle; use codex_mcp::McpOAuthLoginSupport; use codex_mcp::McpPermissionPromptAutoApproveContext; use codex_mcp::mcp_permission_prompt_is_auto_approved; @@ -35,6 +36,7 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( turn_context: &TurnContext, cancellation_token: &CancellationToken, mentioned_skills: &[SkillMetadata], + elicitation_reviewer: Option, ) { let originator_value = originator().value; if !is_first_party_originator(originator_value.as_str()) { @@ -69,7 +71,14 @@ pub(crate) async fn maybe_prompt_and_install_mcp_dependencies( if should_install_mcp_dependencies(sess, turn_context, &unprompted_missing, cancellation_token) .await { - maybe_install_mcp_dependencies(sess, turn_context, config.as_ref(), mentioned_skills).await; + maybe_install_mcp_dependencies( + sess, + turn_context, + config.as_ref(), + mentioned_skills, + elicitation_reviewer, + ) + .await; } } @@ -78,6 +87,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( turn_context: &TurnContext, config: &crate::config::Config, mentioned_skills: &[SkillMetadata], + elicitation_reviewer: Option, ) { if mentioned_skills.is_empty() || !config @@ -194,6 +204,7 @@ pub(crate) async fn maybe_install_mcp_dependencies( turn_context, refresh_servers, config.mcp_oauth_credentials_store_mode, + elicitation_reviewer, ) .await; } diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index a6557b3d6766..c7e665cb02ab 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -54,6 +54,20 @@ use codex_protocol::items::McpToolCallItem; use codex_protocol::items::McpToolCallStatus; use codex_protocol::items::TurnItem; use codex_protocol::mcp::CallToolResult; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY as MCP_TOOL_APPROVAL_KIND_KEY; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_MCP_TOOL_CALL as MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL; +use codex_protocol::mcp_approval_meta::CONNECTOR_DESCRIPTION_KEY as MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_ID_KEY as MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_NAME_KEY as MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY; +use codex_protocol::mcp_approval_meta::PERSIST_ALWAYS as MCP_TOOL_APPROVAL_PERSIST_ALWAYS; +use codex_protocol::mcp_approval_meta::PERSIST_KEY as MCP_TOOL_APPROVAL_PERSIST_KEY; +use codex_protocol::mcp_approval_meta::PERSIST_SESSION as MCP_TOOL_APPROVAL_PERSIST_SESSION; +use codex_protocol::mcp_approval_meta::SOURCE_CONNECTOR as MCP_TOOL_APPROVAL_SOURCE_CONNECTOR; +use codex_protocol::mcp_approval_meta::SOURCE_KEY as MCP_TOOL_APPROVAL_SOURCE_KEY; +use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_DISPLAY_KEY as MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY; +use codex_protocol::mcp_approval_meta::TOOL_TITLE_KEY as MCP_TOOL_APPROVAL_TOOL_TITLE_KEY; use codex_protocol::openai_models::InputModality; use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::McpInvocation; @@ -1097,20 +1111,6 @@ pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this se pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__"; const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again"; const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; -const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind"; -const MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; -const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist"; -const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session"; -const MCP_TOOL_APPROVAL_PERSIST_ALWAYS: &str = "always"; -const MCP_TOOL_APPROVAL_SOURCE_KEY: &str = "source"; -const MCP_TOOL_APPROVAL_SOURCE_CONNECTOR: &str = "connector"; -const MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: &str = "connector_id"; -const MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: &str = "connector_name"; -const MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY: &str = "connector_description"; -const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: &str = "tool_title"; -const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: &str = "tool_description"; -const MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params"; -const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display"; const MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_DEFAULT: &str = "mcp_tool_call__default"; const MCP_TOOL_CALL_ARC_MONITOR_CALLSITE_ALWAYS_ALLOW: &str = "mcp_tool_call__always_allow"; diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 225ee0cf70e0..a556e228ac95 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -1124,6 +1124,7 @@ async fn install_host_owned_codex_apps_manager(session: &Session, turn_context: /*host_owned_codex_apps_enabled*/ true, codex_mcp::ToolPluginProvenance::default(), auth.as_ref(), + /*elicitation_reviewer*/ None, ) .await; *session.services.mcp_connection_manager.write().await = manager; diff --git a/codex-rs/core/src/session/handlers.rs b/codex-rs/core/src/session/handlers.rs index 1dc68a69f3e3..03f29bac53fc 100644 --- a/codex-rs/core/src/session/handlers.rs +++ b/codex-rs/core/src/session/handlers.rs @@ -255,8 +255,11 @@ pub(super) async fn user_input_or_turn_inner( .set_responsesapi_client_metadata(responsesapi_client_metadata); } current_context.session_telemetry.user_prompt(&items); - sess.refresh_mcp_servers_if_requested(¤t_context) - .await; + sess.refresh_mcp_servers_if_requested( + ¤t_context, + Some(sess.mcp_elicitation_reviewer()), + ) + .await; let accepted_items = items.clone(); sess.spawn_task( Arc::clone(¤t_context), @@ -679,7 +682,8 @@ pub async fn review( let turn_context = sess.new_default_turn_with_sub_id(sub_id.clone()).await; sess.maybe_emit_unknown_model_warning_for_turn(turn_context.as_ref()) .await; - sess.refresh_mcp_servers_if_requested(&turn_context).await; + sess.refresh_mcp_servers_if_requested(&turn_context, Some(sess.mcp_elicitation_reviewer())) + .await; match resolve_review_request(review_request, &turn_context.cwd) { Ok(resolved) => { spawn_review_thread( diff --git a/codex-rs/core/src/session/mcp.rs b/codex-rs/core/src/session/mcp.rs index 38ade77ca3e2..ad04d30c66c0 100644 --- a/codex-rs/core/src/session/mcp.rs +++ b/codex-rs/core/src/session/mcp.rs @@ -1,6 +1,66 @@ use super::*; +use codex_mcp::ElicitationReviewRequest; +use codex_mcp::ElicitationReviewer; +use codex_mcp::ElicitationReviewerHandle; +use codex_protocol::config_types::ApprovalsReviewer; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY as MCP_ELICITATION_APPROVAL_KIND_KEY; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_MCP_TOOL_CALL as MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL; +use codex_protocol::mcp_approval_meta::APPROVALS_REVIEWER_KEY as MCP_ELICITATION_APPROVALS_REVIEWER_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_DESCRIPTION_KEY as MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_ID_KEY as MCP_ELICITATION_CONNECTOR_ID_KEY; +use codex_protocol::mcp_approval_meta::CONNECTOR_NAME_KEY as MCP_ELICITATION_CONNECTOR_NAME_KEY; +use codex_protocol::mcp_approval_meta::REQUEST_TYPE_APPROVAL_REQUEST as MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST; +use codex_protocol::mcp_approval_meta::REQUEST_TYPE_KEY as MCP_ELICITATION_REQUEST_TYPE_KEY; +use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_ELICITATION_TOOL_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY as MCP_ELICITATION_TOOL_NAME_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as MCP_ELICITATION_TOOL_PARAMS_KEY; +use codex_protocol::mcp_approval_meta::TOOL_TITLE_KEY as MCP_ELICITATION_TOOL_TITLE_KEY; +use rmcp::model::CreateElicitationRequestParams; +use rmcp::model::ElicitationAction; +use rmcp::model::Meta; +use serde_json::Map; + +const MCP_ELICITATION_DECLINE_MESSAGE_KEY: &str = "message"; + +#[derive(Debug, PartialEq)] +enum GuardianElicitationReview { + NotRequested, + Decline(&'static str), + ApprovalRequest(Box), +} + +struct GuardianMcpElicitationReviewer { + session: std::sync::Weak, +} + +impl GuardianMcpElicitationReviewer { + fn new(session: &Arc) -> Self { + Self { + session: Arc::downgrade(session), + } + } +} + +impl ElicitationReviewer for GuardianMcpElicitationReviewer { + fn review( + &self, + request: ElicitationReviewRequest, + ) -> BoxFuture<'static, anyhow::Result>> { + let session = self.session.clone(); + Box::pin(async move { + let Some(session) = session.upgrade() else { + return Ok(None); + }; + review_guardian_mcp_elicitation(session, request).await + }) + } +} impl Session { + pub(crate) fn mcp_elicitation_reviewer(self: &Arc) -> ElicitationReviewerHandle { + Arc::new(GuardianMcpElicitationReviewer::new(self)) + } + #[expect( clippy::await_holding_invalid_type, reason = "active turn checks and turn state updates must remain atomic" @@ -221,6 +281,7 @@ impl Session { turn_context: &TurnContext, mcp_servers: HashMap, store_mode: OAuthCredentialsStoreMode, + elicitation_reviewer: Option, ) { let auth = self.services.auth_manager.auth().await; let config = self.get_config().await; @@ -269,6 +330,7 @@ impl Session { host_owned_codex_apps_enabled, tool_plugin_provenance, auth.as_ref(), + elicitation_reviewer, ) .await; { @@ -290,7 +352,11 @@ impl Session { old_manager.shutdown().await; } - pub(crate) async fn refresh_mcp_servers_if_requested(&self, turn_context: &TurnContext) { + pub(crate) async fn refresh_mcp_servers_if_requested( + &self, + turn_context: &TurnContext, + elicitation_reviewer: Option, + ) { let refresh_config = { self.pending_mcp_server_refresh_config.lock().await.take() }; let Some(refresh_config) = refresh_config else { return; @@ -319,7 +385,7 @@ impl Session { } }; - self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode) + self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer) .await; } @@ -328,8 +394,9 @@ impl Session { turn_context: &TurnContext, mcp_servers: HashMap, store_mode: OAuthCredentialsStoreMode, + elicitation_reviewer: Option, ) { - self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode) + self.refresh_mcp_servers_inner(turn_context, mcp_servers, store_mode, elicitation_reviewer) .await; } @@ -350,3 +417,219 @@ impl Session { .cancel(); } } + +async fn review_guardian_mcp_elicitation( + session: Arc, + request: ElicitationReviewRequest, +) -> anyhow::Result> { + let Some((turn_context, _cancellation_token)) = + session.active_turn_context_and_cancellation_token().await + else { + return Ok(None); + }; + + if !crate::guardian::routes_approval_to_guardian(turn_context.as_ref()) { + return Ok(None); + } + + let guardian_request = match guardian_elicitation_review_request(&request) { + GuardianElicitationReview::NotRequested => return Ok(None), + GuardianElicitationReview::Decline(reason) => { + warn!( + server_name = %request.server_name, + request_id = %mcp_elicitation_request_id(&request.request_id), + reason, + "declining Guardian MCP elicitation before review" + ); + return Ok(Some(mcp_elicitation_decline_without_message())); + } + GuardianElicitationReview::ApprovalRequest(guardian_request) => *guardian_request, + }; + + let review_id = crate::guardian::new_guardian_review_id(); + let decision = crate::guardian::review_approval_request( + &session, + &turn_context, + review_id.clone(), + guardian_request, + /*retry_reason*/ None, + ) + .await; + Ok(Some( + mcp_elicitation_response_from_guardian_decision(session.as_ref(), &review_id, decision) + .await, + )) +} + +fn guardian_elicitation_review_request( + request: &ElicitationReviewRequest, +) -> GuardianElicitationReview { + let (meta, requested_schema) = match &request.elicitation { + CreateElicitationRequestParams::FormElicitationParams { + meta, + requested_schema, + .. + } => (meta, Some(requested_schema)), + CreateElicitationRequestParams::UrlElicitationParams { meta, .. } => { + return if meta_requests_approval_request(meta) { + GuardianElicitationReview::Decline( + "guardian MCP elicitation review only supports form elicitations", + ) + } else { + GuardianElicitationReview::NotRequested + }; + } + }; + + let Some(meta) = meta.as_ref().map(|meta| &meta.0) else { + return GuardianElicitationReview::NotRequested; + }; + if metadata_str(meta, MCP_ELICITATION_REQUEST_TYPE_KEY) + != Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) + { + return GuardianElicitationReview::NotRequested; + } + if metadata_str(meta, MCP_ELICITATION_APPROVAL_KIND_KEY) + != Some(MCP_ELICITATION_APPROVAL_KIND_MCP_TOOL_CALL) + { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation metadata must declare mcp_tool_call approval kind", + ); + } + if requested_schema.is_some_and(|schema| !schema.properties.is_empty()) { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation review only supports empty form schemas", + ); + } + + let Some(tool_name) = metadata_owned_string(meta, MCP_ELICITATION_TOOL_NAME_KEY) else { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation metadata must include a non-empty tool_name", + ); + }; + let arguments = match meta.get(MCP_ELICITATION_TOOL_PARAMS_KEY) { + Some(value @ Value::Object(_)) => Some(value.clone()), + Some(_) => { + return GuardianElicitationReview::Decline( + "guardian MCP elicitation tool_params must be an object", + ); + } + None => Some(Value::Object(Map::new())), + }; + + GuardianElicitationReview::ApprovalRequest(Box::new( + crate::guardian::GuardianApprovalRequest::McpToolCall { + id: format!( + "mcp_elicitation:{}:{}", + request.server_name, + mcp_elicitation_request_id(&request.request_id) + ), + server: request.server_name.clone(), + tool_name, + arguments, + connector_id: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_ID_KEY), + connector_name: metadata_owned_string(meta, MCP_ELICITATION_CONNECTOR_NAME_KEY), + connector_description: metadata_owned_string( + meta, + MCP_ELICITATION_CONNECTOR_DESCRIPTION_KEY, + ), + tool_title: metadata_owned_string(meta, MCP_ELICITATION_TOOL_TITLE_KEY), + tool_description: metadata_owned_string(meta, MCP_ELICITATION_TOOL_DESCRIPTION_KEY), + annotations: None, + }, + )) +} + +fn meta_requests_approval_request(meta: &Option) -> bool { + meta.as_ref() + .and_then(|meta| metadata_str(&meta.0, MCP_ELICITATION_REQUEST_TYPE_KEY)) + == Some(MCP_ELICITATION_REQUEST_TYPE_APPROVAL_REQUEST) +} + +fn metadata_str<'a>(meta: &'a Map, key: &str) -> Option<&'a str> { + meta.get(key).and_then(Value::as_str) +} + +fn metadata_owned_string(meta: &Map, key: &str) -> Option { + metadata_str(meta, key) + .map(str::trim) + .filter(|value| !value.is_empty()) + .map(ToOwned::to_owned) +} + +fn mcp_elicitation_request_id(id: &RequestId) -> String { + match id { + rmcp::model::NumberOrString::String(value) => value.to_string(), + rmcp::model::NumberOrString::Number(value) => value.to_string(), + } +} + +async fn mcp_elicitation_response_from_guardian_decision( + session: &Session, + review_id: &str, + decision: ReviewDecision, +) -> ElicitationResponse { + let denial_message = match decision { + ReviewDecision::Denied => { + Some(crate::guardian::guardian_rejection_message(session, review_id).await) + } + _ => None, + }; + mcp_elicitation_response_from_guardian_decision_parts(decision, denial_message) +} + +fn mcp_elicitation_response_from_guardian_decision_parts( + decision: ReviewDecision, + denial_message: Option, +) -> ElicitationResponse { + match decision { + ReviewDecision::Approved + | ReviewDecision::ApprovedForSession + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::NetworkPolicyAmendment { .. } => ElicitationResponse { + action: ElicitationAction::Accept, + content: Some(serde_json::json!({})), + meta: Some(mcp_elicitation_auto_meta()), + }, + ReviewDecision::Denied => mcp_elicitation_decline_with_message( + denial_message.unwrap_or_else(|| "Guardian denied this request.".to_string()), + ), + ReviewDecision::TimedOut => { + mcp_elicitation_decline_with_message(crate::guardian::guardian_timeout_message()) + } + ReviewDecision::Abort => ElicitationResponse { + action: ElicitationAction::Cancel, + content: None, + meta: Some(mcp_elicitation_auto_meta()), + }, + } +} + +fn mcp_elicitation_decline_with_message(message: String) -> ElicitationResponse { + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(serde_json::json!({ + MCP_ELICITATION_DECLINE_MESSAGE_KEY: message, + MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, + })), + } +} + +fn mcp_elicitation_decline_without_message() -> ElicitationResponse { + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(mcp_elicitation_auto_meta()), + } +} + +fn mcp_elicitation_auto_meta() -> serde_json::Value { + serde_json::json!({ + MCP_ELICITATION_APPROVALS_REVIEWER_KEY: ApprovalsReviewer::AutoReview, + }) +} + +#[cfg(test)] +#[path = "mcp_tests.rs"] +mod tests; diff --git a/codex-rs/core/src/session/mcp_tests.rs b/codex-rs/core/src/session/mcp_tests.rs new file mode 100644 index 000000000000..31b304faa531 --- /dev/null +++ b/codex-rs/core/src/session/mcp_tests.rs @@ -0,0 +1,212 @@ +use super::*; +use rmcp::model::BooleanSchema; +use rmcp::model::ElicitationSchema; +use rmcp::model::PrimitiveSchema; +use serde_json::json; + +fn meta(value: Value) -> Option { + let Value::Object(map) = value else { + panic!("metadata must be an object"); + }; + Some(Meta(map)) +} + +fn guardian_meta(tool_params: Option) -> Option { + let mut value = json!({ + "codex_approval_kind": "mcp_tool_call", + "codex_request_type": "approval_request", + "connector_id": "browser-use", + "connector_name": "Browser Use", + "tool_name": "access_browser_origin", + "tool_title": "Access browser origin", + }); + if let Some(tool_params) = tool_params { + value["tool_params"] = tool_params; + } + meta(value) +} + +fn form_request(meta: Option) -> ElicitationReviewRequest { + ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(7), + elicitation: CreateElicitationRequestParams::FormElicitationParams { + meta, + message: "Allow origin?".to_string(), + requested_schema: ElicitationSchema::builder() + .build() + .expect("schema should build"), + }, + } +} + +#[test] +fn guardian_elicitation_review_request_builds_mcp_tool_call() { + let request = form_request(guardian_meta(Some(json!({ + "origin": "https://example.com", + })))); + + let GuardianElicitationReview::ApprovalRequest(guardian_request) = + guardian_elicitation_review_request(&request) + else { + panic!("expected Guardian MCP tool call request"); + }; + let crate::guardian::GuardianApprovalRequest::McpToolCall { + id, + server, + tool_name, + arguments, + connector_id, + connector_name, + connector_description, + tool_title, + tool_description, + annotations, + } = *guardian_request + else { + panic!("expected Guardian MCP tool call request"); + }; + + assert_eq!(id, "mcp_elicitation:browser-use:7"); + assert_eq!(server, "browser-use"); + assert_eq!(tool_name, "access_browser_origin"); + assert_eq!(arguments, Some(json!({ "origin": "https://example.com" }))); + assert_eq!(connector_id.as_deref(), Some("browser-use")); + assert_eq!(connector_name.as_deref(), Some("Browser Use")); + assert_eq!(connector_description, None); + assert_eq!(tool_title.as_deref(), Some("Access browser origin")); + assert_eq!(tool_description, None); + assert_eq!(annotations, None); +} + +#[test] +fn guardian_elicitation_review_request_defaults_missing_tool_params() { + let request = form_request(guardian_meta(/*tool_params*/ None)); + + let GuardianElicitationReview::ApprovalRequest(guardian_request) = + guardian_elicitation_review_request(&request) + else { + panic!("expected Guardian MCP tool call request"); + }; + let crate::guardian::GuardianApprovalRequest::McpToolCall { arguments, .. } = *guardian_request + else { + panic!("expected Guardian MCP tool call request"); + }; + + assert_eq!(arguments, Some(json!({}))); +} + +#[test] +fn guardian_elicitation_review_request_requires_opt_in() { + let request = form_request(meta(json!({ + "codex_approval_kind": "mcp_tool_call", + "tool_name": "access_browser_origin", + }))); + + assert_eq!( + guardian_elicitation_review_request(&request), + GuardianElicitationReview::NotRequested + ); +} + +#[test] +fn guardian_elicitation_review_request_declines_unsupported_opt_in_shapes() { + let url_request = ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(8), + elicitation: CreateElicitationRequestParams::UrlElicitationParams { + meta: guardian_meta(Some(json!({}))), + message: "Open URL".to_string(), + url: "https://example.com".to_string(), + elicitation_id: "elicit-1".to_string(), + }, + }; + assert!(matches!( + guardian_elicitation_review_request(&url_request), + GuardianElicitationReview::Decline(_) + )); + + let non_empty_schema_request = ElicitationReviewRequest { + server_name: "browser-use".to_string(), + request_id: rmcp::model::NumberOrString::Number(9), + elicitation: CreateElicitationRequestParams::FormElicitationParams { + meta: guardian_meta(Some(json!({}))), + message: "Allow origin?".to_string(), + requested_schema: ElicitationSchema::builder() + .required_property("confirmed", PrimitiveSchema::Boolean(BooleanSchema::new())) + .build() + .expect("schema should build"), + }, + }; + assert!(matches!( + guardian_elicitation_review_request(&non_empty_schema_request), + GuardianElicitationReview::Decline(_) + )); + + let missing_tool_name_request = form_request(meta(json!({ + "codex_approval_kind": "mcp_tool_call", + "codex_request_type": "approval_request", + }))); + assert!(matches!( + guardian_elicitation_review_request(&missing_tool_name_request), + GuardianElicitationReview::Decline(_) + )); +} + +#[test] +fn guardian_decisions_map_to_elicitation_responses_without_session_state() { + assert_eq!( + mcp_elicitation_response_from_guardian_decision_parts( + ReviewDecision::Approved, + /*denial_message*/ None, + ), + ElicitationResponse { + action: ElicitationAction::Accept, + content: Some(json!({})), + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision_parts( + ReviewDecision::Denied, + Some("Denied by Guardian".to_string()), + ), + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + "message": "Denied by Guardian", + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision_parts( + ReviewDecision::TimedOut, + /*denial_message*/ None, + ), + ElicitationResponse { + action: ElicitationAction::Decline, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + "message": crate::guardian::guardian_timeout_message(), + })), + } + ); + assert_eq!( + mcp_elicitation_response_from_guardian_decision_parts( + ReviewDecision::Abort, + /*denial_message*/ None, + ), + ElicitationResponse { + action: ElicitationAction::Cancel, + content: None, + meta: Some(json!({ + "approvals_reviewer": ApprovalsReviewer::AutoReview, + })), + } + ); +} diff --git a/codex-rs/core/src/session/session.rs b/codex-rs/core/src/session/session.rs index 6ed0c3fc1a53..2ce01e81b3ce 100644 --- a/codex-rs/core/src/session/session.rs +++ b/codex-rs/core/src/session/session.rs @@ -976,6 +976,7 @@ impl Session { host_owned_codex_apps_enabled, tool_plugin_provenance, auth, + Some(sess.mcp_elicitation_reviewer()), ) .instrument(info_span!( "session_init.mcp_manager_init", diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index a5c5ec0ca44f..4cce19f6bcc4 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -5620,7 +5620,7 @@ async fn refresh_mcp_servers_is_deferred_until_next_turn() { ); session - .refresh_mcp_servers_if_requested(&turn_context) + .refresh_mcp_servers_if_requested(&turn_context, /*elicitation_reviewer*/ None) .await; assert!(old_token.is_cancelled()); diff --git a/codex-rs/core/src/session/turn.rs b/codex-rs/core/src/session/turn.rs index 7aa64aae57a7..effbf5372f52 100644 --- a/codex-rs/core/src/session/turn.rs +++ b/codex-rs/core/src/session/turn.rs @@ -240,6 +240,7 @@ pub(crate) async fn run_turn( turn_context.as_ref(), &cancellation_token, &mentioned_skills, + Some(sess.mcp_elicitation_reviewer()), ) .await; diff --git a/codex-rs/protocol/src/lib.rs b/codex-rs/protocol/src/lib.rs index 56248fd27e5f..63053159c6e4 100644 --- a/codex-rs/protocol/src/lib.rs +++ b/codex-rs/protocol/src/lib.rs @@ -15,6 +15,7 @@ pub mod error; pub mod exec_output; pub mod items; pub mod mcp; +pub mod mcp_approval_meta; pub mod memory_citation; pub mod models; pub mod network_policy; diff --git a/codex-rs/protocol/src/mcp_approval_meta.rs b/codex-rs/protocol/src/mcp_approval_meta.rs new file mode 100644 index 000000000000..7a8695a9b6a3 --- /dev/null +++ b/codex-rs/protocol/src/mcp_approval_meta.rs @@ -0,0 +1,19 @@ +pub const APPROVAL_KIND_KEY: &str = "codex_approval_kind"; +pub const APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; +pub const APPROVAL_KIND_TOOL_SUGGESTION: &str = "tool_suggestion"; +pub const REQUEST_TYPE_KEY: &str = "codex_request_type"; +pub const REQUEST_TYPE_APPROVAL_REQUEST: &str = "approval_request"; +pub const APPROVALS_REVIEWER_KEY: &str = "approvals_reviewer"; +pub const PERSIST_KEY: &str = "persist"; +pub const PERSIST_SESSION: &str = "session"; +pub const PERSIST_ALWAYS: &str = "always"; +pub const SOURCE_KEY: &str = "source"; +pub const SOURCE_CONNECTOR: &str = "connector"; +pub const CONNECTOR_ID_KEY: &str = "connector_id"; +pub const CONNECTOR_NAME_KEY: &str = "connector_name"; +pub const CONNECTOR_DESCRIPTION_KEY: &str = "connector_description"; +pub const TOOL_NAME_KEY: &str = "tool_name"; +pub const TOOL_TITLE_KEY: &str = "tool_title"; +pub const TOOL_DESCRIPTION_KEY: &str = "tool_description"; +pub const TOOL_PARAMS_KEY: &str = "tool_params"; +pub const TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display"; diff --git a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs index f307c28d72ca..95e8e8ca1e4a 100644 --- a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs +++ b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs @@ -12,6 +12,15 @@ use codex_app_server_protocol::McpServerElicitationRequest; use codex_app_server_protocol::McpServerElicitationRequestParams; use codex_app_server_protocol::RequestId as AppServerRequestId; use codex_protocol::ThreadId; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_KEY as APPROVAL_META_KIND_KEY; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_MCP_TOOL_CALL as APPROVAL_META_KIND_MCP_TOOL_CALL; +use codex_protocol::mcp_approval_meta::APPROVAL_KIND_TOOL_SUGGESTION as APPROVAL_META_KIND_TOOL_SUGGESTION; +use codex_protocol::mcp_approval_meta::PERSIST_ALWAYS as APPROVAL_PERSIST_ALWAYS_VALUE; +use codex_protocol::mcp_approval_meta::PERSIST_KEY as APPROVAL_PERSIST_KEY; +use codex_protocol::mcp_approval_meta::PERSIST_SESSION as APPROVAL_PERSIST_SESSION_VALUE; +use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_DISPLAY_KEY as APPROVAL_TOOL_PARAMS_DISPLAY_KEY; +use codex_protocol::mcp_approval_meta::TOOL_PARAMS_KEY as APPROVAL_TOOL_PARAMS_KEY; use codex_protocol::user_input::TextElement; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -55,19 +64,10 @@ const APPROVAL_ACCEPT_SESSION_VALUE: &str = "accept_session"; const APPROVAL_ACCEPT_ALWAYS_VALUE: &str = "accept_always"; const APPROVAL_DECLINE_VALUE: &str = "decline"; const APPROVAL_CANCEL_VALUE: &str = "cancel"; -const APPROVAL_META_KIND_KEY: &str = "codex_approval_kind"; -const APPROVAL_META_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; -const APPROVAL_META_KIND_TOOL_SUGGESTION: &str = "tool_suggestion"; -const APPROVAL_PERSIST_KEY: &str = "persist"; -const APPROVAL_PERSIST_SESSION_VALUE: &str = "session"; -const APPROVAL_PERSIST_ALWAYS_VALUE: &str = "always"; -const APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params"; -const APPROVAL_TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display"; const APPROVAL_TOOL_PARAM_DISPLAY_LIMIT: usize = 3; const APPROVAL_TOOL_PARAM_VALUE_TRUNCATE_GRAPHEMES: usize = 60; const TOOL_TYPE_KEY: &str = "tool_type"; const TOOL_ID_KEY: &str = "tool_id"; -const TOOL_NAME_KEY: &str = "tool_name"; const TOOL_SUGGEST_SUGGEST_TYPE_KEY: &str = "suggest_type"; const TOOL_SUGGEST_REASON_KEY: &str = "suggest_reason"; const TOOL_SUGGEST_INSTALL_URL_KEY: &str = "install_url";