From b9f7dbf3c57b61a023581c1a2d98339547503bee Mon Sep 17 00:00:00 2001 From: Anton Panasenko Date: Mon, 29 Jun 2026 16:54:03 -0700 Subject: [PATCH] codex: route tool search parse failures through runtime (#30618) --- .../core/src/context_manager/normalize.rs | 2 +- codex-rs/core/src/session/tests.rs | 6 +- codex-rs/core/src/stream_events_utils.rs | 94 +------------------ codex-rs/core/src/tools/context_tests.rs | 6 +- .../core/src/tools/handlers/tool_search.rs | 12 ++- codex-rs/core/src/tools/router.rs | 38 +++----- codex-rs/core/src/tools/router_tests.rs | 6 +- .../core/src/tools/tool_dispatch_trace.rs | 15 ++- codex-rs/tools/src/lib.rs | 1 + codex-rs/tools/src/tool_payload.rs | 9 +- 10 files changed, 53 insertions(+), 136 deletions(-) diff --git a/codex-rs/core/src/context_manager/normalize.rs b/codex-rs/core/src/context_manager/normalize.rs index 6592705111..2208563cde 100644 --- a/codex-rs/core/src/context_manager/normalize.rs +++ b/codex-rs/core/src/context_manager/normalize.rs @@ -5,6 +5,7 @@ use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::ResponseItem; use codex_protocol::models::SearchToolCallParams; use codex_protocol::openai_models::InputModality; +use codex_tools::INVALID_TOOL_SEARCH_QUERY; use serde::Deserialize; use std::collections::HashSet; use tracing::info; @@ -15,7 +16,6 @@ use crate::util::error_or_panic; const IMAGE_CONTENT_OMITTED_PLACEHOLDER: &str = "image content omitted because you do not support image input"; -const INVALID_TOOL_SEARCH_QUERY: &str = "[invalid tool_search arguments omitted]"; // Changing this value would change model-visible IDs and invalidate prompt caches. const SYNTHETIC_OUTPUT_ID_NAMESPACE: Uuid = Uuid::from_u128(0x90d38d3e_6a5b_4d52_bfe2_2f1e634bfac4); diff --git a/codex-rs/core/src/session/tests.rs b/codex-rs/core/src/session/tests.rs index ece9d446b5..bc3b1a1a25 100644 --- a/codex-rs/core/src/session/tests.rs +++ b/codex-rs/core/src/session/tests.rs @@ -10487,9 +10487,7 @@ async fn fatal_tool_error_stops_turn_and_reports_error() { internal_chat_message_metadata_passthrough: None, }; - let call = ToolRouter::build_tool_call(item.clone()) - .expect("build tool call") - .expect("tool call present"); + let call = ToolRouter::build_tool_call(item.clone()).expect("tool call present"); let tracker = Arc::new(tokio::sync::Mutex::new(TurnDiffTracker::new())); let err = router .dispatch_tool_call_with_code_mode_result( @@ -10826,7 +10824,7 @@ while :; do sleep 1; done"#, call_id: "shell-cleanup-call".to_string(), internal_chat_message_metadata_passthrough: None, }; - let call = ToolRouter::build_tool_call(item)? + let call = ToolRouter::build_tool_call(item) .expect("shell command response item should build a tool call"); let cancellation_token = CancellationToken::new(); let cancellation_tx = cancellation_token.clone(); diff --git a/codex-rs/core/src/stream_events_utils.rs b/codex-rs/core/src/stream_events_utils.rs index 60ea244161..987330e5b3 100644 --- a/codex-rs/core/src/stream_events_utils.rs +++ b/codex-rs/core/src/stream_events_utils.rs @@ -12,7 +12,6 @@ use tokio_util::sync::CancellationToken; use crate::context::ContextualUserFragment; use crate::context::ImageGenerationInstructions; -use crate::function_tool::FunctionCallError; use crate::parse_turn_item; use crate::session::session::Session; use crate::session::turn_context::TurnContext; @@ -23,8 +22,6 @@ use codex_memories_read::citations::thread_ids_from_memory_citation; use codex_protocol::error::CodexErr; use codex_protocol::error::Result; use codex_protocol::memory_citation::MemoryCitation; -use codex_protocol::models::FunctionCallOutputBody; -use codex_protocol::models::FunctionCallOutputPayload; use codex_protocol::models::MessagePhase; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; @@ -412,7 +409,7 @@ pub(crate) async fn handle_output_item_done( match ToolRouter::build_tool_call(item.clone()) { // The model emitted a tool call; log it, persist the item immediately, and queue the tool execution. - Ok(Some(call)) => { + Some(call) => { ctx.sess .input_queue .accept_mailbox_delivery_for_current_turn( @@ -443,7 +440,7 @@ pub(crate) async fn handle_output_item_done( output.tool_future = Some(tool_future); } // No tool call: convert messages/reasoning into turn items and mark them as complete. - Ok(None) => { + None => { let finalized_turn_item = finalize_non_tool_response_item( ctx.sess.as_ref(), ctx.turn_context.as_ref(), @@ -483,46 +480,6 @@ pub(crate) async fn handle_output_item_done( output.last_agent_message = finalized_facts.and_then(|facts| facts.last_agent_message); } - // The tool request should be answered directly (or was denied); push that response into the transcript. - Err(FunctionCallError::RespondToModel(message)) => { - let response = match &item { - ResponseItem::ToolSearchCall { - call_id: Some(call_id), - execution, - .. - } if execution == "client" && !call_id.is_empty() => { - ResponseInputItem::ToolSearchOutput { - call_id: call_id.clone(), - status: "completed".to_string(), - execution: "client".to_string(), - tools: Vec::new(), - } - } - _ => ResponseInputItem::FunctionCallOutput { - call_id: String::new(), - output: FunctionCallOutputPayload { - body: FunctionCallOutputBody::Text(message), - ..Default::default() - }, - }, - }; - record_completed_response_item(ctx.sess.as_ref(), ctx.turn_context.as_ref(), &item) - .await; - if let Some(response_item) = response_input_to_response_item(&response) { - ctx.sess - .record_conversation_items( - &ctx.turn_context, - std::slice::from_ref(&response_item), - ) - .await; - } - - output.needs_follow_up = true; - } - // A fatal error occurred; surface it back into history. - Err(FunctionCallError::Fatal(message)) => { - return Err(CodexErr::Fatal(message)); - } } Ok(output) @@ -634,53 +591,6 @@ fn completed_item_defers_mailbox_delivery_to_next_turn( } } -pub(crate) fn response_input_to_response_item(input: &ResponseInputItem) -> Option { - match input { - ResponseInputItem::FunctionCallOutput { call_id, output } => { - Some(ResponseItem::FunctionCallOutput { - id: None, - call_id: call_id.clone(), - output: output.clone(), - internal_chat_message_metadata_passthrough: None, - }) - } - ResponseInputItem::CustomToolCallOutput { - call_id, - name, - output, - } => Some(ResponseItem::CustomToolCallOutput { - id: None, - call_id: call_id.clone(), - name: name.clone(), - output: output.clone(), - internal_chat_message_metadata_passthrough: None, - }), - ResponseInputItem::McpToolCallOutput { call_id, output } => { - let output = output.as_function_call_output_payload(); - Some(ResponseItem::FunctionCallOutput { - id: None, - call_id: call_id.clone(), - output, - internal_chat_message_metadata_passthrough: None, - }) - } - ResponseInputItem::ToolSearchOutput { - call_id, - status, - execution, - tools, - } => Some(ResponseItem::ToolSearchOutput { - id: None, - call_id: Some(call_id.clone()), - status: status.clone(), - execution: execution.clone(), - tools: tools.clone(), - internal_chat_message_metadata_passthrough: None, - }), - _ => None, - } -} - #[cfg(test)] #[path = "stream_events_utils_tests.rs"] mod tests; diff --git a/codex-rs/core/src/tools/context_tests.rs b/codex-rs/core/src/tools/context_tests.rs index 6ac5b6978f..b9f90e08d4 100644 --- a/codex-rs/core/src/tools/context_tests.rs +++ b/codex-rs/core/src/tools/context_tests.rs @@ -1,6 +1,5 @@ use super::*; use codex_protocol::models::DEFAULT_IMAGE_DETAIL; -use codex_protocol::models::SearchToolCallParams; use core_test_support::assert_regex_match; use pretty_assertions::assert_eq; use serde_json::json; @@ -321,10 +320,7 @@ fn custom_tool_calls_can_derive_text_from_content_items() { #[test] fn tool_search_payloads_roundtrip_as_tool_search_outputs() { let payload = ToolPayload::ToolSearch { - arguments: SearchToolCallParams { - query: "calendar".to_string(), - limit: None, - }, + arguments: json!({ "query": "calendar" }), }; let response = ToolSearchOutput { tools: vec![LoadableToolSpec::Function(codex_tools::ResponsesApiTool { diff --git a/codex-rs/core/src/tools/handlers/tool_search.rs b/codex-rs/core/src/tools/handlers/tool_search.rs index 272da2f226..3b55fc7727 100644 --- a/codex-rs/core/src/tools/handlers/tool_search.rs +++ b/codex-rs/core/src/tools/handlers/tool_search.rs @@ -10,6 +10,7 @@ use bm25::Document; use bm25::Language; use bm25::SearchEngine; use bm25::SearchEngineBuilder; +use codex_protocol::models::SearchToolCallParams; use codex_tools::LoadableToolSpec; use codex_tools::TOOL_SEARCH_DEFAULT_LIMIT; use codex_tools::TOOL_SEARCH_TOOL_NAME; @@ -18,6 +19,7 @@ use codex_tools::ToolSearchEntry; use codex_tools::ToolSearchInfo; use codex_tools::ToolSpec; use codex_tools::coalesce_loadable_tool_specs; +use serde::Deserialize; use std::sync::Arc; use std::sync::Mutex; use tracing::instrument; @@ -119,7 +121,7 @@ impl ToolSearchHandler { ) -> Result, FunctionCallError> { let ToolInvocation { payload, .. } = invocation; - let args = match payload { + let arguments = match payload { ToolPayload::ToolSearch { arguments } => arguments, _ => { return Err(FunctionCallError::Fatal(format!( @@ -127,6 +129,14 @@ impl ToolSearchHandler { ))); } }; + let args = SearchToolCallParams::deserialize(&arguments).map_err(|err| { + FunctionCallError::RespondToModel(format!( + "failed to parse tool_search arguments: {:?} validation at line {}, column {}", + err.classify(), + err.line(), + err.column() + )) + })?; let query = args.query.trim(); if query.is_empty() { diff --git a/codex-rs/core/src/tools/router.rs b/codex-rs/core/src/tools/router.rs index 692801958d..2c66232169 100644 --- a/codex-rs/core/src/tools/router.rs +++ b/codex-rs/core/src/tools/router.rs @@ -12,13 +12,11 @@ use crate::tools::spec_plan::build_tool_router; use codex_mcp::ToolInfo; use codex_protocol::dynamic_tools::DynamicToolSpec; use codex_protocol::models::ResponseItem; -use codex_protocol::models::SearchToolCallParams; use codex_tools::DiscoverableTool; use codex_tools::ToolCall as ExtensionToolCall; use codex_tools::ToolExecutor; use codex_tools::ToolName; use codex_tools::ToolSpec; -use serde::Deserialize; use std::sync::Arc; use std::sync::atomic::AtomicBool; use tokio_util::sync::CancellationToken; @@ -110,8 +108,8 @@ impl ToolRouter { .unwrap_or(false) } - #[instrument(level = "trace", skip_all, err)] - pub fn build_tool_call(item: ResponseItem) -> Result, FunctionCallError> { + #[instrument(level = "trace", skip_all)] + pub fn build_tool_call(item: ResponseItem) -> Option { match item { ResponseItem::FunctionCall { name, @@ -121,45 +119,35 @@ impl ToolRouter { .. } => { let tool_name = ToolName::new(namespace, name); - Ok(Some(ToolCall { + Some(ToolCall { tool_name, call_id, payload: ToolPayload::Function { arguments }, - })) + }) } ResponseItem::ToolSearchCall { call_id: Some(call_id), execution, arguments, .. - } if execution == "client" && !call_id.is_empty() => { - let arguments = SearchToolCallParams::deserialize(&arguments).map_err(|err| { - FunctionCallError::RespondToModel(format!( - "failed to parse tool_search arguments: {:?} validation at line {}, column {}", - err.classify(), - err.line(), - err.column() - )) - })?; - Ok(Some(ToolCall { - tool_name: ToolName::plain("tool_search"), - call_id, - payload: ToolPayload::ToolSearch { arguments }, - })) - } - ResponseItem::ToolSearchCall { .. } => Ok(None), + } if execution == "client" && !call_id.is_empty() => Some(ToolCall { + tool_name: ToolName::plain("tool_search"), + call_id, + payload: ToolPayload::ToolSearch { arguments }, + }), + ResponseItem::ToolSearchCall { .. } => None, ResponseItem::CustomToolCall { name, namespace, input, call_id, .. - } => Ok(Some(ToolCall { + } => Some(ToolCall { tool_name: ToolName::new(namespace, name), call_id, payload: ToolPayload::Custom { input }, - })), - _ => Ok(None), + }), + _ => None, } } diff --git a/codex-rs/core/src/tools/router_tests.rs b/codex-rs/core/src/tools/router_tests.rs index 62391395f8..b99a368b81 100644 --- a/codex-rs/core/src/tools/router_tests.rs +++ b/codex-rs/core/src/tools/router_tests.rs @@ -161,7 +161,7 @@ async fn build_tool_call_uses_namespace_for_registry_name() -> anyhow::Result<() arguments: "{}".to_string(), call_id: "call-namespace".to_string(), internal_chat_message_metadata_passthrough: None, - })? + }) .expect("function_call should produce a tool call"); assert_eq!( @@ -191,7 +191,7 @@ async fn build_custom_tool_call_uses_namespace_for_registry_name() -> anyhow::Re namespace: Some("mcp__python".to_string()), input: "print('hello')".to_string(), internal_chat_message_metadata_passthrough: None, - })? + }) .expect("custom_tool_call should produce a tool call"); assert_eq!( @@ -419,7 +419,7 @@ async fn extension_tool_executors_are_model_visible_and_dispatchable() -> anyhow arguments: json!({ "message": "hello" }).to_string(), call_id: "call-extension".to_string(), internal_chat_message_metadata_passthrough: None, - })? + }) .expect("function_call should produce a tool call"); let result = router .dispatch_tool_call_with_code_mode_result( diff --git a/codex-rs/core/src/tools/tool_dispatch_trace.rs b/codex-rs/core/src/tools/tool_dispatch_trace.rs index 098dc8ddef..267639d951 100644 --- a/codex-rs/core/src/tools/tool_dispatch_trace.rs +++ b/codex-rs/core/src/tools/tool_dispatch_trace.rs @@ -9,12 +9,15 @@ use crate::tools::context::ToolCallSource; use crate::tools::context::ToolInvocation; use crate::tools::context::ToolOutput; use crate::tools::context::ToolPayload; +use codex_protocol::models::SearchToolCallParams; use codex_rollout_trace::ExecutionStatus; use codex_rollout_trace::ToolDispatchInvocation; use codex_rollout_trace::ToolDispatchPayload; use codex_rollout_trace::ToolDispatchRequester; use codex_rollout_trace::ToolDispatchResult; use codex_rollout_trace::ToolDispatchTraceContext; +use codex_tools::INVALID_TOOL_SEARCH_QUERY; +use serde::Deserialize; /// Keeps registry early-return paths paired with trace end events. pub(crate) struct ToolDispatchTrace { @@ -105,9 +108,15 @@ fn tool_dispatch_payload(payload: &ToolPayload) -> ToolDispatchPayload { ToolPayload::Function { arguments } => ToolDispatchPayload::Function { arguments: arguments.clone(), }, - ToolPayload::ToolSearch { arguments } => ToolDispatchPayload::ToolSearch { - arguments: arguments.clone(), - }, + ToolPayload::ToolSearch { arguments } => { + let arguments = SearchToolCallParams::deserialize(arguments).unwrap_or_else(|_| { + SearchToolCallParams { + query: INVALID_TOOL_SEARCH_QUERY.to_string(), + limit: None, + } + }); + ToolDispatchPayload::ToolSearch { arguments } + } ToolPayload::Custom { input } => ToolDispatchPayload::Custom { input: input.clone(), }, diff --git a/codex-rs/tools/src/lib.rs b/codex-rs/tools/src/lib.rs index 8bcad84981..8a18e0a304 100644 --- a/codex-rs/tools/src/lib.rs +++ b/codex-rs/tools/src/lib.rs @@ -98,6 +98,7 @@ pub use tool_executor::ToolExecutorFuture; pub use tool_executor::ToolExposure; pub use tool_output::JsonToolOutput; pub use tool_output::ToolOutput; +pub use tool_payload::INVALID_TOOL_SEARCH_QUERY; pub use tool_payload::ToolPayload; pub use tool_search::ToolSearchEntry; pub use tool_search::ToolSearchInfo; diff --git a/codex-rs/tools/src/tool_payload.rs b/codex-rs/tools/src/tool_payload.rs index 101d95a278..14ad241d87 100644 --- a/codex-rs/tools/src/tool_payload.rs +++ b/codex-rs/tools/src/tool_payload.rs @@ -1,12 +1,15 @@ use std::borrow::Cow; use codex_protocol::models::SearchToolCallParams; +use serde::Deserialize; + +pub const INVALID_TOOL_SEARCH_QUERY: &str = "[invalid tool_search arguments omitted]"; /// Canonical payload shapes accepted by model-visible tool runtimes. #[derive(Clone, Debug, PartialEq)] pub enum ToolPayload { Function { arguments: String }, - ToolSearch { arguments: SearchToolCallParams }, + ToolSearch { arguments: serde_json::Value }, Custom { input: String }, } @@ -14,7 +17,9 @@ impl ToolPayload { pub fn log_payload(&self) -> Cow<'_, str> { match self { ToolPayload::Function { arguments } => Cow::Borrowed(arguments), - ToolPayload::ToolSearch { arguments } => Cow::Owned(arguments.query.clone()), + ToolPayload::ToolSearch { arguments } => SearchToolCallParams::deserialize(arguments) + .map(|arguments| Cow::Owned(arguments.query)) + .unwrap_or(Cow::Borrowed(INVALID_TOOL_SEARCH_QUERY)), ToolPayload::Custom { input } => Cow::Borrowed(input), } }