Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
60 changes: 34 additions & 26 deletions src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -911,42 +911,50 @@ impl App {
call_id,
content,
} => {
let _is_primary = self.agents.primary_id() == Some(agent_id);
let is_primary = self.agents.primary_id() == Some(agent_id);

// Update block status (works for both primary and sub-agent tools)
if let Some(block) = self.chat.transcript.find_tool_block_mut(&call_id) {
block.append_text(&content);
block.set_status(Status::Complete);
}

// Drain injectable notifications and append to content
let injectable = self.notifications.drain_injectable();
let content = if !injectable.is_empty() {
// Finish current assistant turn, add user turn, start new assistant turn
self.chat.transcript.finish_turn();

// Add user turn with notification blocks
let turn_id = self.chat.transcript.add_empty(Role::User);
for notification in &injectable {
let block_id = notification.block_id();
if let Some(mut block) = self.chat.transcript.stage.remove(block_id) {
block.set_status(Status::Complete);
if let Some(turn) = self.chat.transcript.get_mut(turn_id) {
turn.add_block(block);
// Drain injectable notifications and append to content.
// Only inject into the primary agent's tool results - sub-agent tool
// completions don't own a transcript turn, so manipulating the turn
// lifecycle here would create an orphaned turn and panic on the next
// begin_turn() call.
let content = if is_primary {
let injectable = self.notifications.drain_injectable();
if !injectable.is_empty() {
// Finish current assistant turn, add user turn, start new assistant turn
self.chat.transcript.finish_turn();

// Add user turn with notification blocks
let turn_id = self.chat.transcript.add_empty(Role::User);
for notification in &injectable {
let block_id = notification.block_id();
if let Some(mut block) = self.chat.transcript.stage.remove(block_id) {
block.set_status(Status::Complete);
if let Some(turn) = self.chat.transcript.get_mut(turn_id) {
turn.add_block(block);
}
}
}

// Start new assistant turn for continuation
self.chat.begin_turn(Role::Assistant, &mut self.terminal);

// Generate XML for injection
let xml = injectable
.iter()
.filter_map(|n| n.to_xml())
.collect::<Vec<_>>()
.join("\n\n");
format!("{}\n\n{}", content, xml)
} else {
content
}

// Start new assistant turn for continuation
self.chat.begin_turn(Role::Assistant, &mut self.terminal);

// Generate XML for injection
let xml = injectable
.iter()
.filter_map(|n| n.to_xml())
.collect::<Vec<_>>()
.join("\n\n");
format!("{}\n\n{}", content, xml)
} else {
content
};
Expand Down
73 changes: 73 additions & 0 deletions src/transcript.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1550,4 +1550,77 @@ mod tests {
let drained = stage.drain_all();
assert!(drained.is_empty());
}

// ========================================================================
// Turn lifecycle: sub-agent tool completion must not corrupt turn state
// ========================================================================

/// Verifies that begin_turn panics when called with an already-active turn.
/// This is the invariant that was violated before the fix in handle_tool_event:
/// sub-agent tool completions with injectable notifications would call
/// begin_turn() creating an orphaned turn, then the next user submit would
/// call begin_turn() again and hit this panic.
#[test]
#[should_panic(expected = "Cannot begin turn: previous turn not finished")]
fn test_begin_turn_panics_if_previous_not_finished() {
let mut transcript = Transcript::with_path(std::path::PathBuf::from("/tmp/test_double_begin.md"));

transcript.begin_turn(Role::Assistant);
// Second begin_turn without finishing first -> panic
transcript.begin_turn(Role::Assistant);
}

/// Simulates the correct lifecycle when a sub-agent's tool completes
/// after the primary agent has finished its turn. The fix in
/// handle_tool_event guards notification injection behind is_primary,
/// so sub-agent tool completions skip the finish_turn/begin_turn calls
/// that would corrupt the transcript turn state.
#[test]
fn test_spawn_agent_turn_lifecycle_no_orphan() {
let mut transcript = Transcript::with_path(std::path::PathBuf::from("/tmp/test_spawn_ok.md"));

// User sends message
transcript.add_turn(Role::User, TextBlock::new("Spawn an agent to help me"));

// Primary agent's assistant turn begins
transcript.begin_turn(Role::Assistant);
transcript.stream_delta(BlockType::Text, "I'll spawn an agent.");
let tool_block = ToolBlock::new(
"call_spawn_1",
"mcp_spawn_agent",
serde_json::json!({"task": "review code"}),
false,
);
transcript.start_block(Box::new(tool_block));

// Primary agent finishes its turn
transcript.finish_turn();

// Sub-agent runs in the background. Its tool completes, but since
// is_primary is false, the handler does NOT call finish_turn/begin_turn.
// (No transcript manipulation for sub-agent tool completions.)

// Later, user submits next message - this should work without panic
transcript.add_turn(Role::User, TextBlock::new("What did the agent find?"));
transcript.begin_turn(Role::Assistant);
transcript.stream_delta(BlockType::Text, "The agent found...");
transcript.finish_turn();

// Verify transcript integrity
assert_eq!(transcript.turns().len(), 4); // user, assistant, user, assistant
}

/// Verifies that finish_turn is safe to call when no turn is active.
#[test]
fn test_finish_turn_without_active_turn_is_safe() {
let mut transcript = Transcript::with_path(std::path::PathBuf::from("/tmp/test_finish_safe.md"));

// finish_turn with no active turn should not panic
transcript.finish_turn();

// Should still work normally after
transcript.begin_turn(Role::Assistant);
transcript.stream_delta(BlockType::Text, "Hello");
transcript.finish_turn();
}
}