Skip to content

Commit a3d0c9e

Browse files
committed
fix(api): sanitize orphaned tool messages at request-building layer
Adds sanitize_tool_message_pairing() called from build_chat_completion_request() after translate_message() runs. Drops any role:"tool" message whose immediately-preceding non-tool message is role:"assistant" but has no tool_calls entry matching the tool_call_id. This is the second layer of the tool-pairing invariant defense: - 6e301c8: compaction boundary fix (producer layer) - this commit: request-builder sanitizer (sender layer) Together these close the 400-error loop for resumed/compacted multi-turn tool sessions on OpenAI-compatible backends. Sanitization only fires when preceding message is role:assistant (not user/system) to avoid dropping valid translation artifacts from mixed user-message content blocks. Regression tests: sanitize_drops_orphaned_tool_messages covers valid pair, orphaned tool (no tool_calls in preceding assistant), mismatched id, and two tool results both referencing the same assistant turn. 116 api + 159 CLI + 431 runtime tests pass. Fmt clean.
1 parent 78dca71 commit a3d0c9e

1 file changed

Lines changed: 122 additions & 0 deletions

File tree

rust/crates/api/src/providers/openai_compat.rs

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -797,6 +797,14 @@ fn build_chat_completion_request(request: &MessageRequest, config: OpenAiCompatC
797797
for message in &request.messages {
798798
messages.extend(translate_message(message));
799799
}
800+
// Sanitize: drop any `role:"tool"` message that does not have a valid
801+
// paired `role:"assistant"` with a `tool_calls` entry carrying the same
802+
// `id` immediately before it (directly or as part of a run of tool
803+
// results). OpenAI-compatible backends return 400 for orphaned tool
804+
// messages regardless of how they were produced (compaction, session
805+
// editing, resume, etc.). We drop rather than error so the request can
806+
// still proceed with the remaining history intact.
807+
messages = sanitize_tool_message_pairing(messages);
800808

801809
// Strip routing prefix (e.g., "openai/gpt-4" → "gpt-4") for the wire.
802810
let wire_model = strip_routing_prefix(&request.model);
@@ -918,6 +926,75 @@ fn translate_message(message: &InputMessage) -> Vec<Value> {
918926
}
919927
}
920928

