mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Show activity for standalone web search calls (#24693)
## Why Standalone `web.run` calls run in the extension, so they need normal web-search progress activity while a request is in flight and durable completed activity after a thread is reloaded. Follow-up to #23823; uses the extension turn-item emission path added in #24813. ## What changed - Emit standalone `web.run` start/completion items through the host turn-item emitter, preserving standard client delivery and rollout persistence. - Include useful completion detail for queries, image queries, and literal-URL `open`/`find` commands. - Render completed searches as `Searched the web` or `Searched the web for <detail>`, with snapshot coverage for the detail-free case. - Extend the app-server round-trip test to verify completed search activity is reconstructed by `thread/read` after a fresh-process reload. ## Testing - `just test -p codex-web-search-extension` - `just test -p codex-app-server -E "test(standalone_web_search_round_trips_encrypted_output)"`
This commit is contained in:
committed by
GitHub
Unverified
parent
5577a9e148
commit
96f1347fa3
Generated
+1
@@ -4174,6 +4174,7 @@ dependencies = [
|
||||
"pretty_assertions",
|
||||
"schemars 0.8.22",
|
||||
"serde_json",
|
||||
"url",
|
||||
]
|
||||
|
||||
[[package]]
|
||||
|
||||
@@ -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::<ThreadStartResponse>(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::<TurnStartResponse>(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::<ThreadReadResponse>(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<ItemStartedNotification> {
|
||||
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<ItemCompletedNotification> {
|
||||
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"))
|
||||
|
||||
@@ -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 }
|
||||
|
||||
@@ -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::<Vec<_>>();
|
||||
|
||||
assert_eq!(tool_names, vec![ToolName::namespaced("web", "run")]);
|
||||
assert_eq!(
|
||||
tool_names,
|
||||
vec![ToolName::namespaced(WEB_NAMESPACE, RUN_TOOL_NAME)]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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<ToolCall> for WebSearchTool {
|
||||
|
||||
async fn handle(&self, call: ToolCall) -> Result<Box<dyn ToolOutput>, FunctionCallError> {
|
||||
let commands = parse_commands(&call)?;
|
||||
let command_action = command_action(&commands);
|
||||
let provider = self
|
||||
.provider
|
||||
.api_provider()
|
||||
@@ -92,10 +99,16 @@ impl ToolExecutor<ToolCall> 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<SearchCommands, FunctionCallError>
|
||||
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<WebSearchAction> {
|
||||
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<String> {
|
||||
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);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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}"))]
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
+2
-2
@@ -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
|
||||
|
||||
+5
@@ -0,0 +1,5 @@
|
||||
---
|
||||
source: tui/src/history_cell.rs
|
||||
expression: rendered
|
||||
---
|
||||
• Searched the web
|
||||
@@ -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]
|
||||
|
||||
Reference in New Issue
Block a user