From 63c8f36dddce74a3a0c59a1126965ea50eaed251 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 8 Feb 2026 15:10:39 +0000 Subject: [PATCH] Fix panic when sub-agent tool completes after primary agent's turn ends The ToolEvent::Completed handler was unconditionally draining injectable notifications and calling finish_turn()/begin_turn() on the transcript, even for sub-agent tool completions. When the primary agent had already finished its turn, this created an orphaned active turn that nobody would finish, causing a panic ("Cannot begin turn: previous turn not finished") on the next user submit. Guard the notification injection behind an is_primary check so only the primary agent's tool completions manipulate the transcript turn lifecycle. https://claude.ai/code/session_01RimYSQyprjibcxqX8XBmrD --- src/app.rs | 60 +++++++++++++++++++++----------------- src/transcript.rs | 73 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 26 deletions(-) diff --git a/src/app.rs b/src/app.rs index 4165e59..91c9711 100644 --- a/src/app.rs +++ b/src/app.rs @@ -911,7 +911,7 @@ 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) { @@ -919,34 +919,42 @@ impl App { 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::>() + .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::>() - .join("\n\n"); - format!("{}\n\n{}", content, xml) } else { content }; diff --git a/src/transcript.rs b/src/transcript.rs index efc74c3..4e13a64 100644 --- a/src/transcript.rs +++ b/src/transcript.rs @@ -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(); + } }