929+
/// Remove `role:"tool"` messages from `messages` that have no valid paired
930+
/// `role:"assistant"` message with a matching `tool_calls[].id` immediately
931+
/// preceding them. This is a last-resort safety net at the request-building
932+
/// layer — the compaction boundary fix (6e301c8) prevents the most common
933+
/// producer path, but resume, session editing, or future compaction variants
934+
/// could still create orphaned tool messages.
935+
///
936+
/// Algorithm: scan left-to-right. For each `role:"tool"` message, check the
937+
/// immediately preceding non-tool message. If it's `role:"assistant"` with a
938+
/// `tool_calls` array containing an entry whose `id` matches the tool
939+
/// message's `tool_call_id`, the pair is valid and both are kept. Otherwise
940+
/// the tool message is dropped.
941+
fn sanitize_tool_message_pairing(messages: Vec<Value>) -> Vec<Value> {
942+
// Collect indices of tool messages that are orphaned.
943+
let mut drop_indices = std::collections::HashSet::new();
944+
for (i, msg) in messages.iter().enumerate() {
945+
if msg.get("role").and_then(|v| v.as_str()) != Some("tool") {
946+
continue;
947+
}
948+
let tool_call_id = msg
949+
.get("tool_call_id")
950+
.and_then(|v| v.as_str())
951+
.unwrap_or("");
952+
// Find the nearest preceding non-tool message.
953+
let preceding = messages[..i]
954+
.iter()
955+
.rev()
956+
.find(|m| m.get("role").and_then(|v| v.as_str()) != Some("tool"));
957+
// A tool message is considered paired when:
958+
// (a) the nearest preceding non-tool message is an assistant message
959+
// whose `tool_calls` array contains an entry with the matching id, OR
960+
// (b) there's no clear preceding context (e.g. the message comes right
961+
// after a user turn — this can happen with translated mixed-content
962+
// user messages). In case (b) we allow the message through rather
963+
// than silently dropping potentially valid history.
964+
let preceding_role = preceding
965+
.and_then(|m| m.get("role"))
966+
.and_then(|v| v.as_str())
967+
.unwrap_or("");
968+
// Only apply sanitization when the preceding message is an assistant
969+
// turn (the invariant is: assistant-with-tool_calls must precede tool).
970+
// If the preceding is something else (user, system) don't drop — it
971+
// may be a valid translation artifact or a path we don't understand.
972+
if preceding_role != "assistant" {
973+
continue;
974+
}
975+
let paired = preceding
976+
.and_then(|m| m.get("tool_calls").and_then(|tc| tc.as_array()))
977+
.map(|tool_calls| {
978+
tool_calls
979+
.iter()
980+
.any(|tc| tc.get("id").and_then(|v| v.as_str()) == Some(tool_call_id))
981+
})
982+
.unwrap_or(false);
983+
if !paired {
984+
drop_indices.insert(i);
985+
}
986+
}
987+
if drop_indices.is_empty() {
988+
return messages;
989+
}
990+
messages
991+
.into_iter()
992+
.enumerate()
993+
.filter(|(i, _)| !drop_indices.contains(i))
994+
.map(|(_, m)| m)
995+
.collect()
996+
}
997+
921998
fn flatten_tool_result_content(content: &[ToolResultContentBlock]) -> String {
922999
content
9231000
.iter()
@@ -1656,6 +1733,51 @@ mod tests {
16561733
assert_eq!(tool_calls.as_array().unwrap().len(), 1);
16571734
}
16581735

1736+
/// Orphaned tool messages (no preceding assistant tool_calls) must be
1737+
/// dropped by the request-builder sanitizer. Regression for the second
1738+
/// layer of the tool-pairing invariant fix (gaebal-gajae 2026-04-10).
1739+
#[test]
1740+
fn sanitize_drops_orphaned_tool_messages() {
1741+
use super::sanitize_tool_message_pairing;
1742+
1743+
// Valid pair: assistant with tool_calls → tool result
1744+
let valid = vec![
1745+
json!({"role": "assistant", "content": null, "tool_calls": [{"id": "call_1", "type": "function", "function": {"name": "search", "arguments": "{}"}}]}),
1746+
json!({"role": "tool", "tool_call_id": "call_1", "content": "result"}),
1747+
];
1748+
let out = sanitize_tool_message_pairing(valid);
1749+
assert_eq!(out.len(), 2, "valid pair must be preserved");
1750+
1751+
// Orphaned tool message: no preceding assistant tool_calls
1752+
let orphaned = vec![
1753+
json!({"role": "assistant", "content": "hi"}),
1754+
json!({"role": "tool", "tool_call_id": "call_2", "content": "orphaned"}),
1755+
];
1756+
let out = sanitize_tool_message_pairing(orphaned);
1757+
assert_eq!(out.len(), 1, "orphaned tool message must be dropped");
1758+
assert_eq!(out[0]["role"], json!("assistant"));
1759+
1760+
// Mismatched tool_call_id
1761+
let mismatched = vec![
1762+
json!({"role": "assistant", "content": null, "tool_calls": [{"id": "call_3", "type": "function", "function": {"name": "f", "arguments": "{}"}}]}),
1763+
json!({"role": "tool", "tool_call_id": "call_WRONG", "content": "bad"}),
1764+
];
1765+
let out = sanitize_tool_message_pairing(mismatched);
1766+
assert_eq!(out.len(), 1, "tool message with wrong id must be dropped");
1767+
1768+
// Two tool results both valid (same preceding assistant)
1769+
let two_results = vec![
1770+
json!({"role": "assistant", "content": null, "tool_calls": [
1771+
{"id": "call_a", "type": "function", "function": {"name": "fa", "arguments": "{}"}},
1772+
{"id": "call_b", "type": "function", "function": {"name": "fb", "arguments": "{}"}}
1773+
]}),
1774+
json!({"role": "tool", "tool_call_id": "call_a", "content": "ra"}),
1775+
json!({"role": "tool", "tool_call_id": "call_b", "content": "rb"}),
1776+
];
1777+
let out = sanitize_tool_message_pairing(two_results);
1778+
assert_eq!(out.len(), 3, "both valid tool results must be preserved");
1779+
}
1780+
16591781
#[test]
16601782
fn non_gpt5_uses_max_tokens() {
16611783
// Older OpenAI models expect `max_tokens`; verify gpt-4o is unaffected.

0 commit comments

Comments
 (0)