diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index eacf5aa59f..85aee998f7 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -71,7 +71,9 @@ use codex_protocol::mcp_approval_meta::PERSIST_KEY as MCP_TOOL_APPROVAL_PERSIST_ 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_ANNOTATIONS_KEY as MCP_TOOL_APPROVAL_TOOL_ANNOTATIONS_KEY; use codex_protocol::mcp_approval_meta::TOOL_DESCRIPTION_KEY as MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY; +use codex_protocol::mcp_approval_meta::TOOL_NAME_KEY as MCP_TOOL_APPROVAL_TOOL_NAME_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; @@ -1201,7 +1203,7 @@ impl McpToolApprovalPolicy { } } -#[derive(Clone)] +#[derive(Clone, Default)] pub(crate) struct McpToolApprovalMetadata { annotations: Option, pub(crate) connector_id: Option, @@ -1470,6 +1472,7 @@ struct McpToolApprovalPromptOptions { struct McpToolApprovalElicitationRequest<'a> { server: &'a str, + tool_name: &'a str, metadata: Option<&'a McpToolApprovalMetadata>, tool_params: Option<&'a serde_json::Value>, tool_params_display: Option<&'a [RenderedMcpToolApprovalParam]>, @@ -1695,28 +1698,23 @@ pub(crate) async fn request_mcp_tool_user_approval( .map(|rendered_template| rendered_template.question.as_str()), ); let (request_dispatched, decision, cancellation_reason) = if tool_call_mcp_elicitation_enabled { - let link_id = sess + let metadata = sess .mcp_tool_approval_metadata(server, id) - .and_then(|context| context.metadata.link_id); - let metadata = McpToolApprovalMetadata { - annotations: None, - connector_id: connector_id.clone(), - link_id, - connector_name: connector_name.clone(), - connector_description: connector_description.clone(), - connected_account_email: connected_account_email.clone(), - plugin_id: None, - tool_title: tool_title.clone(), - tool_description: tool_description.clone(), - mcp_app_resource_uri: None, - mcp_app_ui: None, - codex_apps_meta: None, - openai_file_input_optional_fields: None, - }; + .map(|context| context.metadata) + .unwrap_or_else(|| McpToolApprovalMetadata { + connector_id: connector_id.clone(), + connector_name: connector_name.clone(), + connector_description: connector_description.clone(), + connected_account_email: connected_account_email.clone(), + tool_title: tool_title.clone(), + tool_description: tool_description.clone(), + ..Default::default() + }); let request_id = rmcp::model::RequestId::String(question_id.clone().into()); let request = build_mcp_tool_approval_elicitation_request(McpToolApprovalElicitationRequest { server, + tool_name, metadata: Some(&metadata), tool_params: rendered_template .as_ref() @@ -2068,6 +2066,7 @@ fn build_mcp_tool_approval_elicitation_request( ElicitationRequest::Form { meta: build_mcp_tool_approval_elicitation_meta( request.server, + request.tool_name, request.metadata, request.tool_params, request.tool_params_display, @@ -2083,6 +2082,7 @@ fn build_mcp_tool_approval_elicitation_request( fn build_mcp_tool_approval_elicitation_meta( server: &str, + tool_name: &str, metadata: Option<&McpToolApprovalMetadata>, tool_params: Option<&serde_json::Value>, tool_params_display: Option<&[RenderedMcpToolApprovalParam]>, @@ -2120,7 +2120,24 @@ fn build_mcp_tool_approval_elicitation_meta( } (false, false) => {} } + meta.insert( + MCP_TOOL_APPROVAL_TOOL_NAME_KEY.to_string(), + serde_json::Value::String(tool_name.to_owned()), + ); if let Some(metadata) = metadata { + if let Some(annotations) = &metadata.annotations { + let hints = ToolAnnotations::from_raw( + /*title*/ None, + annotations.read_only_hint, + annotations.destructive_hint, + annotations.idempotent_hint, + annotations.open_world_hint, + ); + meta.insert( + MCP_TOOL_APPROVAL_TOOL_ANNOTATIONS_KEY.to_string(), + serde_json::json!(hints), + ); + } if let Some(tool_title) = metadata.tool_title.as_ref() { meta.insert( MCP_TOOL_APPROVAL_TOOL_TITLE_KEY.to_string(), diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 9f4ef92042..c0ac2d401b 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -734,10 +734,24 @@ fn truncates_strings_on_char_boundaries() { #[test] fn approval_elicitation_request_uses_message_override_and_preserves_tool_params_keys() { + let mut metadata = approval_metadata( + Some("calendar"), + Some("Calendar"), + Some("Manage events and schedules."), + Some("Create Event"), + Some("Create a calendar event."), + ); + metadata.annotations = Some(ToolAnnotations::from_raw( + /*title*/ Some("Calendar Event".to_string()), + /*read_only_hint*/ Some(false), + /*destructive_hint*/ Some(false), + /*idempotent_hint*/ Some(true), + /*open_world_hint*/ Some(true), + )); let question = build_mcp_tool_approval_question( "q".to_string(), CODEX_APPS_MCP_SERVER_NAME, - "create_event", + "calendar.create_event", Some("Calendar"), prompt_options( /*allow_session_remember*/ true, /*allow_persistent_approval*/ true, @@ -747,13 +761,8 @@ fn approval_elicitation_request_uses_message_override_and_preserves_tool_params_ let request = build_mcp_tool_approval_elicitation_request(McpToolApprovalElicitationRequest { server: CODEX_APPS_MCP_SERVER_NAME, - metadata: Some(&approval_metadata( - Some("calendar"), - Some("Calendar"), - Some("Manage events and schedules."), - Some("Create Event"), - Some("Create a calendar event."), - )), + tool_name: "calendar.create_event", + metadata: Some(&metadata), tool_params: Some(&serde_json::json!({ "calendar_id": "primary", "title": "Roadmap review", @@ -790,6 +799,13 @@ fn approval_elicitation_request_uses_message_override_and_preserves_tool_params_ MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: "calendar", MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: "Calendar", MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY: "Manage events and schedules.", + "tool_name": "calendar.create_event", + "tool_annotations": { + "readOnlyHint": false, + "destructiveHint": false, + "idempotentHint": true, + "openWorldHint": true, + }, MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: "Create Event", MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: "Create a calendar event.", MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: { @@ -1923,6 +1939,7 @@ fn approval_elicitation_meta_marks_tool_approvals() { assert_eq!( build_mcp_tool_approval_elicitation_meta( "custom_server", + "run_action", /*metadata*/ None, /*tool_params*/ None, /*tool_params_display*/ None, @@ -1932,15 +1949,46 @@ fn approval_elicitation_meta_marks_tool_approvals() { ), Some(serde_json::json!({ MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL, + MCP_TOOL_APPROVAL_TOOL_NAME_KEY: "run_action", })) ); } +#[test] +fn approval_elicitation_meta_emits_only_supplied_hints_for_custom_servers() { + let metadata = McpToolApprovalMetadata { + annotations: Some(annotations( + Some(false), + /*destructive*/ None, + /*open_world*/ None, + )), + ..Default::default() + }; + assert_eq!( + build_mcp_tool_approval_elicitation_meta( + "custom_server", + "run_action", + Some(&metadata), + /*tool_params*/ None, + /*tool_params_display*/ None, + prompt_options( + /*allow_session_remember*/ false, /*allow_persistent_approval*/ false + ), + ), + Some(serde_json::json!({ + "codex_approval_kind": "mcp_tool_call", + "tool_name": "run_action", + "tool_annotations": { "readOnlyHint": false }, + })), + ); +} + #[test] fn approval_elicitation_meta_merges_session_and_always_persist_for_custom_servers() { assert_eq!( build_mcp_tool_approval_elicitation_meta( "custom_server", + "run_action", Some(&approval_metadata( /*connector_id*/ None, /*connector_name*/ None, @@ -1956,6 +2004,7 @@ fn approval_elicitation_meta_merges_session_and_always_persist_for_custom_server ), Some(serde_json::json!({ MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL, + MCP_TOOL_APPROVAL_TOOL_NAME_KEY: "run_action", MCP_TOOL_APPROVAL_PERSIST_KEY: [ MCP_TOOL_APPROVAL_PERSIST_SESSION, MCP_TOOL_APPROVAL_PERSIST_ALWAYS, @@ -2095,6 +2144,7 @@ fn approval_elicitation_meta_includes_connector_source_for_codex_apps() { assert_eq!( build_mcp_tool_approval_elicitation_meta( CODEX_APPS_MCP_SERVER_NAME, + "calendar.run_action", Some(&approval_metadata( Some("calendar"), Some("Calendar"), @@ -2113,6 +2163,7 @@ fn approval_elicitation_meta_includes_connector_source_for_codex_apps() { Some(serde_json::json!({ MCP_TOOL_APPROVAL_KIND_KEY: MCP_TOOL_APPROVAL_KIND_MCP_TOOL_CALL, MCP_TOOL_APPROVAL_SOURCE_KEY: MCP_TOOL_APPROVAL_SOURCE_CONNECTOR, + MCP_TOOL_APPROVAL_TOOL_NAME_KEY: "calendar.run_action", MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: "calendar", MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: "Calendar", MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY: "Manage events and schedules.", @@ -2130,6 +2181,7 @@ fn approval_elicitation_meta_merges_session_and_always_persist_with_connector_so assert_eq!( build_mcp_tool_approval_elicitation_meta( CODEX_APPS_MCP_SERVER_NAME, + "calendar.run_action", Some(&approval_metadata( Some("calendar"), Some("Calendar"), @@ -2152,6 +2204,7 @@ fn approval_elicitation_meta_merges_session_and_always_persist_with_connector_so MCP_TOOL_APPROVAL_PERSIST_ALWAYS, ], MCP_TOOL_APPROVAL_SOURCE_KEY: MCP_TOOL_APPROVAL_SOURCE_CONNECTOR, + MCP_TOOL_APPROVAL_TOOL_NAME_KEY: "calendar.run_action", MCP_TOOL_APPROVAL_CONNECTOR_ID_KEY: "calendar", MCP_TOOL_APPROVAL_CONNECTOR_NAME_KEY: "Calendar", MCP_TOOL_APPROVAL_CONNECTOR_DESCRIPTION_KEY: "Manage events and schedules.", diff --git a/codex-rs/core/tests/suite/mcp_turn_metadata.rs b/codex-rs/core/tests/suite/mcp_turn_metadata.rs index b396a72d15..e8648e44a3 100644 --- a/codex-rs/core/tests/suite/mcp_turn_metadata.rs +++ b/codex-rs/core/tests/suite/mcp_turn_metadata.rs @@ -1075,7 +1075,7 @@ default_tools_approval_mode = "{selected_link_mode}" "Link identifier for the calendar resource.".to_string() }; let tool = json!({ - "name": "calendar_create_event", + "name": "calendar.create_event", "description": "Create a calendar event.", "annotations": { "readOnlyHint": false }, "inputSchema": { @@ -1094,7 +1094,7 @@ default_tools_approval_mode = "{selected_link_mode}" "connector_name": "Calendar", "_codex_apps": { "connector_id": "calendar", - "resource_uri": "connector://calendar/tools/calendar_create_event", + "resource_uri": "connector://calendar/tools/create_event", "contains_mcp_source": true, "requires_explicit_link_id": requires_explicit_link_id } @@ -1188,6 +1188,7 @@ default_tools_approval_mode = "{selected_link_mode}" matches!(&event, EventMsg::ElicitationRequest(_)), expected_prompt ); + let mut approval_tool_name = None; if let EventMsg::ElicitationRequest(request) = event { let ElicitationRequest::Form { meta: Some(meta), .. @@ -1195,6 +1196,8 @@ default_tools_approval_mode = "{selected_link_mode}" else { panic!("expected a native MCP tool approval with metadata"); }; + approval_tool_name = Some(meta["tool_name"].clone()); + assert_eq!(meta["tool_annotations"]["readOnlyHint"], json!(false)); assert_eq!( ( meta.get("link_id").and_then(Value::as_str), @@ -1218,10 +1221,11 @@ default_tools_approval_mode = "{selected_link_mode}" .await; } assert_eq!(mock.requests().len(), 2); - assert_eq!( - recorded_apps_tool_call_by_call_id(&server, call_id).await["params"]["arguments"], - calendar_args - ); + let tool_call = recorded_apps_tool_call_by_call_id(&server, call_id).await; + if let Some(tool_name) = approval_tool_name { + assert_eq!(tool_name, tool_call["params"]["name"]); + } + assert_eq!(tool_call["params"]["arguments"], calendar_args); assert_eq!(completed_links, vec![expected_link_id.map(str::to_string)]); Ok(()) } diff --git a/codex-rs/protocol/src/mcp_approval_meta.rs b/codex-rs/protocol/src/mcp_approval_meta.rs index 674be8244c..00b9b88bd7 100644 --- a/codex-rs/protocol/src/mcp_approval_meta.rs +++ b/codex-rs/protocol/src/mcp_approval_meta.rs @@ -35,6 +35,7 @@ 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_ANNOTATIONS_KEY: &str = "tool_annotations"; pub const TOOL_TITLE_KEY: &str = "tool_title"; pub const TOOL_DESCRIPTION_KEY: &str = "tool_description"; pub const TOOL_PARAMS_KEY: &str = "tool_params";