diff --git a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs index 5bef33a26c..03ec181fd2 100644 --- a/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs +++ b/codex-rs/app-server/tests/suite/v2/dynamic_tools.rs @@ -17,6 +17,7 @@ use codex_app_server_protocol::ThreadStartResponse; use codex_app_server_protocol::TurnStartParams; use codex_app_server_protocol::TurnStartResponse; use codex_app_server_protocol::UserInput as V2UserInput; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use core_test_support::responses; @@ -223,11 +224,7 @@ async fn dynamic_tool_call_round_trip_sends_output_to_model() -> Result<()> { .iter() .find_map(|body| function_call_output_payload(body, call_id)) .context("expected function_call_output in follow-up request")?; - let expected_payload = FunctionCallOutputPayload { - content: "dynamic-ok".to_string(), - content_items: None, - success: None, - }; + let expected_payload = FunctionCallOutputPayload::from_text("dynamic-ok".to_string()); assert_eq!(payload, expected_payload); Ok(()) @@ -377,10 +374,15 @@ async fn dynamic_tool_call_round_trip_sends_content_items_to_model() -> Result<( .iter() .find_map(|body| function_call_output_payload(body, call_id)) .context("expected function_call_output in follow-up request")?; - assert_eq!(payload.content_items, Some(content_items.clone())); + assert_eq!( + payload.body, + FunctionCallOutputBody::ContentItems(content_items.clone()) + ); assert_eq!(payload.success, None); - // The deserializer keeps a compatibility text mirror in `content`. - assert_eq!(payload.content, serde_json::to_string(&content_items)?); + assert_eq!( + serde_json::to_string(&payload)?, + serde_json::to_string(&content_items)? + ); Ok(()) } diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index 2614ce83ef..5a4eea8836 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -3,6 +3,7 @@ use crate::config::types::Personality; use crate::error::Result; pub use codex_api::common::ResponseEvent; use codex_protocol::models::BaseInstructions; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::ResponseItem; use futures::Stream; use serde::Deserialize; @@ -97,9 +98,11 @@ fn reserialize_shell_outputs(items: &mut [ResponseItem]) { } ResponseItem::FunctionCallOutput { call_id, output } => { if shell_call_ids.remove(call_id) - && let Some(structured) = parse_structured_shell_output(&output.content) + && let Some(structured) = output + .text_content() + .and_then(parse_structured_shell_output) { - output.content = structured + output.body = FunctionCallOutputBody::Text(structured); } } _ => {} diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index ba34af5ed8..fd8c2aef97 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -5114,13 +5114,14 @@ mod tests { let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { - content: serde_json::to_string(&json!({ - "ok": true, - "value": 42 - })) - .unwrap(), + body: codex_protocol::models::FunctionCallOutputBody::Text( + serde_json::to_string(&json!({ + "ok": true, + "value": 42 + })) + .unwrap(), + ), success: Some(true), - ..Default::default() }; assert_eq!(expected, got); @@ -5157,10 +5158,10 @@ mod tests { let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { - content: serde_json::to_string(&vec![text_block("hello"), text_block("world")]) - .unwrap(), + body: codex_protocol::models::FunctionCallOutputBody::Text( + serde_json::to_string(&vec![text_block("hello"), text_block("world")]).unwrap(), + ), success: Some(true), - ..Default::default() }; assert_eq!(expected, got); @@ -5177,9 +5178,10 @@ mod tests { let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { - content: serde_json::to_string(&json!({ "message": "bad" })).unwrap(), + body: codex_protocol::models::FunctionCallOutputBody::Text( + serde_json::to_string(&json!({ "message": "bad" })).unwrap(), + ), success: Some(false), - ..Default::default() }; assert_eq!(expected, got); @@ -5196,9 +5198,10 @@ mod tests { let got = FunctionCallOutputPayload::from(&ctr); let expected = FunctionCallOutputPayload { - content: serde_json::to_string(&vec![text_block("alpha")]).unwrap(), + body: codex_protocol::models::FunctionCallOutputBody::Text( + serde_json::to_string(&vec![text_block("alpha")]).unwrap(), + ), success: Some(true), - ..Default::default() }; assert_eq!(expected, got); diff --git a/codex-rs/core/src/context_manager/history.rs b/codex-rs/core/src/context_manager/history.rs index a29f7df7e0..65a7cf9960 100644 --- a/codex-rs/core/src/context_manager/history.rs +++ b/codex-rs/core/src/context_manager/history.rs @@ -10,6 +10,7 @@ use crate::truncate::truncate_function_output_items_with_policy; use crate::truncate::truncate_text; use crate::user_shell_command::is_user_shell_command_text; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; @@ -136,7 +137,7 @@ impl ContextManager { match &mut self.items[index] { ResponseItem::FunctionCallOutput { output, .. } => { - let Some(content_items) = output.content_items.as_mut() else { + let Some(content_items) = output.content_items_mut() else { return false; }; let mut replaced = false; @@ -270,19 +271,23 @@ impl ContextManager { let policy_with_serialization_budget = policy * 1.2; match item { ResponseItem::FunctionCallOutput { call_id, output } => { - let truncated = - truncate_text(output.content.as_str(), policy_with_serialization_budget); - let truncated_items = output.content_items.as_ref().map(|items| { - truncate_function_output_items_with_policy( - items, - policy_with_serialization_budget, - ) - }); + let body = match &output.body { + FunctionCallOutputBody::Text(content) => FunctionCallOutputBody::Text( + truncate_text(content, policy_with_serialization_budget), + ), + FunctionCallOutputBody::ContentItems(items) => { + FunctionCallOutputBody::ContentItems( + truncate_function_output_items_with_policy( + items, + policy_with_serialization_budget, + ), + ) + } + }; ResponseItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { - content: truncated, - content_items: truncated_items, + body, success: output.success, }, } diff --git a/codex-rs/core/src/context_manager/history_tests.rs b/codex-rs/core/src/context_manager/history_tests.rs index a6eba62f1a..c1801e202d 100644 --- a/codex-rs/core/src/context_manager/history_tests.rs +++ b/codex-rs/core/src/context_manager/history_tests.rs @@ -3,6 +3,7 @@ use crate::truncate; use crate::truncate::TruncationPolicy; use codex_git::GhostCommit; use codex_protocol::models::ContentItem; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::LocalShellAction; @@ -63,10 +64,7 @@ fn user_input_text_msg(text: &str) -> ResponseItem { fn function_call_output(call_id: &str, content: &str) -> ResponseItem { ResponseItem::FunctionCallOutput { call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - content: content.to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text(content.to_string()), } } @@ -263,10 +261,7 @@ fn remove_first_item_removes_matching_output_for_function_call() { }, ResponseItem::FunctionCallOutput { call_id: "call-1".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ]; let mut h = create_history_with_items(items); @@ -279,10 +274,7 @@ fn remove_first_item_removes_matching_call_for_output() { let items = vec![ ResponseItem::FunctionCallOutput { call_id: "call-2".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ResponseItem::FunctionCall { id: None, @@ -308,10 +300,7 @@ fn remove_last_item_removes_matching_call_for_output() { }, ResponseItem::FunctionCallOutput { call_id: "call-delete-last".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ]; let mut h = create_history_with_items(items); @@ -327,10 +316,11 @@ fn replace_last_turn_images_replaces_tool_output_images() { ResponseItem::FunctionCallOutput { call_id: "call-1".to_string(), output: FunctionCallOutputPayload { - content: "ok".to_string(), - content_items: Some(vec![FunctionCallOutputContentItem::InputImage { - image_url: "data:image/png;base64,AAA".to_string(), - }]), + body: FunctionCallOutputBody::ContentItems(vec![ + FunctionCallOutputContentItem::InputImage { + image_url: "data:image/png;base64,AAA".to_string(), + }, + ]), success: Some(true), }, }, @@ -346,10 +336,11 @@ fn replace_last_turn_images_replaces_tool_output_images() { ResponseItem::FunctionCallOutput { call_id: "call-1".to_string(), output: FunctionCallOutputPayload { - content: "ok".to_string(), - content_items: Some(vec![FunctionCallOutputContentItem::InputText { - text: "Invalid image".to_string(), - }]), + body: FunctionCallOutputBody::ContentItems(vec![ + FunctionCallOutputContentItem::InputText { + text: "Invalid image".to_string(), + }, + ]), success: Some(true), }, }, @@ -391,10 +382,7 @@ fn remove_first_item_handles_local_shell_pair() { }, ResponseItem::FunctionCallOutput { call_id: "call-3".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ]; let mut h = create_history_with_items(items); @@ -560,10 +548,7 @@ fn normalization_retains_local_shell_outputs() { }, ResponseItem::FunctionCallOutput { call_id: "shell-1".to_string(), - output: FunctionCallOutputPayload { - content: "Total output lines: 1\n\nok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("Total output lines: 1\n\nok".to_string()), }, ]; @@ -583,9 +568,8 @@ fn record_items_truncates_function_call_output_content() { let item = ResponseItem::FunctionCallOutput { call_id: "call-100".to_string(), output: FunctionCallOutputPayload { - content: long_output.clone(), + body: FunctionCallOutputBody::Text(long_output.clone()), success: Some(true), - ..Default::default() }, }; @@ -594,16 +578,15 @@ fn record_items_truncates_function_call_output_content() { assert_eq!(history.items.len(), 1); match &history.items[0] { ResponseItem::FunctionCallOutput { output, .. } => { - assert_ne!(output.content, long_output); + let content = output.text_content().unwrap_or_default(); + assert_ne!(content, long_output); assert!( - output.content.contains("tokens truncated"), - "expected token-based truncation marker, got {}", - output.content + content.contains("tokens truncated"), + "expected token-based truncation marker, got {content}" ); assert!( - output.content.contains("tokens truncated"), - "expected truncation marker, got {}", - output.content + content.contains("tokens truncated"), + "expected truncation marker, got {content}" ); } other => panic!("unexpected history item: {other:?}"), @@ -648,9 +631,8 @@ fn record_items_respects_custom_token_limit() { let item = ResponseItem::FunctionCallOutput { call_id: "call-custom-limit".to_string(), output: FunctionCallOutputPayload { - content: long_output, + body: FunctionCallOutputBody::Text(long_output), success: Some(true), - ..Default::default() }, }; @@ -660,7 +642,11 @@ fn record_items_respects_custom_token_limit() { ResponseItem::FunctionCallOutput { output, .. } => output, other => panic!("unexpected history item: {other:?}"), }; - assert!(stored.content.contains("tokens truncated")); + assert!( + stored + .text_content() + .is_some_and(|content| content.contains("tokens truncated")) + ); } fn assert_truncated_message_matches(message: &str, line: &str, expected_removed: usize) { @@ -782,10 +768,7 @@ fn normalize_adds_missing_output_for_function_call() { }, ResponseItem::FunctionCallOutput { call_id: "call-x".to_string(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ] ); @@ -859,10 +842,7 @@ fn normalize_adds_missing_output_for_local_shell_call_with_id() { }, ResponseItem::FunctionCallOutput { call_id: "shell-1".to_string(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ] ); @@ -873,10 +853,7 @@ fn normalize_adds_missing_output_for_local_shell_call_with_id() { fn normalize_removes_orphan_function_call_output() { let items = vec![ResponseItem::FunctionCallOutput { call_id: "orphan-1".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }]; let mut h = create_history_with_items(items); @@ -913,10 +890,7 @@ fn normalize_mixed_inserts_and_removals() { // Orphan output that should be removed ResponseItem::FunctionCallOutput { call_id: "c2".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, // Will get an inserted custom tool output ResponseItem::CustomToolCall { @@ -955,10 +929,7 @@ fn normalize_mixed_inserts_and_removals() { }, ResponseItem::FunctionCallOutput { call_id: "c1".to_string(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ResponseItem::CustomToolCall { id: None, @@ -985,10 +956,7 @@ fn normalize_mixed_inserts_and_removals() { }, ResponseItem::FunctionCallOutput { call_id: "s1".to_string(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ] ); @@ -1015,10 +983,7 @@ fn normalize_adds_missing_output_for_function_call_inserts_output() { }, ResponseItem::FunctionCallOutput { call_id: "call-x".to_string(), - output: FunctionCallOutputPayload { - content: "aborted".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("aborted".to_string()), }, ] ); @@ -1065,10 +1030,7 @@ fn normalize_adds_missing_output_for_local_shell_call_with_id_panics_in_debug() fn normalize_removes_orphan_function_call_output_panics_in_debug() { let items = vec![ResponseItem::FunctionCallOutput { call_id: "orphan-1".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }]; let mut h = create_history_with_items(items); h.normalize_history(); @@ -1099,10 +1061,7 @@ fn normalize_mixed_inserts_and_removals_panics_in_debug() { }, ResponseItem::FunctionCallOutput { call_id: "c2".to_string(), - output: FunctionCallOutputPayload { - content: "ok".to_string(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".to_string()), }, ResponseItem::CustomToolCall { id: None, diff --git a/codex-rs/core/src/context_manager/normalize.rs b/codex-rs/core/src/context_manager/normalize.rs index 85e25e32aa..37e177900f 100644 --- a/codex-rs/core/src/context_manager/normalize.rs +++ b/codex-rs/core/src/context_manager/normalize.rs @@ -1,5 +1,6 @@ use std::collections::HashSet; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; @@ -29,7 +30,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { ResponseItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { - content: "aborted".to_string(), + body: FunctionCallOutputBody::Text("aborted".to_string()), ..Default::default() }, }, @@ -76,7 +77,7 @@ pub(crate) fn ensure_call_outputs_present(items: &mut Vec) { ResponseItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { - content: "aborted".to_string(), + body: FunctionCallOutputBody::Text("aborted".to_string()), ..Default::default() }, }, diff --git a/codex-rs/core/src/mcp_tool_call.rs b/codex-rs/core/src/mcp_tool_call.rs index 75248f34cc..95a41e1ebf 100644 --- a/codex-rs/core/src/mcp_tool_call.rs +++ b/codex-rs/core/src/mcp_tool_call.rs @@ -11,6 +11,7 @@ use crate::protocol::McpInvocation; use crate::protocol::McpToolCallBeginEvent; use crate::protocol::McpToolCallEndEvent; use codex_protocol::mcp::CallToolResult; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::protocol::AskForApproval; @@ -44,9 +45,8 @@ pub(crate) async fn handle_mcp_tool_call( return ResponseInputItem::FunctionCallOutput { call_id: call_id.clone(), output: FunctionCallOutputPayload { - content: format!("err: {e}"), + body: FunctionCallOutputBody::Text(format!("err: {e}")), success: Some(false), - ..Default::default() }, }; } diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 02d9822510..a81519daf2 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -14,6 +14,7 @@ use crate::parse_turn_item; use crate::proposed_plan_parser::strip_proposed_plan_blocks; use crate::tools::parallel::ToolCallRuntime; use crate::tools::router::ToolRouter; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; @@ -108,7 +109,7 @@ pub(crate) async fn handle_output_item_done( let response = ResponseInputItem::FunctionCallOutput { call_id: String::new(), output: FunctionCallOutputPayload { - content: msg.to_string(), + body: FunctionCallOutputBody::Text(msg.to_string()), ..Default::default() }, }; @@ -131,7 +132,7 @@ pub(crate) async fn handle_output_item_done( let response = ResponseInputItem::FunctionCallOutput { call_id: String::new(), output: FunctionCallOutputPayload { - content: message, + body: FunctionCallOutputBody::Text(message), ..Default::default() }, }; @@ -236,9 +237,8 @@ pub(crate) fn response_input_to_response_item(input: &ResponseInputItem) -> Opti let output = match result { Ok(call_tool_result) => FunctionCallOutputPayload::from(call_tool_result), Err(err) => FunctionCallOutputPayload { - content: err.clone(), + body: FunctionCallOutputBody::Text(err.clone()), success: Some(false), - ..Default::default() }, }; Some(ResponseItem::FunctionCallOutput { diff --git a/codex-rs/core/src/tools/context.rs b/codex-rs/core/src/tools/context.rs index f0bbb158f5..22e2d0b69f 100644 --- a/codex-rs/core/src/tools/context.rs +++ b/codex-rs/core/src/tools/context.rs @@ -5,6 +5,7 @@ use crate::tools::TELEMETRY_PREVIEW_MAX_LINES; use crate::tools::TELEMETRY_PREVIEW_TRUNCATION_NOTICE; use crate::turn_diff_tracker::TurnDiffTracker; use codex_protocol::mcp::CallToolResult; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputContentItem; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -97,13 +98,13 @@ impl ToolOutput { output: content, } } else { + let body = match content_items { + Some(content_items) => FunctionCallOutputBody::ContentItems(content_items), + None => FunctionCallOutputBody::Text(content), + }; ResponseInputItem::FunctionCallOutput { call_id: call_id.to_string(), - output: FunctionCallOutputPayload { - content, - content_items, - success, - }, + output: FunctionCallOutputPayload { body, success }, } } } @@ -196,8 +197,8 @@ mod tests { match response { ResponseInputItem::FunctionCallOutput { call_id, output } => { assert_eq!(call_id, "fn-1"); - assert_eq!(output.content, "ok"); - assert!(output.content_items.is_none()); + assert_eq!(output.text_content(), Some("ok")); + assert!(output.content_items().is_none()); assert_eq!(output.success, Some(true)); } other => panic!("expected FunctionCallOutput, got {other:?}"), diff --git a/codex-rs/core/src/tools/handlers/mcp.rs b/codex-rs/core/src/tools/handlers/mcp.rs index 5138b3cd0d..0f9d4c592a 100644 --- a/codex-rs/core/src/tools/handlers/mcp.rs +++ b/codex-rs/core/src/tools/handlers/mcp.rs @@ -57,11 +57,18 @@ impl ToolHandler for McpHandler { Ok(ToolOutput::Mcp { result }) } codex_protocol::models::ResponseInputItem::FunctionCallOutput { output, .. } => { - let codex_protocol::models::FunctionCallOutputPayload { - content, - content_items, - success, - } = output; + let success = output.success; + let (content, content_items) = match output.body { + codex_protocol::models::FunctionCallOutputBody::Text(content) => { + (content, None) + } + codex_protocol::models::FunctionCallOutputBody::ContentItems(content_items) => { + ( + serde_json::to_string(&content_items).unwrap_or_default(), + Some(content_items), + ) + } + }; Ok(ToolOutput::Function { content, content_items, diff --git a/codex-rs/core/src/tools/parallel.rs b/codex-rs/core/src/tools/parallel.rs index dcd3ae40ad..ca08048bd8 100644 --- a/codex-rs/core/src/tools/parallel.rs +++ b/codex-rs/core/src/tools/parallel.rs @@ -17,6 +17,7 @@ use crate::tools::context::SharedTurnDiffTracker; use crate::tools::context::ToolPayload; use crate::tools::router::ToolCall; use crate::tools::router::ToolRouter; +use codex_protocol::models::FunctionCallOutputBody; use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseInputItem; @@ -119,7 +120,7 @@ impl ToolCallRuntime { _ => ResponseInputItem::FunctionCallOutput { call_id: call.call_id.clone(), output: FunctionCallOutputPayload { - content: Self::abort_message(call, secs), + body: FunctionCallOutputBody::Text(Self::abort_message(call, secs)), ..Default::default() }, }, diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 51328ccc9f..d0390027d0 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -181,9 +181,8 @@ impl ToolRouter { ResponseInputItem::FunctionCallOutput { call_id, output: codex_protocol::models::FunctionCallOutputPayload { - content: message, + body: codex_protocol::models::FunctionCallOutputBody::Text(message), success: Some(false), - ..Default::default() }, } } diff --git a/codex-rs/core/tests/suite/client.rs b/codex-rs/core/tests/suite/client.rs index bd50708a2c..ed2ae66fa0 100644 --- a/codex-rs/core/tests/suite/client.rs +++ b/codex-rs/core/tests/suite/client.rs @@ -1251,10 +1251,7 @@ async fn azure_responses_request_includes_store_and_reasoning_ids() { }); prompt.input.push(ResponseItem::FunctionCallOutput { call_id: "function-call-id".into(), - output: FunctionCallOutputPayload { - content: "ok".into(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".into()), }); prompt.input.push(ResponseItem::LocalShellCall { id: Some("local-shell-id".into()), diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index bfa0e9466a..4f6f95a3ae 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -131,8 +131,8 @@ pub enum ResponseItem { }, // NOTE: The `output` field for `function_call_output` uses a dedicated payload type with // custom serialization. On the wire it is either: - // • a plain string (`content`) - // • an array of structured content items (`content_items`) + // - a plain string (`content`) + // - an array of structured content items (`content_items`) // We keep this behavior centralized in `FunctionCallOutputPayload`. FunctionCallOutput { call_id: String, @@ -617,9 +617,8 @@ impl From for ResponseItem { let output = match result { Ok(result) => FunctionCallOutputPayload::from(&result), Err(tool_call_err) => FunctionCallOutputPayload { - content: format!("err: {tool_call_err:?}"), + body: FunctionCallOutputBody::Text(format!("err: {tool_call_err:?}")), success: Some(false), - ..Default::default() }, }; Self::FunctionCallOutput { call_id, output } @@ -782,39 +781,82 @@ pub enum FunctionCallOutputContentItem { /// The payload we send back to OpenAI when reporting a tool call result. /// -/// `content` preserves a historical plain-text representation that downstream -/// code still uses for logs/history/tests. -/// -/// `content_items` holds structured tool output. When present, custom -/// serialization sends these items directly on the wire as the `output` value -/// (an array), rather than serializing `content`. +/// `body` serializes directly as the wire value for `function_call_output.output`. +/// `success` remains internal metadata for downstream handling. #[derive(Debug, Default, Clone, PartialEq, JsonSchema, TS)] pub struct FunctionCallOutputPayload { - pub content: String, - #[serde(skip_serializing_if = "Option::is_none")] - pub content_items: Option>, + pub body: FunctionCallOutputBody, pub success: Option, } -#[derive(Deserialize)] +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq, JsonSchema, TS)] #[serde(untagged)] -enum FunctionCallOutputPayloadSerde { +pub enum FunctionCallOutputBody { Text(String), - Items(Vec), + ContentItems(Vec), +} + +impl Default for FunctionCallOutputBody { + fn default() -> Self { + Self::Text(String::new()) + } +} + +impl FunctionCallOutputPayload { + pub fn from_text(content: String) -> Self { + Self { + body: FunctionCallOutputBody::Text(content), + success: None, + } + } + + pub fn from_content_items(content_items: Vec) -> Self { + Self { + body: FunctionCallOutputBody::ContentItems(content_items), + success: None, + } + } + + pub fn text_content(&self) -> Option<&str> { + match &self.body { + FunctionCallOutputBody::Text(content) => Some(content), + FunctionCallOutputBody::ContentItems(_) => None, + } + } + + pub fn text_content_mut(&mut self) -> Option<&mut String> { + match &mut self.body { + FunctionCallOutputBody::Text(content) => Some(content), + FunctionCallOutputBody::ContentItems(_) => None, + } + } + + pub fn content_items(&self) -> Option<&[FunctionCallOutputContentItem]> { + match &self.body { + FunctionCallOutputBody::Text(_) => None, + FunctionCallOutputBody::ContentItems(items) => Some(items), + } + } + + pub fn content_items_mut(&mut self) -> Option<&mut Vec> { + match &mut self.body { + FunctionCallOutputBody::Text(_) => None, + FunctionCallOutputBody::ContentItems(items) => Some(items), + } + } } // `function_call_output.output` is encoded as either: -// • an array of structured content items, when `content_items` is present -// • a plain string, otherwise +// - an array of structured content items +// - a plain string impl Serialize for FunctionCallOutputPayload { fn serialize(&self, serializer: S) -> Result where S: Serializer, { - if let Some(items) = &self.content_items { - items.serialize(serializer) - } else { - serializer.serialize_str(&self.content) + match &self.body { + FunctionCallOutputBody::Text(content) => serializer.serialize_str(content), + FunctionCallOutputBody::ContentItems(items) => items.serialize(serializer), } } } @@ -824,22 +866,11 @@ impl<'de> Deserialize<'de> for FunctionCallOutputPayload { where D: Deserializer<'de>, { - match FunctionCallOutputPayloadSerde::deserialize(deserializer)? { - FunctionCallOutputPayloadSerde::Text(content) => Ok(FunctionCallOutputPayload { - content, - ..Default::default() - }), - FunctionCallOutputPayloadSerde::Items(items) => { - // Preserve a text mirror for compatibility with legacy callers - // that still inspect `content`. - let content = serde_json::to_string(&items).map_err(serde::de::Error::custom)?; - Ok(FunctionCallOutputPayload { - content, - content_items: Some(items), - success: None, - }) - } - } + let body = FunctionCallOutputBody::deserialize(deserializer)?; + Ok(FunctionCallOutputPayload { + body, + success: None, + }) } } @@ -860,16 +891,14 @@ impl From<&CallToolResult> for FunctionCallOutputPayload { match serde_json::to_string(structured_content) { Ok(serialized_structured_content) => { return FunctionCallOutputPayload { - content: serialized_structured_content, + body: FunctionCallOutputBody::Text(serialized_structured_content), success: Some(is_success), - ..Default::default() }; } Err(err) => { return FunctionCallOutputPayload { - content: err.to_string(), + body: FunctionCallOutputBody::Text(err.to_string()), success: Some(false), - ..Default::default() }; } } @@ -879,18 +908,21 @@ impl From<&CallToolResult> for FunctionCallOutputPayload { Ok(serialized_content) => serialized_content, Err(err) => { return FunctionCallOutputPayload { - content: err.to_string(), + body: FunctionCallOutputBody::Text(err.to_string()), success: Some(false), - ..Default::default() }; } }; let content_items = convert_mcp_content_to_items(content); + let body = match content_items { + Some(content_items) => FunctionCallOutputBody::ContentItems(content_items), + None => FunctionCallOutputBody::Text(serialized_content), + }; + FunctionCallOutputPayload { - content: serialized_content, - content_items, + body, success: Some(is_success), } } @@ -941,19 +973,18 @@ fn convert_mcp_content_to_items( } // Implement Display so callers can treat the payload like a plain string when logging or doing -// trivial substring checks in tests (existing tests call `.contains()` on the output). Display -// returns the raw `content` field. +// trivial substring checks in tests (existing tests call `.contains()` on the output). For +// `ContentItems`, Display emits a JSON representation. impl std::fmt::Display for FunctionCallOutputPayload { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { - f.write_str(&self.content) - } -} - -impl std::ops::Deref for FunctionCallOutputPayload { - type Target = str; - fn deref(&self) -> &Self::Target { - &self.content + match &self.body { + FunctionCallOutputBody::Text(content) => f.write_str(content), + FunctionCallOutputBody::ContentItems(items) => { + let content = serde_json::to_string(items).unwrap_or_default(); + f.write_str(content.as_str()) + } + } } } @@ -1160,10 +1191,7 @@ mod tests { fn serializes_success_as_plain_string() -> Result<()> { let item = ResponseInputItem::FunctionCallOutput { call_id: "call1".into(), - output: FunctionCallOutputPayload { - content: "ok".into(), - ..Default::default() - }, + output: FunctionCallOutputPayload::from_text("ok".into()), }; let json = serde_json::to_string(&item)?; @@ -1179,9 +1207,8 @@ mod tests { let item = ResponseInputItem::FunctionCallOutput { call_id: "call1".into(), output: FunctionCallOutputPayload { - content: "bad".into(), + body: FunctionCallOutputBody::Text("bad".into()), success: Some(false), - ..Default::default() }, }; @@ -1206,7 +1233,10 @@ mod tests { let payload = FunctionCallOutputPayload::from(&call_tool_result); assert_eq!(payload.success, Some(true)); - let items = payload.content_items.clone().expect("content items"); + let Some(items) = payload.content_items() else { + panic!("expected content items"); + }; + let items = items.to_vec(); assert_eq!( items, vec![ @@ -1247,9 +1277,10 @@ mod tests { }; let payload = FunctionCallOutputPayload::from(&call_tool_result); - let Some(items) = payload.content_items else { + let Some(items) = payload.content_items() else { panic!("expected content items"); }; + let items = items.to_vec(); assert_eq!( items, vec![FunctionCallOutputContentItem::InputImage { @@ -1278,10 +1309,14 @@ mod tests { image_url: "data:image/png;base64,XYZ".into(), }, ]; - assert_eq!(payload.content_items, Some(expected_items.clone())); - - let expected_content = serde_json::to_string(&expected_items)?; - assert_eq!(payload.content, expected_content); + assert_eq!( + payload.body, + FunctionCallOutputBody::ContentItems(expected_items.clone()) + ); + assert_eq!( + serde_json::to_string(&payload)?, + serde_json::to_string(&expected_items)? + ); Ok(()) }