mirror of
https://github.com/openai/codex.git
synced 2026-04-30 19:32:04 +03:00
[elicitation] User-friendly tool call messages. (#14403)
- [x] Add a curated set of tool call messages and human-readable tool param names.
This commit is contained in:
@@ -40,6 +40,8 @@ use crate::bottom_pane::selection_popup_common::menu_surface_padding_height;
|
||||
use crate::bottom_pane::selection_popup_common::render_menu_surface;
|
||||
use crate::bottom_pane::selection_popup_common::render_rows;
|
||||
use crate::render::renderable::Renderable;
|
||||
use crate::text_formatting::format_json_compact;
|
||||
use crate::text_formatting::truncate_text;
|
||||
|
||||
const ANSWER_PLACEHOLDER: &str = "Type your answer";
|
||||
const OPTIONAL_ANSWER_PLACEHOLDER: &str = "Type your answer (optional)";
|
||||
@@ -58,6 +60,10 @@ const APPROVAL_META_KIND_TOOL_SUGGESTION: &str = "tool_suggestion";
|
||||
const APPROVAL_PERSIST_KEY: &str = "persist";
|
||||
const APPROVAL_PERSIST_SESSION_VALUE: &str = "session";
|
||||
const APPROVAL_PERSIST_ALWAYS_VALUE: &str = "always";
|
||||
const APPROVAL_TOOL_PARAMS_KEY: &str = "tool_params";
|
||||
const APPROVAL_TOOL_PARAMS_DISPLAY_KEY: &str = "tool_params_display";
|
||||
const APPROVAL_TOOL_PARAM_DISPLAY_LIMIT: usize = 3;
|
||||
const APPROVAL_TOOL_PARAM_VALUE_TRUNCATE_GRAPHEMES: usize = 60;
|
||||
const TOOL_TYPE_KEY: &str = "tool_type";
|
||||
const TOOL_ID_KEY: &str = "tool_id";
|
||||
const TOOL_NAME_KEY: &str = "tool_name";
|
||||
@@ -146,12 +152,19 @@ pub(crate) struct ToolSuggestionRequest {
|
||||
pub(crate) install_url: String,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
struct McpToolApprovalDisplayParam {
|
||||
name: String,
|
||||
value: Value,
|
||||
}
|
||||
|
||||
#[derive(Clone, Debug, PartialEq)]
|
||||
pub(crate) struct McpServerElicitationFormRequest {
|
||||
thread_id: ThreadId,
|
||||
server_name: String,
|
||||
request_id: McpRequestId,
|
||||
message: String,
|
||||
approval_display_params: Vec<McpToolApprovalDisplayParam>,
|
||||
response_mode: McpServerElicitationResponseMode,
|
||||
fields: Vec<McpServerElicitationField>,
|
||||
tool_suggestion: Option<ToolSuggestionRequest>,
|
||||
@@ -216,6 +229,11 @@ impl McpServerElicitationFormRequest {
|
||||
});
|
||||
let is_tool_approval_action =
|
||||
is_tool_approval && (requested_schema.is_null() || is_empty_object_schema);
|
||||
let approval_display_params = if is_tool_approval_action {
|
||||
parse_tool_approval_display_params(meta.as_ref())
|
||||
} else {
|
||||
Vec::new()
|
||||
};
|
||||
|
||||
let (response_mode, fields) = if tool_suggestion.is_some()
|
||||
&& (requested_schema.is_null() || is_empty_object_schema)
|
||||
@@ -297,6 +315,7 @@ impl McpServerElicitationFormRequest {
|
||||
server_name: request.server_name,
|
||||
request_id: request.id,
|
||||
message,
|
||||
approval_display_params,
|
||||
response_mode,
|
||||
fields,
|
||||
tool_suggestion,
|
||||
@@ -376,6 +395,99 @@ fn tool_approval_supports_persist_mode(meta: Option<&Value>, expected_mode: &str
|
||||
}
|
||||
}
|
||||
|
||||
fn parse_tool_approval_display_params(meta: Option<&Value>) -> Vec<McpToolApprovalDisplayParam> {
|
||||
let Some(meta) = meta.and_then(Value::as_object) else {
|
||||
return Vec::new();
|
||||
};
|
||||
|
||||
let display_params = meta
|
||||
.get(APPROVAL_TOOL_PARAMS_DISPLAY_KEY)
|
||||
.and_then(Value::as_array)
|
||||
.map(|display_params| {
|
||||
display_params
|
||||
.iter()
|
||||
.filter_map(parse_tool_approval_display_param)
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
if !display_params.is_empty() {
|
||||
return display_params;
|
||||
}
|
||||
|
||||
let mut fallback_params = meta
|
||||
.get(APPROVAL_TOOL_PARAMS_KEY)
|
||||
.and_then(Value::as_object)
|
||||
.map(|tool_params| {
|
||||
tool_params
|
||||
.iter()
|
||||
.map(|(name, value)| McpToolApprovalDisplayParam {
|
||||
name: name.clone(),
|
||||
value: value.clone(),
|
||||
})
|
||||
.collect::<Vec<_>>()
|
||||
})
|
||||
.unwrap_or_default();
|
||||
fallback_params.sort_by(|left, right| left.name.cmp(&right.name));
|
||||
fallback_params
|
||||
}
|
||||
|
||||
fn parse_tool_approval_display_param(value: &Value) -> Option<McpToolApprovalDisplayParam> {
|
||||
let value = value.as_object()?;
|
||||
let name = value.get("name")?.as_str()?.trim();
|
||||
if name.is_empty() {
|
||||
return None;
|
||||
}
|
||||
Some(McpToolApprovalDisplayParam {
|
||||
name: name.to_string(),
|
||||
value: value.get("value")?.clone(),
|
||||
})
|
||||
}
|
||||
|
||||
fn format_tool_approval_display_message(
|
||||
message: &str,
|
||||
approval_display_params: &[McpToolApprovalDisplayParam],
|
||||
) -> String {
|
||||
let message = message.trim();
|
||||
if approval_display_params.is_empty() {
|
||||
return message.to_string();
|
||||
}
|
||||
|
||||
let mut sections = Vec::new();
|
||||
if !message.is_empty() {
|
||||
sections.push(message.to_string());
|
||||
}
|
||||
let param_lines = approval_display_params
|
||||
.iter()
|
||||
.take(APPROVAL_TOOL_PARAM_DISPLAY_LIMIT)
|
||||
.map(format_tool_approval_display_param_line)
|
||||
.collect::<Vec<_>>();
|
||||
if !param_lines.is_empty() {
|
||||
sections.push(param_lines.join("\n"));
|
||||
}
|
||||
let mut message = sections.join("\n\n");
|
||||
message.push('\n');
|
||||
message
|
||||
}
|
||||
|
||||
fn format_tool_approval_display_param_line(param: &McpToolApprovalDisplayParam) -> String {
|
||||
format!(
|
||||
"{}: {}",
|
||||
param.name,
|
||||
format_tool_approval_display_param_value(¶m.value)
|
||||
)
|
||||
}
|
||||
|
||||
fn format_tool_approval_display_param_value(value: &Value) -> String {
|
||||
let formatted = match value {
|
||||
Value::String(text) => text.split_whitespace().collect::<Vec<_>>().join(" "),
|
||||
_ => {
|
||||
let compact_json = value.to_string();
|
||||
format_json_compact(&compact_json).unwrap_or(compact_json)
|
||||
}
|
||||
};
|
||||
truncate_text(&formatted, APPROVAL_TOOL_PARAM_VALUE_TRUNCATE_GRAPHEMES)
|
||||
}
|
||||
|
||||
fn parse_fields_from_schema(requested_schema: &Value) -> Option<Vec<McpServerElicitationField>> {
|
||||
let schema = requested_schema.as_object()?;
|
||||
if schema.get("type").and_then(Value::as_str) != Some("object") {
|
||||
@@ -779,12 +891,16 @@ impl McpServerElicitationOverlay {
|
||||
}
|
||||
|
||||
fn current_prompt_text(&self) -> String {
|
||||
let request_message = format_tool_approval_display_message(
|
||||
&self.request.message,
|
||||
&self.request.approval_display_params,
|
||||
);
|
||||
let Some(field) = self.current_field() else {
|
||||
return self.request.message.clone();
|
||||
return request_message;
|
||||
};
|
||||
let mut sections = Vec::new();
|
||||
if !self.request.message.trim().is_empty() {
|
||||
sections.push(self.request.message.trim().to_string());
|
||||
if !request_message.trim().is_empty() {
|
||||
sections.push(request_message);
|
||||
}
|
||||
let field_prompt = if field.label.trim().is_empty()
|
||||
|| field.prompt.trim().is_empty()
|
||||
@@ -1549,7 +1665,11 @@ mod tests {
|
||||
})
|
||||
}
|
||||
|
||||
fn tool_approval_meta(persist_modes: &[&str]) -> Option<Value> {
|
||||
fn tool_approval_meta(
|
||||
persist_modes: &[&str],
|
||||
tool_params: Option<Value>,
|
||||
tool_params_display: Option<Vec<(&str, Value)>>,
|
||||
) -> Option<Value> {
|
||||
let mut meta = serde_json::Map::from_iter([(
|
||||
APPROVAL_META_KIND_KEY.to_string(),
|
||||
Value::String(APPROVAL_META_KIND_MCP_TOOL_CALL.to_string()),
|
||||
@@ -1565,6 +1685,25 @@ mod tests {
|
||||
),
|
||||
);
|
||||
}
|
||||
if let Some(tool_params) = tool_params {
|
||||
meta.insert(APPROVAL_TOOL_PARAMS_KEY.to_string(), tool_params);
|
||||
}
|
||||
if let Some(tool_params_display) = tool_params_display {
|
||||
meta.insert(
|
||||
APPROVAL_TOOL_PARAMS_DISPLAY_KEY.to_string(),
|
||||
Value::Array(
|
||||
tool_params_display
|
||||
.into_iter()
|
||||
.map(|(name, value)| {
|
||||
serde_json::json!({
|
||||
"name": name,
|
||||
"value": value,
|
||||
})
|
||||
})
|
||||
.collect(),
|
||||
),
|
||||
);
|
||||
}
|
||||
Some(Value::Object(meta))
|
||||
}
|
||||
|
||||
@@ -1616,6 +1755,7 @@ mod tests {
|
||||
server_name: "server-1".to_string(),
|
||||
request_id: McpRequestId::String("request-1".to_string()),
|
||||
message: "Allow this request?".to_string(),
|
||||
approval_display_params: Vec::new(),
|
||||
response_mode: McpServerElicitationResponseMode::FormContent,
|
||||
fields: vec![McpServerElicitationField {
|
||||
id: "confirmed".to_string(),
|
||||
@@ -1681,6 +1821,7 @@ mod tests {
|
||||
server_name: "server-1".to_string(),
|
||||
request_id: McpRequestId::String("request-1".to_string()),
|
||||
message: "Allow this request?".to_string(),
|
||||
approval_display_params: Vec::new(),
|
||||
response_mode: McpServerElicitationResponseMode::ApprovalAction,
|
||||
fields: vec![McpServerElicitationField {
|
||||
id: APPROVAL_FIELD_ID.to_string(),
|
||||
@@ -1723,7 +1864,7 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[]),
|
||||
tool_approval_meta(&[], None, None),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -1735,6 +1876,7 @@ mod tests {
|
||||
server_name: "server-1".to_string(),
|
||||
request_id: McpRequestId::String("request-1".to_string()),
|
||||
message: "Allow this request?".to_string(),
|
||||
approval_display_params: Vec::new(),
|
||||
response_mode: McpServerElicitationResponseMode::ApprovalAction,
|
||||
fields: vec![McpServerElicitationField {
|
||||
id: APPROVAL_FIELD_ID.to_string(),
|
||||
@@ -1805,6 +1947,43 @@ mod tests {
|
||||
assert_eq!(request, None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn tool_approval_display_params_prefer_explicit_display_order() {
|
||||
let request = McpServerElicitationFormRequest::from_event(
|
||||
ThreadId::default(),
|
||||
form_request(
|
||||
"Allow Calendar to create an event",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(
|
||||
&[],
|
||||
Some(serde_json::json!({
|
||||
"zeta": 3,
|
||||
"alpha": 1,
|
||||
})),
|
||||
Some(vec![
|
||||
("Calendar", Value::String("primary".to_string())),
|
||||
("Title", Value::String("Roadmap review".to_string())),
|
||||
]),
|
||||
),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
|
||||
assert_eq!(
|
||||
request.approval_display_params,
|
||||
vec![
|
||||
McpToolApprovalDisplayParam {
|
||||
name: "Calendar".to_string(),
|
||||
value: Value::String("primary".to_string()),
|
||||
},
|
||||
McpToolApprovalDisplayParam {
|
||||
name: "Title".to_string(),
|
||||
value: Value::String("Roadmap review".to_string()),
|
||||
},
|
||||
]
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn submit_sends_accept_with_typed_content() {
|
||||
let (tx, mut rx) = test_sender();
|
||||
@@ -1865,10 +2044,14 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
tool_approval_meta(
|
||||
&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
],
|
||||
None,
|
||||
None,
|
||||
),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -1912,10 +2095,14 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
tool_approval_meta(
|
||||
&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
],
|
||||
None,
|
||||
None,
|
||||
),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -2105,7 +2292,7 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[]),
|
||||
tool_approval_meta(&[], None, None),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -2125,10 +2312,14 @@ mod tests {
|
||||
form_request(
|
||||
"Allow this request?",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
]),
|
||||
tool_approval_meta(
|
||||
&[
|
||||
APPROVAL_PERSIST_SESSION_VALUE,
|
||||
APPROVAL_PERSIST_ALWAYS_VALUE,
|
||||
],
|
||||
None,
|
||||
None,
|
||||
),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
@@ -2139,4 +2330,41 @@ mod tests {
|
||||
render_snapshot(&overlay, Rect::new(0, 0, 120, 16))
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn approval_form_tool_approval_with_param_summary_snapshot() {
|
||||
let (tx, _rx) = test_sender();
|
||||
let request = McpServerElicitationFormRequest::from_event(
|
||||
ThreadId::default(),
|
||||
form_request(
|
||||
"Allow Calendar to create an event",
|
||||
empty_object_schema(),
|
||||
tool_approval_meta(
|
||||
&[],
|
||||
Some(serde_json::json!({
|
||||
"calendar_id": "primary",
|
||||
"title": "Roadmap review",
|
||||
"notes": "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.",
|
||||
"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()),
|
||||
),
|
||||
("Ignored", Value::String("fourth param".to_string())),
|
||||
]),
|
||||
),
|
||||
),
|
||||
)
|
||||
.expect("expected approval fallback");
|
||||
let overlay = McpServerElicitationOverlay::new(request, tx, true, false, false);
|
||||
|
||||
insta::assert_snapshot!(
|
||||
"mcp_server_elicitation_approval_form_with_param_summary",
|
||||
render_snapshot(&overlay, Rect::new(0, 0, 120, 16))
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,19 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/mcp_server_elicitation.rs
|
||||
expression: "render_snapshot(&overlay, Rect::new(0, 0, 120, 16))"
|
||||
---
|
||||
|
||||
Field 1/1
|
||||
Allow Calendar to create an event
|
||||
|
||||
Calendar: primary
|
||||
Title: Roadmap review
|
||||
Notes: This is a deliberately long note that should truncate bef...
|
||||
|
||||
› 1. Allow Run the tool and continue.
|
||||
2. Cancel Cancel this tool call
|
||||
|
||||
|
||||
|
||||
|
||||
enter to submit | esc to cancel
|
||||
Reference in New Issue
Block a user