From 9dbe7284d25a4b50d2163484f3af8a174ef263e7 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim Date: Thu, 28 Aug 2025 19:24:38 -0700 Subject: [PATCH] Following up on #2371 post commit feedback (#2852) - Introduce websearch end to complement the begin - Moves the logic of adding the sebsearch tool to create_tools_json_for_responses_api - Making it the client responsibility to toggle the tool on or off - Other misc in #2371 post commit feedback - Show the query: image --- codex-rs/core/src/chat_completions.rs | 11 ++++---- codex-rs/core/src/client.rs | 26 ++++--------------- codex-rs/core/src/client_common.rs | 1 - codex-rs/core/src/codex.rs | 18 ++++++++++--- codex-rs/core/src/config.rs | 9 +++---- codex-rs/core/src/conversation_history.rs | 2 +- codex-rs/core/src/openai_tools.rs | 12 +++++---- codex-rs/core/src/rollout.rs | 6 ++--- .../src/event_processor_with_human_output.rs | 6 +++-- codex-rs/mcp-server/src/codex_tool_runner.rs | 1 + codex-rs/protocol/src/models.rs | 26 +++++++++++++++++++ codex-rs/protocol/src/protocol.rs | 7 +++++ codex-rs/tui/src/chatwidget.rs | 13 ++++++++-- 13 files changed, 89 insertions(+), 49 deletions(-) diff --git a/codex-rs/core/src/chat_completions.rs b/codex-rs/core/src/chat_completions.rs index 64157407..6eca119f 100644 --- a/codex-rs/core/src/chat_completions.rs +++ b/codex-rs/core/src/chat_completions.rs @@ -129,7 +129,9 @@ pub(crate) async fn stream_chat_completions( "content": output, })); } - ResponseItem::Reasoning { .. } | ResponseItem::Other => { + ResponseItem::Reasoning { .. } + | ResponseItem::WebSearchCall { .. } + | ResponseItem::Other => { // Omit these items from the conversation history. continue; } @@ -623,11 +625,8 @@ where Poll::Ready(Some(Ok(ResponseEvent::ReasoningSummaryPartAdded))) => { continue; } - Poll::Ready(Some(Ok(ResponseEvent::WebSearchCallBegin { .. }))) => { - return Poll::Ready(Some(Ok(ResponseEvent::WebSearchCallBegin { - call_id: String::new(), - query: None, - }))); + Poll::Ready(Some(Ok(ResponseEvent::WebSearchCallBegin { call_id }))) => { + return Poll::Ready(Some(Ok(ResponseEvent::WebSearchCallBegin { call_id }))); } } } diff --git a/codex-rs/core/src/client.rs b/codex-rs/core/src/client.rs index e0ab334f..b7bfeaa7 100644 --- a/codex-rs/core/src/client.rs +++ b/codex-rs/core/src/client.rs @@ -160,21 +160,7 @@ impl ModelClient { let store = prompt.store && auth_mode != Some(AuthMode::ChatGPT); let full_instructions = prompt.get_full_instructions(&self.config.model_family); - let mut tools_json = create_tools_json_for_responses_api(&prompt.tools)?; - // ChatGPT backend expects the preview name for web search. - if auth_mode == Some(AuthMode::ChatGPT) { - for tool in &mut tools_json { - if let Some(map) = tool.as_object_mut() - && map.get("type").and_then(|v| v.as_str()) == Some("web_search") - { - map.insert( - "type".to_string(), - serde_json::Value::String("web_search_preview".to_string()), - ); - } - } - } - + let tools_json = create_tools_json_for_responses_api(&prompt.tools)?; let reasoning = create_reasoning_param_for_request( &self.config.model_family, self.effort, @@ -607,11 +593,9 @@ async fn process_sse( | "response.custom_tool_call_input.delta" | "response.custom_tool_call_input.done" // also emitted as response.output_item.done | "response.in_progress" - | "response.output_item.added" - | "response.output_text.done" => { - if event.kind == "response.output_item.added" - && let Some(item) = event.item.as_ref() - { + | "response.output_text.done" => {} + "response.output_item.added" => { + if let Some(item) = event.item.as_ref() { // Detect web_search_call begin and forward a synthetic event upstream. if let Some(ty) = item.get("type").and_then(|v| v.as_str()) && ty == "web_search_call" @@ -621,7 +605,7 @@ async fn process_sse( .and_then(|v| v.as_str()) .unwrap_or("") .to_string(); - let ev = ResponseEvent::WebSearchCallBegin { call_id, query: None }; + let ev = ResponseEvent::WebSearchCallBegin { call_id }; if tx_event.send(Ok(ev)).await.is_err() { return; } diff --git a/codex-rs/core/src/client_common.rs b/codex-rs/core/src/client_common.rs index e2c191f4..27401051 100644 --- a/codex-rs/core/src/client_common.rs +++ b/codex-rs/core/src/client_common.rs @@ -95,7 +95,6 @@ pub enum ResponseEvent { ReasoningSummaryPartAdded, WebSearchCallBegin { call_id: String, - query: Option, }, } diff --git a/codex-rs/core/src/codex.rs b/codex-rs/core/src/codex.rs index 40bcdc2c..99c59d97 100644 --- a/codex-rs/core/src/codex.rs +++ b/codex-rs/core/src/codex.rs @@ -101,6 +101,7 @@ use crate::protocol::Submission; use crate::protocol::TaskCompleteEvent; use crate::protocol::TurnDiffEvent; use crate::protocol::WebSearchBeginEvent; +use crate::protocol::WebSearchEndEvent; use crate::rollout::RolloutRecorder; use crate::safety::SafetyCheck; use crate::safety::assess_command_safety; @@ -120,6 +121,7 @@ use codex_protocol::models::ReasoningItemReasoningSummary; use codex_protocol::models::ResponseInputItem; use codex_protocol::models::ResponseItem; use codex_protocol::models::ShellToolCallParams; +use codex_protocol::models::WebSearchAction; // A convenience extension trait for acquiring mutex locks where poisoning is // unrecoverable and should abort the program. This avoids scattered `.unwrap()` @@ -1769,13 +1771,12 @@ async fn try_run_turn( .await?; output.push(ProcessedResponseItem { item, response }); } - ResponseEvent::WebSearchCallBegin { call_id, query } => { - let q = query.unwrap_or_else(|| "Searching Web...".to_string()); + ResponseEvent::WebSearchCallBegin { call_id } => { let _ = sess .tx_event .send(Event { id: sub_id.to_string(), - msg: EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id, query: q }), + msg: EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id }), }) .await; } @@ -2074,6 +2075,17 @@ async fn handle_response_item( debug!("unexpected CustomToolCallOutput from stream"); None } + ResponseItem::WebSearchCall { id, action, .. } => { + if let WebSearchAction::Search { query } = action { + let call_id = id.unwrap_or_else(|| "".to_string()); + let event = Event { + id: sub_id.to_string(), + msg: EventMsg::WebSearchEnd(WebSearchEndEvent { call_id, query }), + }; + sess.tx_event.send(event).await.ok(); + } + None + } ResponseItem::Other => None, }; Ok(output) diff --git a/codex-rs/core/src/config.rs b/codex-rs/core/src/config.rs index b47f717c..7845f5d4 100644 --- a/codex-rs/core/src/config.rs +++ b/codex-rs/core/src/config.rs @@ -506,7 +506,6 @@ pub struct ProjectConfig { #[derive(Deserialize, Debug, Clone, Default)] pub struct ToolsToml { - // Renamed from `web_search_request`; keep alias for backwards compatibility. #[serde(default, alias = "web_search_request")] pub web_search: Option, @@ -672,7 +671,7 @@ impl Config { })? .clone(); - let shell_environment_policy = cfg.shell_environment_policy.clone().into(); + let shell_environment_policy = cfg.shell_environment_policy.into(); let resolved_cwd = { use std::env; @@ -693,7 +692,7 @@ impl Config { } }; - let history = cfg.history.clone().unwrap_or_default(); + let history = cfg.history.unwrap_or_default(); let tools_web_search_request = override_tools_web_search_request .or(cfg.tools.as_ref().and_then(|t| t.web_search)) @@ -775,7 +774,7 @@ impl Config { codex_home, history, file_opener: cfg.file_opener.unwrap_or(UriBasedFileOpener::VsCode), - tui: cfg.tui.clone().unwrap_or_default(), + tui: cfg.tui.unwrap_or_default(), codex_linux_sandbox_exe, hide_agent_reasoning: cfg.hide_agent_reasoning.unwrap_or(false), @@ -794,7 +793,7 @@ impl Config { model_verbosity: config_profile.model_verbosity.or(cfg.model_verbosity), chatgpt_base_url: config_profile .chatgpt_base_url - .or(cfg.chatgpt_base_url.clone()) + .or(cfg.chatgpt_base_url) .unwrap_or("https://chatgpt.com/backend-api/".to_string()), experimental_resume, diff --git a/codex-rs/core/src/conversation_history.rs b/codex-rs/core/src/conversation_history.rs index a8995f57..ed15bd41 100644 --- a/codex-rs/core/src/conversation_history.rs +++ b/codex-rs/core/src/conversation_history.rs @@ -72,7 +72,7 @@ fn is_api_message(message: &ResponseItem) -> bool { | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::LocalShellCall { .. } | ResponseItem::Reasoning { .. } => true, - ResponseItem::Other => false, + ResponseItem::WebSearchCall { .. } | ResponseItem::Other => false, } } diff --git a/codex-rs/core/src/openai_tools.rs b/codex-rs/core/src/openai_tools.rs index f7418816..59ca7a83 100644 --- a/codex-rs/core/src/openai_tools.rs +++ b/codex-rs/core/src/openai_tools.rs @@ -47,7 +47,9 @@ pub(crate) enum OpenAiTool { Function(ResponsesApiTool), #[serde(rename = "local_shell")] LocalShell {}, - #[serde(rename = "web_search")] + // TODO: Understand why we get an error on web_search although the API docs say it's supported. + // https://platform.openai.com/docs/guides/tools-web-search?api-mode=responses#:~:text=%7B%20type%3A%20%22web_search%22%20%7D%2C + #[serde(rename = "web_search_preview")] WebSearch {}, #[serde(rename = "custom")] Freeform(FreeformTool), @@ -335,12 +337,12 @@ pub fn create_tools_json_for_responses_api( let mut tools_json = Vec::new(); for tool in tools { - tools_json.push(serde_json::to_value(tool)?); + let json = serde_json::to_value(tool)?; + tools_json.push(json); } Ok(tools_json) } - /// Returns JSON values that are compatible with Function Calling in the /// Chat Completions API: /// https://platform.openai.com/docs/guides/function-calling?api-mode=chat @@ -702,8 +704,8 @@ mod tests { "number_property": { "type": "number" }, }, "required": [ - "string_property".to_string(), - "number_property".to_string() + "string_property", + "number_property", ], "additionalProperties": Some(false), }, diff --git a/codex-rs/core/src/rollout.rs b/codex-rs/core/src/rollout.rs index 46098c16..3c7162e8 100644 --- a/codex-rs/core/src/rollout.rs +++ b/codex-rs/core/src/rollout.rs @@ -135,7 +135,7 @@ impl RolloutRecorder { | ResponseItem::CustomToolCall { .. } | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::Reasoning { .. } => filtered.push(item.clone()), - ResponseItem::Other => { + ResponseItem::WebSearchCall { .. } | ResponseItem::Other => { // These should never be serialized. continue; } @@ -199,7 +199,7 @@ impl RolloutRecorder { | ResponseItem::CustomToolCall { .. } | ResponseItem::CustomToolCallOutput { .. } | ResponseItem::Reasoning { .. } => items.push(item), - ResponseItem::Other => {} + ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {} }, Err(e) => { warn!("failed to parse item: {v:?}, error: {e}"); @@ -326,7 +326,7 @@ async fn rollout_writer( | ResponseItem::Reasoning { .. } => { writer.write_line(&item).await?; } - ResponseItem::Other => {} + ResponseItem::WebSearchCall { .. } | ResponseItem::Other => {} } } } diff --git a/codex-rs/exec/src/event_processor_with_human_output.rs b/codex-rs/exec/src/event_processor_with_human_output.rs index 6d1bc123..8403bbde 100644 --- a/codex-rs/exec/src/event_processor_with_human_output.rs +++ b/codex-rs/exec/src/event_processor_with_human_output.rs @@ -25,6 +25,7 @@ use codex_core::protocol::TaskCompleteEvent; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; use codex_core::protocol::WebSearchBeginEvent; +use codex_core::protocol::WebSearchEndEvent; use owo_colors::OwoColorize; use owo_colors::Style; use shlex::try_join; @@ -362,8 +363,9 @@ impl EventProcessor for EventProcessorWithHumanOutput { } } } - EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id: _, query }) => { - ts_println!(self, "🌐 {query}"); + EventMsg::WebSearchBegin(WebSearchBeginEvent { call_id: _ }) => {} + EventMsg::WebSearchEnd(WebSearchEndEvent { call_id: _, query }) => { + ts_println!(self, "🌐 Searched: {query}"); } EventMsg::PatchApplyBegin(PatchApplyBeginEvent { call_id, diff --git a/codex-rs/mcp-server/src/codex_tool_runner.rs b/codex-rs/mcp-server/src/codex_tool_runner.rs index 72cac591..b297dadd 100644 --- a/codex-rs/mcp-server/src/codex_tool_runner.rs +++ b/codex-rs/mcp-server/src/codex_tool_runner.rs @@ -274,6 +274,7 @@ async fn run_codex_tool_session_inner( | EventMsg::PatchApplyEnd(_) | EventMsg::TurnDiff(_) | EventMsg::WebSearchBegin(_) + | EventMsg::WebSearchEnd(_) | EventMsg::GetHistoryEntryResponse(_) | EventMsg::PlanUpdate(_) | EventMsg::TurnAborted(_) diff --git a/codex-rs/protocol/src/models.rs b/codex-rs/protocol/src/models.rs index 31e61d8b..6cdfd933 100644 --- a/codex-rs/protocol/src/models.rs +++ b/codex-rs/protocol/src/models.rs @@ -95,6 +95,22 @@ pub enum ResponseItem { call_id: String, output: String, }, + // Emitted by the Responses API when the agent triggers a web search. + // Example payload (from SSE `response.output_item.done`): + // { + // "id":"ws_...", + // "type":"web_search_call", + // "status":"completed", + // "action": {"type":"search","query":"weather: San Francisco, CA"} + // } + WebSearchCall { + #[serde(default, skip_serializing_if = "Option::is_none")] + id: Option, + #[serde(default, skip_serializing_if = "Option::is_none")] + status: Option, + action: WebSearchAction, + }, + #[serde(other)] Other, } @@ -162,6 +178,16 @@ pub struct LocalShellExecAction { pub user: Option, } +#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] +#[serde(tag = "type", rename_all = "snake_case")] +pub enum WebSearchAction { + Search { + query: String, + }, + #[serde(other)] + Other, +} + #[derive(Debug, Clone, Serialize, Deserialize, PartialEq)] #[serde(tag = "type", rename_all = "snake_case")] pub enum ReasoningItemReasoningSummary { diff --git a/codex-rs/protocol/src/protocol.rs b/codex-rs/protocol/src/protocol.rs index 8707e1a8..1f6cbe07 100644 --- a/codex-rs/protocol/src/protocol.rs +++ b/codex-rs/protocol/src/protocol.rs @@ -443,6 +443,8 @@ pub enum EventMsg { WebSearchBegin(WebSearchBeginEvent), + WebSearchEnd(WebSearchEndEvent), + /// Notification that the server is about to execute a command. ExecCommandBegin(ExecCommandBeginEvent), @@ -675,6 +677,11 @@ impl McpToolCallEndEvent { #[derive(Debug, Clone, Deserialize, Serialize)] pub struct WebSearchBeginEvent { pub call_id: String, +} + +#[derive(Debug, Clone, Deserialize, Serialize)] +pub struct WebSearchEndEvent { + pub call_id: String, pub query: String, } diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 26928545..4695c656 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -31,6 +31,7 @@ use codex_core::protocol::TokenUsage; use codex_core::protocol::TurnAbortReason; use codex_core::protocol::TurnDiffEvent; use codex_core::protocol::WebSearchBeginEvent; +use codex_core::protocol::WebSearchEndEvent; use codex_protocol::parse_command::ParsedCommand; use crossterm::event::KeyCode; use crossterm::event::KeyEvent; @@ -358,9 +359,16 @@ impl ChatWidget { self.defer_or_handle(|q| q.push_mcp_end(ev), |s| s.handle_mcp_end_now(ev2)); } - fn on_web_search_begin(&mut self, ev: WebSearchBeginEvent) { + fn on_web_search_begin(&mut self, _ev: WebSearchBeginEvent) { self.flush_answer_stream_with_separator(); - self.add_to_history(history_cell::new_web_search_call(ev.query)); + } + + fn on_web_search_end(&mut self, ev: WebSearchEndEvent) { + self.flush_answer_stream_with_separator(); + self.add_to_history(history_cell::new_web_search_call(format!( + "Searched: {}", + ev.query + ))); } fn on_get_history_entry_response( @@ -992,6 +1000,7 @@ impl ChatWidget { EventMsg::McpToolCallBegin(ev) => self.on_mcp_tool_call_begin(ev), EventMsg::McpToolCallEnd(ev) => self.on_mcp_tool_call_end(ev), EventMsg::WebSearchBegin(ev) => self.on_web_search_begin(ev), + EventMsg::WebSearchEnd(ev) => self.on_web_search_end(ev), EventMsg::GetHistoryEntryResponse(ev) => self.on_get_history_entry_response(ev), EventMsg::McpListToolsResponse(ev) => self.on_list_mcp_tools(ev), EventMsg::ListCustomPromptsResponse(ev) => self.on_list_custom_prompts(ev),