Skip to content

Commit 763ab20

Browse files
apanasenko-oaiapanasenko-oai
committed
feedback
1 parent a164336 commit 763ab20

12 files changed

Lines changed: 326 additions & 212 deletions

File tree

‎codex-rs/core/src/context_manager/history_tests.rs‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1309,6 +1309,39 @@ fn normalize_adds_missing_output_for_function_call_inserts_output() {
13091309
);
13101310
}
13111311

1312+
#[test]
1313+
fn normalize_adds_missing_output_for_tool_search_call() {
1314+
let items = vec![ResponseItem::ToolSearchCall {
1315+
id: None,
1316+
call_id: Some("search-call-x".to_string()),
1317+
status: Some("completed".to_string()),
1318+
execution: "client".to_string(),
1319+
arguments: "{}".into(),
1320+
}];
1321+
let mut h = create_history_with_items(items);
1322+
1323+
h.normalize_history(&default_input_modalities());
1324+
1325+
assert_eq!(
1326+
h.raw_items(),
1327+
vec![
1328+
ResponseItem::ToolSearchCall {
1329+
id: None,
1330+
call_id: Some("search-call-x".to_string()),
1331+
status: Some("completed".to_string()),
1332+
execution: "client".to_string(),
1333+
arguments: "{}".into(),
1334+
},
1335+
ResponseItem::ToolSearchOutput {
1336+
call_id: Some("search-call-x".to_string()),
1337+
status: "completed".to_string(),
1338+
execution: "client".to_string(),
1339+
tools: Vec::new(),
1340+
},
1341+
]
1342+
);
1343+
}
1344+
13121345
#[cfg(debug_assertions)]
13131346
#[test]
13141347
#[should_panic]
@@ -1368,6 +1401,59 @@ fn normalize_removes_orphan_custom_tool_call_output_panics_in_debug() {
13681401
h.normalize_history(&default_input_modalities());
13691402
}
13701403

1404+
#[cfg(not(debug_assertions))]
1405+
#[test]
1406+
fn normalize_removes_orphan_client_tool_search_output() {
1407+
let items = vec![ResponseItem::ToolSearchOutput {
1408+
call_id: Some("orphan-search".to_string()),
1409+
status: "completed".to_string(),
1410+
execution: "client".to_string(),
1411+
tools: Vec::new(),
1412+
}];
1413+
let mut h = create_history_with_items(items);
1414+
1415+
h.normalize_history(&default_input_modalities());
1416+
1417+
assert_eq!(h.raw_items(), vec![]);
1418+
}
1419+
1420+
#[cfg(debug_assertions)]
1421+
#[test]
1422+
#[should_panic]
1423+
fn normalize_removes_orphan_client_tool_search_output_panics_in_debug() {
1424+
let items = vec![ResponseItem::ToolSearchOutput {
1425+
call_id: Some("orphan-search".to_string()),
1426+
status: "completed".to_string(),
1427+
execution: "client".to_string(),
1428+
tools: Vec::new(),
1429+
}];
1430+
let mut h = create_history_with_items(items);
1431+
h.normalize_history(&default_input_modalities());
1432+
}
1433+
1434+
#[test]
1435+
fn normalize_keeps_server_tool_search_output_without_matching_call() {
1436+
let items = vec![ResponseItem::ToolSearchOutput {
1437+
call_id: Some("server-search".to_string()),
1438+
status: "completed".to_string(),
1439+
execution: "server".to_string(),
1440+
tools: Vec::new(),
1441+
}];
1442+
let mut h = create_history_with_items(items);
1443+
1444+
h.normalize_history(&default_input_modalities());
1445+
1446+
assert_eq!(
1447+
h.raw_items(),
1448+
vec![ResponseItem::ToolSearchOutput {
1449+
call_id: Some("server-search".to_string()),
1450+
status: "completed".to_string(),
1451+
execution: "server".to_string(),
1452+
tools: Vec::new(),
1453+
}]
1454+
);
1455+
}
1456+
13711457
#[cfg(debug_assertions)]
13721458
#[test]
13731459
#[should_panic]

