From 76dd924c7fcb791f4d48b1acac509ffaa566c205 Mon Sep 17 00:00:00 2001 From: Hunter Bown Date: Mon, 4 May 2026 22:27:11 -0500 Subject: [PATCH] fix(engine): turn_meta must skip tool-result messages (HTTP 400 fix) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Live-test repro: typing a single user message in the TUI triggered a tool call (read_file Cargo.toml), and the *next* request to DeepSeek's API returned HTTP 400: "An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'. (insufficient tool messages following tool_calls message)" Root cause: `messages_with_turn_metadata` walked the message list from the tail and prepended a `` Text block to the *last* message with role="user". But tool-result messages also use role="user" internally (they serialize to role="tool" on the wire). Inserting a Text content block at index 0 of a tool-result message changed the shape from `[ToolResult(...)]` to `[Text("turn_meta..."), ToolResult(...)]`, which on the wire becomes a role="user" message with text instead of the role="tool" message the API needs to satisfy the assistant's prior tool_call. Hence the 400. The fix: * Restrict the injection target to messages that have at least one Text content block AND no ToolResult blocks. This identifies actual user-typed messages and skips tool-result envelopes. * When the trailing slice has no eligible user message (e.g. mid-turn when a tool result is the most recent message), skip injection entirely. The working_set will surface again on the next genuine user prompt; we don't retroactively prepend onto an earlier user message because that would also confuse the API's tool-call continuity checks. Two regression tests pin the contract: * `turn_metadata_skips_tool_result_messages` — assistant tool_call + tool_result + earlier user message: only the user message gets the prefix, the tool_result message stays a single-block ToolResult. * `turn_metadata_skips_when_only_tool_results_trail` — the corner case where the trailing user-role message is solely a tool result (no real user message in the slice): no injection happens, the message returns unchanged. Verified locally: * 2038 tests passed in TUI bin (2 ignored, was 2036 — these are the +2 new regressions). * `cargo fmt`, `cargo clippy --locked -D warnings`, parity gates all clean. Co-Authored-By: Claude Opus 4.7 (1M context) --- crates/tui/src/core/engine/tests.rs | 126 ++++++++++++++++++++++++ crates/tui/src/core/engine/turn_loop.rs | 26 ++++- 2 files changed, 147 insertions(+), 5 deletions(-) diff --git a/crates/tui/src/core/engine/tests.rs b/crates/tui/src/core/engine/tests.rs index 9c28826d..00a82681 100644 --- a/crates/tui/src/core/engine/tests.rs +++ b/crates/tui/src/core/engine/tests.rs @@ -567,6 +567,132 @@ fn working_set_reaches_model_as_turn_metadata() { assert!(text.contains("src/lib.rs")); } +/// v0.8.11 regression: tool-result messages serialize to role="tool" on +/// the wire but are stored as role="user" internally. Prepending +/// `` text onto a tool-result message broke the +/// assistant→tool_result invariant and caused HTTP 400 from DeepSeek's +/// API ("insufficient tool messages following tool_calls"). The fix: +/// inject only into messages that have a Text content block and no +/// ToolResult blocks; mid-turn (tool-result is the trailing user +/// message) the injection skips. +#[test] +fn turn_metadata_skips_tool_result_messages() { + let tmp = tempdir().expect("tempdir"); + fs::create_dir_all(tmp.path().join("src")).expect("mkdir"); + fs::write(tmp.path().join("src/lib.rs"), "pub fn sample() {}").expect("write"); + + let config = EngineConfig { + workspace: tmp.path().to_path_buf(), + ..Default::default() + }; + let (mut engine, _handle) = Engine::new(config, &Config::default()); + engine + .session + .working_set + .observe_user_message("inspect src/lib.rs", tmp.path()); + + // Real user message — should be eligible for injection. + engine.session.add_message(Message { + role: "user".to_string(), + content: vec![ContentBlock::Text { + text: "inspect src/lib.rs".to_string(), + cache_control: None, + }], + }); + // Assistant tool-call. + engine.session.add_message(Message { + role: "assistant".to_string(), + content: vec![ContentBlock::ToolUse { + id: "call_42".to_string(), + name: "read_file".to_string(), + input: serde_json::json!({"path": "src/lib.rs"}), + caller: None, + }], + }); + // Tool result, stored as role="user" internally. + engine.session.add_message(Message { + role: "user".to_string(), + content: vec![ContentBlock::ToolResult { + tool_use_id: "call_42".to_string(), + content: "pub fn sample() {}".to_string(), + is_error: None, + content_blocks: None, + }], + }); + + let messages = engine.messages_with_turn_metadata(); + + // The trailing message is the tool result and MUST be untouched — + // no Text block sneaking in front of the ToolResult block. + let trailing = messages.last().expect("trailing message"); + assert_eq!(trailing.role, "user"); + assert_eq!(trailing.content.len(), 1); + assert!(matches!( + trailing.content.first(), + Some(ContentBlock::ToolResult { .. }) + )); + + // The earlier real user message receives the turn_meta prefix. + let real_user = messages.first().expect("first user message"); + assert_eq!(real_user.role, "user"); + let ContentBlock::Text { text, .. } = real_user + .content + .first() + .expect("user text content") + else { + panic!("expected Text block on real user message"); + }; + assert!(text.starts_with("\n")); + assert!(text.contains("src/lib.rs")); +} + +/// When the turn is mid-execution and the trailing user message is a +/// tool result, no turn_meta is injected at all (rather than landing on +/// some earlier user message and confusing the API's tool-call +/// continuity check). The working_set surfaces again on the next +/// genuine user prompt. +#[test] +fn turn_metadata_skips_when_only_tool_results_trail() { + let tmp = tempdir().expect("tempdir"); + fs::create_dir_all(tmp.path().join("src")).expect("mkdir"); + fs::write(tmp.path().join("src/lib.rs"), "pub fn sample() {}").expect("write"); + + let config = EngineConfig { + workspace: tmp.path().to_path_buf(), + ..Default::default() + }; + let (mut engine, _handle) = Engine::new(config, &Config::default()); + engine + .session + .working_set + .observe_user_message("inspect src/lib.rs", tmp.path()); + + // Only a tool-result message in history — simulates the corner case + // where the prior real user message has already been compacted away + // but a tool-result is still pending. We must not retroactively + // inject. + engine.session.add_message(Message { + role: "user".to_string(), + content: vec![ContentBlock::ToolResult { + tool_use_id: "call_42".to_string(), + content: "pub fn sample() {}".to_string(), + is_error: None, + content_blocks: None, + }], + }); + + let messages = engine.messages_with_turn_metadata(); + + // Returned unchanged: the single tool-result message, no Text + // prefix, content length == 1. + let only = messages.last().expect("trailing message"); + assert_eq!(only.content.len(), 1); + assert!(matches!( + only.content.first(), + Some(ContentBlock::ToolResult { .. }) + )); +} + #[test] fn refresh_system_prompt_is_noop_when_unchanged() { let tmp = tempdir().expect("tempdir"); diff --git a/crates/tui/src/core/engine/turn_loop.rs b/crates/tui/src/core/engine/turn_loop.rs index 2220dc9a..18071ef5 100644 --- a/crates/tui/src/core/engine/turn_loop.rs +++ b/crates/tui/src/core/engine/turn_loop.rs @@ -1607,11 +1607,27 @@ impl Engine { }; let mut messages = self.session.messages.clone(); - let Some(last_user) = messages - .iter_mut() - .rev() - .find(|message| message.role == "user") - else { + // v0.8.11 hotfix: tool-result messages are stored as role="user" in + // our internal representation but serialize to role="tool" on the + // wire. Prepending a Text block onto a tool-result message breaks + // the assistant→tool_result invariant — the API rejects the request + // with `"insufficient tool messages following tool_calls"`. Inject + // only into actual user-typed messages, recognizable by having at + // least one Text content block (and no ToolResult blocks). + let Some(last_user) = messages.iter_mut().rev().find(|message| { + message.role == "user" + && message + .content + .iter() + .all(|block| !matches!(block, ContentBlock::ToolResult { .. })) + && message + .content + .iter() + .any(|block| matches!(block, ContentBlock::Text { .. })) + }) else { + // No real user message in the trailing slice (e.g. mid-turn + // after a tool call). Skip injection — the working_set will + // surface again on the next genuine user prompt. return messages; };