github/codex
4
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-20 00:18:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve MCP tool output as content items (#40737)

Browse Source 
## What changed

- Convert unstructured MCP results into typed function-call output items instead of serializing the entire content array as a text string.
- Keep structured MCP results as serialized text and preserve media, encrypted content, and unknown content through their existing item conversions.
- Drop empty text items during output truncation so they do not consume API array slots.

## Testing

- Cover text-only, mixed unstructured, structured, and image-sanitized MCP results.
- Verify empty text items are discarded under byte- and token-based truncation policies.

GitOrigin-RevId: fa1c5b7dee6f003a094265379dcabdd060386a48
This commit is contained in:
pakrym-oai
2026-08-25 23:42:50 +00:00
committed by copyberry
parent 62aacbb2c9
commit 75cb7c903d
12 changed files with 288 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/app-server/tests/suite/v2/mcp_server_elicitation.rs
+12 -13
View File
@@ -907,20 +907,19 @@ impl ElicitationRoundTripFixture {
function_call_output.get("call_id"),
Some(&Value::String(call_id.to_string()))
);
let output = function_call_output
.get("output")
.and_then(Value::as_str)
.expect("function_call_output output should be a JSON string");
let payload = assert_regex_match(
r#"(?s)^Wall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:\n(.*)$"#,
output,
)
.get(1)
.expect("wall-time wrapped output should include payload")
.as_str();
let header = function_call_output["output"][0]["text"]
.as_str()
.expect("first content item should contain the wall-time header");
assert_regex_match(
r#"^Wall time: [0-9]+(?:\.[0-9]+)? seconds\nOutput:$"#,
header,
);
assert_eq!(
serde_json::from_str::<Value>(payload)?,
json!([{ "type": "text", "text": expected_text }])
function_call_output["output"],
json!([
{ "type": "input_text", "text": header },
{ "type": "input_text", "text": expected_text },
])
);
if matches!(self.scenario, ElicitationScenario::Strict(Review::Approved)) && !self.next_turn
codex-rs/core/src/session/tests.rs
+13 -6
View File
@@ -4726,9 +4726,14 @@ fn falls_back_to_content_when_structured_is_null() {
let got = ctr.into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&vec![text_block("hello"), text_block("world")]).unwrap(),
),
body: FunctionCallOutputBody::ContentItems(vec![
FunctionCallOutputContentItem::InputText {
text: "hello".to_string(),
},
FunctionCallOutputContentItem::InputText {
text: "world".to_string(),
},
]),
success: Some(true),
};
@@ -4766,9 +4771,11 @@ fn success_flag_true_with_no_error_and_content_used() {
let got = ctr.into_function_call_output_payload();
let expected = FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::to_string(&vec![text_block("alpha")]).unwrap(),
),
body: FunctionCallOutputBody::ContentItems(vec![
FunctionCallOutputContentItem::InputText {
text: "alpha".to_string(),
},
]),
success: Some(true),
};
codex-rs/core/src/tools/context_tests.rs
+16 -22
View File
@@ -108,29 +108,23 @@ fn mcp_tool_output_response_item_includes_wall_time() {
},
);
match response {
ResponseInputItem::FunctionCallOutput { call_id, output } => {
assert_eq!(call_id, "mcp-call-1");
assert_eq!(output.success, Some(true));
let Some(text) = output.body.to_text() else {
panic!("MCP output should serialize as text");
};
let Some(payload) = text.strip_prefix("Wall time: 1.2500 seconds\nOutput:\n") else {
panic!("MCP output should include wall-time header: {text}");
};
let parsed: serde_json::Value = serde_json::from_str(payload).unwrap_or_else(|err| {
panic!("MCP output should serialize JSON content: {err}");
});
assert_eq!(
parsed,
json!([{
"type": "text",
"text": "done",
}])
);
assert_eq!(
response,
ResponseInputItem::FunctionCallOutput {
call_id: "mcp-call-1".to_string(),
output: FunctionCallOutputPayload {
body: FunctionCallOutputBody::ContentItems(vec![
FunctionCallOutputContentItem::InputText {
text: "Wall time: 1.2500 seconds\nOutput:".to_string(),
},
FunctionCallOutputContentItem::InputText {
text: "done".to_string(),
},
]),
success: Some(true),
},
}
other => panic!("expected FunctionCallOutput, got {other:?}"),
}
);
}
#[test]
codex-rs/core/tests/suite/codex_delegate.rs
+2 -2
View File
@@ -240,9 +240,9 @@ default_tools_approval_mode = "prompt"
panic!("expected the model request and MCP-denial continuation");
};
let output = completion_request.function_call_output(call_id);
let response = output["output"]
let response = output["output"][1]["text"]
.as_str()
.expect("MCP tool output should be a string");
.expect("MCP tool output should contain an input_text item");
assert!(
response.contains("approval policy is never"),
"MCP tool should be rejected by the delegate's approval policy: {response}"
codex-rs/core/tests/suite/hooks_mcp.rs
+5 -4
View File
@@ -410,10 +410,11 @@ async fn run_mcp_permission_request_hook_test(outcome: PermissionRequestHookOutc
);
let output_item = requests[1].function_call_output(call_id);
let output = output_item
.get("output")
.and_then(Value::as_str)
.expect("MCP tool output should be a string");
let output = match outcome {
PermissionRequestHookOutcome::Allow => output_item["output"].as_str(),
PermissionRequestHookOutcome::Deny(_) => output_item["output"][1]["text"].as_str(),
}
.expect("MCP tool output should contain text");
match outcome {
PermissionRequestHookOutcome::Allow => assert!(
output.contains(&format!("ECHOING: {RMCP_ECHO_MESSAGE}")),
codex-rs/core/tests/suite/mcp_auth_elicitation.rs
+11 -7
View File
@@ -199,13 +199,17 @@ default_tools_approval_mode = "auto"
let requests = responses.requests();
assert_eq!(requests.len(), 2);
let output = requests[1]
.function_call_output_text(call_id)
.expect("follow-up request should contain the auth elicitation result");
assert!(output.contains(
"Authentication for Calendar was requested and accepted. Retry this tool call now."
));
assert!(!output.contains("Connector reauthentication required"));
let output = requests[1].function_call_output(call_id);
let items = output["output"]
.as_array()
.expect("auth elicitation result should contain content items");
assert_eq!(
&items[1..],
&[json!({
"type": "input_text",
"text": "Authentication for Calendar was requested and accepted. Retry this tool call now.",
})]
);
Ok(())
}
codex-rs/core/tests/suite/mcp_tool_cache.rs
+4 -2
View File
@@ -677,9 +677,11 @@ async fn cached_mcp_startup_is_eager_for_root_and_lazy_for_subagents() -> anyhow
&format!("Use the tools from {second_process}."),
&format!("Echo from {second_process}."),
);
let output = cached_done_response
let output_item = cached_done_response
.single_request()
.function_call_output_text(app_only_call_id)
.function_call_output(app_only_call_id);
let output = output_item["output"][1]["text"]
.as_str()
.expect("app-only tool error should be returned to the model");
assert!(
output.contains(&expected_error),
codex-rs/core/tests/suite/openai_file_mcp.rs
+3 -2
View File
@@ -396,8 +396,9 @@ async fn codex_apps_file_params_reject_denied_file_before_upload() -> Result<()>
let responses = run_extract_turn(&test, &server, permission_profile).await?;
let requests = responses.requests();
let output = requests[2]
.function_call_output_text("extract-call-1")
let output_item = requests[2].function_call_output("extract-call-1");
let output = output_item["output"][1]["text"]
.as_str()
.context("denied Apps upload should return a tool error")?;
assert!(
codex-rs/core/tests/suite/rmcp_client.rs
+97 -12
View File
@@ -587,6 +587,91 @@ async fn mcp_namespace_instructions_are_preserved_without_hiding_tools() -> anyh
Ok(())
}
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
async fn text_only_mcp_content_uses_content_items() -> anyhow::Result<()> {
skip_if_wine_exec!(
Ok(()),
"requires a Windows test_stdio_server in the Wine-exec environment"
);
skip_if_no_network!(Ok(()));
let server = responses::start_mock_server().await;
let call_id = "content-items-1";
let server_name = "rmcp";
let namespace = format!("mcp__{server_name}");
mount_sse_once(
&server,
responses::sse(vec![
responses::ev_response_created("resp-1"),
responses::ev_function_call_with_namespace(
call_id,
&namespace,
"image_scenario",
r#"{"scenario":"text_only","caption":"content item fixture result"}"#,
),
responses::ev_completed("resp-1"),
]),
)
.await;
let final_mock = mount_sse_once(
&server,
responses::sse(vec![
responses::ev_assistant_message("msg-1", "done"),
responses::ev_completed("resp-2"),
]),
)
.await;
let command = remote_aware_stdio_server_bin()?;
let fixture = test_codex()
.with_model_info_override("gpt-5.4", |model| model.supports_search_tool = false)
.with_config(move |config| {
insert_mcp_server(
config,
server_name,
stdio_transport(command, /*env*/ None, Vec::new()),
TestMcpServerOptions {
environment_id: remote_aware_environment_id(),
..Default::default()
},
);
})
.build_with_auto_env(&server)
.await?;
wait_for_mcp_server(&fixture.codex, server_name).await?;
fixture
.codex
.start_or_steer_turn(read_only_user_turn(&fixture, "return content items"))
.await?;
wait_for_event(&fixture.codex, |event| {
matches!(event, EventMsg::TurnComplete(_))
})
.await;
let output = final_mock.single_request().function_call_output(call_id);
let header = output["output"][0]["text"]
.as_str()
.expect("first content item should contain the wall-time header");
assert_wall_time_header(header);
assert_eq!(
output["output"],
json!([
{
"type": "input_text",
"text": header,
},
{
"type": "input_text",
"text": "content item fixture result",
},
])
);
server.verify().await;
Ok(())
}
#[test_case(false; "configured servers")]
#[test_case(true; "plugin servers")]
#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
@@ -2704,19 +2789,19 @@ async fn stdio_image_responses_are_sanitized_for_text_only_model() -> anyhow::Re
wait_for_event(&fixture.codex, |ev| matches!(ev, EventMsg::TurnComplete(_))).await;
let output_item = final_mock.single_request().function_call_output(call_id);
let output_text = output_item
.get("output")
.and_then(Value::as_str)
.expect("function_call_output output should be a JSON string");
let wrapped_payload = split_wall_time_wrapped_output(output_text);
let output_json: Value = serde_json::from_str(wrapped_payload)
.expect("function_call_output output should be valid JSON");
let header = output_item["output"][0]["text"]
.as_str()
.expect("first content item should contain the wall-time header");
assert_wall_time_header(header);
assert_eq!(
output_json,
json!([{
"type": "text",
"text": "<image content omitted because you do not support image input>"
}])
output_item["output"],
json!([
{"type": "input_text", "text": header},
{
"type": "input_text",
"text": "<image content omitted because you do not support image input>",
},
])
);
server.verify().await;
Ok(())
codex-rs/protocol/src/models.rs
+82 -34
View File
@@ -2230,13 +2230,12 @@ impl CallToolResult {
pub fn as_function_call_output_payload(&self) -> FunctionCallOutputPayload {
let content_items = convert_mcp_content_to_items(&self.content);
if content_items.as_ref().is_some_and(|items| {
items
.iter()
.any(|item| matches!(item, FunctionCallOutputContentItem::EncryptedContent { .. }))
}) {
if content_items
.iter()
.any(|item| matches!(item, FunctionCallOutputContentItem::EncryptedContent { .. }))
{
return FunctionCallOutputPayload {
body: FunctionCallOutputBody::ContentItems(content_items.unwrap_or_default()),
body: FunctionCallOutputBody::ContentItems(content_items),
success: Some(self.success()),
};
}
@@ -2260,23 +2259,8 @@ impl CallToolResult {
}
}
let serialized_content = match serde_json::to_string(&self.content) {
Ok(serialized_content) => serialized_content,
Err(err) => {
return FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(err.to_string()),
success: Some(false),
};
}
};
let body = match content_items {
Some(content_items) => FunctionCallOutputBody::ContentItems(content_items),
None => FunctionCallOutputBody::Text(serialized_content),
};
FunctionCallOutputPayload {
body,
body: FunctionCallOutputBody::ContentItems(content_items),
success: Some(self.success()),
}
}
@@ -2288,7 +2272,7 @@ impl CallToolResult {
fn convert_mcp_content_to_items(
contents: &[serde_json::Value],
) -> Option<Vec<FunctionCallOutputContentItem>> {
) -> Vec<FunctionCallOutputContentItem> {
const CODEX_ENCRYPTED_CONTENT_META_KEY: &str = "codex/encryptedContent";
const CODEX_IMAGE_DETAIL_META_KEY: &str = "codex/imageDetail";
@@ -2321,7 +2305,6 @@ fn convert_mcp_content_to_items(
Unknown,
}
let mut saw_content_item = false;
let mut items = Vec::with_capacity(contents.len());
for content in contents {
@@ -2333,7 +2316,6 @@ fn convert_mcp_content_to_items(
.and_then(serde_json::Value::as_bool)
== Some(true)
{
saw_content_item = true;
FunctionCallOutputContentItem::EncryptedContent {
encrypted_content: text,
}
@@ -2346,7 +2328,6 @@ fn convert_mcp_content_to_items(
mime_type,
meta,
}) => {
saw_content_item = true;
let image_url = if data.starts_with("data:") {
data
} else {
@@ -2373,7 +2354,6 @@ fn convert_mcp_content_to_items(
Ok(McpContent::Audio {
data, mime_type, ..
}) => {
saw_content_item = true;
let audio_url = if data.starts_with("data:") {
data
} else {
@@ -2389,7 +2369,7 @@ fn convert_mcp_content_to_items(
items.push(item);
}
if saw_content_item { Some(items) } else { None }
items
}
// Implement Display so callers can treat the payload like a plain string when logging or doing
@@ -2679,7 +2659,7 @@ mod tests {
"mimeType": "image/png",
})];
let items = convert_mcp_content_to_items(&contents).expect("expected image items");
let items = convert_mcp_content_to_items(&contents);
assert_eq!(
items,
vec![FunctionCallOutputContentItem::InputImage {
@@ -3011,7 +2991,7 @@ mod tests {
"mimeType": "image/png",
})];
let items = convert_mcp_content_to_items(&contents).expect("expected image items");
let items = convert_mcp_content_to_items(&contents);
assert_eq!(
items,
vec![FunctionCallOutputContentItem::InputImage {
@@ -3039,25 +3019,30 @@ mod tests {
assert_eq!(
convert_mcp_content_to_items(&contents),
Some(vec![
vec![
FunctionCallOutputContentItem::InputAudio {
audio_url: "data:audio/wav;base64,Zm9v".to_string(),
},
FunctionCallOutputContentItem::InputAudio {
audio_url: "data:audio/ogg;base64,YmFy".to_string(),
},
])
]
);
}
#[test]
fn convert_mcp_content_to_items_returns_none_without_media() {
fn convert_mcp_content_to_items_converts_text_without_media() {
let contents = vec![serde_json::json!({
"type": "text",
"text": "hello",
})];
assert_eq!(convert_mcp_content_to_items(&contents), None);
assert_eq!(
convert_mcp_content_to_items(&contents),
vec![FunctionCallOutputContentItem::InputText {
text: "hello".to_string(),
}]
);
}
#[test]
@@ -3302,6 +3287,69 @@ mod tests {
Ok(())
}
#[test]
fn converts_unstructured_mcp_content_to_items() {
let content = vec![
serde_json::json!({"type":"text","text":"caption"}),
serde_json::json!({
"type": "resource_link",
"uri": "file:///notes.txt",
"name": "notes",
}),
serde_json::json!({
"type": "audio",
"mimeType": "audio/wav",
}),
];
let call_tool_result = CallToolResult {
content: content.clone(),
structured_content: Some(serde_json::Value::Null),
is_error: Some(false),
meta: None,
};
let resource_link = serde_json::to_string(&content[1]).expect("serialize resource link");
let malformed_audio =
serde_json::to_string(&content[2]).expect("serialize malformed audio");
assert_eq!(
call_tool_result.as_function_call_output_payload(),
FunctionCallOutputPayload {
body: FunctionCallOutputBody::ContentItems(vec![
FunctionCallOutputContentItem::InputText {
text: "caption".to_string(),
},
FunctionCallOutputContentItem::InputText {
text: resource_link,
},
FunctionCallOutputContentItem::InputText {
text: malformed_audio,
},
]),
success: Some(true),
}
);
}
#[test]
fn preserves_structured_mcp_content() {
let call_tool_result = CallToolResult {
content: vec![serde_json::json!({"type":"text","text":"ignored"})],
structured_content: Some(serde_json::json!({"result":"structured"})),
is_error: Some(false),
meta: None,
};
assert_eq!(
call_tool_result.as_function_call_output_payload(),
FunctionCallOutputPayload {
body: FunctionCallOutputBody::Text(
serde_json::json!({"result":"structured"}).to_string(),
),
success: Some(true),
}
);
}
#[test]
fn serializes_image_outputs_as_array() -> Result<()> {
let call_tool_result = CallToolResult {
codex-rs/utils/output-truncation/src/lib.rs
+4
View File
@@ -102,6 +102,10 @@ pub fn truncate_function_output_items_with_policy(
for item in items {
match item {
FunctionCallOutputContentItem::InputText { text } => {
// Empty text contributes no model content but still consumes an API array slot.
if text.is_empty() {
continue;
}
if remaining_budget == 0 {
omitted_text_items += 1;
continue;
codex-rs/utils/output-truncation/src/truncate_tests.rs
+39
View File
@@ -163,6 +163,45 @@ fn truncates_across_multiple_under_limit_texts_and_reports_omitted() {
assert!(summary_text.contains("omitted 2 text items"));
}
#[test]
fn truncate_function_output_items_with_policy_discards_empty_text() {
let mut items = vec![
FunctionCallOutputContentItem::InputText {
text: String::new(),
};
16_384
];
for policy in [TruncationPolicy::Bytes(0), TruncationPolicy::Tokens(1)] {
assert_eq!(
truncate_function_output_items_with_policy(&items, policy, |_| 0),
Vec::new()
);
}
let content = vec![
FunctionCallOutputContentItem::InputText {
text: "caption".to_string(),
},
FunctionCallOutputContentItem::InputImage {
image_url: "img:one".to_string(),
detail: Some(DEFAULT_IMAGE_DETAIL),
},
FunctionCallOutputContentItem::InputAudio {
audio_url: "audio:one".to_string(),
},
FunctionCallOutputContentItem::EncryptedContent {
encrypted_content: "enc_opaque".to_string(),
},
];
items.extend(content.clone());
for policy in [TruncationPolicy::Bytes(16), TruncationPolicy::Tokens(4)] {
assert_eq!(
truncate_function_output_items_with_policy(&items, policy, |_| 1),
content
);
}
}
#[test]
fn formatted_truncate_text_content_items_with_policy_returns_original_under_limit() {
let items = vec![