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
2026-10-30 19:18:49 +00:00 · 2026-10-29 19:34:46 +00:00
parent fa4ebe87af
commit 4bc746c94f
4 changed files with 107 additions and 32 deletions
@@ -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<ToolAnnotations>,
    pub(crate) connector_id: Option<String>,
@@ -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(),
@@ -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.",
@@ -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(())
 }
@@ -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";