fix/acp-replay-loaded-session-history
claude/github-issue-40-0au5ua
claude/linked-issue-acp-tui-u4vvpa
claude/minor-release-cut-vkugow
claude/sigit-acp-local-chat-cx6380
claude/sigit-cloud-agent-expansion-reox0a
claude/tool-permission-system
claude/zen-feynman-0u78dk
development
feature/acp-cancel-request
feature/acp-session-list-cursor
feature/acp-v1-conformance
feature/agent-tools-multiedit-glob-todos-remember
feature/cargo-workspace
feature/hooks
feature/load-local-model-explicitly
feature/sigit-agent-profile
feature/sigit-code-cloud
feature/xcode-mcp-server
fix/acp-replay-loaded-session-history
fix/acp-replay-reasoning-and-attachments
fix/dsml-doubled-bar-markers
main
release/v1.5.10
release/v1.5.6
release/v1.6.1
| 1 | //! Recovery for tool calls a model emits as literal text instead of the |
| 2 | //! endpoint's structured `tool_calls` field. |
| 3 | //! |
| 4 | //! Several open-weight families siGit Code can drive — Qwen 3, GLM, DeepSeek — are |
| 5 | //! fine-tuned on a chat template that renders a tool call as |
| 6 | //! `<tool_call>NAME<arg_key>K</arg_key><arg_value>V</arg_value>...</tool_call>`. |
| 7 | //! A serving stack is supposed to parse that back into the OpenAI-shaped |
| 8 | //! `tool_calls` field before it reaches us. When it doesn't — seen against the |
| 9 | //! hosted GLM tiers once the model has just been told a call was rejected as a |
| 10 | //! repeat (getsigit/sigit#73) — the raw tag arrives as ordinary content. |
| 11 | //! |
| 12 | //! Nothing then executes it. Worse, `consume_stream` forwards content to the |
| 13 | //! UI as it arrives, so the tag is rendered verbatim in the editor and the |
| 14 | //! turn ends with no tool calls, which reads to the agent loop as "the model |
| 15 | //! chose to answer in prose". Whatever it was actually trying to do is |
| 16 | //! dropped. |
| 17 | //! |
| 18 | //! This lives here rather than only in the gateway because siGit Code talks to |
| 19 | //! more than one kind of endpoint: siGit Code Cloud, an arbitrary |
| 20 | //! OpenAI-compatible base URL from `providers.toml` or `OPENAI_BASE_URL`, and |
| 21 | //! on-device models. A fix in any single upstream leaves the others exposed. |
| 22 | //! |
| 23 | //! Only the well-formed shape is recovered: a flat sequence of key/value pairs |
| 24 | //! with no nesting. A block that doesn't match is left in the text untouched, |
| 25 | //! deliberately. Reissuing a `run_command` is cheap, but guessing wrong at a |
| 26 | //! half-parsed `edit_file` would write the wrong change to a file, so a |
| 27 | //! malformed block stays visible rather than being silently misinterpreted. |
| 28 | |
| 29 | use crate::backend::ToolSpec; |
| 30 | |
| 31 | const OPEN_TAG: &str = "<tool_call>"; |
| 32 | const CLOSE_TAG: &str = "</tool_call>"; |
| 33 | |
| 34 | /// One tool call recovered from inline text. |
| 35 | #[derive(Debug, Clone, PartialEq, Eq)] |
| 36 | pub struct Recovered { |
| 37 | pub name: String, |
| 38 | /// Arguments as a JSON-encoded string, matching `ToolCall::arguments`. |
| 39 | pub arguments: String, |
| 40 | } |
| 41 | |
| 42 | /// Scan a complete text blob for inline tool-call blocks. Returns the text |
| 43 | /// with recovered blocks removed, plus the calls recovered from them. Blocks |
| 44 | /// that don't parse cleanly, and an unterminated trailing tag, are left in the |
| 45 | /// returned text untouched. |
| 46 | /// |
| 47 | /// `tools` are the specs offered for this turn, used to type each argument |
| 48 | /// value: a field the schema declares as an integer has to arrive as a JSON |
| 49 | /// number or the tool's own argument parsing rejects it (`command_output` |
| 50 | /// reads `task_id` with `as_u64`, which refuses the string `"2"`). Anything |
| 51 | /// the schema doesn't cover stays a string. |
| 52 | pub fn extract(text: &str, tools: &[ToolSpec]) -> (String, Vec<Recovered>) { |
| 53 | let mut out = String::with_capacity(text.len()); |
| 54 | let mut calls = Vec::new(); |
| 55 | let mut cursor = text; |
| 56 | |
| 57 | while let Some(open_idx) = cursor.find(OPEN_TAG) { |
| 58 | let Some(close_rel) = cursor[open_idx..].find(CLOSE_TAG) else { |
| 59 | // Unterminated tag: leave the rest alone rather than guess at a |
| 60 | // block we never saw the end of. |
| 61 | break; |
| 62 | }; |
| 63 | let close_idx = open_idx + close_rel; |
| 64 | let inner = &cursor[open_idx + OPEN_TAG.len()..close_idx]; |
| 65 | |
| 66 | match parse_block(inner, tools) { |
| 67 | Some(call) => { |
| 68 | out.push_str(&cursor[..open_idx]); |
| 69 | calls.push(call); |
| 70 | } |
| 71 | // Didn't parse cleanly: keep the whole tag as visible text. |
| 72 | None => out.push_str(&cursor[..close_idx + CLOSE_TAG.len()]), |
| 73 | } |
| 74 | cursor = &cursor[close_idx + CLOSE_TAG.len()..]; |
| 75 | } |
| 76 | out.push_str(cursor); |
| 77 | (out, calls) |
| 78 | } |
| 79 | |
| 80 | /// Parse one block's inner text: a name, then zero or more |
| 81 | /// `<arg_key>K</arg_key><arg_value>V</arg_value>` pairs. Returns `None` the |
| 82 | /// moment anything departs from that shape. |
| 83 | fn parse_block(inner: &str, tools: &[ToolSpec]) -> Option<Recovered> { |
| 84 | let (name, mut rest) = match inner.find("<arg_key>") { |
| 85 | Some(idx) => (inner[..idx].trim(), &inner[idx..]), |
| 86 | None => (inner.trim(), ""), |
| 87 | }; |
| 88 | if name.is_empty() || name.contains(['<', '>']) { |
| 89 | return None; |
| 90 | } |
| 91 | |
| 92 | let mut args = serde_json::Map::new(); |
| 93 | while !rest.is_empty() { |
| 94 | let (key, after_key) = split_once(rest.strip_prefix("<arg_key>")?, "</arg_key>")?; |
| 95 | let (value, after_value) = |
| 96 | split_once(after_key.strip_prefix("<arg_value>")?, "</arg_value>")?; |
| 97 | let key = key.trim(); |
| 98 | if key.is_empty() { |
| 99 | return None; |
| 100 | } |
| 101 | args.insert( |
| 102 | key.to_string(), |
| 103 | coerce(value, declared_type(tools, name, key).as_deref()), |
| 104 | ); |
| 105 | rest = after_value; |
| 106 | } |
| 107 | |
| 108 | Some(Recovered { |
| 109 | name: name.to_string(), |
| 110 | arguments: serde_json::Value::Object(args).to_string(), |
| 111 | }) |
| 112 | } |
| 113 | |
| 114 | fn split_once<'a>(s: &'a str, delim: &str) -> Option<(&'a str, &'a str)> { |
| 115 | let idx = s.find(delim)?; |
| 116 | Some((&s[..idx], &s[idx + delim.len()..])) |
| 117 | } |
| 118 | |
| 119 | /// The JSON Schema `type` declared for `tool_name`'s `key` parameter — e.g. |
| 120 | /// `"integer"` for `command_output`'s `task_id`. [`ToolSpec`] carries the |
| 121 | /// schema as an unparsed string, so this parses it per lookup; the argument |
| 122 | /// counts involved are tiny and this only runs on the recovery path. |
| 123 | fn declared_type(tools: &[ToolSpec], tool_name: &str, key: &str) -> Option<String> { |
| 124 | let spec = tools.iter().find(|spec| spec.name == tool_name)?; |
| 125 | let schema: serde_json::Value = serde_json::from_str(&spec.parameters_schema).ok()?; |
| 126 | Some( |
| 127 | schema |
| 128 | .get("properties")? |
| 129 | .get(key)? |
| 130 | .get("type")? |
| 131 | .as_str()? |
| 132 | .to_string(), |
| 133 | ) |
| 134 | } |
| 135 | |
| 136 | /// Convert a raw (unescaped) text value to JSON per its declared schema type. |
| 137 | /// Falls back to a plain string for `"string"`, an unknown type, or a value |
| 138 | /// that doesn't parse as what it claims to be. This only ever narrows a value |
| 139 | /// to what the schema already promises — it never invents structure the text |
| 140 | /// doesn't have, since a bad coercion would corrupt a call that was otherwise |
| 141 | /// recoverable. |
| 142 | fn coerce(raw: &str, declared: Option<&str>) -> serde_json::Value { |
| 143 | match declared { |
| 144 | Some("integer") => raw |
| 145 | .trim() |
| 146 | .parse::<i64>() |
| 147 | .map(serde_json::Value::from) |
| 148 | .unwrap_or_else(|_| serde_json::Value::String(raw.to_string())), |
| 149 | Some("number") => raw |
| 150 | .trim() |
| 151 | .parse::<f64>() |
| 152 | .ok() |
| 153 | .and_then(serde_json::Number::from_f64) |
| 154 | .map(serde_json::Value::Number) |
| 155 | .unwrap_or_else(|| serde_json::Value::String(raw.to_string())), |
| 156 | Some("boolean") => match raw.trim() { |
| 157 | "true" => serde_json::Value::Bool(true), |
| 158 | "false" => serde_json::Value::Bool(false), |
| 159 | _ => serde_json::Value::String(raw.to_string()), |
| 160 | }, |
| 161 | _ => serde_json::Value::String(raw.to_string()), |
| 162 | } |
| 163 | } |
| 164 | |
| 165 | /// One event from incrementally scanning a stream of content deltas. |
| 166 | #[derive(Debug, Clone, PartialEq, Eq)] |
| 167 | pub enum ScanEvent { |
| 168 | /// Plain text, safe to forward to the UI. |
| 169 | Text(String), |
| 170 | /// A tool call recovered from a completed block. |
| 171 | ToolCall(Recovered), |
| 172 | } |
| 173 | |
| 174 | /// Incremental scanner for a live delta stream. |
| 175 | /// |
| 176 | /// Holds back only the text that could still turn into a `<tool_call>` tag, so |
| 177 | /// ordinary answers — the overwhelming majority of turns, which never contain |
| 178 | /// one — still reach the UI token by token. Buffering starts only once an |
| 179 | /// opening tag actually appears, and lasts only until it closes. |
| 180 | pub struct StreamScanner<'a> { |
| 181 | tools: &'a [ToolSpec], |
| 182 | pending: String, |
| 183 | } |
| 184 | |
| 185 | impl<'a> StreamScanner<'a> { |
| 186 | pub fn new(tools: &'a [ToolSpec]) -> Self { |
| 187 | Self { |
| 188 | tools, |
| 189 | pending: String::new(), |
| 190 | } |
| 191 | } |
| 192 | |
| 193 | /// Feed the next content delta. Returns events to emit now, in order; may |
| 194 | /// be empty if the chunk was absorbed into a tag that hasn't closed yet. |
| 195 | pub fn push(&mut self, chunk: &str) -> Vec<ScanEvent> { |
| 196 | self.pending.push_str(chunk); |
| 197 | self.drain() |
| 198 | } |
| 199 | |
| 200 | /// Take whatever is still held back — e.g. a `<tool_call` prefix that |
| 201 | /// never closed. Text we never saw the end of can't be interpreted |
| 202 | /// safely, so the honest thing is to hand it back as-is, exactly as if |
| 203 | /// recovery had never run. Call this wherever the stream can end, or |
| 204 | /// held-back text is lost. |
| 205 | pub fn take_pending(&mut self) -> Option<String> { |
| 206 | if self.pending.is_empty() { |
| 207 | None |
| 208 | } else { |
| 209 | Some(std::mem::take(&mut self.pending)) |
| 210 | } |
| 211 | } |
| 212 | |
| 213 | fn drain(&mut self) -> Vec<ScanEvent> { |
| 214 | let mut events = Vec::new(); |
| 215 | loop { |
| 216 | let Some(open_idx) = self.pending.find(OPEN_TAG) else { |
| 217 | // No tag yet: flush all but a suffix that could still grow |
| 218 | // into "<tool_call>" once the next chunk lands. |
| 219 | let keep = partial_prefix_len(&self.pending, OPEN_TAG); |
| 220 | let flush_len = self.pending.len() - keep; |
| 221 | if flush_len > 0 { |
| 222 | events.push(ScanEvent::Text( |
| 223 | self.pending.drain(..flush_len).collect::<String>(), |
| 224 | )); |
| 225 | } |
| 226 | break; |
| 227 | }; |
| 228 | if open_idx > 0 { |
| 229 | events.push(ScanEvent::Text( |
| 230 | self.pending.drain(..open_idx).collect::<String>(), |
| 231 | )); |
| 232 | } |
| 233 | let Some(close_rel) = self.pending[OPEN_TAG.len()..].find(CLOSE_TAG) else { |
| 234 | break; // opened but not closed: wait for more |
| 235 | }; |
| 236 | let close_idx = OPEN_TAG.len() + close_rel; |
| 237 | let block: String = self.pending.drain(..close_idx + CLOSE_TAG.len()).collect(); |
| 238 | let inner = &block[OPEN_TAG.len()..block.len() - CLOSE_TAG.len()]; |
| 239 | match parse_block(inner, self.tools) { |
| 240 | Some(call) => events.push(ScanEvent::ToolCall(call)), |
| 241 | None => events.push(ScanEvent::Text(block)), |
| 242 | } |
| 243 | } |
| 244 | events |
| 245 | } |
| 246 | } |
| 247 | |
| 248 | /// Length of the longest suffix of `buf` that is a prefix of `needle` — how |
| 249 | /// much of the tail could still become `needle`. Lets a split opening tag |
| 250 | /// (one chunk ending `"<tool_c"`, the next starting `"all>"`) be caught |
| 251 | /// without delaying text that can't possibly be part of one. |
| 252 | fn partial_prefix_len(buf: &str, needle: &str) -> usize { |
| 253 | let max = buf.len().min(needle.len() - 1); |
| 254 | for len in (1..=max).rev() { |
| 255 | let start = buf.len() - len; |
| 256 | if buf.is_char_boundary(start) && needle.starts_with(&buf[start..]) { |
| 257 | return len; |
| 258 | } |
| 259 | } |
| 260 | 0 |
| 261 | } |
| 262 | |
| 263 | #[cfg(test)] |
| 264 | mod tests { |
| 265 | use super::*; |
| 266 | |
| 267 | fn spec(name: &str, schema: serde_json::Value) -> ToolSpec { |
| 268 | ToolSpec { |
| 269 | name: name.to_string(), |
| 270 | description: String::new(), |
| 271 | parameters_schema: schema.to_string(), |
| 272 | } |
| 273 | } |
| 274 | |
| 275 | fn command_output_spec() -> ToolSpec { |
| 276 | spec( |
| 277 | "command_output", |
| 278 | serde_json::json!({ |
| 279 | "type": "object", |
| 280 | "properties": { "task_id": { "type": "integer" } } |
| 281 | }), |
| 282 | ) |
| 283 | } |
| 284 | |
| 285 | #[test] |
| 286 | fn an_integer_argument_is_recovered_as_a_json_number() { |
| 287 | // `exec_command_output` reads task_id with `as_u64`, so a string "2" |
| 288 | // here would parse as JSON and then fail at the tool. |
| 289 | let tools = vec![command_output_spec()]; |
| 290 | let (text, calls) = extract( |
| 291 | "Checking.<tool_call>command_output<arg_key>task_id</arg_key><arg_value>2</arg_value></tool_call>", |
| 292 | &tools, |
| 293 | ); |
| 294 | assert_eq!(text, "Checking."); |
| 295 | assert_eq!(calls.len(), 1); |
| 296 | assert_eq!(calls[0].name, "command_output"); |
| 297 | let args: serde_json::Value = serde_json::from_str(&calls[0].arguments).unwrap(); |
| 298 | assert_eq!(args["task_id"], 2); |
| 299 | assert!(args["task_id"].is_number()); |
| 300 | } |
| 301 | |
| 302 | #[test] |
| 303 | fn recovers_multiple_string_arguments_in_order() { |
| 304 | let tools = vec![spec( |
| 305 | "run_command", |
| 306 | serde_json::json!({ |
| 307 | "type": "object", |
| 308 | "properties": { |
| 309 | "command": { "type": "string" }, |
| 310 | "cwd": { "type": "string" }, |
| 311 | "run_in_background": { "type": "boolean" } |
| 312 | } |
| 313 | }), |
| 314 | )]; |
| 315 | let (text, calls) = extract( |
| 316 | "<tool_call>run_command<arg_key>command</arg_key><arg_value>cargo test --locked 2>&1 | tail -20</arg_value><arg_key>cwd</arg_key><arg_value>/Users/x/sigit</arg_value><arg_key>run_in_background</arg_key><arg_value>true</arg_value></tool_call>", |
| 317 | &tools, |
| 318 | ); |
| 319 | assert_eq!(text, ""); |
| 320 | let args: serde_json::Value = serde_json::from_str(&calls[0].arguments).unwrap(); |
| 321 | assert_eq!(args["command"], "cargo test --locked 2>&1 | tail -20"); |
| 322 | assert_eq!(args["cwd"], "/Users/x/sigit"); |
| 323 | assert_eq!(args["run_in_background"], true); |
| 324 | } |
| 325 | |
| 326 | #[test] |
| 327 | fn a_malformed_block_is_left_alone_rather_than_guessed_at() { |
| 328 | // The mis-tagged shape seen in practice: an opening `<arg_value>` |
| 329 | // where `<arg_key>` was meant. Guessing at an edit_file call is worse |
| 330 | // than leaving it visible. |
| 331 | let text = "<tool_call>edit_file<arg_value>new_text</arg_key><arg_value>def x; end</arg_value></tool_call>"; |
| 332 | let (out, calls) = extract(text, &[]); |
| 333 | assert_eq!(out, text); |
| 334 | assert!(calls.is_empty()); |
| 335 | } |
| 336 | |
| 337 | #[test] |
| 338 | fn an_unterminated_tag_is_left_alone() { |
| 339 | let text = "working on it <tool_call>run_command<arg_key>command</arg_key>"; |
| 340 | let (out, calls) = extract(text, &[]); |
| 341 | assert_eq!(out, text); |
| 342 | assert!(calls.is_empty()); |
| 343 | } |
| 344 | |
| 345 | #[test] |
| 346 | fn ordinary_text_passes_through_untouched() { |
| 347 | let (out, calls) = extract("if x < y then a<b, nothing to recover", &[]); |
| 348 | assert_eq!(out, "if x < y then a<b, nothing to recover"); |
| 349 | assert!(calls.is_empty()); |
| 350 | } |
| 351 | |
| 352 | #[test] |
| 353 | fn scanner_recovers_a_tag_split_across_chunk_boundaries() { |
| 354 | let tools = vec![command_output_spec()]; |
| 355 | let mut scanner = StreamScanner::new(&tools); |
| 356 | let mut text = String::new(); |
| 357 | let mut calls = Vec::new(); |
| 358 | |
| 359 | for chunk in [ |
| 360 | "polling ", |
| 361 | "<tool_c", |
| 362 | "all>command_output<arg_", |
| 363 | "key>task_id</arg_key><arg_value>2</arg_v", |
| 364 | "alue></tool_call>", |
| 365 | " done", |
| 366 | ] { |
| 367 | for event in scanner.push(chunk) { |
| 368 | match event { |
| 369 | ScanEvent::Text(t) => text.push_str(&t), |
| 370 | ScanEvent::ToolCall(c) => calls.push(c), |
| 371 | } |
| 372 | } |
| 373 | } |
| 374 | if let Some(rest) = scanner.take_pending() { |
| 375 | text.push_str(&rest); |
| 376 | } |
| 377 | |
| 378 | assert_eq!(text, "polling done", "no tag fragment may reach the UI"); |
| 379 | assert_eq!(calls.len(), 1); |
| 380 | assert_eq!(calls[0].name, "command_output"); |
| 381 | } |
| 382 | |
| 383 | #[test] |
| 384 | fn scanner_forwards_ordinary_text_without_holding_it_back() { |
| 385 | let mut scanner = StreamScanner::new(&[]); |
| 386 | let events = scanner.push("a normal streamed answer"); |
| 387 | assert_eq!( |
| 388 | events, |
| 389 | vec![ScanEvent::Text("a normal streamed answer".to_string())] |
| 390 | ); |
| 391 | assert!(scanner.take_pending().is_none()); |
| 392 | } |
| 393 | |
| 394 | #[test] |
| 395 | fn scanner_releases_a_near_miss_prefix_once_it_cannot_match() { |
| 396 | let mut scanner = StreamScanner::new(&[]); |
| 397 | // "<tool" could still become "<tool_call>", so it is held back... |
| 398 | assert_eq!( |
| 399 | scanner.push("see <tool"), |
| 400 | vec![ScanEvent::Text("see ".to_string())] |
| 401 | ); |
| 402 | // ...and released as soon as the next chunk rules that out. |
| 403 | assert_eq!( |
| 404 | scanner.push("box for details"), |
| 405 | vec![ScanEvent::Text("<toolbox for details".to_string())] |
| 406 | ); |
| 407 | assert!(scanner.take_pending().is_none()); |
| 408 | } |
| 409 | |
| 410 | #[test] |
| 411 | fn scanner_hands_back_an_unclosed_tag_at_stream_end() { |
| 412 | let mut scanner = StreamScanner::new(&[]); |
| 413 | let events = scanner.push("partial <tool_call>run_command<arg_key>cmd"); |
| 414 | assert_eq!(events, vec![ScanEvent::Text("partial ".to_string())]); |
| 415 | assert_eq!( |
| 416 | scanner.take_pending().as_deref(), |
| 417 | Some("<tool_call>run_command<arg_key>cmd") |
| 418 | ); |
| 419 | } |
| 420 | } |