Skip to content

runtime: persist tool-call identity so restarts replay valid history (#5823) #5840

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Hmbown merged 1 commit into from
Sep 2, 2026
+352 −35
Merged

runtime: persist tool-call identity so restarts replay valid history (#5823) #5840

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 74 additions & 35 deletions crates/tui/src/runtime_threads.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8319,51 +8319,57 @@ impl RuntimeThreadManager {
}
TurnItemKind::ToolCall => {
let meta = item.metadata.as_ref();
let is_tool_result = meta.and_then(|m| m.get("tool_result_for")).is_some();
if is_tool_result {
let meta_str = |key: &str| {
meta.and_then(|m| m.get(key))
.and_then(Value::as_str)
.unwrap_or_default()
.to_string()
};
let tool_use_id = meta_str("tool_use_id");
let tool_name = meta_str("tool_name");
let tool_result_for = meta_str("tool_result_for");
// Completed live turns persist the call and its result
// on one item; seeded history persists them as two.
// Both shapes must rebuild the paired tool_call /
// tool_result. Snapshots persisted before tool identity
// was durable carry neither side: skip them rather than
// replay an empty tool_call shell that strict
// OpenAI-compatible endpoints reject (#5823).
if !tool_use_id.is_empty() && !tool_name.is_empty() {
flush_user(&mut user_blocks, &mut messages);
let input_str = meta
.and_then(|m| m.get("tool_input"))
.and_then(Value::as_str)
.map(str::to_string)
.or_else(|| item.detail.clone())
.unwrap_or_default();
let input: serde_json::Value =
serde_json::from_str(&input_str).unwrap_or(serde_json::Value::Null);
assistant_blocks.push(ContentBlock::ToolUse {
id: tool_use_id,
name: tool_name,
input,
caller: None,
thought_signature: None,
});
}
if !tool_result_for.is_empty() {
flush_assistant(&mut assistant_blocks, &mut messages);

@codewhale-agent codewhale-agent Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Completed live tool-call items can split parallel tool calls into separate assistant/tool turns

Each completed live tool-call item now carries both call identity and tool_result_for, and the result branch flushes assistant_blocks immediately after each item. If one assistant turn issued multiple parallel tool calls, reconstruction will produce assistant(tool A) -> user(result A) -> assistant(tool B) -> user(result B) instead of a single assistant message containing all tool calls followed by their results. This changes the replayed history semantics and may be rejected by strict providers.

let tool_use_id = meta
.and_then(|m| m.get("tool_result_for"))
.and_then(|v| v.as_str())
.unwrap_or("")
.to_string();
let content = item.detail.unwrap_or_default();
let is_error = meta
.and_then(|m| m.get("is_error"))
.and_then(|v| v.as_bool())
.and_then(Value::as_bool)
.unwrap_or(false);
let content_blocks = meta
.and_then(|m| m.get("content_blocks"))
.and_then(|v| v.as_array())
.and_then(Value::as_array)
.cloned();
user_blocks.push(ContentBlock::ToolResult {
tool_use_id,
tool_use_id: tool_result_for,
content,
is_error: if is_error { Some(true) } else { None },
content_blocks,
});
} else {
flush_user(&mut user_blocks, &mut messages);
let tool_use_id = meta
.and_then(|m| m.get("tool_use_id"))
.and_then(|v| v.as_str())
.unwrap_or("")
.to_string();
let tool_name = meta
.and_then(|m| m.get("tool_name"))
.and_then(|v| v.as_str())
.unwrap_or("")
.to_string();
let input_str = item.detail.unwrap_or_default();
let input: serde_json::Value =
serde_json::from_str(&input_str).unwrap_or(serde_json::Value::Null);
assistant_blocks.push(ContentBlock::ToolUse {
id: tool_use_id,
name: tool_name,
input,
caller: None,
thought_signature: None,
});
}
}
_ => {}
Expand Down Expand Up @@ -8702,15 +8708,25 @@ impl RuntimeThreadManager {
tool_items.insert(id.clone(), item_id.clone());
let kind = tool_kind_for_name(&name);
let summary = summarize_text(&format!("{name} started"), SUMMARY_LIMIT);
let input_str = serde_json::to_string(&input).unwrap_or_default();
let item = TurnItemRecord {
schema_version: CURRENT_RUNTIME_SCHEMA_VERSION,
id: item_id.clone(),
turn_id: turn_id.clone(),
kind,
status: TurnItemLifecycleStatus::InProgress,
summary,
detail: Some(serde_json::to_string(&input).unwrap_or_default()),
metadata: None,
detail: Some(input_str.clone()),
// The tool identity must live in the durable item
// snapshot: restart history rebuild reads it back to
// re-emit provider tool_calls. Without it a restart
// replays empty id/name/arguments shells that strict
// OpenAI-compatible endpoints reject (#5823).
metadata: Some(json!({
"tool_use_id": id.clone(),
"tool_name": name.clone(),
"tool_input": input_str,
})),
artifact_refs: Vec::new(),
started_at: Some(Utc::now()),
ended_at: None,
Expand Down Expand Up @@ -8771,7 +8787,30 @@ impl RuntimeThreadManager {
SUMMARY_LIMIT,
);
item.detail = Some(output.content.clone());
item.metadata = output.metadata.clone();
// `detail` is now the tool output, so the
// call identity persisted at start must be
// carried through metadata. Mark the
// terminal result too so restart history
// rebuild can re-emit the paired

@codewhale-agent codewhale-agent Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Completion merge discards non-object output metadata

The new match output.metadata { Some(Value::Object(map)) => Value::Object(map), _ => json!({}) } resets any non-object metadata to an empty object. The previous code assigned output.metadata.clone() directly, preserving arbitrary serde_json::Value metadata. If a tool output can carry string/array/null metadata, that data is now lost at completion and may affect consumers of item.metadata.

// tool_call/tool_result (#5823).
let mut meta = match output.metadata {
Some(Value::Object(map)) => Value::Object(map),
_ => json!({}),
};
if let Some(obj) = meta.as_object_mut() {
if let Some(started) =
item.metadata.as_ref().and_then(Value::as_object)
{
for key in ["tool_use_id", "tool_name", "tool_input"] {
if let Some(value) = started.get(key) {
obj.insert(key.to_string(), value.clone());
}
}

@codewhale-agent codewhale-agent Bot Sep 2, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[WARNING] Legacy in-progress item can still become a standalone ToolResult after upgrade

For an item persisted before tool identity existed (item.metadata is None), completion still inserts tool_result_for and is_error. Reconstruction then sees a non-empty tool_result_for but empty tool_use_id/tool_name, so it emits a ToolResult with no preceding ToolUse. The PR explicitly plans to skip legacy items without durable identity, so the completion merge should avoid adding tool_result_for unless the start identity was copied.

}
obj.insert("tool_result_for".to_string(), json!(id));
obj.insert("is_error".to_string(), json!(!output.success));
}
item.metadata = Some(meta);
Comment on lines +8796 to +8813

@claude claude Bot Sep 2, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good fix for the Ok branch, but note the Err(err) => branch a few lines below (crates/tui/src/runtime_threads.rs:8816-8821, outside this diff) has no equivalent: it never sets tool_result_for/is_error, so it keeps only the tool_use_id/tool_name/tool_input set at ToolCallStarted.

After a restart, reconstruct_messages_from_turns will replay that item as a ToolUse with no paired ToolResult. tool_history_repair::repair_tool_call_pairs_for_provider catches this before it hits the wire, but it synthesizes a generic "Tool call interrupted by process exit; terminal status: crashed_and_repaired." placeholder — discarding the real error text already captured in item.detail = Some(err.to_string()) and mislabeling a normal tool-execution failure as a process-crash repair.

Worth mirroring this same merge (tool_result_for: id, is_error: true) in the Err branch so the actual error survives replay.

}
}
Err(err) => {
Expand Down
Loading
Loading