‎codex-rs/core/src/mcp_connection_manager.rs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1542,7 +1542,7 @@ async fn list_tools_for_client_uncached(
15421542
}
15431543
ToolInfo {
15441544
server_name: server_name.to_owned(),
1545-
tool_name: tool_name.to_owned(),
1545+
tool_name,
15461546
tool: tool_def,
15471547
connector_id: tool.connector_id,
15481548
connector_name,

‎codex-rs/core/src/tools/code_mode.rs‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -374,7 +374,10 @@ fn enabled_tool_from_spec(spec: ToolSpec) -> Option<EnabledTool> {
374374
let (description, kind) = match spec {
375375
ToolSpec::Function(tool) => (tool.description, CodeModeToolKind::Function),
376376
ToolSpec::Freeform(tool) => (tool.description, CodeModeToolKind::Freeform),
377-
ToolSpec::LocalShell {} | ToolSpec::ImageGeneration { .. } | ToolSpec::WebSearch { .. } => {
377+
ToolSpec::LocalShell {}
378+
| ToolSpec::ImageGeneration { .. }
379+
| ToolSpec::ToolSearch { .. }
380+
| ToolSpec::WebSearch { .. } => {
378381
return None;
379382
}
380383
};

‎codex-rs/core/src/tools/context.rs‎

Lines changed: 49 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
use crate::client_common::tools::ToolSearchOutputTool;
12
use crate::codex::Session;
23
use crate::codex::TurnContext;
34
use crate::tools::TELEMETRY_PREVIEW_MAX_BYTES;
@@ -12,6 +13,7 @@ use codex_protocol::models::FunctionCallOutputBody;
1213
use codex_protocol::models::FunctionCallOutputContentItem;
1314
use codex_protocol::models::FunctionCallOutputPayload;
1415
use codex_protocol::models::ResponseInputItem;
16+
use codex_protocol::models::SearchToolCallParams;
1517
use codex_protocol::models::ShellToolCallParams;
1618
use codex_protocol::models::function_call_output_content_items_to_text;
1719
use codex_utils_string::take_bytes_at_char_boundary;
@@ -48,7 +50,7 @@ pub enum ToolPayload {
4850
arguments: String,
4951
},
5052
ToolSearch {
51-
arguments: serde_json::Value,
53+
arguments: SearchToolCallParams,
5254
},
5355
Custom {
5456
input: String,
@@ -67,7 +69,7 @@ impl ToolPayload {
6769
pub fn log_payload(&self) -> Cow<'_, str> {
6870
match self {
6971
ToolPayload::Function { arguments } => Cow::Borrowed(arguments),
70-
ToolPayload::ToolSearch { arguments } => Cow::Owned(arguments.to_string()),
72+
ToolPayload::ToolSearch { arguments } => Cow::Owned(arguments.query.clone()),
7173
ToolPayload::Custom { input } => Cow::Borrowed(input),
7274
ToolPayload::LocalShell { params } => Cow::Owned(params.command.join(" ")),
7375
ToolPayload::Mcp { raw_arguments, .. } => Cow::Borrowed(raw_arguments),
@@ -114,12 +116,21 @@ impl ToolOutput for CallToolResult {
114116

115117
#[derive(Clone)]
116118
pub struct ToolSearchOutput {
117-
pub tools: Vec<JsonValue>,
119+
pub tools: Vec<ToolSearchOutputTool>,
118120
}
119121

120122
impl ToolOutput for ToolSearchOutput {
121123
fn log_preview(&self) -> String {
122-
telemetry_preview(&JsonValue::Array(self.tools.clone()).to_string())
124+
let tools = self
125+
.tools
126+
.iter()
127+
.map(|tool| {
128+
serde_json::to_value(tool).unwrap_or_else(|err| {
129+
JsonValue::String(format!("failed to serialize tool_search output: {err}"))
130+
})
131+
})
132+
.collect();
133+
telemetry_preview(&JsonValue::Array(tools).to_string())
123134
}
124135

125136
fn success_for_logging(&self) -> bool {
@@ -131,7 +142,15 @@ impl ToolOutput for ToolSearchOutput {
131142
call_id: call_id.to_string(),
132143
status: "completed".to_string(),
133144
execution: "client".to_string(),
134-
tools: self.tools.clone(),
145+
tools: self
146+
.tools
147+
.iter()
148+
.map(|tool| {
149+
serde_json::to_value(tool).unwrap_or_else(|err| {
150+
JsonValue::String(format!("failed to serialize tool_search output: {err}"))
151+
})
152+
})
153+
.collect(),
135154
}
136155
}
137156
}
@@ -539,13 +558,26 @@ mod tests {
539558
#[test]
540559
fn tool_search_payloads_roundtrip_as_tool_search_outputs() {
541560
let payload = ToolPayload::ToolSearch {
542-
arguments: json!({"query": "calendar"}),
561+
arguments: SearchToolCallParams {
562+
query: "calendar".to_string(),
563+
limit: None,
564+
},
543565
};
544566
let response = ToolSearchOutput {
545-
tools: vec![json!({
546-
"type": "function",
547-
"name": "create_event",
548-
})],
567+
tools: vec![ToolSearchOutputTool::Function(
568+
crate::client_common::tools::ResponsesApiTool {
569+
name: "create_event".to_string(),
570+
description: String::new(),
571+
strict: false,
572+
defer_loading: Some(true),
573+
parameters: crate::tools::spec::JsonSchema::Object {
574+
properties: Default::default(),
575+
required: None,
576+
additional_properties: None,
577+
},
578+
output_schema: None,
579+
},
580+
)],
549581
}
550582
.to_response_item("search-1", &payload);
551583

@@ -564,6 +596,13 @@ mod tests {
564596
vec![json!({
565597
"type": "function",
566598
"name": "create_event",
599+
"description": "",
600+
"strict": false,
601+
"defer_loading": true,
602+
"parameters": {
603+
"type": "object",
604+
"properties": {}
605+
}
567606
})]
568607
);
569608
}

0 commit comments

Comments
 (0)