Skip to content

Commit 273aada

Browse files
yogthosYogthos
andauthored
feat(phase 3): structured tool-call persistence with interrupted-state pairing (#71)
Phase 3 of the 6-phase plan. Reference pattern: opencode's `packages/opencode/src/session/message-v2.ts:310-320, 630-899` where `ToolPart` carries a `state: pending|running|completed|error` and is materialized into provider-format tool_use/tool_result blocks on resume. Anthropic + OpenAI reject orphan tool_use blocks, so opencode always emits a paired result — even "[Tool execution was interrupted]" for unfinished calls. dirge adopts the same shape. ## Problem Before Phase 3, dirge's `SessionMessage` was (role + text). Tool calls and results streamed to the UI but were never persisted. On session resume, `convert_history` emitted assistants as text-only — the LLM lost all structured knowledge of prior tool work. It could re-attempt the same bash command, re-read the same file, or hallucinate that "the file said X" without seeing the actual prior tool_result. ## Fix New `ToolCallEntry` + `ToolCallState` types in `src/session/mod.rs`: ```rust pub struct ToolCallEntry { pub id: String, // rig's ToolCall.id for correlation pub name: String, // tool name pub args: serde_json::Value, // unparsed args pub state: ToolCallState, } pub enum ToolCallState { Completed { result: String }, Interrupted, Failed { error: String }, } ``` `SessionMessage` gains `#[serde(default)] tool_calls: Vec<ToolCallEntry>` — back-compat with old session files (missing field → empty Vec). `Session::add_message_with_tool_calls(role, content, tool_calls)` is the new constructor; the existing `add_message` becomes a thin wrapper that passes an empty Vec. ### Event shape `AgentEvent::ToolCall` + `AgentEvent::ToolResult` now both carry an `id: CompactString` (rig's `ToolCall.id` / `ToolResult.id`). Empty when the provider didn't emit one — the UI falls back to positional pairing in that case. ### UI capture (src/ui/mod.rs) - New `tool_calls_buf: Vec<ToolCallEntry>` lives alongside `response_buf` for the duration of an agent run. - `AgentEvent::ToolCall` pushes a new entry with `state: Interrupted` (defensive default — if the user aborts before the result arrives, the saved state is already correct). - `AgentEvent::ToolResult` finds the matching entry (by id, or last-pending if id is empty) and flips state to `Completed { result }`. - `AgentEvent::Done` and `AgentEvent::Interjected` call `add_message_with_tool_calls(Assistant, response, std::mem::take(&mut tool_calls_buf))` — the run's tool calls attach to the final assistant message. - `capture_partial_on_abort` (Ctrl+C, Esc) also drains the buffer onto the stashed message; any still-Interrupted entries stay that way, completed ones keep their state. Empty buf alone no longer counts as a no-op when there are pending tool calls — the abort still stashes a message with just the trailer + the tool_calls so the LLM sees the interrupted state. ### convert_history (src/agent/runner.rs) When an assistant message has `tool_calls`, emit: 1. `Message::Assistant` with content = [text (if any), tool_call(...)...] built via `OneOrMany::many(...)` / `OneOrMany::one(...)`. 2. `Message::tool_result(id, body)` per call, where: - `Completed { result }` → body = result text verbatim - `Interrupted` → "[Tool execution was interrupted]" - `Failed { error }` → "[Tool error: <msg>]" Bare assistant messages (no tool_calls) keep the prior simple `Message::assistant(text)` shape — full backward compatibility with existing session files. ## Tests 4 new session tests, written failing first: - `session_message_tool_calls_default_when_field_missing`: old session JSON without `tool_calls` field deserializes with empty Vec — back-compat guard. - `session_message_tool_calls_roundtrip`: write a message with tool_calls, read back via serde, fields intact. - `convert_history_emits_tool_use_and_tool_result_blocks`: builds a session with one bash call → completed; asserts history has [User, Assistant(text+tool_use), User(tool_result)] with the id correlation intact. - `convert_history_pairs_interrupted_tool_calls_with_error_marker`: Interrupted entry → result body contains "interrupted". 1 new UI test: - `capture_partial_on_abort_preserves_pending_tool_calls_as_interrupted`: mix of Interrupted + Completed entries in the buffer → saved assistant message has both with their states intact. 5 new tests total. 644 pass (was 639), 0 fail, all build profiles clean. ## Test plan - [x] `cargo test --features plugin` -> 644 pass. - [x] `cargo build --all-features` -> compiles. - [x] `cargo build --no-default-features` -> compiles. ## Up next: Phase 4 (optional, pi-style branch summaries) Phase 2 drops sibling branches with a notification. If users report this feels too lossy, Phase 4 would add pi's `BranchSummaryMessage` pattern — generate per-branch LLM summaries and persist them as a new message variant so forks are preserved across compactions instead of discarded. Co-authored-by: Yogthos <yogthos@gmail.com>
1 parent e8af8c2 commit 273aada

5 files changed

Lines changed: 429 additions & 16 deletions

File tree

src/agent/runner.rs

Lines changed: 60 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,8 @@ pub struct AgentRunner {
123123
}
124124

125125
pub fn convert_history(session: &Session) -> Vec<Message> {
126+
use rig::OneOrMany;
127+
use rig::completion::message::AssistantContent;
126128
let (summary, first_kept) = session.compacted_context();
127129
let mut messages = Vec::new();
128130

@@ -136,8 +138,63 @@ pub fn convert_history(session: &Session) -> Vec<Message> {
136138
for msg in &session.messages[first_kept..] {
137139
match msg.role {
138140
MessageRole::User => messages.push(Message::user(msg.content.to_string())),
139-
MessageRole::Assistant => messages.push(Message::assistant(msg.content.to_string())),
140141
MessageRole::System => messages.push(Message::system(msg.content.to_string())),
142+
MessageRole::Assistant => {
143+
// Phase 3: if this assistant message has structured
144+
// tool calls, emit a single Assistant message with
145+
// text + tool_use content parts, followed by ONE
146+
// tool_result User message per call. The pairing
147+
// matches opencode's `toModelMessagesEffect`
148+
// (`message-v2.ts:630-899`); Anthropic + OpenAI
149+
// reject orphan tool_use blocks so we always emit a
150+
// result, marking Interrupted/Failed as error text
151+
// rather than skipping. Bare assistant messages
152+
// (no tool_calls) keep the prior simple shape.
153+
if msg.tool_calls.is_empty() {
154+
messages.push(Message::assistant(msg.content.to_string()));
155+
continue;
156+
}
157+
158+
// Build the Assistant message's content blocks: text
159+
// first (if any) then each ToolCall.
160+
let mut parts: Vec<AssistantContent> = Vec::new();
161+
if !msg.content.is_empty() {
162+
parts.push(AssistantContent::text(msg.content.to_string()));
163+
}
164+
for tc in &msg.tool_calls {
165+
parts.push(AssistantContent::tool_call(
166+
tc.id.clone(),
167+
tc.name.clone(),
168+
tc.args.clone(),
169+
));
170+
}
171+
// OneOrMany::many requires at least one element; we
172+
// always have at least one ToolCall here since
173+
// tool_calls is non-empty.
174+
let content = if parts.len() == 1 {
175+
OneOrMany::one(parts.pop().unwrap())
176+
} else {
177+
OneOrMany::many(parts).expect("non-empty parts vec")
178+
};
179+
messages.push(Message::Assistant { id: None, content });
180+
181+
// One User tool_result per call. State maps to:
182+
// Completed → result text verbatim
183+
// Interrupted → "[Tool execution was interrupted]"
184+
// Failed → "[Tool error: <message>]"
185+
for tc in &msg.tool_calls {
186+
let body = match &tc.state {
187+
crate::session::ToolCallState::Completed { result } => result.clone(),
188+
crate::session::ToolCallState::Interrupted => {
189+
"[Tool execution was interrupted]".to_string()
190+
}
191+
crate::session::ToolCallState::Failed { error } => {
192+
format!("[Tool error: {}]", error)
193+
}
194+
};
195+
messages.push(Message::tool_result(tc.id.clone(), body));
196+
}
197+
}
141198
}
142199
}
143200

@@ -217,6 +274,7 @@ where
217274
outcome.had_tool_calls = true;
218275
let _ = event_tx
219276
.send(AgentEvent::ToolCall {
277+
id: CompactString::from(tool_call.id),
220278
name: CompactString::from(tool_call.function.name),
221279
args: tool_call.function.arguments,
222280
})
@@ -238,6 +296,7 @@ where
238296
}
239297
let _ = event_tx
240298
.send(AgentEvent::ToolResult {
299+
id: CompactString::from(tool_result.id),
241300
output: CompactString::from(output),
242301
})
243302
.await;

src/event.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,20 @@ pub enum AgentEvent {
55
Token(CompactString),
66
Reasoning(CompactString),
77
ToolCall {
8+
/// Provider call id (rig's `ToolCall.id`). Empty for older
9+
/// rig versions or providers that don't emit one; the UI
10+
/// uses it to pair this call with the corresponding
11+
/// `ToolResult` event for structured persistence (Phase 3).
12+
id: CompactString,
813
name: CompactString,
914
args: serde_json::Value,
1015
},
1116
ToolResult {
17+
/// Matching call id from the `ToolCall` event. Empty if the
18+
/// provider didn't emit one — the UI falls back to
19+
/// positional pairing (this result belongs to the most-
20+
/// recent unanswered ToolCall in the same turn).
21+
id: CompactString,
1222
output: CompactString,
1323
},
1424
Error(CompactString),

src/extras/acp/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,7 @@ async fn run_prompt(
209209
);
210210
let _ = cx.send_notification(notif);
211211
}
212-
AgentEvent::ToolCall { name, args } => {
212+
AgentEvent::ToolCall { id: _, name, args } => {
213213
let args_str = args.to_string();
214214
let call_id = ToolCallId::new(uuid::Uuid::new_v4().to_string());
215215
last_tool_call_id = Some(call_id.clone());
@@ -221,7 +221,7 @@ async fn run_prompt(
221221
);
222222
let _ = cx.send_notification(notif);
223223
}
224-
AgentEvent::ToolResult { output } => {
224+
AgentEvent::ToolResult { id: _, output } => {
225225
// Use the most recent ToolCall id so the client can
226226
// correlate result → call. Falls back to an empty id
227227
// only if a stray ToolResult arrives without a prior

src/session/mod.rs

Lines changed: 212 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,48 @@ pub enum MessageRole {
1414
System,
1515
}
1616

17+
/// State of a tool call attached to an assistant message. Mirrors
18+
/// opencode's `ToolPart.state` (`message-v2.ts:310-320`). The point
19+
/// of preserving state — rather than just "this tool ran" — is so
20+
/// that resumed sessions can emit a paired tool_result block to the
21+
/// LLM even for tool calls that didn't complete (e.g. user hit
22+
/// Ctrl+C mid-execution). Anthropic + OpenAI reject orphan tool_use
23+
/// blocks; we always emit a result, even if its content is an
24+
/// interrupted marker.
25+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
26+
#[serde(tag = "kind", rename_all = "snake_case")]
27+
pub enum ToolCallState {
28+
/// Tool ran to completion. `result` is the output text the LLM
29+
/// would see (the same string the UI rendered in the chamber).
30+
Completed { result: String },
31+
/// Tool was dispatched but the agent was aborted before its
32+
/// result came back. Resumed sessions emit a tool_result with
33+
/// "[Tool execution was interrupted]" so the LLM knows the
34+
/// effect is undefined.
35+
Interrupted,
36+
/// Tool dispatched but the call errored (e.g. permission denied,
37+
/// runtime panic). `error` is the message the LLM saw.
38+
Failed { error: String },
39+
}
40+
41+
/// One tool invocation attached to an assistant message. We keep
42+
/// the original call id (rig's `ToolCall.id`) so resumed sessions
43+
/// emit tool_result blocks with the right correlation id.
44+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
45+
pub struct ToolCallEntry {
46+
/// Provider-supplied call id (e.g. `tooluse_abc123` for
47+
/// Anthropic, `call_xyz` for OpenAI). Used as the
48+
/// `tool_use_id` / `tool_call_id` correlation on resume.
49+
pub id: String,
50+
/// Tool name as the LLM saw it (`bash`, `read`, `mcp_tool:...`).
51+
pub name: String,
52+
/// Arguments the LLM sent. JSON value so it round-trips
53+
/// without re-parsing.
54+
pub args: serde_json::Value,
55+
/// Outcome — completed, interrupted, or failed.
56+
pub state: ToolCallState,
57+
}
58+
1759
#[derive(Debug, Clone, Serialize, Deserialize)]
1860
pub struct SessionMessage {
1961
pub role: MessageRole,
@@ -31,6 +73,16 @@ pub struct SessionMessage {
3173
/// chat messages with plugin entries by timestamp.
3274
#[serde(default)]
3375
pub timestamp: i64,
76+
/// Tool calls + results attached to this assistant message.
77+
/// Empty for User / System messages and for assistants that
78+
/// didn't invoke any tools. Phase 3 added persistence so
79+
/// resumed sessions re-emit structured tool_use/tool_result
80+
/// blocks to the LLM instead of only the assistant's text;
81+
/// previously the LLM lost all context of prior tool work on
82+
/// session resume. Defaulted on deserialize for back-compat
83+
/// with pre-Phase-3 session files.
84+
#[serde(default)]
85+
pub tool_calls: Vec<ToolCallEntry>,
3486
}
3587

3688
/// Generate a fresh message id. Extracted for `#[serde(default = ...)]`.
@@ -282,6 +334,21 @@ impl Session {
282334
}
283335

284336
pub fn add_message(&mut self, role: MessageRole, content: &str) {
337+
self.add_message_with_tool_calls(role, content, Vec::new());
338+
}
339+
340+
/// Same as `add_message` but attaches structured tool-call
341+
/// entries to the new message. Used by the runner to persist
342+
/// assistant turns that invoked tools so `convert_history`
343+
/// can re-emit structured tool_use/tool_result blocks on
344+
/// session resume. Empty `tool_calls` is equivalent to the
345+
/// plain `add_message`.
346+
pub fn add_message_with_tool_calls(
347+
&mut self,
348+
role: MessageRole,
349+
content: &str,
350+
tool_calls: Vec<ToolCallEntry>,
351+
) {
285352
// Make sure tree + store mirror any messages that were loaded
286353
// from a pre-P4b/P4c session file BEFORE we append the new
287354
// one — otherwise the rebuild would re-insert this new message
@@ -299,6 +366,7 @@ impl Session {
299366
estimated_tokens: tokens,
300367
id: id.clone(),
301368
timestamp,
369+
tool_calls,
302370
};
303371
self.messages.push(msg.clone());
304372
self.message_store.insert(id.clone(), msg);
@@ -597,6 +665,7 @@ impl Session {
597665
estimated_tokens: summary_tokens,
598666
id: summary_id.clone(),
599667
timestamp: summary_ts,
668+
tool_calls: Vec::new(),
600669
};
601670

602671
// Collect the IDs of the messages we're about to drop so we
@@ -1243,6 +1312,147 @@ mod tests {
12431312
assert_eq!(s.messages.len(), 1);
12441313
}
12451314

1315+
/// Phase 3 — tool calls round-trip through serde with default
1316+
/// for back-compat. Old session files without the field
1317+
/// deserialize into an empty Vec.
1318+
#[test]
1319+
fn session_message_tool_calls_default_when_field_missing() {
1320+
let json = r#"{
1321+
"role": "assistant",
1322+
"content": "Done.",
1323+
"estimated_tokens": 5
1324+
}"#;
1325+
let msg: SessionMessage = serde_json::from_str(json).unwrap();
1326+
assert!(
1327+
msg.tool_calls.is_empty(),
1328+
"missing field must default to []"
1329+
);
1330+
}
1331+
1332+
/// Round-trip: write a message WITH tool_calls, read back, fields intact.
1333+
#[test]
1334+
fn session_message_tool_calls_roundtrip() {
1335+
let mut s = Session::new("p", "m", 0);
1336+
let calls = vec![
1337+
ToolCallEntry {
1338+
id: "tc_1".to_string(),
1339+
name: "bash".to_string(),
1340+
args: serde_json::json!({"cmd": "ls"}),
1341+
state: ToolCallState::Completed {
1342+
result: "file1\nfile2".to_string(),
1343+
},
1344+
},
1345+
ToolCallEntry {
1346+
id: "tc_2".to_string(),
1347+
name: "read".to_string(),
1348+
args: serde_json::json!({"path": "/tmp/x"}),
1349+
state: ToolCallState::Interrupted,
1350+
},
1351+
];
1352+
s.add_message_with_tool_calls(MessageRole::Assistant, "Let me check.", calls.clone());
1353+
1354+
let blob = serde_json::to_string(&s).unwrap();
1355+
let s2: Session = serde_json::from_str(&blob).unwrap();
1356+
let last = s2.messages.last().unwrap();
1357+
assert_eq!(last.tool_calls.len(), 2);
1358+
assert_eq!(last.tool_calls[0].id, "tc_1");
1359+
assert!(matches!(
1360+
last.tool_calls[0].state,
1361+
ToolCallState::Completed { .. },
1362+
));
1363+
assert!(matches!(
1364+
last.tool_calls[1].state,
1365+
ToolCallState::Interrupted,
1366+
));
1367+
}
1368+
1369+
/// Convert history materializes prior tool calls as structured
1370+
/// rig Message blocks (Assistant with ToolCall content +
1371+
/// User with ToolResult content). Without this, resumed sessions
1372+
/// lose tool-call context and the LLM may re-call the same
1373+
/// tools. Matches opencode's `message-v2.ts:630-899` pattern.
1374+
#[test]
1375+
fn convert_history_emits_tool_use_and_tool_result_blocks() {
1376+
let mut s = Session::new("p", "m", 0);
1377+
s.add_message(MessageRole::User, "list files");
1378+
s.add_message_with_tool_calls(
1379+
MessageRole::Assistant,
1380+
"Here:",
1381+
vec![ToolCallEntry {
1382+
id: "tc_42".to_string(),
1383+
name: "bash".to_string(),
1384+
args: serde_json::json!({"cmd": "ls"}),
1385+
state: ToolCallState::Completed {
1386+
result: "a\nb".to_string(),
1387+
},
1388+
}],
1389+
);
1390+
1391+
let history = crate::agent::runner::convert_history(&s);
1392+
// Expect: User("list files"), Assistant(text + tool_use),
1393+
// User(tool_result). 3 messages total.
1394+
assert_eq!(history.len(), 3, "history shape: {:#?}", history);
1395+
1396+
// Last is a User with tool_result content carrying the id.
1397+
match &history[2] {
1398+
rig::completion::Message::User { content } => {
1399+
let s = format!("{:?}", content);
1400+
assert!(s.contains("tc_42"), "tool_result missing call id: {s}");
1401+
// Debug format escapes newlines, so check the
1402+
// escaped form. The underlying string still has the
1403+
// real newline; this is just an assertion-side
1404+
// formatting consideration.
1405+
assert!(
1406+
s.contains("a\\nb") || s.contains("a\nb"),
1407+
"tool_result missing output: {s}",
1408+
);
1409+
}
1410+
other => panic!("expected User tool_result message; got {other:?}"),
1411+
}
1412+
1413+
// Middle is Assistant with both text and a ToolCall.
1414+
match &history[1] {
1415+
rig::completion::Message::Assistant { content, .. } => {
1416+
let s = format!("{:?}", content);
1417+
assert!(s.contains("tc_42"), "tool_use missing id: {s}");
1418+
assert!(s.contains("\"bash\""), "tool_use missing name: {s}");
1419+
}
1420+
other => panic!("expected Assistant message; got {other:?}"),
1421+
}
1422+
}
1423+
1424+
/// Interrupted tool calls must be emitted as tool_result with
1425+
/// an "[interrupted]" marker, NOT skipped. Anthropic + OpenAI
1426+
/// reject orphan tool_use blocks; opencode handles this
1427+
/// (`message-v2.ts:848-857`) by emitting an error tool_result.
1428+
#[test]
1429+
fn convert_history_pairs_interrupted_tool_calls_with_error_marker() {
1430+
let mut s = Session::new("p", "m", 0);
1431+
s.add_message_with_tool_calls(
1432+
MessageRole::Assistant,
1433+
"About to bash...",
1434+
vec![ToolCallEntry {
1435+
id: "tc_99".to_string(),
1436+
name: "bash".to_string(),
1437+
args: serde_json::json!({"cmd": "sleep 9999"}),
1438+
state: ToolCallState::Interrupted,
1439+
}],
1440+
);
1441+
1442+
let history = crate::agent::runner::convert_history(&s);
1443+
// 2 messages: Assistant(text + tool_use) + User(tool_result-interrupted).
1444+
assert_eq!(history.len(), 2);
1445+
let last_str = format!("{:?}", &history[1]);
1446+
assert!(
1447+
last_str.contains("tc_99"),
1448+
"interrupted result must reference call id: {last_str}",
1449+
);
1450+
assert!(
1451+
last_str.contains("interrupted") || last_str.contains("Interrupted"),
1452+
"interrupted result must say so: {last_str}",
1453+
);
1454+
}
1455+
12461456
/// Phase 2 — compress drops a parent that has a sibling branch
12471457
/// underneath. The sibling subtree must also be pruned;
12481458
/// otherwise its nodes have `parent` pointing at a removed id
@@ -1289,6 +1499,7 @@ mod tests {
12891499
estimated_tokens: 1,
12901500
id: sib1_id.clone(),
12911501
timestamp: 0,
1502+
tool_calls: Vec::new(),
12921503
},
12931504
);
12941505
s.message_store.insert(
@@ -1299,6 +1510,7 @@ mod tests {
12991510
estimated_tokens: 1,
13001511
id: sib2_id.clone(),
13011512
timestamp: 0,
1513+
tool_calls: Vec::new(),
13021514
},
13031515
);
13041516

0 commit comments

Comments
 (0)