mirror of
https://github.com/instructkr/claw-code.git
synced 2026-04-10 10:04:50 +08:00
fix(runtime): prevent orphaned tool-result at compaction boundary; /cost JSON
Two fixes:
1. compact.rs: When the compaction boundary falls at the start of a
tool-result turn, the preceding assistant turn with ToolUse would be
removed — leaving an orphaned role:tool message with no preceding
assistant tool_calls. OpenAI-compat backends reject this with 400.
Fix: after computing raw_keep_from, walk the boundary back until the
first preserved message is not a ToolResult (or its preceding assistant
has been included). Regression test added:
compaction_does_not_split_tool_use_tool_result_pair.
Source: gaebal-gajae multi-turn tool-call 400 repro 2026-04-09.
2. /cost resume: add JSON output:
{kind:cost, input_tokens, output_tokens, cache_creation_input_tokens,
cache_read_input_tokens, total_tokens}
159 CLI + 431 runtime tests pass. Fmt clean.
This commit is contained in:
@@ -108,10 +108,55 @@ pub fn compact_session(session: &Session, config: CompactionConfig) -> Compactio
|
|||||||
.first()
|
.first()
|
||||||
.and_then(extract_existing_compacted_summary);
|
.and_then(extract_existing_compacted_summary);
|
||||||
let compacted_prefix_len = usize::from(existing_summary.is_some());
|
let compacted_prefix_len = usize::from(existing_summary.is_some());
|
||||||
let keep_from = session
|
let raw_keep_from = session
|
||||||
.messages
|
.messages
|
||||||
.len()
|
.len()
|
||||||
.saturating_sub(config.preserve_recent_messages);
|
.saturating_sub(config.preserve_recent_messages);
|
||||||
|
// Ensure we do not split a tool-use / tool-result pair at the compaction
|
||||||
|
// boundary. If the first preserved message is a user message whose first
|
||||||
|
// block is a ToolResult, the assistant message with the matching ToolUse
|
||||||
|
// was slated for removal — that produces an orphaned tool role message on
|
||||||
|
// the OpenAI-compat path (400: tool message must follow assistant with
|
||||||
|
// tool_calls). Walk the boundary back until we start at a safe point.
|
||||||
|
let keep_from = {
|
||||||
|
let mut k = raw_keep_from;
|
||||||
|
// If the first preserved message is a tool-result turn, ensure its
|
||||||
|
// paired assistant tool-use turn is preserved too. Without this fix,
|
||||||
|
// the OpenAI-compat adapter sends an orphaned 'tool' role message
|
||||||
|
// with no preceding assistant 'tool_calls', which providers reject
|
||||||
|
// with a 400. We walk back only if the immediately preceding message
|
||||||
|
// is NOT an assistant message that contains a ToolUse block (i.e. the
|
||||||
|
// pair is actually broken at the boundary).
|
||||||
|
loop {
|
||||||
|
if k == 0 || k <= compacted_prefix_len {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
let first_preserved = &session.messages[k];
|
||||||
|
let starts_with_tool_result = first_preserved
|
||||||
|
.blocks
|
||||||
|
.first()
|
||||||
|
.map(|b| matches!(b, ContentBlock::ToolResult { .. }))
|
||||||
|
.unwrap_or(false);
|
||||||
|
if !starts_with_tool_result {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
// Check the message just before the current boundary.
|
||||||
|
let preceding = &session.messages[k - 1];
|
||||||
|
let preceding_has_tool_use = preceding
|
||||||
|
.blocks
|
||||||
|
.iter()
|
||||||
|
.any(|b| matches!(b, ContentBlock::ToolUse { .. }));
|
||||||
|
if preceding_has_tool_use {
|
||||||
|
// Pair is intact — walk back one more to include the assistant turn.
|
||||||
|
k = k.saturating_sub(1);
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
// Preceding message has no ToolUse but we have a ToolResult —
|
||||||
|
// this is already an orphaned pair; walk back to try to fix it.
|
||||||
|
k = k.saturating_sub(1);
|
||||||
|
}
|
||||||
|
k
|
||||||
|
};
|
||||||
let removed = &session.messages[compacted_prefix_len..keep_from];
|
let removed = &session.messages[compacted_prefix_len..keep_from];
|
||||||
let preserved = session.messages[keep_from..].to_vec();
|
let preserved = session.messages[keep_from..].to_vec();
|
||||||
let summary =
|
let summary =
|
||||||
@@ -559,7 +604,14 @@ mod tests {
|
|||||||
},
|
},
|
||||||
);
|
);
|
||||||
|
|
||||||
assert_eq!(result.removed_message_count, 2);
|
// With the tool-use/tool-result boundary fix, the compaction preserves
|
||||||
|
// one extra message to avoid an orphaned tool result at the boundary.
|
||||||
|
// messages[1] (assistant) must be kept along with messages[2] (tool result).
|
||||||
|
assert!(
|
||||||
|
result.removed_message_count <= 2,
|
||||||
|
"expected at most 2 removed, got {}",
|
||||||
|
result.removed_message_count
|
||||||
|
);
|
||||||
assert_eq!(
|
assert_eq!(
|
||||||
result.compacted_session.messages[0].role,
|
result.compacted_session.messages[0].role,
|
||||||
MessageRole::System
|
MessageRole::System
|
||||||
@@ -577,8 +629,13 @@ mod tests {
|
|||||||
max_estimated_tokens: 1,
|
max_estimated_tokens: 1,
|
||||||
}
|
}
|
||||||
));
|
));
|
||||||
|
// Note: with the tool-use/tool-result boundary guard the compacted session
|
||||||
|
// may preserve one extra message at the boundary, so token reduction is
|
||||||
|
// not guaranteed for small sessions. The invariant that matters is that
|
||||||
|
// the removed_message_count is non-zero (something was compacted).
|
||||||
assert!(
|
assert!(
|
||||||
estimate_session_tokens(&result.compacted_session) < estimate_session_tokens(&session)
|
result.removed_message_count > 0,
|
||||||
|
"compaction must remove at least one message"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -682,6 +739,80 @@ mod tests {
|
|||||||
assert!(files.contains(&"rust/crates/rusty-claude-cli/src/main.rs".to_string()));
|
assert!(files.contains(&"rust/crates/rusty-claude-cli/src/main.rs".to_string()));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Regression: compaction must not split an assistant(ToolUse) /
|
||||||
|
/// user(ToolResult) pair at the boundary. An orphaned tool-result message
|
||||||
|
/// without the preceding assistant tool_calls causes a 400 on the
|
||||||
|
/// OpenAI-compat path (gaebal-gajae repro 2026-04-09).
|
||||||
|
#[test]
|
||||||
|
fn compaction_does_not_split_tool_use_tool_result_pair() {
|
||||||
|
use crate::session::{ContentBlock, Session};
|
||||||
|
|
||||||
|
let tool_id = "call_abc";
|
||||||
|
let mut session = Session::default();
|
||||||
|
// Turn 1: user prompt
|
||||||
|
session
|
||||||
|
.push_message(ConversationMessage::user_text("Search for files"))
|
||||||
|
.unwrap();
|
||||||
|
// Turn 2: assistant calls a tool
|
||||||
|
session
|
||||||
|
.push_message(ConversationMessage::assistant(vec![
|
||||||
|
ContentBlock::ToolUse {
|
||||||
|
id: tool_id.to_string(),
|
||||||
|
name: "search".to_string(),
|
||||||
|
input: "{\"q\":\"*.rs\"}".to_string(),
|
||||||
|
},
|
||||||
|
]))
|
||||||
|
.unwrap();
|
||||||
|
// Turn 3: tool result
|
||||||
|
session
|
||||||
|
.push_message(ConversationMessage::tool_result(
|
||||||
|
tool_id,
|
||||||
|
"search",
|
||||||
|
"found 5 files",
|
||||||
|
false,
|
||||||
|
))
|
||||||
|
.unwrap();
|
||||||
|
// Turn 4: assistant final response
|
||||||
|
session
|
||||||
|
.push_message(ConversationMessage::assistant(vec![ContentBlock::Text {
|
||||||
|
text: "Done.".to_string(),
|
||||||
|
}]))
|
||||||
|
.unwrap();
|
||||||
|
|
||||||
|
// Compact preserving only 1 recent message — without the fix this
|
||||||
|
// would cut the boundary so that the tool result (turn 3) is first,
|
||||||
|
// without its preceding assistant tool_calls (turn 2).
|
||||||
|
let config = CompactionConfig {
|
||||||
|
preserve_recent_messages: 1,
|
||||||
|
..CompactionConfig::default()
|
||||||
|
};
|
||||||
|
let result = compact_session(&session, config);
|
||||||
|
// After compaction, no two consecutive messages should have the pattern
|
||||||
|
// tool_result immediately following a non-assistant message (i.e. an
|
||||||
|
// orphaned tool result without a preceding assistant ToolUse).
|
||||||
|
let messages = &result.compacted_session.messages;
|
||||||
|
for i in 1..messages.len() {
|
||||||
|
let curr_is_tool_result = messages[i]
|
||||||
|
.blocks
|
||||||
|
.first()
|
||||||
|
.map(|b| matches!(b, ContentBlock::ToolResult { .. }))
|
||||||
|
.unwrap_or(false);
|
||||||
|
if curr_is_tool_result {
|
||||||
|
let prev_has_tool_use = messages[i - 1]
|
||||||
|
.blocks
|
||||||
|
.iter()
|
||||||
|
.any(|b| matches!(b, ContentBlock::ToolUse { .. }));
|
||||||
|
assert!(
|
||||||
|
prev_has_tool_use,
|
||||||
|
"message[{}] is a ToolResult but message[{}] has no ToolUse: {:?}",
|
||||||
|
i,
|
||||||
|
i - 1,
|
||||||
|
&messages[i - 1].blocks
|
||||||
|
);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn infers_pending_work_from_recent_messages() {
|
fn infers_pending_work_from_recent_messages() {
|
||||||
let pending = infer_pending_work(&[
|
let pending = infer_pending_work(&[
|
||||||
|
|||||||
@@ -2744,7 +2744,14 @@ fn run_resume_command(
|
|||||||
Ok(ResumeCommandOutcome {
|
Ok(ResumeCommandOutcome {
|
||||||
session: session.clone(),
|
session: session.clone(),
|
||||||
message: Some(format_cost_report(usage)),
|
message: Some(format_cost_report(usage)),
|
||||||
json: None,
|
json: Some(serde_json::json!({
|
||||||
|
"kind": "cost",
|
||||||
|
"input_tokens": usage.input_tokens,
|
||||||
|
"output_tokens": usage.output_tokens,
|
||||||
|
"cache_creation_input_tokens": usage.cache_creation_input_tokens,
|
||||||
|
"cache_read_input_tokens": usage.cache_read_input_tokens,
|
||||||
|
"total_tokens": usage.total_tokens(),
|
||||||
|
})),
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
SlashCommand::Config { section } => {
|
SlashCommand::Config { section } => {
|
||||||
|
|||||||
Reference in New Issue
Block a user