From 4bc746c94fce95ec7ca5f1a91a786156d240c8a2 Mon Sep 17 00:00:00 2001 From: Greg Brisebois Date: Thu, 29 Oct 2026 19:18:42 +0000 Subject: [PATCH] Include tool names and annotation hints in MCP approval metadata (#58161) ## What changed Include `tool_name` in MCP tool approval elicitation metadata and forward available `readOnlyHint`, `destructiveHint`, `idempotentHint`, and `openWorldHint` values under `tool_annotations`. Omit unspecified hints and the annotation title. Reuse the stored tool approval metadata when available so annotation hints reach the approval request. ## Testing Extend unit tests to cover tool names, annotation hints, and omission of unspecified hints. Update integration coverage to verify that the approval tool name matches the executed MCP call and that `readOnlyHint` is preserved. GitOrigin-RevId: f082fe7fd87fde9f4895149e09210c7cb9d1e582 --- codex-rs/core/src/mcp_tool_call.rs | 53 +++++++++----- codex-rs/core/src/mcp_tool_call_tests.rs | 69 ++++++++++++++++--- .../core/tests/suite/mcp_turn_metadata.rs | 16 +++-- codex-rs/protocol/src/mcp_approval_meta.rs | 1 + 4 files changed, 107 insertions(+), 32 deletions(-) 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";