diff --git a/codex-rs/core/src/mcp_tool_approval_templates.rs b/codex-rs/core/src/mcp_tool_approval_templates.rs index f8fbad3ed..b1e0ee27d 100644 --- a/codex-rs/core/src/mcp_tool_approval_templates.rs +++ b/codex-rs/core/src/mcp_tool_approval_templates.rs @@ -26,6 +26,7 @@ pub(crate) struct RenderedMcpToolApprovalTemplate { pub(crate) struct RenderedMcpToolApprovalParam { pub(crate) name: String, pub(crate) value: Value, + pub(crate) display_name: String, } #[derive(Debug, Deserialize)] @@ -142,8 +143,8 @@ fn render_tool_params( tool_params: &Map, template_params: &[ConsequentialToolTemplateParam], ) -> Option<(Option, Vec)> { - let mut relabeled = Map::new(); let mut display_params = Vec::new(); + let mut display_names = HashSet::new(); let mut handled_names = HashSet::new(); for template_param in template_params { @@ -154,12 +155,13 @@ fn render_tool_params( let Some(value) = tool_params.get(&template_param.name) else { continue; }; - if relabeled.insert(label.to_string(), value.clone()).is_some() { + if !display_names.insert(label.to_string()) { return None; } display_params.push(RenderedMcpToolApprovalParam { - name: label.to_string(), + name: template_param.name.clone(), value: value.clone(), + display_name: label.to_string(), }); handled_names.insert(template_param.name.as_str()); } @@ -174,16 +176,17 @@ fn render_tool_params( if handled_names.contains(name.as_str()) { continue; } - if relabeled.insert(name.clone(), value.clone()).is_some() { + if !display_names.insert(name.clone()) { return None; } display_params.push(RenderedMcpToolApprovalParam { name: name.clone(), value: value.clone(), + display_name: name.clone(), }); } - Some((Some(Value::Object(relabeled)), display_params)) + Some((Some(Value::Object(tool_params.clone())), display_params)) } #[cfg(test)] @@ -231,22 +234,25 @@ mod tests { question: "Allow Calendar to create an event?".to_string(), elicitation_message: "Allow Calendar to create an event?".to_string(), tool_params: Some(json!({ - "Calendar": "primary", - "Title": "Roadmap review", + "title": "Roadmap review", + "calendar_id": "primary", "timezone": "UTC", })), tool_params_display: vec![ RenderedMcpToolApprovalParam { - name: "Calendar".to_string(), + name: "calendar_id".to_string(), value: json!("primary"), + display_name: "Calendar".to_string(), }, RenderedMcpToolApprovalParam { - name: "Title".to_string(), + name: "title".to_string(), value: json!("Roadmap review"), + display_name: "Title".to_string(), }, RenderedMcpToolApprovalParam { name: "timezone".to_string(), value: json!("UTC"), + display_name: "timezone".to_string(), }, ], }) diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 656e4bead..77193847a 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -1039,6 +1039,7 @@ fn build_mcp_tool_approval_display_params( |(name, value)| crate::mcp_tool_approval_templates::RenderedMcpToolApprovalParam { name: name.clone(), value: value.clone(), + display_name: name.clone(), }, ) .collect::>(); diff --git a/codex-rs/core/src/mcp_tool_call_tests.rs b/codex-rs/core/src/mcp_tool_call_tests.rs index 55f47f674..7b1da0f9d 100644 --- a/codex-rs/core/src/mcp_tool_call_tests.rs +++ b/codex-rs/core/src/mcp_tool_call_tests.rs @@ -109,7 +109,7 @@ fn approval_question_text_prepends_safety_reason() { } #[tokio::test] -async fn approval_elicitation_request_uses_message_override_and_readable_tool_params() { +async fn approval_elicitation_request_uses_message_override_and_preserves_tool_params_keys() { let (session, turn_context) = make_session_and_context().await; let question = build_mcp_tool_approval_question( "q".to_string(), @@ -133,10 +133,21 @@ async fn approval_elicitation_request_uses_message_override_and_readable_tool_pa Some("Create a calendar event."), )), tool_params: Some(&serde_json::json!({ - "Calendar": "primary", - "Title": "Roadmap review", + "calendar_id": "primary", + "title": "Roadmap review", })), - tool_params_display: None, + tool_params_display: Some(&[ + RenderedMcpToolApprovalParam { + name: "calendar_id".to_string(), + value: serde_json::json!("primary"), + display_name: "Calendar".to_string(), + }, + RenderedMcpToolApprovalParam { + name: "title".to_string(), + value: serde_json::json!("Roadmap review"), + display_name: "Title".to_string(), + }, + ]), question, message_override: Some("Allow Calendar to create an event?"), prompt_options: prompt_options(true, true), @@ -163,9 +174,21 @@ async fn approval_elicitation_request_uses_message_override_and_readable_tool_pa MCP_TOOL_APPROVAL_TOOL_TITLE_KEY: "Create Event", MCP_TOOL_APPROVAL_TOOL_DESCRIPTION_KEY: "Create a calendar event.", MCP_TOOL_APPROVAL_TOOL_PARAMS_KEY: { - "Calendar": "primary", - "Title": "Roadmap review", + "calendar_id": "primary", + "title": "Roadmap review", }, + MCP_TOOL_APPROVAL_TOOL_PARAMS_DISPLAY_KEY: [ + { + "name": "calendar_id", + "value": "primary", + "display_name": "Calendar", + }, + { + "name": "title", + "value": "Roadmap review", + "display_name": "Title", + }, + ], })), message: "Allow Calendar to create an event?".to_string(), requested_schema: McpElicitationSchema { 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 975b8701d..4860a61f2 100644 --- a/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs +++ b/codex-rs/tui/src/bottom_pane/mcp_server_elicitation.rs @@ -156,6 +156,7 @@ pub(crate) struct ToolSuggestionRequest { struct McpToolApprovalDisplayParam { name: String, value: Value, + display_name: String, } #[derive(Clone, Debug, PartialEq)] @@ -423,6 +424,7 @@ fn parse_tool_approval_display_params(meta: Option<&Value>) -> Vec>() }) @@ -437,9 +439,18 @@ fn parse_tool_approval_display_param(value: &Value) -> Option String { format!( "{}: {}", - param.name, + param.display_name, format_tool_approval_display_param_value(¶m.value) ) } @@ -1668,7 +1679,7 @@ mod tests { fn tool_approval_meta( persist_modes: &[&str], tool_params: Option, - tool_params_display: Option>, + tool_params_display: Option>, ) -> Option { let mut meta = serde_json::Map::from_iter([( APPROVAL_META_KIND_KEY.to_string(), @@ -1694,10 +1705,11 @@ mod tests { Value::Array( tool_params_display .into_iter() - .map(|(name, value)| { + .map(|(name, value, display_name)| { serde_json::json!({ "name": name, "value": value, + "display_name": display_name, }) }) .collect(), @@ -1961,8 +1973,16 @@ mod tests { "alpha": 1, })), Some(vec![ - ("Calendar", Value::String("primary".to_string())), - ("Title", Value::String("Roadmap review".to_string())), + ( + "calendar_id", + Value::String("primary".to_string()), + "Calendar", + ), + ( + "title", + Value::String("Roadmap review".to_string()), + "Title", + ), ]), ), ), @@ -1973,12 +1993,14 @@ mod tests { request.approval_display_params, vec![ McpToolApprovalDisplayParam { - name: "Calendar".to_string(), + name: "calendar_id".to_string(), value: Value::String("primary".to_string()), + display_name: "Calendar".to_string(), }, McpToolApprovalDisplayParam { - name: "Title".to_string(), + name: "title".to_string(), value: Value::String("Roadmap review".to_string()), + display_name: "Title".to_string(), }, ] ); @@ -2348,13 +2370,26 @@ mod tests { "ignored_after_limit": "fourth param", })), Some(vec![ - ("Calendar", Value::String("primary".to_string())), - ("Title", Value::String("Roadmap review".to_string())), ( - "Notes", - Value::String("This is a deliberately long note that should truncate before it turns the approval body into a giant wall of text in the TUI overlay.".to_string()), + "calendar_id", + Value::String("primary".to_string()), + "Calendar", + ), + ( + "title", + Value::String("Roadmap review".to_string()), + "Title", + ), + ( + "notes", + Value::String("This is a deliberately long note that should truncate before it turns the approval body into a giant wall of text in the TUI overlay.".to_string()), + "Notes", + ), + ( + "ignored_after_limit", + Value::String("fourth param".to_string()), + "Ignored", ), - ("Ignored", Value::String("fourth param".to_string())), ]), ), ),