diff --git a/codex-rs/Cargo.lock b/codex-rs/Cargo.lock index e873e689f4f..ab279abe8d8 100644 --- a/codex-rs/Cargo.lock +++ b/codex-rs/Cargo.lock @@ -4174,6 +4174,7 @@ dependencies = [ "pretty_assertions", "schemars 0.8.22", "serde_json", + "url", ] [[package]] diff --git a/codex-rs/app-server/tests/suite/v2/web_search.rs b/codex-rs/app-server/tests/suite/v2/web_search.rs index 5cc02c7f6f1..ef8d562703e 100644 --- a/codex-rs/app-server/tests/suite/v2/web_search.rs +++ b/codex-rs/app-server/tests/suite/v2/web_search.rs @@ -7,13 +7,19 @@ use app_test_support::ChatGptAuthFixture; use app_test_support::McpProcess; use app_test_support::to_response; use app_test_support::write_chatgpt_auth; +use codex_app_server_protocol::ItemCompletedNotification; +use codex_app_server_protocol::ItemStartedNotification; use codex_app_server_protocol::JSONRPCResponse; use codex_app_server_protocol::RequestId; +use codex_app_server_protocol::ThreadItem; +use codex_app_server_protocol::ThreadReadParams; +use codex_app_server_protocol::ThreadReadResponse; use codex_app_server_protocol::ThreadStartParams; 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_app_server_protocol::WebSearchAction; use codex_config::types::AuthCredentialsStoreMode; use core_test_support::responses; use pretty_assertions::assert_eq; @@ -84,10 +90,11 @@ async fn standalone_web_search_round_trips_encrypted_output() -> Result<()> { ) .await??; let ThreadStartResponse { thread, .. } = to_response::(thread_resp)?; + let thread_id = thread.id.clone(); let turn_req = mcp .send_turn_start_request(TurnStartParams { - thread_id: thread.id, + thread_id: thread_id.clone(), client_user_message_id: None, input: vec![V2UserInput::Text { text: "Search the web".to_string(), @@ -103,6 +110,13 @@ async fn standalone_web_search_round_trips_encrypted_output() -> Result<()> { .await??; let _turn: TurnStartResponse = to_response::(turn_resp)?; + let started = timeout(DEFAULT_READ_TIMEOUT, wait_for_web_search_started(&mut mcp)).await??; + let completed = timeout( + DEFAULT_READ_TIMEOUT, + wait_for_web_search_completed(&mut mcp), + ) + .await??; + timeout( DEFAULT_READ_TIMEOUT, mcp.read_stream_until_notification_message("turn/completed"), @@ -159,10 +173,83 @@ async fn standalone_web_search_round_trips_encrypted_output() -> Result<()> { }], }) ); + assert_eq!( + started.item, + ThreadItem::WebSearch { + id: call_id.to_string(), + query: String::new(), + action: Some(WebSearchAction::Other), + } + ); + let expected_completed_item = ThreadItem::WebSearch { + id: call_id.to_string(), + query: "standalone web search".to_string(), + action: Some(WebSearchAction::Search { + query: Some("standalone web search".to_string()), + queries: None, + }), + }; + assert_eq!(completed.item, expected_completed_item); + + drop(mcp); + let mut reloaded_mcp = + McpProcess::new_with_env(codex_home.path(), &[("OPENAI_API_KEY", None)]).await?; + timeout(DEFAULT_READ_TIMEOUT, reloaded_mcp.initialize()).await??; + let read_req = reloaded_mcp + .send_thread_read_request(ThreadReadParams { + thread_id, + include_turns: true, + }) + .await?; + let read_resp: JSONRPCResponse = timeout( + DEFAULT_READ_TIMEOUT, + reloaded_mcp.read_stream_until_response_message(RequestId::Integer(read_req)), + ) + .await??; + let ThreadReadResponse { thread, .. } = to_response::(read_resp)?; + let persisted_web_searches: Vec<&ThreadItem> = thread + .turns + .iter() + .flat_map(|turn| &turn.items) + .filter(|item| matches!(item, ThreadItem::WebSearch { .. })) + .collect(); + assert_eq!(persisted_web_searches, vec![&expected_completed_item]); Ok(()) } +async fn wait_for_web_search_started(mcp: &mut McpProcess) -> Result { + loop { + let notification = mcp + .read_stream_until_notification_message("item/started") + .await?; + let started: ItemStartedNotification = serde_json::from_value( + notification + .params + .context("item/started notification should include params")?, + )?; + if matches!(&started.item, ThreadItem::WebSearch { .. }) { + return Ok(started); + } + } +} + +async fn wait_for_web_search_completed(mcp: &mut McpProcess) -> Result { + loop { + let notification = mcp + .read_stream_until_notification_message("item/completed") + .await?; + let completed: ItemCompletedNotification = serde_json::from_value( + notification + .params + .context("item/completed notification should include params")?, + )?; + if matches!(&completed.item, ThreadItem::WebSearch { .. }) { + return Ok(completed); + } + } +} + async fn mount_search_response(server: &MockServer) { Mock::given(method("POST")) .and(path("/api/codex/alpha/search")) diff --git a/codex-rs/ext/web-search/Cargo.toml b/codex-rs/ext/web-search/Cargo.toml index 978aa4c45d2..0faabf21a86 100644 --- a/codex-rs/ext/web-search/Cargo.toml +++ b/codex-rs/ext/web-search/Cargo.toml @@ -26,6 +26,7 @@ codex-tools = { workspace = true } http = { workspace = true } schemars = { workspace = true } serde_json = { workspace = true } +url = { workspace = true } [dev-dependencies] pretty_assertions = { workspace = true } diff --git a/codex-rs/ext/web-search/src/extension.rs b/codex-rs/ext/web-search/src/extension.rs index b7b6c83951d..2a7f3b882fd 100644 --- a/codex-rs/ext/web-search/src/extension.rs +++ b/codex-rs/ext/web-search/src/extension.rs @@ -146,6 +146,8 @@ mod tests { use super::Config; use super::WebSearchExtensionConfig; use super::install; + use crate::tool::RUN_TOOL_NAME; + use crate::tool::WEB_NAMESPACE; #[test] fn installed_extension_contributes_web_run_when_enabled() { @@ -170,6 +172,9 @@ mod tests { .map(|tool| tool.tool_name()) .collect::>(); - assert_eq!(tool_names, vec![ToolName::namespaced("web", "run")]); + assert_eq!( + tool_names, + vec![ToolName::namespaced(WEB_NAMESPACE, RUN_TOOL_NAME)] + ); } } diff --git a/codex-rs/ext/web-search/src/tool.rs b/codex-rs/ext/web-search/src/tool.rs index 875476db9a3..2cf25b007c0 100644 --- a/codex-rs/ext/web-search/src/tool.rs +++ b/codex-rs/ext/web-search/src/tool.rs @@ -1,8 +1,11 @@ use codex_api::ReqwestTransport; use codex_api::SearchClient; use codex_api::SearchCommands; +use codex_api::SearchQuery; use codex_api::SearchRequest; use codex_api::SearchSettings; +use codex_core::web_search_action_detail; +use codex_extension_api::ExtensionTurnItem; use codex_extension_api::FunctionCallError; use codex_extension_api::ResponsesApiTool; use codex_extension_api::ToolCall; @@ -13,18 +16,21 @@ use codex_extension_api::ToolSpec; use codex_extension_api::parse_tool_input_schema_without_compaction; use codex_login::default_client::build_reqwest_client; use codex_model_provider::SharedModelProvider; +use codex_protocol::items::WebSearchItem; +use codex_protocol::models::WebSearchAction; use codex_tools::ResponsesApiNamespace; use codex_tools::ResponsesApiNamespaceTool; use codex_tools::ToolExposure; use codex_tools::default_namespace_description; use http::HeaderMap; +use url::Url; use crate::history::recent_input; use crate::output::EncryptedSearchOutput; use crate::schema::commands_schema; -const WEB_NAMESPACE: &str = "web"; -const RUN_TOOL_NAME: &str = "run"; +pub(crate) const WEB_NAMESPACE: &str = "web"; +pub(crate) const RUN_TOOL_NAME: &str = "run"; const WEB_RUN_DESCRIPTION: &str = include_str!("../web_run_description.md"); pub(crate) struct WebSearchTool { @@ -66,6 +72,7 @@ impl ToolExecutor for WebSearchTool { async fn handle(&self, call: ToolCall) -> Result, FunctionCallError> { let commands = parse_commands(&call)?; + let command_action = command_action(&commands); let provider = self .provider .api_provider() @@ -92,10 +99,16 @@ impl ToolExecutor for WebSearchTool { u64::try_from(call.truncation_policy.token_budget()).unwrap_or(u64::MAX), ), }; + call.turn_item_emitter + .emit_started(web_search_item(&call.call_id, WebSearchAction::Other)) + .await; let response = client .search(&request, HeaderMap::new()) .await .map_err(|err| FunctionCallError::Fatal(err.to_string()))?; + call.turn_item_emitter + .emit_completed(web_search_item(&call.call_id, command_action)) + .await; Ok(Box::new(EncryptedSearchOutput::new( response.encrypted_output, @@ -112,3 +125,110 @@ fn parse_commands(call: &ToolCall) -> Result serde_json::from_str(arguments) .map_err(|err| FunctionCallError::RespondToModel(err.to_string())) } + +fn command_action(commands: &SearchCommands) -> WebSearchAction { + commands + .search_query + .as_deref() + .and_then(query_action) + .or_else(|| commands.image_query.as_deref().and_then(query_action)) + .or_else(|| { + commands + .open + .as_deref() + .and_then(|operations| operations.first()) + .and_then(|operation| { + literal_url(&operation.ref_id) + .map(|url| WebSearchAction::OpenPage { url: Some(url) }) + }) + }) + .or_else(|| { + commands + .find + .as_deref() + .and_then(|operations| operations.first()) + .map(|operation| WebSearchAction::FindInPage { + url: literal_url(&operation.ref_id), + pattern: Some(operation.pattern.clone()), + }) + }) + .unwrap_or(WebSearchAction::Other) +} + +fn query_action(queries: &[SearchQuery]) -> Option { + match queries { + [] => None, + [query] => Some(WebSearchAction::Search { + query: Some(query.q.clone()), + queries: None, + }), + queries => Some(WebSearchAction::Search { + query: None, + queries: Some(queries.iter().map(|query| query.q.clone()).collect()), + }), + } +} + +fn literal_url(ref_id: &str) -> Option { + Url::parse(ref_id).is_ok().then(|| ref_id.to_string()) +} + +fn web_search_item(call_id: &str, action: WebSearchAction) -> ExtensionTurnItem { + ExtensionTurnItem::WebSearch(WebSearchItem { + id: call_id.to_string(), + query: web_search_action_detail(&action), + action, + }) +} + +#[cfg(test)] +mod tests { + use codex_api::SearchCommands; + use codex_protocol::models::WebSearchAction; + use pretty_assertions::assert_eq; + + use super::command_action; + + #[test] + fn command_action_reports_queries_and_navigation_detail() { + let cases = [ + ( + r#"{"image_query":[{"q":"waterfalls"},{"q":"mountains"}]}"#, + WebSearchAction::Search { + query: None, + queries: Some(vec!["waterfalls".to_string(), "mountains".to_string()]), + }, + ), + ( + r#"{"open":[{"ref_id":"https://example.com/docs"}]}"#, + WebSearchAction::OpenPage { + url: Some("https://example.com/docs".to_string()), + }, + ), + ( + r#"{"find":[{"ref_id":"https://example.com/docs","pattern":"install"}]}"#, + WebSearchAction::FindInPage { + url: Some("https://example.com/docs".to_string()), + pattern: Some("install".to_string()), + }, + ), + ( + r#"{"find":[{"ref_id":"turn0search0","pattern":"install"}]}"#, + WebSearchAction::FindInPage { + url: None, + pattern: Some("install".to_string()), + }, + ), + ( + r#"{"open":[{"ref_id":"turn0search0"}]}"#, + WebSearchAction::Other, + ), + ]; + + for (arguments, expected) in cases { + let commands: SearchCommands = + serde_json::from_str(arguments).expect("valid search command arguments"); + assert_eq!(command_action(&commands), expected); + } + } +} diff --git a/codex-rs/tui/src/history_cell/search.rs b/codex-rs/tui/src/history_cell/search.rs index d75ac50fdce..5373f68ec23 100644 --- a/codex-rs/tui/src/history_cell/search.rs +++ b/codex-rs/tui/src/history_cell/search.rs @@ -4,7 +4,7 @@ use super::*; fn web_search_header(completed: bool) -> &'static str { if completed { - "Searched" + "Searched the web" } else { "Searching the web" } @@ -104,7 +104,8 @@ impl HistoryCell for WebSearchCell { let text: Text<'static> = if detail.is_empty() { Line::from(vec![header.bold()]).into() } else { - Line::from(vec![header.bold(), " ".into(), detail.into()]).into() + let separator = if self.completed { " for " } else { " " }; + Line::from(vec![header.bold(), separator.into(), detail.into()]).into() }; PrefixedWrappedHistoryCell::new(text, vec![bullet, " ".into()], " ").display_lines(width) } @@ -115,7 +116,8 @@ impl HistoryCell for WebSearchCell { if detail.is_empty() { vec![Line::from(header)] } else { - vec![Line::from(format!("{header} {detail}"))] + let separator = if self.completed { " for " } else { " " }; + vec![Line::from(format!("{header}{separator}{detail}"))] } } } diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_snapshot.snap index e119420f18c..2a4e8b3abb1 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_snapshot.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_snapshot.snap @@ -2,5 +2,5 @@ source: tui/src/history_cell.rs expression: rendered --- -• Searched example search query with several generic words to - exercise wrapping +• Searched the web for example search query with several generic + words to exercise wrapping diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_transcript_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_transcript_snapshot.snap index e119420f18c..2a4e8b3abb1 100644 --- a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_transcript_snapshot.snap +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_transcript_snapshot.snap @@ -2,5 +2,5 @@ source: tui/src/history_cell.rs expression: rendered --- -• Searched example search query with several generic words to - exercise wrapping +• Searched the web for example search query with several generic + words to exercise wrapping diff --git a/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_without_detail_snapshot.snap b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_without_detail_snapshot.snap new file mode 100644 index 00000000000..ffda6cb13fe --- /dev/null +++ b/codex-rs/tui/src/history_cell/snapshots/codex_tui__history_cell__tests__web_search_history_cell_without_detail_snapshot.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/history_cell.rs +expression: rendered +--- +• Searched the web diff --git a/codex-rs/tui/src/history_cell/tests.rs b/codex-rs/tui/src/history_cell/tests.rs index 76fe17f027e..63a04d19d4f 100644 --- a/codex-rs/tui/src/history_cell/tests.rs +++ b/codex-rs/tui/src/history_cell/tests.rs @@ -1084,6 +1084,14 @@ fn standalone_windows_update_available_history_cell_snapshot() { insta::assert_snapshot!(rendered); } +#[test] +fn web_search_history_cell_without_detail_snapshot() { + let cell = new_web_search_call("call-1".to_string(), String::new(), WebSearchAction::Other); + let rendered = render_lines(&cell.display_lines(/*width*/ 64)).join("\n"); + + insta::assert_snapshot!(rendered); +} + #[test] fn web_search_history_cell_wraps_with_indented_continuation() { let query = "example search query with several generic words to exercise wrapping".to_string(); @@ -1100,8 +1108,8 @@ fn web_search_history_cell_wraps_with_indented_continuation() { assert_eq!( rendered, vec![ - "• Searched example search query with several generic words to".to_string(), - " exercise wrapping".to_string(), + "• Searched the web for example search query with several generic".to_string(), + " words to exercise wrapping".to_string(), ] ); } @@ -1119,7 +1127,10 @@ fn web_search_history_cell_short_query_does_not_wrap() { ); let rendered = render_lines(&cell.display_lines(/*width*/ 64)); - assert_eq!(rendered, vec!["• Searched short query".to_string()]); + assert_eq!( + rendered, + vec!["• Searched the web for short query".to_string()] + ); } #[test]