diff --git a/PATCHES.md b/PATCHES.md index 29f54034..1e008f72 100644 --- a/PATCHES.md +++ b/PATCHES.md @@ -1,5 +1,18 @@ # Vendored local patches on serdes-ai 0.2.6 +## Issue 326: preserve raw streamed tool arguments + +`ToolCallPartDelta::apply` now distinguishes the typed initial empty-object +placeholder from raw argument bytes. A raw string spelling `{}` must be appended +verbatim, not replaced by the next delta. The previous placeholder heuristic +could lose a valid object followed by whitespace and could erase a malformed +concatenated-object prefix before strict host validation. The Codex HTTP compiled +regressions cover these cases, interleaved calls/reasoning, fragmented UTF-8 and +JSON, malformed external inputs, typed initialization and request boundaries. +No JSON repair, request retry, done-event fallback or provider shim is added. +This controlled current-source defect does not establish the cause of the +historical empty `read_file` argument failure. + ## Issue 79: stateless prompt-cache routing The retained Responses client adds an optional `prompt_cache_key` request field and @@ -65,7 +78,7 @@ Each vendored crate archive is SerdesAI 0.2.6 from crates.io. Every shipped | `serdes-ai-tools` | `ae4c635d97827560acaa8d3af32a78fc50fece538d1e4638c889c7588f490777` | | `serdes-ai-toolsets` | `85e7ab76a1546ce6aa858c7a0fd438dd4235b3927fcf5a907bec26bacb6f2588` | -`SERDES-AI-0.2.6.patch` is the complete diff from those extracted archives and the retained Responses Git snapshot to `vendor/`, including path-dependency rewrites, the bounded client-only Responses selection, and source compatibility changes. Its SHA-256 is `ec2a3dc2574fae2efa5f8f759e84b7bd0600c385ddc311fb2599d1bfa3338163`. +`SERDES-AI-0.2.6.patch` is the complete diff from those extracted archives and the retained Responses Git snapshot to `vendor/`, including path-dependency rewrites, the bounded client-only Responses selection, and source compatibility changes. Its SHA-256 is `715a5681c2836676c8c10d667c4fd6e47c818a7262316112e7ee5aa16b45ceb3`. `bash scripts/regenerate-serdes-patch.sh` recreates the patch from all 11 crates.io archives and the pinned Git snapshot in a temporary Git repository. It uses a committed archive baseline plus `git add -N` before the binary diff so new files, modifications, and deletions are all represented. The 11 exact crates.io archives and the Git archive of the Responses subtree are retained under `vendor-upstream/`. The snapshot identity and SHA-256 are recorded in `provenance/serdes-ai-responses-git.json`. To reproduce the vendored tree: diff --git a/SERDES-AI-0.2.6.patch b/SERDES-AI-0.2.6.patch index 04bddcaa..200df3a1 100644 --- a/SERDES-AI-0.2.6.patch +++ b/SERDES-AI-0.2.6.patch @@ -3874,7 +3874,7 @@ index 5c3e9c9..020eb12 100644 //! //! ## Example diff --git a/vendor/serdes-ai-core/src/messages/events.rs b/vendor/serdes-ai-core/src/messages/events.rs -index 64d62f5..1f84dff 100644 +index 64d62f5..d2086d0 100644 --- a/vendor/serdes-ai-core/src/messages/events.rs +++ b/vendor/serdes-ai-core/src/messages/events.rs @@ -6,6 +6,8 @@ @@ -3916,7 +3916,38 @@ index 64d62f5..1f84dff 100644 } /// Event indicating a new part has started. -@@ -653,6 +664,37 @@ impl PartEndEvent { +@@ -420,20 +431,17 @@ impl ToolCallPartDelta { + /// Apply this delta to an existing ToolCallPart. + pub fn apply(&self, part: &mut ToolCallPart) { + if !self.args_delta.is_empty() { +- // Check if current args are empty (just "{}") - if so, replace instead of append +- let current = part +- .args +- .to_json_string() +- .unwrap_or_else(|_| part.args.to_json().to_string()); +- +- let new_args = if current == "{}" || current.is_empty() { +- // Start fresh with the delta +- self.args_delta.clone() +- } else { +- // Append to existing args +- format!("{}{}", current, self.args_delta) ++ // A typed empty object is the initial placeholder. Raw strings are ++ // received argument bytes, even when they happen to spell "{}". ++ let mut arguments = match &part.args { ++ ToolCallArgs::String(raw) => raw.clone(), ++ ToolCallArgs::Json(value) if value.as_object().is_some_and(|v| v.is_empty()) => { ++ String::new() ++ } ++ ToolCallArgs::Json(value) => value.to_string(), + }; +- part.args = ToolCallArgs::String(new_args); ++ arguments.push_str(&self.args_delta); ++ part.args = ToolCallArgs::String(arguments); + } + if self.tool_call_id.is_some() && part.tool_call_id.is_none() { + part.tool_call_id = self.tool_call_id.clone(); +@@ -653,6 +661,37 @@ impl PartEndEvent { } } diff --git a/scripts/verify-vendor-licenses.sh b/scripts/verify-vendor-licenses.sh index 03ce0ddd..94ccd8e4 100644 --- a/scripts/verify-vendor-licenses.sh +++ b/scripts/verify-vendor-licenses.sh @@ -51,7 +51,7 @@ for manifest in "${required_vendor[@]}"; do done patch_file=SERDES-AI-0.2.6.patch -expected_patch='ec2a3dc2574fae2efa5f8f759e84b7bd0600c385ddc311fb2599d1bfa3338163' +expected_patch='715a5681c2836676c8c10d667c4fd6e47c818a7262316112e7ee5aa16b45ceb3' if [[ ! -f "$patch_file" ]]; then echo "missing reproducible vendor patch: $patch_file" >&2 fail=1 diff --git a/tests/codex_tool_argument.rs b/tests/codex_tool_argument.rs new file mode 100644 index 00000000..98ecae70 --- /dev/null +++ b/tests/codex_tool_argument.rs @@ -0,0 +1,101 @@ +//! Current Codex HTTP framing: argument bytes survive transport, folding and adapter. +mod tool_argument_framing; + +use llxprt_code_rs::adapter::LlmResult; +use llxprt_code_rs::agent::parse_object_args; +use serdes_ai::core::{FinishReason, ModelRequest, ModelSettings}; +use serdes_ai::models::{Model, ModelRequestParameters}; +use tool_argument_framing::{payload, serve}; + +fn request(model: &serdes_ai_responses::client::OpenResponsesModel) -> LlmResult { + let runtime = tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .unwrap(); + let response = runtime + .block_on(model.request( + &[ModelRequest::default()], + &ModelSettings::default(), + &ModelRequestParameters::default(), + )) + .unwrap(); + LlmResult::from_response(&response, &[]) +} + +#[test] +fn codex_raw_empty_object_is_not_a_placeholder() { + for fragments in [vec!["{}", " \n"], vec!["{", "}", "\t"]] { + let expected = fragments.concat(); + let (model, server) = serve(vec![payload(&[fragments], false)], true); + let result = request(&model); + server.join().unwrap(); + assert_eq!(result.finish_reason, Some(FinishReason::ToolCall)); + assert_eq!(result.calls.len(), 1); + assert_eq!(result.calls[0].args_json, expected); + assert_eq!( + parse_object_args(&result.calls[0]).unwrap(), + serde_json::json!({}) + ); + } +} + +#[test] +fn codex_malformed_external_arguments_remain_strict_failures() { + for fragments in [ + vec!["{}", "{\"path\":\".\"}"], + vec![""], + vec!["{\"path\":"], + vec!["[", "]"], + ] { + let expected = fragments.concat(); + let (model, server) = serve(vec![payload(&[fragments], false)], true); + let result = request(&model); + server.join().unwrap(); + assert_eq!(result.calls.len(), 1); + assert_eq!(result.calls[0].args_json, expected); + assert!(parse_object_args(&result.calls[0]).is_err()); + } +} + +#[test] +fn codex_interleaved_calls_reasoning_and_split_utf8_preserve_identity() { + let fragments = [vec!["{\"path\":", "\"é\"}"], vec!["{\"path\":", "\".\"}"]]; + let (model, server) = serve(vec![payload(&fragments, true)], true); + let result = request(&model); + server.join().unwrap(); + assert_eq!(result.finish_reason, Some(FinishReason::ToolCall)); + assert_eq!(result.thinking, "framing é"); + assert_eq!(result.calls.len(), 2); + for (index, call) in result.calls.iter().enumerate() { + assert_eq!(call.id, format!("call-{index}")); + assert_eq!(call.name, "list_directory"); + assert_eq!(call.args_json, fragments[index].concat()); + assert!(parse_object_args(call).is_ok()); + } + assert_eq!(result.usage.cache_read_tokens, Some(5)); +} + +#[test] +fn codex_arguments_do_not_cross_request_boundaries() { + let bodies = ["first", "second"] + .map(|path| payload(&[vec![&format!("{{\"path\":\"{path}\"}}")]], false)); + let (model, server) = serve(bodies.into(), false); + for path in ["first", "second"] { + let result = request(&model); + assert_eq!(result.calls.len(), 1); + assert_eq!( + parse_object_args(&result.calls[0]).unwrap(), + serde_json::json!({"path":path}) + ); + } + server.join().unwrap(); +} + +#[test] +fn typed_initial_empty_placeholder_still_starts_a_delta() { + use serdes_ai::core::messages::{ToolCallArgs, ToolCallPart, ToolCallPartDelta}; + let mut call = ToolCallPart::new("list_directory", ToolCallArgs::default()); + ToolCallPartDelta::new("{\"path\":").apply(&mut call); + ToolCallPartDelta::new("\".\"}").apply(&mut call); + assert_eq!(call.args.to_json_string().unwrap(), "{\"path\":\".\"}"); +} diff --git a/tests/tool_argument_framing/mod.rs b/tests/tool_argument_framing/mod.rs new file mode 100644 index 00000000..074e9d54 --- /dev/null +++ b/tests/tool_argument_framing/mod.rs @@ -0,0 +1,145 @@ +use serde_json::{json, Value}; +use serdes_ai_responses::types::{ + CreateResponseRequest, ResponseObject, ResponseStatus, StreamEvent, +}; +use std::io::{Read, Write}; +use std::net::TcpListener; +use std::time::Duration; +fn event(value: Value) -> StreamEvent { + serde_json::from_value(value).expect("payload must satisfy the current owning wire type") +} +fn call(index: usize, done: bool, args: &str) -> Value { + json!({"type":"function_call","id":format!("item-{index}"),"call_id":format!("call-{index}"),"name":"list_directory","arguments":args,"status":if done {"completed"} else {"in_progress"}}) +} +pub fn payload(deltas: &[Vec<&str>], reasoning: bool) -> String { + let request: CreateResponseRequest = + serde_json::from_value(json!({"model":"fixture","input":[]})).unwrap(); + let mut response = ResponseObject::in_progress("response", 1, "fixture", &request); + let mut events = vec![StreamEvent::ResponseCreated { + sequence_number: 0, + response: response.clone(), + }]; + for index in 0..deltas.len() { + events.push(event(json!({"type":"response.output_item.added","sequence_number":1,"output_index":index,"item":call(index,false,"")}))); + } + let ri = deltas.len(); + if reasoning { + events.push(event(json!({"type":"response.output_item.added","sequence_number":2,"output_index":ri,"item":{"type":"reasoning","id":"reason","status":"in_progress","summary":[]}}))); + events.push(event(json!({"type":"response.reasoning_summary_part.added","sequence_number":3,"output_index":ri,"item_id":"reason","summary_index":0,"part":{"type":"summary_text","text":""}}))); + events.push(event(json!({"type":"response.reasoning_summary_text.delta","sequence_number":4,"output_index":ri,"item_id":"reason","summary_index":0,"delta":"framing é"}))); + } + // Ordered starts, interleaved argument fragments, then per-call done boundaries. + for n in 0..deltas.iter().map(Vec::len).max().unwrap_or(0) { + for (index, fragments) in deltas.iter().enumerate() { + if let Some(delta) = fragments.get(n) { + events.push(event(json!({"type":"response.function_call_arguments.delta","sequence_number":5,"output_index":index,"item_id":format!("item-{index}"),"delta":delta}))); + } + } + } + let mut output = Vec::new(); + for (index, fragments) in deltas.iter().enumerate() { + let args = fragments.concat(); + let item = call(index, true, &args); + events.push(event(json!({"type":"response.function_call_arguments.done","sequence_number":6,"output_index":index,"item_id":format!("item-{index}"),"arguments":args}))); + events.push(event(json!({"type":"response.output_item.done","sequence_number":7,"output_index":index,"item":item}))); + output.push(item); + } + if reasoning { + let part = json!({"type":"summary_text","text":"framing é"}); + events.push(event(json!({"type":"response.reasoning_summary_text.done","sequence_number":8,"output_index":ri,"item_id":"reason","summary_index":0,"text":"framing é"}))); + events.push(event(json!({"type":"response.reasoning_summary_part.done","sequence_number":9,"output_index":ri,"item_id":"reason","summary_index":0,"part":part}))); + let item = json!({"type":"reasoning","id":"reason","status":"completed","summary":[part]}); + events.push(event(json!({"type":"response.output_item.done","sequence_number":10,"output_index":ri,"item":item}))); + output.push(item); + } + response.status = ResponseStatus::Completed; + response.usage=Some(serde_json::from_value(json!({"input_tokens":10,"output_tokens":3,"total_tokens":13,"input_tokens_details":{"cached_tokens":5}})).unwrap()); + response.output = output + .into_iter() + .map(|v| serde_json::from_value(v).unwrap()) + .collect(); + events.push(StreamEvent::ResponseCompleted { + sequence_number: 11, + response, + }); + events + .into_iter() + .enumerate() + .map(|(sequence, event)| { + let mut value = serde_json::to_value(event).unwrap(); + value["sequence_number"] = json!(sequence); + format!("data: {value}\r\n\r\n") + }) + .collect() +} +fn read_request(socket: &mut std::net::TcpStream) -> Value { + let mut bytes = Vec::new(); + loop { + let mut chunk = [0; 4096]; + let n = socket.read(&mut chunk).unwrap(); + assert!(n > 0); + bytes.extend_from_slice(&chunk[..n]); + if let Some(start) = bytes.windows(4).position(|v| v == b"\r\n\r\n") { + let headers = String::from_utf8_lossy(&bytes[..start]).to_lowercase(); + let len: usize = headers + .lines() + .find_map(|l| l.strip_prefix("content-length: ")) + .unwrap() + .parse() + .unwrap(); + if bytes.len() >= start + 4 + len { + return serde_json::from_slice(&bytes[start + 4..start + 4 + len]).unwrap(); + } + } + assert!(bytes.len() < 2 * 1024 * 1024); + } +} +pub fn serve( + bodies: Vec, + fragments: bool, +) -> ( + serdes_ai_responses::client::OpenResponsesModel, + std::thread::JoinHandle<()>, +) { + use serdes_ai_responses::client::OpenResponsesModel; + let turns = bodies.len(); + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + + let server = std::thread::spawn(move || { + let mut requests = Vec::new(); + for body in bodies { + let (mut socket, _) = listener.accept().unwrap(); + socket + .set_read_timeout(Some(Duration::from_secs(5))) + .unwrap(); + requests.push(read_request(&mut socket)); + write!(socket,"HTTP/1.1 200 OK\r\nContent-Type: text/event-stream\r\nContent-Length: {}\r\nConnection: close\r\n\r\n",body.len()).unwrap(); + if fragments { + for chunk in body.as_bytes().chunks(1) { + socket.write_all(chunk).unwrap(); + } + } else { + socket.write_all(body.as_bytes()).unwrap(); + } + } + listener.set_nonblocking(true).unwrap(); + std::thread::sleep(Duration::from_millis(100)); + assert!( + matches!(listener.accept(),Err(e) if e.kind()==std::io::ErrorKind::WouldBlock), + "request replay" + ); + assert_eq!(requests.len(), turns); + for request in requests { + assert_eq!(request["stream"], true); + assert_eq!(request["store"], false); + for key in ["previous_response_id", "max_output_tokens"] { + assert!(request.get(key).is_none()); + } + } + }); + let model = OpenResponsesModel::new("fixture", format!("http://{address}/responses")) + .with_http_client(reqwest::Client::builder().no_proxy().build().unwrap()) + .codex_http(); + (model, server) +} diff --git a/vendor/serdes-ai-core/src/messages/events.rs b/vendor/serdes-ai-core/src/messages/events.rs index 1f84dff1..d2086d05 100644 --- a/vendor/serdes-ai-core/src/messages/events.rs +++ b/vendor/serdes-ai-core/src/messages/events.rs @@ -431,20 +431,17 @@ impl ToolCallPartDelta { /// Apply this delta to an existing ToolCallPart. pub fn apply(&self, part: &mut ToolCallPart) { if !self.args_delta.is_empty() { - // Check if current args are empty (just "{}") - if so, replace instead of append - let current = part - .args - .to_json_string() - .unwrap_or_else(|_| part.args.to_json().to_string()); - - let new_args = if current == "{}" || current.is_empty() { - // Start fresh with the delta - self.args_delta.clone() - } else { - // Append to existing args - format!("{}{}", current, self.args_delta) + // A typed empty object is the initial placeholder. Raw strings are + // received argument bytes, even when they happen to spell "{}". + let mut arguments = match &part.args { + ToolCallArgs::String(raw) => raw.clone(), + ToolCallArgs::Json(value) if value.as_object().is_some_and(|v| v.is_empty()) => { + String::new() + } + ToolCallArgs::Json(value) => value.to_string(), }; - part.args = ToolCallArgs::String(new_args); + arguments.push_str(&self.args_delta); + part.args = ToolCallArgs::String(arguments); } if self.tool_call_id.is_some() && part.tool_call_id.is_none() { part.tool_call_id = self.tool_call_id.clone(); diff --git a/xtask/src/vendor_provenance.rs b/xtask/src/vendor_provenance.rs index 23912ee6..6edbc61f 100644 --- a/xtask/src/vendor_provenance.rs +++ b/xtask/src/vendor_provenance.rs @@ -55,7 +55,7 @@ pub fn run(root: &Path) -> Result { checked(&mut python(root, "verify-upstream-evidence.py"))?; checked(&mut python(root, "verify-serdes-responses-evidence.py"))?; let patch = root.join("SERDES-AI-0.2.6.patch"); - if digest(&patch)? != "ec2a3dc2574fae2efa5f8f759e84b7bd0600c385ddc311fb2599d1bfa3338163" { + if digest(&patch)? != "715a5681c2836676c8c10d667c4fd6e47c818a7262316112e7ee5aa16b45ceb3" { return Err("retained SerdesAI patch digest mismatch".into()); } documentation(root)?;