From f06870f376d803c7f6d6526e2b83c2fe9d11ee91 Mon Sep 17 00:00:00 2001 From: Abhinav Vedmala Date: Sat, 2 May 2026 00:31:09 -0700 Subject: [PATCH] move MCP approval prompts back to MCP --- codex-rs/core/src/codex_delegate.rs | 5 +- codex-rs/core/src/codex_delegate_tests.rs | 4 +- .../core/src/guardian/approval_request.rs | 453 +----------------- codex-rs/core/src/guardian/mod.rs | 43 -- codex-rs/core/src/mcp_tool_call.rs | 359 +++++++++++++- codex-rs/core/src/mcp_tool_call_tests.rs | 205 +++++--- 6 files changed, 500 insertions(+), 569 deletions(-) diff --git a/codex-rs/core/src/codex_delegate.rs b/codex-rs/core/src/codex_delegate.rs index e3c23ab859..1dd8cf7180 100644 --- a/codex-rs/core/src/codex_delegate.rs +++ b/codex-rs/core/src/codex_delegate.rs @@ -32,12 +32,13 @@ use tokio_util::sync::CancellationToken; use crate::config::Config; use crate::environment_selection::ResolvedTurnEnvironments; use crate::guardian::GuardianApprovalRequest; -use crate::guardian::is_mcp_tool_approval_question_id; use crate::guardian::new_guardian_review_id; use crate::guardian::routes_approval_to_guardian; use crate::guardian::spawn_approval_request_review; use crate::mcp_tool_call::build_mcp_tool_approval_request; +use crate::mcp_tool_call::is_mcp_tool_approval_question_id; use crate::mcp_tool_call::lookup_mcp_tool_metadata; +use crate::mcp_tool_call::mcp_tool_approval_compat_response; use crate::session::Codex; use crate::session::CodexSpawnArgs; use crate::session::CodexSpawnOk; @@ -707,7 +708,7 @@ async fn maybe_auto_review_mcp_request_user_input( Some(&review_cancel), ) .await; - approval_request.mcp_tool_approval_compat_response(question, decision) + mcp_tool_approval_compat_response(&approval_request, question, decision) } async fn handle_request_permissions( diff --git a/codex-rs/core/src/codex_delegate_tests.rs b/codex-rs/core/src/codex_delegate_tests.rs index 9260acaeca..454e4aa660 100644 --- a/codex-rs/core/src/codex_delegate_tests.rs +++ b/codex-rs/core/src/codex_delegate_tests.rs @@ -1,6 +1,6 @@ use super::*; -use crate::guardian::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; -use crate::guardian::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX; +use crate::mcp_tool_call::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; +use crate::mcp_tool_call::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX; use async_channel::bounded; use codex_protocol::config_types::ApprovalsReviewer; use codex_protocol::models::NetworkPermissions; diff --git a/codex-rs/core/src/guardian/approval_request.rs b/codex-rs/core/src/guardian/approval_request.rs index 4002dc91ea..8deb4720c0 100644 --- a/codex-rs/core/src/guardian/approval_request.rs +++ b/codex-rs/core/src/guardian/approval_request.rs @@ -16,18 +16,12 @@ use codex_protocol::protocol::FileChange; use codex_protocol::protocol::ReviewDecision; use codex_protocol::request_permissions::RequestPermissionProfile; use codex_protocol::request_permissions::RequestPermissionsEvent; -use codex_protocol::request_user_input::RequestUserInputAnswer; -use codex_protocol::request_user_input::RequestUserInputQuestion; -use codex_protocol::request_user_input::RequestUserInputQuestionOption; -use codex_protocol::request_user_input::RequestUserInputResponse; use codex_utils_absolute_path::AbsolutePathBuf; use serde::Serialize; use serde_json::Value; use super::GUARDIAN_MAX_ACTION_STRING_TOKENS; use super::prompt::guardian_truncate_text; -use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam; -use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template; use crate::tools::hook_names::HookToolName; use crate::tools::sandboxing::PermissionRequestPayload; @@ -105,41 +99,6 @@ pub(crate) enum GuardianApprovalRequest { }, } -#[derive(Clone, Copy, Debug, Eq, PartialEq)] -pub(crate) struct McpToolApprovalPromptOptions { - pub(crate) allow_session_remember: bool, - pub(crate) allow_persistent_approval: bool, -} - -#[derive(Clone, Debug, PartialEq)] -pub(crate) struct McpToolApprovalPrompt { - pub(crate) question: RequestUserInputQuestion, - pub(crate) message_override: Option, - pub(crate) tool_params: Option, - pub(crate) tool_params_display: Option>, -} - -pub(crate) const MCP_TOOL_APPROVAL_ACCEPT: &str = "Allow"; -pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this session"; -pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again"; -pub(crate) const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; -pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__"; -pub(crate) const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; -pub(crate) const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind"; -pub(crate) const MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session"; -pub(crate) const MCP_TOOL_APPROVAL_PERSIST_ALWAYS: &str = "always"; -pub(crate) const MCP_TOOL_APPROVAL_SOURCE_KEY: &str = "source"; -pub(crate) const MCP_TOOL_APPROVAL_SOURCE_CONNECTOR: &str = "connector"; -pub(crate) const MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: &str = "connector_id"; -pub(crate) const MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: &str = "connector_name"; -pub(crate) const MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY: &str = "connector_description"; -pub(crate) const MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: &str = "tool_title"; -pub(crate) const MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: &str = "tool_description"; -pub(crate) const MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params"; -pub(crate) const MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display"; - impl GuardianApprovalRequest { pub(crate) fn permission_request_payload(&self) -> Option { match self { @@ -350,311 +309,6 @@ impl GuardianApprovalRequest { Self::Execve { .. } => None, } } - - pub(crate) fn mcp_tool_approval_prompt( - &self, - question_id: String, - prompt_options: McpToolApprovalPromptOptions, - monitor_reason: Option<&str>, - ) -> Option { - let Self::McpToolCall { - server, - tool_name, - arguments, - connector_id, - connector_name, - tool_title, - .. - } = self - else { - return None; - }; - - let rendered_template = render_mcp_tool_approval_template( - server, - connector_id.as_deref(), - connector_name.as_deref(), - tool_title.as_deref(), - arguments.as_ref(), - ); - let tool_params_display = rendered_template - .as_ref() - .map(|rendered_template| rendered_template.tool_params_display.clone()) - .or_else(|| build_mcp_tool_approval_display_params(arguments.as_ref())); - let question_override = rendered_template - .as_ref() - .map(|rendered_template| rendered_template.question.as_str()); - let mut question = build_mcp_tool_approval_question( - question_id, - server, - tool_name, - connector_name.as_deref(), - prompt_options, - question_override, - ); - question.question = mcp_tool_approval_question_text(question.question, monitor_reason); - - Some(McpToolApprovalPrompt { - question, - message_override: rendered_template.as_ref().and_then(|rendered_template| { - monitor_reason - .is_none() - .then_some(rendered_template.elicitation_message.clone()) - }), - tool_params: rendered_template - .as_ref() - .and_then(|rendered_template| rendered_template.tool_params.clone()) - .or_else(|| arguments.clone()), - tool_params_display, - }) - } - - pub(crate) fn mcp_tool_approval_elicitation_meta( - &self, - tool_params: Option<&Value>, - tool_params_display: Option<&[RenderedMcpToolApprovalParam]>, - prompt_options: McpToolApprovalPromptOptions, - ) -> Option { - let Self::McpToolCall { - server, - connector_id, - connector_name, - connector_description, - tool_title, - tool_description, - .. - } = self - else { - return None; - }; - - let mut meta = serde_json::Map::new(); - meta.insert( - MCP_TOOL_APPROVAL_KIND_KEY.to_string(), - serde_json::Value::String(MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL.to_string()), - ); - match ( - prompt_options.allow_session_remember, - prompt_options.allow_persistent_approval, - ) { - (true, true) => { - meta.insert( - MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), - serde_json::json!([ - MCP_TOOL_APPROVAL_PERSIST_SESSION, - MCP_TOOL_APPROVAL_PERSIST_ALWAYS, - ]), - ); - } - (true, false) => { - meta.insert( - MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), - serde_json::Value::String(MCP_TOOL_APPROVAL_PERSIST_SESSION.to_string()), - ); - } - (false, true) => { - meta.insert( - MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), - serde_json::Value::String(MCP_TOOL_APPROVAL_PERSIST_ALWAYS.to_string()), - ); - } - (false, false) => {} - } - if let Some(tool_title) = tool_title.as_ref() { - meta.insert( - MCP_TOOL_APPROVAL_TOOL_TITLE_KEY.to_string(), - serde_json::Value::String(tool_title.clone()), - ); - } - if let Some(tool_description) = tool_description.as_ref() { - meta.insert( - MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY.to_string(), - serde_json::Value::String(tool_description.clone()), - ); - } - if server == "codex_apps" - && (connector_id.is_some() - || connector_name.is_some() - || connector_description.is_some()) - { - meta.insert( - MCP_TOOL_APPROVAL_SOURCE_KEY.to_string(), - serde_json::Value::String(MCP_TOOL_APPROVAL_SOURCE_CONNECTOR.to_string()), - ); - if let Some(connector_id) = connector_id.as_deref() { - meta.insert( - MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY.to_string(), - serde_json::Value::String(connector_id.to_string()), - ); - } - if let Some(connector_name) = connector_name.as_ref() { - meta.insert( - MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY.to_string(), - serde_json::Value::String(connector_name.clone()), - ); - } - if let Some(connector_description) = connector_description.as_ref() { - meta.insert( - MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY.to_string(), - serde_json::Value::String(connector_description.clone()), - ); - } - } - if let Some(tool_params) = tool_params { - meta.insert( - MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY.to_string(), - tool_params.clone(), - ); - } - if let Some(tool_params_display) = tool_params_display - && let Ok(tool_params_display) = serde_json::to_value(tool_params_display) - { - meta.insert( - MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY.to_string(), - tool_params_display, - ); - } - - (!meta.is_empty()).then_some(serde_json::Value::Object(meta)) - } - - pub(crate) fn mcp_tool_approval_compat_response( - &self, - question: &RequestUserInputQuestion, - decision: ReviewDecision, - ) -> Option { - let Self::McpToolCall { .. } = self else { - return None; - }; - - let selected_label = match decision { - ReviewDecision::ApprovedForSession => question - .options - .as_ref() - .and_then(|options| { - options - .iter() - .find(|option| option.label == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION) - }) - .map(|option| option.label.clone()) - .unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()), - ReviewDecision::Approved - | ReviewDecision::ApprovedExecpolicyAmendment { .. } - | ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(), - ReviewDecision::Denied | ReviewDecision::TimedOut | ReviewDecision::Abort => { - MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string() - } - }; - - Some(RequestUserInputResponse { - answers: HashMap::from([( - question.id.clone(), - RequestUserInputAnswer { - answers: vec![selected_label], - }, - )]), - }) - } -} - -pub(crate) fn mcp_tool_approval_question_id(call_id: &str) -> String { - format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}") -} - -pub(crate) fn is_mcp_tool_approval_question_id(question_id: &str) -> bool { - question_id - .strip_prefix(MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX) - .is_some_and(|suffix| suffix.starts_with('_')) -} - -fn build_mcp_tool_approval_question( - question_id: String, - server: &str, - tool_name: &str, - connector_name: Option<&str>, - prompt_options: McpToolApprovalPromptOptions, - question_override: Option<&str>, -) -> RequestUserInputQuestion { - let question = question_override - .map(ToString::to_string) - .unwrap_or_else(|| { - build_mcp_tool_approval_fallback_message(server, tool_name, connector_name) - }); - let question = format!("{}?", question.trim_end_matches('?')); - - let mut options = vec![RequestUserInputQuestionOption { - label: MCP_TOOL_APPROVAL_ACCEPT.to_string(), - description: "Run the tool and continue.".to_string(), - }]; - if prompt_options.allow_session_remember { - options.push(RequestUserInputQuestionOption { - label: MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(), - description: "Run the tool and remember this choice for this session.".to_string(), - }); - } - if prompt_options.allow_persistent_approval { - options.push(RequestUserInputQuestionOption { - label: MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(), - description: "Run the tool and remember this choice for future tool calls.".to_string(), - }); - } - options.push(RequestUserInputQuestionOption { - label: MCP_TOOL_APPROVAL_CANCEL.to_string(), - description: "Cancel this tool call.".to_string(), - }); - - RequestUserInputQuestion { - id: question_id, - header: "Approve app tool call?".to_string(), - question, - is_other: false, - is_secret: false, - options: Some(options), - } -} - -fn build_mcp_tool_approval_fallback_message( - server: &str, - tool_name: &str, - connector_name: Option<&str>, -) -> String { - let actor = connector_name - .map(str::trim) - .filter(|name| !name.is_empty()) - .map(ToString::to_string) - .unwrap_or_else(|| { - if server == "codex_apps" { - "this app".to_string() - } else { - format!("the {server} MCP server") - } - }); - format!("Allow {actor} to run tool \"{tool_name}\"?") -} - -fn mcp_tool_approval_question_text(question: String, monitor_reason: Option<&str>) -> String { - match monitor_reason.map(str::trim) { - Some(reason) if !reason.is_empty() => { - format!("Tool call needs your approval. Reason: {reason}") - } - _ => question, - } -} - -fn build_mcp_tool_approval_display_params( - tool_params: Option<&Value>, -) -> Option> { - let tool_params = tool_params?.as_object()?; - let mut display_params = tool_params - .iter() - .map(|(name, value)| RenderedMcpToolApprovalParam { - name: name.clone(), - value: value.clone(), - display_name: name.clone(), - }) - .collect::>(); - display_params.sort_by(|left, right| left.name.cmp(&right.name)); - Some(display_params) } #[derive(Debug, Clone, PartialEq, Serialize)] @@ -959,7 +613,9 @@ pub(crate) fn guardian_approval_request_to_json( } } -pub(crate) fn guardian_assessment_action(action: &GuardianApprovalRequest) -> GuardianAssessmentAction { +pub(crate) fn guardian_assessment_action( + action: &GuardianApprovalRequest, +) -> GuardianAssessmentAction { match action { GuardianApprovalRequest::Shell { command, cwd, .. } => { command_assessment_action(GuardianCommandSource::Shell, command, cwd) @@ -980,10 +636,12 @@ pub(crate) fn guardian_assessment_action(action: &GuardianApprovalRequest) -> Gu argv: argv.clone(), cwd: cwd.clone(), }, - GuardianApprovalRequest::ApplyPatch { cwd, files, .. } => GuardianAssessmentAction::ApplyPatch { - cwd: cwd.clone(), - files: files.clone(), - }, + GuardianApprovalRequest::ApplyPatch { cwd, files, .. } => { + GuardianAssessmentAction::ApplyPatch { + cwd: cwd.clone(), + files: files.clone(), + } + } GuardianApprovalRequest::NetworkAccess { id: _id, turn_id: _turn_id, @@ -1024,7 +682,9 @@ pub(crate) fn guardian_assessment_action(action: &GuardianApprovalRequest) -> Gu } } -pub(crate) fn guardian_reviewed_action(request: &GuardianApprovalRequest) -> GuardianReviewedAction { +pub(crate) fn guardian_reviewed_action( + request: &GuardianApprovalRequest, +) -> GuardianReviewedAction { match request { GuardianApprovalRequest::Shell { sandbox_permissions, @@ -1076,7 +736,9 @@ pub(crate) fn guardian_reviewed_action(request: &GuardianApprovalRequest) -> Gua connector_name: connector_name.clone(), tool_title: tool_title.clone(), }, - GuardianApprovalRequest::RequestPermissions { .. } => GuardianReviewedAction::RequestPermissions {}, + GuardianApprovalRequest::RequestPermissions { .. } => { + GuardianReviewedAction::RequestPermissions {} + } } } @@ -1124,7 +786,6 @@ pub(crate) fn format_guardian_action_pretty( mod tests { use super::*; use codex_protocol::protocol::FileChange; - use codex_protocol::protocol::ReviewDecision; use codex_utils_absolute_path::test_support::PathBufExt; use codex_utils_absolute_path::test_support::test_path_buf; use pretty_assertions::assert_eq; @@ -1296,88 +957,4 @@ mod tests { ]) ); } - - #[test] - fn mcp_tool_approval_compat_response_uses_session_label_when_present() { - let request = GuardianApprovalRequest::McpToolCall { - id: "call-1".to_string(), - server: "custom_server".to_string(), - tool_name: "dangerous_tool".to_string(), - hook_tool_name: "custom_server__dangerous_tool".to_string(), - arguments: None, - connector_id: None, - connector_name: None, - connector_description: None, - tool_title: None, - tool_description: None, - annotations: None, - }; - let question = RequestUserInputQuestion { - id: "q-1".to_string(), - header: "Approve app tool call?".to_string(), - question: "Allow this app tool?".to_string(), - is_other: false, - is_secret: false, - options: Some(vec![RequestUserInputQuestionOption { - label: MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(), - description: "Remember until session ends".to_string(), - }]), - }; - - let response = request - .mcp_tool_approval_compat_response(&question, ReviewDecision::ApprovedForSession) - .expect("compat response"); - - assert_eq!( - response.answers.get("q-1"), - Some(&RequestUserInputAnswer { - answers: vec![MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string()], - }) - ); - } - - #[test] - fn mcp_tool_approval_compat_response_uses_synthetic_decline_for_abort() { - let request = GuardianApprovalRequest::McpToolCall { - id: "call-1".to_string(), - server: "custom_server".to_string(), - tool_name: "dangerous_tool".to_string(), - hook_tool_name: "custom_server__dangerous_tool".to_string(), - arguments: None, - connector_id: None, - connector_name: None, - connector_description: None, - tool_title: None, - tool_description: None, - annotations: None, - }; - let question = RequestUserInputQuestion { - id: "q-1".to_string(), - header: "Approve app tool call?".to_string(), - question: "Allow this app tool?".to_string(), - is_other: false, - is_secret: false, - options: None, - }; - - let response = request - .mcp_tool_approval_compat_response(&question, ReviewDecision::Abort) - .expect("compat response"); - - assert_eq!( - response.answers.get("q-1"), - Some(&RequestUserInputAnswer { - answers: vec![MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()], - }) - ); - } - - #[test] - fn mcp_tool_approval_question_id_helpers_round_trip() { - let question_id = mcp_tool_approval_question_id("call-1"); - - assert_eq!(question_id, "mcp_tool_call_approval_call-1"); - assert!(is_mcp_tool_approval_question_id(&question_id)); - assert!(!is_mcp_tool_approval_question_id("other_question")); - } } diff --git a/codex-rs/core/src/guardian/mod.rs b/codex-rs/core/src/guardian/mod.rs index 8bd2621546..256a616b97 100644 --- a/codex-rs/core/src/guardian/mod.rs +++ b/codex-rs/core/src/guardian/mod.rs @@ -26,50 +26,7 @@ use serde::Serialize; pub(crate) use approval_request::GuardianApprovalRequest; pub(crate) use approval_request::GuardianMcpAnnotations; pub(crate) use approval_request::GuardianNetworkAccessTrigger; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_ACCEPT; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_CANCEL; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_KIND_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_PERSIST_ALWAYS; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_PERSIST_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_PERSIST_SESSION; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_SOURCE_CONNECTOR; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_SOURCE_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY; -#[allow(unused_imports)] -pub(crate) use approval_request::MCP_TOOL_APPROVAL_TOOL_TITLE_KEY; -pub(crate) use approval_request::McpToolApprovalPromptOptions; pub(crate) use approval_request::guardian_approval_request_to_json; -pub(crate) use approval_request::is_mcp_tool_approval_question_id; -pub(crate) use approval_request::mcp_tool_approval_question_id; pub(crate) use review::guardian_rejection_message; pub(crate) use review::guardian_timeout_message; pub(crate) use review::is_guardian_reviewer_source; diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index a63051186c..60cfc03c9f 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -19,24 +19,16 @@ use crate::config::load_global_mcp_servers; use crate::connectors; use crate::guardian::GuardianApprovalRequest; use crate::guardian::GuardianMcpAnnotations; -use crate::guardian::MCP_TOOL_APPROVAL_ACCEPT; -use crate::guardian::MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER; -use crate::guardian::MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION; -use crate::guardian::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; -use crate::guardian::MCP_TOOL_APPROVAL_PERSIST_ALWAYS; -use crate::guardian::MCP_TOOL_APPROVAL_PERSIST_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_PERSIST_SESSION; -use crate::guardian::McpToolApprovalPromptOptions; use crate::guardian::guardian_approval_request_to_json; use crate::guardian::guardian_rejection_message; use crate::guardian::guardian_timeout_message; -use crate::guardian::mcp_tool_approval_question_id; use crate::guardian::new_guardian_review_id; use crate::guardian::review_approval_request; use crate::guardian::routes_approval_to_guardian; use crate::hook_runtime::run_permission_request_hooks; use crate::mcp_openai_file::rewrite_mcp_tool_arguments_for_openai_files; use crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam; +use crate::mcp_tool_approval_templates::render_mcp_tool_approval_template; use crate::session::session::Session; use crate::session::turn_context::TurnContext; use codex_analytics::AppInvocation; @@ -61,6 +53,7 @@ use codex_protocol::protocol::ReviewDecision; use codex_protocol::request_user_input::RequestUserInputAnswer; use codex_protocol::request_user_input::RequestUserInputArgs; use codex_protocol::request_user_input::RequestUserInputQuestion; +use codex_protocol::request_user_input::RequestUserInputQuestionOption; use codex_protocol::request_user_input::RequestUserInputResponse; use codex_rmcp_client::ElicitationAction; use codex_rmcp_client::ElicitationResponse; @@ -882,6 +875,45 @@ fn with_mcp_tool_call_thread_id_meta( } } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +struct McpToolApprovalPromptOptions { + allow_session_remember: bool, + allow_persistent_approval: bool, +} + +#[derive(Clone, Debug, PartialEq)] +struct McpToolApprovalPrompt { + question: RequestUserInputQuestion, + message_override: Option, + tool_params: Option, + tool_params_display: Option>, +} + +pub(crate) const MCP_TOOL_APPROVAL_ACCEPT: &str = "Allow"; +pub(crate) const MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION: &str = "Allow for this session"; +const MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER: &str = "Allow and don't ask me again"; +const MCP_TOOL_APPROVAL_CANCEL: &str = "Cancel"; +// Internal-only token used when guardian auto-reviews delegated MCP approvals on the +// RequestUserInput compatibility path. That legacy MCP prompt has allow/cancel labels but no +// real "Decline" answer, so this lets guardian denials round-trip distinctly from user cancel. +// This is not a user-facing option. +pub(crate) const MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC: &str = "__codex_mcp_decline__"; +pub(crate) const MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX: &str = "mcp_tool_call_approval"; +const MCP_TOOL_APPROVAL_KIND_KEY: &str = "codex_approval_kind"; +const MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL: &str = "mcp_tool_call"; +pub(crate) const MCP_TOOL_APPROVAL_PERSIST_KEY: &str = "persist"; +pub(crate) const MCP_TOOL_APPROVAL_PERSIST_SESSION: &str = "session"; +pub(crate) 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"; + struct McpToolApprovalElicitationRequest<'a> { server: &'a str, approval_request: &'a GuardianApprovalRequest, @@ -902,6 +934,309 @@ struct McpToolApprovalKey { tool_name: String, } +pub(crate) fn mcp_tool_approval_question_id(call_id: &str) -> String { + format!("{MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX}_{call_id}") +} + +pub(crate) fn is_mcp_tool_approval_question_id(question_id: &str) -> bool { + question_id + .strip_prefix(MCP_TOOL_APPROVAL_QUESTION_ID_PREFIX) + .is_some_and(|suffix| suffix.starts_with('_')) +} + +fn mcp_tool_approval_prompt( + approval_request: &GuardianApprovalRequest, + question_id: String, + prompt_options: McpToolApprovalPromptOptions, + monitor_reason: Option<&str>, +) -> Option { + let GuardianApprovalRequest::McpToolCall { + server, + tool_name, + arguments, + connector_id, + connector_name, + tool_title, + .. + } = approval_request + else { + return None; + }; + + let rendered_template = render_mcp_tool_approval_template( + server, + connector_id.as_deref(), + connector_name.as_deref(), + tool_title.as_deref(), + arguments.as_ref(), + ); + let tool_params_display = rendered_template + .as_ref() + .map(|rendered_template| rendered_template.tool_params_display.clone()) + .or_else(|| build_mcp_tool_approval_display_params(arguments.as_ref())); + let question_override = rendered_template + .as_ref() + .map(|rendered_template| rendered_template.question.as_str()); + let mut question = build_mcp_tool_approval_question( + question_id, + server, + tool_name, + connector_name.as_deref(), + prompt_options, + question_override, + ); + question.question = mcp_tool_approval_question_text(question.question, monitor_reason); + + Some(McpToolApprovalPrompt { + question, + message_override: rendered_template.as_ref().and_then(|rendered_template| { + monitor_reason + .is_none() + .then_some(rendered_template.elicitation_message.clone()) + }), + tool_params: rendered_template + .as_ref() + .and_then(|rendered_template| rendered_template.tool_params.clone()) + .or_else(|| arguments.clone()), + tool_params_display, + }) +} + +fn mcp_tool_approval_elicitation_meta( + approval_request: &GuardianApprovalRequest, + tool_params: Option<&JsonValue>, + tool_params_display: Option<&[RenderedMcpToolApprovalParam]>, + prompt_options: McpToolApprovalPromptOptions, +) -> Option { + let GuardianApprovalRequest::McpToolCall { + server, + connector_id, + connector_name, + connector_description, + tool_title, + tool_description, + .. + } = approval_request + else { + return None; + }; + + let mut meta = serde_json::Map::new(); + meta.insert( + MCP_TOOL_APPROVAL_KIND_KEY.to_string(), + JsonValue::String(MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL.to_string()), + ); + match ( + prompt_options.allow_session_remember, + prompt_options.allow_persistent_approval, + ) { + (true, true) => { + meta.insert( + MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), + serde_json::json!([ + MCP_TOOL_APPROVAL_PERSIST_SESSION, + MCP_TOOL_APPROVAL_PERSIST_ALWAYS, + ]), + ); + } + (true, false) => { + meta.insert( + MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), + JsonValue::String(MCP_TOOL_APPROVAL_PERSIST_SESSION.to_string()), + ); + } + (false, true) => { + meta.insert( + MCP_TOOL_APPROVAL_PERSIST_KEY.to_string(), + JsonValue::String(MCP_TOOL_APPROVAL_PERSIST_ALWAYS.to_string()), + ); + } + (false, false) => {} + } + if let Some(tool_title) = tool_title.as_ref() { + meta.insert( + MCP_TOOL_APPROVAL_TOOL_TITLE_KEY.to_string(), + JsonValue::String(tool_title.clone()), + ); + } + if let Some(tool_description) = tool_description.as_ref() { + meta.insert( + MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY.to_string(), + JsonValue::String(tool_description.clone()), + ); + } + if server == "codex_apps" + && (connector_id.is_some() || connector_name.is_some() || connector_description.is_some()) + { + meta.insert( + MCP_TOOL_APPROVAL_SOURCE_KEY.to_string(), + JsonValue::String(MCP_TOOL_APPROVAL_SOURCE_CONNECTOR.to_string()), + ); + if let Some(connector_id) = connector_id.as_deref() { + meta.insert( + MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY.to_string(), + JsonValue::String(connector_id.to_string()), + ); + } + if let Some(connector_name) = connector_name.as_ref() { + meta.insert( + MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY.to_string(), + JsonValue::String(connector_name.clone()), + ); + } + if let Some(connector_description) = connector_description.as_ref() { + meta.insert( + MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY.to_string(), + JsonValue::String(connector_description.clone()), + ); + } + } + if let Some(tool_params) = tool_params { + meta.insert( + MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY.to_string(), + tool_params.clone(), + ); + } + if let Some(tool_params_display) = tool_params_display + && let Ok(tool_params_display) = serde_json::to_value(tool_params_display) + { + meta.insert( + MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY.to_string(), + tool_params_display, + ); + } + + (!meta.is_empty()).then_some(JsonValue::Object(meta)) +} + +pub(crate) fn mcp_tool_approval_compat_response( + approval_request: &GuardianApprovalRequest, + question: &RequestUserInputQuestion, + decision: ReviewDecision, +) -> Option { + let GuardianApprovalRequest::McpToolCall { .. } = approval_request else { + return None; + }; + + let selected_label = match decision { + ReviewDecision::ApprovedForSession => question + .options + .as_ref() + .and_then(|options| { + options + .iter() + .find(|option| option.label == MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION) + }) + .map(|option| option.label.clone()) + .unwrap_or_else(|| MCP_TOOL_APPROVAL_ACCEPT.to_string()), + ReviewDecision::Approved + | ReviewDecision::ApprovedExecpolicyAmendment { .. } + | ReviewDecision::NetworkPolicyAmendment { .. } => MCP_TOOL_APPROVAL_ACCEPT.to_string(), + ReviewDecision::Denied | ReviewDecision::TimedOut | ReviewDecision::Abort => { + MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string() + } + }; + + Some(RequestUserInputResponse { + answers: HashMap::from([( + question.id.clone(), + RequestUserInputAnswer { + answers: vec![selected_label], + }, + )]), + }) +} + +fn build_mcp_tool_approval_question( + question_id: String, + server: &str, + tool_name: &str, + connector_name: Option<&str>, + prompt_options: McpToolApprovalPromptOptions, + question_override: Option<&str>, +) -> RequestUserInputQuestion { + let question = question_override + .map(ToString::to_string) + .unwrap_or_else(|| { + build_mcp_tool_approval_fallback_message(server, tool_name, connector_name) + }); + let question = format!("{}?", question.trim_end_matches('?')); + + let mut options = vec![RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_ACCEPT.to_string(), + description: "Run the tool and continue.".to_string(), + }]; + if prompt_options.allow_session_remember { + options.push(RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(), + description: "Run the tool and remember this choice for this session.".to_string(), + }); + } + if prompt_options.allow_persistent_approval { + options.push(RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_ACCEPT_AND_REMEMBER.to_string(), + description: "Run the tool and remember this choice for future tool calls.".to_string(), + }); + } + options.push(RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_CANCEL.to_string(), + description: "Cancel this tool call.".to_string(), + }); + + RequestUserInputQuestion { + id: question_id, + header: "Approve app tool call?".to_string(), + question, + is_other: false, + is_secret: false, + options: Some(options), + } +} + +fn build_mcp_tool_approval_fallback_message( + server: &str, + tool_name: &str, + connector_name: Option<&str>, +) -> String { + let actor = connector_name + .map(str::trim) + .filter(|name| !name.is_empty()) + .map(ToString::to_string) + .unwrap_or_else(|| { + if server == "codex_apps" { + "this app".to_string() + } else { + format!("the {server} MCP server") + } + }); + format!("Allow {actor} to run tool \\\"{tool_name}\\\"?") +} + +fn mcp_tool_approval_question_text(question: String, monitor_reason: Option<&str>) -> String { + match monitor_reason.map(str::trim) { + Some(reason) if !reason.is_empty() => { + format!("Tool call needs your approval. Reason: {reason}") + } + _ => question, + } +} + +fn build_mcp_tool_approval_display_params( + tool_params: Option<&JsonValue>, +) -> Option> { + let tool_params = tool_params?.as_object()?; + let mut display_params = tool_params + .iter() + .map(|(name, value)| RenderedMcpToolApprovalParam { + name: name.clone(), + value: value.clone(), + display_name: name.clone(), + }) + .collect::>(); + display_params.sort_by(|left, right| left.name.cmp(&right.name)); + Some(display_params) +} + fn mcp_tool_approval_prompt_options( session_approval_key: Option<&McpToolApprovalKey>, persistent_approval_key: Option<&McpToolApprovalKey>, @@ -1029,7 +1364,8 @@ async fn maybe_request_mcp_tool_approval( tool_call_mcp_elicitation_enabled, ); let question_id = mcp_tool_approval_question_id(call_id); - let Some(prompt) = approval_request.mcp_tool_approval_prompt( + let Some(prompt) = mcp_tool_approval_prompt( + &approval_request, question_id.clone(), prompt_options, monitor_reason.as_deref(), @@ -1358,7 +1694,8 @@ fn build_mcp_tool_approval_elicitation_request( turn_id: Some(turn_context.sub_id.clone()), server_name: request.server.to_string(), request: McpServerElicitationRequest::Form { - meta: request.approval_request.mcp_tool_approval_elicitation_meta( + meta: mcp_tool_approval_elicitation_meta( + request.approval_request, request.tool_params, request.tool_params_display, request.prompt_options, diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index a48be0d030..bbfe891127 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -2,19 +2,6 @@ use super::*; use crate::config::ConfigBuilder; use crate::guardian::GuardianApprovalRequest; use crate::guardian::GuardianMcpAnnotations; -use crate::guardian::MCP_TOOL_APPROVAL_CANCEL; -use crate::guardian::MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC; -use crate::guardian::MCP_TOOL_APPROVAL_KIND_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL; -use crate::guardian::MCP_TOOL_APPROVAL_SOURCE_CONNECTOR; -use crate::guardian::MCP_TOOL_APPROVAL_SOURCE_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY; -use crate::guardian::MCP_TOOL_APPROVAL_TOOL_TITLE_KEY; use crate::session::tests::make_session_and_context; use crate::session::tests::make_session_and_context_with_rx; use crate::state::ActiveTurn; @@ -320,22 +307,90 @@ fn approval_question_text_prepends_safety_reason() { /*metadata*/ None, ); assert_eq!( - request - .mcp_tool_approval_prompt( - "q".to_string(), - prompt_options( - /*allow_session_remember*/ false, - /*allow_persistent_approval*/ false, - ), - Some("This tool may contact an external system."), - ) - .expect("mcp approval prompt") - .question - .question, + mcp_tool_approval_prompt( + &request, + "q".to_string(), + prompt_options( + /*allow_session_remember*/ false, /*allow_persistent_approval*/ false, + ), + Some("This tool may contact an external system."), + ) + .expect("mcp approval prompt") + .question + .question, "Tool call needs your approval. Reason: This tool may contact an external system." ); } +#[test] +fn mcp_tool_approval_compat_response_uses_session_label_when_present() { + let request = approval_prompt_request( + "custom_server", + "dangerous_tool", + /*arguments*/ None, + /*metadata*/ None, + ); + let question = RequestUserInputQuestion { + id: "q-1".to_string(), + header: "Approve app tool call?".to_string(), + question: "Allow this app tool?".to_string(), + is_other: false, + is_secret: false, + options: Some(vec![RequestUserInputQuestionOption { + label: MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string(), + description: "Remember until session ends".to_string(), + }]), + }; + + let response = + mcp_tool_approval_compat_response(&request, &question, ReviewDecision::ApprovedForSession) + .expect("compat response"); + + assert_eq!( + response.answers.get("q-1"), + Some(&RequestUserInputAnswer { + answers: vec![MCP_TOOL_APPROVAL_ACCEPT_FOR_SESSION.to_string()], + }) + ); +} + +#[test] +fn mcp_tool_approval_compat_response_uses_synthetic_decline_for_abort() { + let request = approval_prompt_request( + "custom_server", + "dangerous_tool", + /*arguments*/ None, + /*metadata*/ None, + ); + let question = RequestUserInputQuestion { + id: "q-1".to_string(), + header: "Approve app tool call?".to_string(), + question: "Allow this app tool?".to_string(), + is_other: false, + is_secret: false, + options: None, + }; + + let response = mcp_tool_approval_compat_response(&request, &question, ReviewDecision::Abort) + .expect("compat response"); + + assert_eq!( + response.answers.get("q-1"), + Some(&RequestUserInputAnswer { + answers: vec![MCP_TOOL_APPROVAL_DECLINE_SYNTHETIC.to_string()], + }) + ); +} + +#[test] +fn mcp_tool_approval_question_id_helpers_round_trip() { + let question_id = mcp_tool_approval_question_id("call-1"); + + assert_eq!(question_id, "mcp_tool_call_approval_call-1"); + assert!(is_mcp_tool_approval_question_id(&question_id)); + assert!(!is_mcp_tool_approval_question_id("other_question")); +} + #[tokio::test] async fn mcp_tool_call_span_records_expected_fields() { let buffer: &'static std::sync::Mutex> = @@ -543,15 +598,15 @@ async fn approval_elicitation_request_uses_message_override_and_preserves_tool_p })), Some(&metadata), ); - let prompt = approval_request - .mcp_tool_approval_prompt( - "q".to_string(), - prompt_options( - /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, - ), - /*monitor_reason*/ None, - ) - .expect("mcp approval prompt"); + let prompt = mcp_tool_approval_prompt( + &approval_request, + "q".to_string(), + prompt_options( + /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, + ), + /*monitor_reason*/ None, + ) + .expect("mcp approval prompt"); let request = build_mcp_tool_approval_elicitation_request( &session, @@ -625,13 +680,13 @@ async fn approval_elicitation_request_uses_message_override_and_preserves_tool_p #[test] fn custom_mcp_tool_question_mentions_server_name() { - let question = approval_prompt_request( - "custom_server", - "run_action", - /*arguments*/ None, - /*metadata*/ None, - ) - .mcp_tool_approval_prompt( + let question = mcp_tool_approval_prompt( + &approval_prompt_request( + "custom_server", + "run_action", + /*arguments*/ None, + /*metadata*/ None, + ), "q".to_string(), prompt_options( /*allow_session_remember*/ false, /*allow_persistent_approval*/ false, @@ -658,13 +713,13 @@ fn custom_mcp_tool_question_mentions_server_name() { #[test] fn codex_apps_tool_question_uses_fallback_app_label() { - let question = approval_prompt_request( - CODEX_APPS_MCP_SERVER_NAME, - "run_action", - /*arguments*/ None, - /*metadata*/ None, - ) - .mcp_tool_approval_prompt( + let question = mcp_tool_approval_prompt( + &approval_prompt_request( + CODEX_APPS_MCP_SERVER_NAME, + "run_action", + /*arguments*/ None, + /*metadata*/ None, + ), "q".to_string(), prompt_options( /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, @@ -689,13 +744,13 @@ fn trusted_codex_apps_tool_question_offers_always_allow() { /*tool_title*/ None, /*tool_description*/ None, ); - let question = approval_prompt_request( - CODEX_APPS_MCP_SERVER_NAME, - "run_action", - /*arguments*/ None, - Some(&metadata), - ) - .mcp_tool_approval_prompt( + let question = mcp_tool_approval_prompt( + &approval_prompt_request( + CODEX_APPS_MCP_SERVER_NAME, + "run_action", + /*arguments*/ None, + Some(&metadata), + ), "q".to_string(), prompt_options( /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, @@ -743,13 +798,13 @@ fn codex_apps_tool_question_without_elicitation_omits_always_allow() { /*tool_title*/ None, /*tool_description*/ None, ); - let question = approval_prompt_request( - CODEX_APPS_MCP_SERVER_NAME, - "run_action", - /*arguments*/ None, - Some(&metadata), - ) - .mcp_tool_approval_prompt( + let question = mcp_tool_approval_prompt( + &approval_prompt_request( + CODEX_APPS_MCP_SERVER_NAME, + "run_action", + /*arguments*/ None, + Some(&metadata), + ), "q".to_string(), mcp_tool_approval_prompt_options( Some(&session_key), @@ -778,13 +833,13 @@ fn codex_apps_tool_question_without_elicitation_omits_always_allow() { #[test] fn custom_mcp_tool_question_offers_session_remember_and_always_allow() { - let question = approval_prompt_request( - "custom_server", - "run_action", - /*arguments*/ None, - /*metadata*/ None, - ) - .mcp_tool_approval_prompt( + let question = mcp_tool_approval_prompt( + &approval_prompt_request( + "custom_server", + "run_action", + /*arguments*/ None, + /*metadata*/ None, + ), "q".to_string(), prompt_options( /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, @@ -1181,7 +1236,8 @@ fn approval_elicitation_meta_marks_tool_approvals() { /*metadata*/ None, ); assert_eq!( - request.mcp_tool_approval_elicitation_meta( + mcp_tool_approval_elicitation_meta( + &request, /*tool_params*/ None, /*tool_params_display*/ None, prompt_options( @@ -1210,7 +1266,8 @@ fn approval_elicitation_meta_merges_session_and_always_persist_for_custom_server Some(&metadata), ); assert_eq!( - request.mcp_tool_approval_elicitation_meta( + mcp_tool_approval_elicitation_meta( + &request, Some(&serde_json::json!({"id": 1})), /*tool_params_display*/ None, prompt_options( @@ -1433,7 +1490,8 @@ fn approval_elicitation_meta_includes_connector_source_for_codex_apps() { Some(&metadata), ); assert_eq!( - request.mcp_tool_approval_elicitation_meta( + mcp_tool_approval_elicitation_meta( + &request, Some(&serde_json::json!({ "calendar_id": "primary", })), @@ -1475,7 +1533,8 @@ fn approval_elicitation_meta_merges_session_and_always_persist_with_connector_so Some(&metadata), ); assert_eq!( - request.mcp_tool_approval_elicitation_meta( + mcp_tool_approval_elicitation_meta( + &request, Some(&serde_json::json!({ "calendar_id": "primary", })),