mirror of
https://github.com/openai/codex.git
synced 2026-11-12 21:20:34 +00:00
codex: route tool search parse failures through runtime (#30618)
This commit is contained in:
@@ -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);
|
||||
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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<ResponseItem> {
|
||||
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;
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<Box<dyn crate::tools::context::ToolOutput>, 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() {
|
||||
|
||||
@@ -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<Option<ToolCall>, FunctionCallError> {
|
||||
#[instrument(level = "trace", skip_all)]
|
||||
pub fn build_tool_call(item: ResponseItem) -> Option<ToolCall> {
|
||||
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,
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -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(
|
||||
|
||||
@@ -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(),
|
||||
},
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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),
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user