From 515c291d875e07faa64e3fa43dc9bbffed8db31f Mon Sep 17 00:00:00 2001 From: ningyi-oai Date: Thu, 8 Oct 2026 20:25:48 +0000 Subject: [PATCH] Preserve large arguments in executed tool call metadata (#52268) ## Why Fixed argument limits truncated recorded tool calls even when their metadata fit within the available message budget. ## What changed - Remove the 8 KiB per-call and 32 KiB per-output argument limits for direct calls and Code Mode. - Preserve full recorded arguments while retaining message metadata budgets, pending-call limits, and the direct metadata retention limit. - Rename the normalization helper to `normalize_executed_tool_call_completeness`; it now only clears completeness when calls or tool names were lost. ## Testing Update regression coverage for large arguments, escaped inputs, reserved-key wrappers, and argument and result metadata preservation across Code Mode waits. GitOrigin-RevId: 8c199ccf39191d5d44eef02d28b1773654735b43 --- .../core/src/tools/executed_tool_calls.rs | 50 +++++------------- .../executed_tool_calls/request_metadata.rs | 3 +- .../request_metadata_tests.rs | 39 +++++--------- .../tools/executed_tool_calls_direct_tests.rs | 14 ++--- codex-rs/core/tests/suite/code_mode.rs | 51 ++++++++++--------- .../core/tests/suite/direct_tool_metadata.rs | 15 +++--- codex-rs/core/tests/suite/tools.rs | 7 +-- codex-rs/protocol/src/models.rs | 2 +- .../src/models/executed_tool_calls.rs | 45 +++++----------- .../src/models/executed_tool_calls_tests.rs | 39 ++++++-------- 10 files changed, 97 insertions(+), 168 deletions(-) diff --git a/codex-rs/core/src/tools/executed_tool_calls.rs b/codex-rs/core/src/tools/executed_tool_calls.rs index 6ca54d44a3..853c88a972 100644 --- a/codex-rs/core/src/tools/executed_tool_calls.rs +++ b/codex-rs/core/src/tools/executed_tool_calls.rs @@ -28,7 +28,7 @@ use codex_protocol::models::ExecutedToolCallArguments; use codex_protocol::models::ResponseItem; use codex_protocol::models::ToolResultMetadata; use codex_protocol::models::executed_tool_call_metadata_bytes; -use codex_protocol::models::normalize_executed_tool_call_arguments; +use codex_protocol::models::normalize_executed_tool_call_completeness; use codex_protocol::openai_models::ToolMode; use indexmap::IndexMap; use serde_json::Value as JsonValue; @@ -46,8 +46,6 @@ mod seen_ids; use seen_ids::SeenIds; -const MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES: usize = 8 * 1024; -const MAX_EXECUTED_TOOL_CALL_FULL_ARGUMENT_BYTES_PER_OUTPUT: usize = 32 * 1024; const MAX_PENDING_EXECUTED_TOOL_CALLS: usize = 256; // Limits new Direct metadata retained in history and the append-only rollout // during one recorder lifetime. A new process cannot recover the exact prior @@ -150,7 +148,6 @@ enum CellCompletion { #[derive(Default)] struct RecordedCell { pending_calls: IndexMap, - pending_full_argument_bytes: usize, completion: CellCompletion, originating_call_id: Option, truncated_metadata_binding_valid: bool, @@ -365,17 +362,7 @@ impl ExecutedToolCalls { return None; } let permit = self.reserve_direct_call()?; - let (mut recorded, _) = recorded_call(call); - let argument_bytes = serialized_json_bytes(recorded.arguments()).unwrap_or(usize::MAX); - if matches!(recorded.arguments(), ExecutedToolCallArguments::Raw(_)) - && argument_bytes > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES - { - recorded = ExecutedToolCall::truncated( - recorded.name, - argument_bytes, - MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES, - ); - } + let (recorded, _) = recorded_call(call); Some((recorded, permit)) } @@ -561,19 +548,10 @@ impl ExecutedToolCalls { if !unique_nested_id || duplicate_call_id { cell.truncated_metadata_binding_valid = false; } - let max_bytes = MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES.min( - MAX_EXECUTED_TOOL_CALL_FULL_ARGUMENT_BYTES_PER_OUTPUT - .saturating_sub(cell.pending_full_argument_bytes), - ); let call = if at_pending_call_limit { ExecutedToolCall::truncated(call.name, original_bytes, /*max_bytes*/ 0) - } else if original_bytes <= max_bytes { - cell.pending_full_argument_bytes = cell - .pending_full_argument_bytes - .saturating_add(original_bytes); - call } else { - ExecutedToolCall::truncated(call.name, original_bytes, max_bytes) + call }; let truncated = matches!( call.arguments(), @@ -748,20 +726,16 @@ fn recorded_call(call: &ToolCall) -> (ExecutedToolCall, usize) { } }; let name = codex_tools::code_mode_name_for_tool_name(&call.tool_name); - let recorded_call = if original_bytes > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES { - ExecutedToolCall::truncated(name, original_bytes, MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES) - } else { - let arguments = match &call.payload { - ToolPayload::Function { arguments } => serde_json::from_str(arguments) - .unwrap_or_else(|_| JsonValue::String(arguments.clone())), - ToolPayload::Custom { input } => JsonValue::String(input.clone()), - ToolPayload::ToolSearch { arguments } => { - serde_json::to_value(arguments).unwrap_or_default() - } - }; - ExecutedToolCall::new(name, arguments) + let arguments = match &call.payload { + ToolPayload::Function { arguments } => { + serde_json::from_str(arguments).unwrap_or_else(|_| JsonValue::String(arguments.clone())) + } + ToolPayload::Custom { input } => JsonValue::String(input.clone()), + ToolPayload::ToolSearch { arguments } => { + serde_json::to_value(arguments).unwrap_or_default() + } }; - (recorded_call, original_bytes) + (ExecutedToolCall::new(name, arguments), original_bytes) } fn input_call_id(item: &ResponseItem) -> Option<&str> { diff --git a/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs b/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs index 346bc01ff7..01d0056b59 100644 --- a/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs +++ b/codex-rs/core/src/tools/executed_tool_calls/request_metadata.rs @@ -57,7 +57,7 @@ impl ExecutedToolCalls { // Argument truncation preserves inventory completeness; omitted calls or // names still invalidate completion across the cell's outputs. - normalize_executed_tool_call_arguments(items); + normalize_executed_tool_call_completeness(items); } fn attach_pending_to_prompt_with_state( @@ -258,7 +258,6 @@ impl ExecutedToolCalls { } calls.push(call); } - cell.pending_full_argument_bytes = 0; complete = cell.completion == CellCompletion::Complete && (state.can_prove_wait_completion || matches!(item, ResponseItem::CustomToolCallOutput { .. })); diff --git a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs index e168c9a660..1c504bf872 100644 --- a/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs +++ b/codex-rs/core/src/tools/executed_tool_calls/request_metadata_tests.rs @@ -886,11 +886,11 @@ fn tool_call_completeness_survives_waits_without_changing_deltas() { if index < 2 { // Identical calls are distinct submissions; truncation stays sticky across waits. let original_bytes = if truncated && index == 1 { 9_000 } else { 2 }; - let call = if original_bytes > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES { + let call = if truncated && index == 1 { ExecutedToolCall::truncated( "nested_tool".to_string(), original_bytes, - MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES, + /*max_bytes*/ 8_192, ) } else { ExecutedToolCall::new("nested_tool".to_string(), json!({})) @@ -1156,40 +1156,33 @@ fn completeness_rejects_direct_id_collision_and_late_calls() { } #[test] -fn newly_normalized_arguments_preserve_later_wait_completeness() { +fn large_wrapped_arguments_preserve_later_wait_completeness() { let recorder = new_recorder(InitialHistory::New); let cell = CellId::new("active-cell".to_string()); recorder.start_cell(&cell, "exec"); let arguments = json!({ "_codex_executed_tool_call_truncated": true, - "padding": "x".repeat(8_120), - }) - .to_string(); + "padding": "x".repeat(40 * 1024), + }); let (call, original_bytes) = recorded_call(&ToolCall { tool_name: codex_tools::ToolName::plain("nested_tool"), call_id: "nested".to_string(), - payload: ToolPayload::Function { arguments }, + payload: ToolPayload::Function { + arguments: arguments.to_string(), + }, encrypted_function_args: None, }); - assert!(original_bytes <= MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES); - assert!( - serialized_json_bytes(call.arguments()).unwrap() > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES - ); recorder.record_nested_tool_call(cell.clone(), "nested".to_string(), call, original_bytes); let mut initial = [exec_input("exec"), exec_output("exec")]; recorder.attach_to_prompt(&mut initial, &mut HashMap::new()); - assert!( + assert_eq!( initial[1] .executed_tool_call_metadata() .unwrap() .executed_tool_calls .as_ref() - .unwrap() - .iter() - .any(|call| matches!( - call.arguments(), - ExecutedToolCallArguments::Truncated { .. } - )) + .unwrap(), + &vec![ExecutedToolCall::new("nested_tool".to_string(), arguments)], ); recorder.register_cell(&cell, "wait"); recorder.finish_cell_recording(&cell); @@ -1612,13 +1605,12 @@ fn late_truncated_metadata_preserves_large_result_on_retry() { } #[test] -fn late_truncated_metadata_is_not_reused_after_budget_changes_calls() { +fn late_truncated_metadata_with_wrapped_arguments_rejects_ambiguous_outputs() { let recorder = new_recorder(InitialHistory::New); let cell = CellId::new("late-truncated-cell".to_string()); recorder.start_cell(&cell, "exec"); record_truncated_call(&recorder, &cell, "nested"); - // The reserved key requires a wrapper in recorded arguments. The wrapper can - // put an otherwise accepted function argument over the later prompt limit. + // The reserved key requires a wrapper in recorded arguments. let arguments = json!({ "_codex_executed_tool_call_truncated": true, "padding": "x".repeat(8_120), @@ -1630,11 +1622,6 @@ fn late_truncated_metadata_is_not_reused_after_budget_changes_calls() { payload: ToolPayload::Function { arguments }, encrypted_function_args: None, }); - assert!(original_bytes <= MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES); - assert!( - serialized_json_bytes(wrapped_call.arguments()).unwrap() - > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES - ); recorder.record_nested_tool_call( cell.clone(), "other".to_string(), diff --git a/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs b/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs index bda3cca798..304375bbb8 100644 --- a/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs +++ b/codex-rs/core/src/tools/executed_tool_calls_direct_tests.rs @@ -245,7 +245,7 @@ async fn direct_pending_limit_releases_on_completion_or_dropped_future() { } #[tokio::test] -async fn direct_budget_counts_the_encoded_argument_before_it_enters_history() { +async fn direct_recording_preserves_encoded_arguments_in_history() { let (_, turn) = crate::session::tests::make_session_and_context().await; let step = StepContext::for_test(Arc::new(turn)); let mut features = Features::default(); @@ -253,13 +253,12 @@ async fn direct_budget_counts_the_encoded_argument_before_it_enters_history() { let recorder = ExecutedToolCalls::new(&features, &InitialHistory::New); let invalid_json = "\"".repeat(5_000); let wire_bytes = serialized_json_bytes(&JsonValue::String(invalid_json.clone())).unwrap(); - assert!(invalid_json.len() < MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES); - assert!(wire_bytes > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES); + assert!(wire_bytes > 8 * 1024); let call = ToolCall { tool_name: codex_tools::ToolName::plain("test_tool"), call_id: "direct".to_string(), payload: ToolPayload::Function { - arguments: invalid_json, + arguments: invalid_json.clone(), }, encrypted_function_args: None, }; @@ -273,10 +272,7 @@ async fn direct_budget_counts_the_encoded_argument_before_it_enters_history() { assert_eq!(wire["output"], ordinary_output); let metadata = &wire["internal_chat_message_metadata_passthrough"]; let arguments = &metadata["executed_tool_calls"][0]["arguments"]; - assert!(serialized_json_bytes(arguments).unwrap() < MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES); - assert_eq!( - arguments["_codex_executed_tool_call_truncated"]["original_bytes"], - wire_bytes, - ); + assert_eq!(arguments, &JsonValue::String(invalid_json)); + assert_eq!(serialized_json_bytes(arguments).unwrap(), wire_bytes); assert_eq!(metadata["tool_calls_complete"], true); } diff --git a/codex-rs/core/tests/suite/code_mode.rs b/codex-rs/core/tests/suite/code_mode.rs index 40cd9bd5e5..b2371a0c9e 100644 --- a/codex-rs/core/tests/suite/code_mode.rs +++ b/codex-rs/core/tests/suite/code_mode.rs @@ -1548,7 +1548,7 @@ async fn code_mode_tool_call_completeness_is_private_and_opt_in( let code = if oversized { r#" const args = { barrier: { id: "", participants: 1 } }; -args.barrier.id = "x".repeat(8192 - JSON.stringify(args).length); +args.barrier.id = "x".repeat(9000 - JSON.stringify(args).length); for (let index = 0; index < 17; index++) await tools.test_sync_tool(args); text("done"); yield_control(); @@ -1577,12 +1577,17 @@ await new Promise(() => {}); let calls = serde_json::json!([{ "name": "test_sync_tool", "arguments": {} }]); let complete = Value::Bool(true); if oversized { - assert!( - metadata["executed_tool_calls"] - .as_array() - .is_some_and(|calls| calls.iter().any(|call| call["arguments"] - .get("_codex_executed_tool_call_truncated") - .is_some())) + let mut arguments = serde_json::json!({ "barrier": { "id": "", "participants": 1 } }); + arguments["barrier"]["id"] = + serde_json::json!("x".repeat(9000 - arguments.to_string().len())); + assert_eq!( + metadata["executed_tool_calls"], + serde_json::json!(vec![ + serde_json::json!({ + "name": "test_sync_tool", "arguments": arguments, + }); + 17 + ]), ); } else { assert_eq!( @@ -2976,7 +2981,7 @@ async fn code_mode_result_metadata_keeps_prepared_call_binding_across_runtime_re } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn code_mode_late_truncated_result_metadata_survives_waits() -> Result<()> { +async fn code_mode_late_result_metadata_with_large_arguments_survives_waits() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; let metadata = serde_json::json!({ @@ -3028,7 +3033,7 @@ async fn code_mode_late_truncated_result_metadata_survives_waits() -> Result<()> .context("first call did not reach the result gate")??; // The call may be dispatched after the first yield. Wait while its result is held so - // the request has already recorded the truncated call before accepting its metadata. + // the request has already recorded the call before accepting its metadata. let held = responses::mount_function_call_agent_response( &server, "call-2", @@ -3044,16 +3049,16 @@ async fn code_mode_late_truncated_result_metadata_survives_waits() -> Result<()> let original_output = held_calls[0]; let original_id = original_output["call_id"].as_str().unwrap().to_string(); let original_type = original_output["type"].as_str().unwrap().to_string(); - let truncated = + let recorded = &original_output["internal_chat_message_metadata_passthrough"]["executed_tool_calls"][0]; assert!( - truncated["name"] + recorded["name"] .as_str() .unwrap() .ends_with(RESULT_METADATA_TOOL) ); - assert!(truncated["arguments"]["_codex_executed_tool_call_truncated"].is_object()); - assert!(truncated.get("tool_result_metadata").is_none()); + assert_eq!(recorded["arguments"], arguments); + assert!(recorded.get("tool_result_metadata").is_none()); release_tx.send(()).unwrap(); let resumed = responses::mount_function_call_agent_response( @@ -3096,7 +3101,7 @@ async fn code_mode_late_truncated_result_metadata_survives_waits() -> Result<()> let captured_calls = &captured_output["internal_chat_message_metadata_passthrough"]["executed_tool_calls"]; assert_eq!(captured_calls.as_array().unwrap().len(), 1); - assert_eq!(captured_calls[0]["arguments"], truncated["arguments"]); + assert_eq!(captured_calls[0]["arguments"], recorded["arguments"]); assert_eq!(captured_calls[0]["tool_result_metadata"], metadata); assert_ne!( captured_output["internal_chat_message_metadata_passthrough"]["tool_calls_complete"], @@ -3124,7 +3129,7 @@ async fn code_mode_late_truncated_result_metadata_survives_waits() -> Result<()> let original = terminal_request.call_output(&original_id, &original_type); assert_eq!( original["internal_chat_message_metadata_passthrough"]["executed_tool_calls"][0]["arguments"], - truncated["arguments"] + recorded["arguments"] ); assert!( original["internal_chat_message_metadata_passthrough"]["executed_tool_calls"][0] @@ -3548,7 +3553,7 @@ text("pressure ready");"#, } #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn code_mode_complete_call_survives_unrelated_truncation() -> Result<()> { +async fn code_mode_complete_calls_preserve_large_arguments() -> Result<()> { skip_if_no_network!(Ok(())); let server = responses::start_mock_server().await; @@ -3589,12 +3594,11 @@ async fn code_mode_complete_call_survives_unrelated_truncation() -> Result<()> { let request = follow_up.single_request(); let overflow = &request.custom_tool_call_output("call-1")["internal_chat_message_metadata_passthrough"]; - assert!( - overflow["executed_tool_calls"] - .as_array() - .is_some_and(|calls| calls.iter().any(|call| call["arguments"] - .get("_codex_executed_tool_call_truncated") - .is_some())) + assert_eq!( + overflow["executed_tool_calls"], + serde_json::json!([{ "name": "test_sync_tool", "arguments": { + "barrier": { "id": "x".repeat(8192), "participants": 1 } + } }]), ); assert_eq!(overflow["tool_calls_complete"], true); @@ -9278,8 +9282,7 @@ async fn code_mode_argument_truncation_preserves_later_wait_completeness() -> Re probe.codex.shutdown_and_wait().await?; probe_server.shutdown().await; - // One 7 KiB invocation stays under the recorder's per-output argument - // budget. It pushes the yielded delta over the message budget only. + // One 7 KiB invocation pushes the yielded delta over the message budget. let padding = "x".repeat(LIMIT - base_bytes - 4 * 1024); let code = r#" await tools.test_sync_tool({ barrier: { id: "x".repeat(7000), participants: 1 } }); diff --git a/codex-rs/core/tests/suite/direct_tool_metadata.rs b/codex-rs/core/tests/suite/direct_tool_metadata.rs index 67285f14ac..68fc43e217 100644 --- a/codex-rs/core/tests/suite/direct_tool_metadata.rs +++ b/codex-rs/core/tests/suite/direct_tool_metadata.rs @@ -538,6 +538,8 @@ async fn direct_function_and_tool_search_mark_complete_attempts( // These calls exceed the old request budget but stay below Direct's retained-history limit. let budget_arguments = json!({"plan": [{"step": "x".repeat(7 * 1024), "status": "in_progress"}]}); + let oversized_arguments = + json!({"plan": [{"step": "x".repeat(9 * 1024), "status": "in_progress"}]}); let budget_arguments_json = budget_arguments.to_string(); assert!(budget_arguments_json.len() < 8 * 1024); if budget_calls > 0 { @@ -549,7 +551,7 @@ async fn direct_function_and_tool_search_mark_complete_attempts( events.push(ev_function_call( "plan-oversized", "update_plan", - &json!({"plan": [{"step": "x".repeat(9 * 1024), "status": "in_progress"}]}).to_string(), + &oversized_arguments.to_string(), )); } for index in 0..budget_calls { @@ -625,12 +627,13 @@ async fn direct_function_and_tool_search_mark_complete_attempts( assert_eq!(output["output"], json!("Plan updated")); let metadata = tool_call_metadata(output); metadata_bytes += serde_json::to_vec(&metadata)?.len(); - assert!( - metadata["executed_tool_calls"][0]["arguments"] - .get("_codex_executed_tool_call_truncated") - .is_some() + assert_eq!( + metadata, + json!({ + "executed_tool_calls": [{"name": "update_plan", "arguments": oversized_arguments}], + "tool_calls_complete": true, + }), ); - assert_eq!(metadata["tool_calls_complete"], true); } for index in 0..budget_calls { let output = request.function_call_output(&format!("plan-budget-{index}")); diff --git a/codex-rs/core/tests/suite/tools.rs b/codex-rs/core/tests/suite/tools.rs index cf663b0be4..e371db1e3f 100644 --- a/codex-rs/core/tests/suite/tools.rs +++ b/codex-rs/core/tests/suite/tools.rs @@ -527,12 +527,7 @@ async fn namespaced_custom_tool_call_preserves_namespace_through_dispatch_and_re ); let expected_escaped_calls = json!([{ "name": format!("{namespace}__{tool_name}"), - "arguments": { - "_codex_executed_tool_call_truncated": { - "original_bytes": serde_json::to_vec(&escaped_input)?.len(), - "max_bytes": 8 * 1024, - }, - }, + "arguments": escaped_input, }]); assert_eq!( tool_call_metadata(escaped_request.custom_tool_call_output(escaped_call_id)), diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 58e20012d1..28b70155ad 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -50,7 +50,7 @@ pub use executed_tool_calls::ToolResultSource; pub use executed_tool_calls::ToolResultSources; pub use executed_tool_calls::bound_executed_tool_calls_for_message; pub use executed_tool_calls::executed_tool_call_metadata_bytes; -pub use executed_tool_calls::normalize_executed_tool_call_arguments; +pub use executed_tool_calls::normalize_executed_tool_call_completeness; pub use item_metadata::ContentItemKind; pub use item_metadata::ContentItemMetadata; pub use item_metadata::ContentItemNamespace; diff --git a/codex-rs/protocol/src/models/executed_tool_calls.rs b/codex-rs/protocol/src/models/executed_tool_calls.rs index 2a817a6a0e..95151c2694 100644 --- a/codex-rs/protocol/src/models/executed_tool_calls.rs +++ b/codex-rs/protocol/src/models/executed_tool_calls.rs @@ -8,7 +8,6 @@ use ts_rs::TS; use super::InternalChatMessageMetadataPassthrough; use super::ResponseItem; -const MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES: usize = 8 * 1024; /// Maximum distinct result sources retained for one tool invocation. const MAX_TOOL_RESULT_SOURCES: usize = 32; /// Maximum UTF-8 bytes for each source's `type` and `id` separately, not the source list. @@ -86,12 +85,10 @@ impl InternalChatMessageMetadataPassthrough { } } -/// Bounds recorded arguments, preserving completion unless calls or tool names were lost. -/// Returns cells whose arguments were newly truncated. Ordinary tool-call arguments and -/// outputs are not changed. -pub fn normalize_executed_tool_call_arguments(items: &mut [ResponseItem]) -> HashSet { +/// Clears completion across a cell when recorded calls or tool names were lost. +/// Argument truncation preserves inventory completeness; arguments and outputs are unchanged. +pub fn normalize_executed_tool_call_completeness(items: &mut [ResponseItem]) { let mut damaged_cells = HashSet::new(); - let mut newly_truncated_cells = HashSet::new(); for item in items.iter_mut() { let Some(metadata) = item .internal_chat_message_metadata_passthrough_mut() @@ -99,33 +96,17 @@ pub fn normalize_executed_tool_call_arguments(items: &mut [ResponseItem]) -> Has else { continue; }; - let mut inventory_lost = false; - let mut newly_truncated = false; - for call in metadata.executed_tool_calls.iter_mut().flatten() { - let argument_bytes = serde_json::to_vec(&call.arguments) - .map(|bytes| bytes.len()) - .unwrap_or(usize::MAX); - if call.truncation().is_none() && argument_bytes > MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES - { - call.set_truncation( - argument_bytes, - MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES, - /*omitted_calls*/ None, - ); - newly_truncated = true; - } - inventory_lost |= !call.has_complete_inventory(); - } - if inventory_lost { + if metadata + .executed_tool_calls + .iter() + .flatten() + .any(|call| !call.has_complete_inventory()) + { metadata.tool_calls_complete = None; damaged_cells.extend(metadata.cell_id.clone()); } - if newly_truncated { - newly_truncated_cells.extend(metadata.cell_id.clone()); - } } clear_damaged_cell_completeness(items, &damaged_cells); - newly_truncated_cells } /// Bounds optional observations to the space left in the actual outgoing message. @@ -339,9 +320,7 @@ fn truncate_call_arguments_to_fit( let truncated = ExecutedToolCallArguments::Truncated { truncation: ExecutedToolCallTruncation { original_bytes, - max_bytes: original_bytes - .saturating_sub(overage_bytes) - .min(MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES), + max_bytes: original_bytes.saturating_sub(overage_bytes), omitted_calls: None, original_name_bytes: None, }, @@ -849,7 +828,7 @@ impl ResponseItem { let omitted_calls = (represented_calls > 1).then_some(represented_calls - 1); call.set_truncation_with_name( original_bytes, - max_call_bytes.min(MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES), + max_call_bytes, omitted_calls, original_name_bytes, ); @@ -862,7 +841,7 @@ impl ResponseItem { let call = &mut calls[0]; call.set_truncation_with_name( original_bytes, - max_call_bytes.min(MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES), + max_call_bytes, omitted_calls, Some(original_name_bytes.unwrap_or(call.name.len())), ); diff --git a/codex-rs/protocol/src/models/executed_tool_calls_tests.rs b/codex-rs/protocol/src/models/executed_tool_calls_tests.rs index 311d6c2a3c..7ce7159974 100644 --- a/codex-rs/protocol/src/models/executed_tool_calls_tests.rs +++ b/codex-rs/protocol/src/models/executed_tool_calls_tests.rs @@ -132,7 +132,7 @@ fn model_arguments_cannot_forge_executed_tool_call_truncation() -> Result<()> { forged_marker.clone(), )]); - normalize_executed_tool_call_arguments(std::slice::from_mut(&mut item)); + normalize_executed_tool_call_completeness(std::slice::from_mut(&mut item)); let call = item .executed_tool_call_metadata() .and_then(|metadata| metadata.executed_tool_calls.as_ref()) @@ -177,7 +177,7 @@ fn tool_call_completeness_is_host_only_and_fail_closed() -> Result<()> { let mut item = output("call-1"); item.set_tool_call_cell_id("cell-1"); item.mark_tool_calls_complete(); - normalize_executed_tool_call_arguments(std::slice::from_mut(&mut item)); + normalize_executed_tool_call_completeness(std::slice::from_mut(&mut item)); assert_eq!( serde_json::to_value(&item)?["internal_chat_message_metadata_passthrough"], serde_json::json!({ @@ -188,21 +188,15 @@ fn tool_call_completeness_is_host_only_and_fail_closed() -> Result<()> { item.clear_executed_tool_calls(); assert!(item.executed_tool_call_metadata().is_none()); - for (call, newly_truncated) in [ - ( - ExecutedToolCall::new( - "test_tool".to_string(), - serde_json::json!({ "payload": "x".repeat(MAX_EXECUTED_TOOL_CALL_ARGUMENT_BYTES + 1) }), - ), - true, + for call in [ + ExecutedToolCall::new( + "test_tool".to_string(), + serde_json::json!({ "payload": "x".repeat(9_000) }), ), - ( - ExecutedToolCall::truncated( - "test_tool".to_string(), - /*original_bytes*/ 9_000, - /*max_bytes*/ 0, - ), - false, + ExecutedToolCall::truncated( + "test_tool".to_string(), + /*original_bytes*/ 9_000, + /*max_bytes*/ 0, ), ] { for same_cell in [false, true] { @@ -214,12 +208,11 @@ fn tool_call_completeness_is_host_only_and_fail_closed() -> Result<()> { } item.mark_tool_calls_complete(); } - let changed_cells = normalize_executed_tool_call_arguments(&mut items); - assert_eq!( - changed_cells.contains("cell-1"), - same_cell && newly_truncated - ); - assert!(normalize_executed_tool_call_arguments(&mut items).is_empty()); + let expected = items.clone(); + normalize_executed_tool_call_completeness(&mut items); + assert_eq!(items, expected); + normalize_executed_tool_call_completeness(&mut items); + assert_eq!(items, expected); assert_eq!( items.map(|item| item .executed_tool_call_metadata() @@ -605,7 +598,7 @@ fn inventory_loss_still_clears_completeness() { let mut item = output("call"); item.append_executed_tool_calls(vec![call]); item.mark_tool_calls_complete(); - normalize_executed_tool_call_arguments(std::slice::from_mut(&mut item)); + normalize_executed_tool_call_completeness(std::slice::from_mut(&mut item)); assert_eq!( item.executed_tool_call_metadata() .unwrap()