From 0ffd1181d1bce8896cde5d1920a32454d9929d2f Mon Sep 17 00:00:00 2001 From: Ryan Ward Date: Tue, 18 Aug 2026 16:22:05 -0500 Subject: [PATCH] Fix agent cancellation and monitor UI --- app/src/ai/blocklist/block/status_bar.rs | 202 ++++++++++++++--------- app/src/ai/blocklist/controller.rs | 59 ++++--- app/src/terminal/view.rs | 39 ++++- app/src/terminal/view_tests.rs | 53 ++++++ 4 files changed, 243 insertions(+), 110 deletions(-) diff --git a/app/src/ai/blocklist/block/status_bar.rs b/app/src/ai/blocklist/block/status_bar.rs index 533e949a..5db16301 100644 --- a/app/src/ai/blocklist/block/status_bar.rs +++ b/app/src/ai/blocklist/block/status_bar.rs @@ -125,6 +125,7 @@ pub struct BlocklistAIStatusBar { /// The random loading message chosen for the current conversation run, stable across /// re-renders and tool-call follow-up exchanges. warping_message: Option<&'static str>, + warping_message_ticks: u8, /// Handle for the periodic timer that updates the warping elapsed timer UI. warping_timer_handle: Option, @@ -319,7 +320,7 @@ impl BlocklistAIStatusBar { &summarization_cancel_dialog, |me, _, event, ctx| match event { SummarizationCancelDialogEvent::ConfirmCancel => { - me.cancel_active_request_or_action(ctx); + me.cancel_active_request_or_action(None, ctx); me.close_summarization_cancel_dialog(ctx); } SummarizationCancelDialogEvent::Continue => { @@ -431,6 +432,7 @@ impl BlocklistAIStatusBar { last_read_refresh_handle: None, warping_start_time: None, warping_message: None, + warping_message_ticks: 0, warping_timer_handle: None, ambient_agent_view_model, current_tip: None, @@ -451,18 +453,17 @@ impl BlocklistAIStatusBar { } pub fn handle_ctrl_c(&mut self, ctx: &mut ViewContext) { - let Some(model) = self.active_exchange_model.as_ref() else { - return; - }; - // Show confirmation dialog if summarization is active and feature flag enabled if FeatureFlag::SummarizationCancellationConfirmation.is_enabled() - && model.is_conversation_summarization_active(ctx) + && self + .active_exchange_model + .as_ref() + .is_some_and(|model| model.is_conversation_summarization_active(ctx)) { // If the dialog is already open, treat Ctrl-C as confirm-cancel if self.is_summarization_cancel_dialog_open { // Confirm cancel the running/pending action/request, then close the dialog overlay. - self.cancel_active_request_or_action(ctx); + self.cancel_active_request_or_action(None, ctx); self.close_summarization_cancel_dialog(ctx); return; } @@ -472,7 +473,7 @@ impl BlocklistAIStatusBar { } // If summarization isn't active, fall back to canceling the running request/action directly - self.cancel_active_request_or_action(ctx); + self.cancel_active_request_or_action(None, ctx); ctx.notify(); } @@ -488,19 +489,50 @@ impl BlocklistAIStatusBar { .as_ref() .and_then(|model| model.conversation_id(ctx)) == Some(conversation_id); - if active_exchange_matches { + if active_exchange_matches + && FeatureFlag::SummarizationCancellationConfirmation.is_enabled() + && self + .active_exchange_model + .as_ref() + .is_some_and(|model| model.is_conversation_summarization_active(ctx)) + { self.handle_ctrl_c(ctx); } else { - self.controller.update(ctx, |controller, ctx| { - controller.cancel_conversation_progress( - conversation_id, - CancellationReason::ManuallyCancelled, - ctx, - ); - }); + self.cancel_active_request_or_action(Some(conversation_id), ctx); } } + /// Cancels a conversation by identity and immediately clears status-bar activity UI. + /// This path is used by the terminal-level Stop/Ctrl+C handler because the status bar's + /// rendered exchange may already be stale while provider work is still running. + pub fn cancel_conversation_for_terminal( + &mut self, + conversation_id: AIConversationId, + ctx: &mut ViewContext, + ) { + self.is_summarization_cancel_dialog_open = false; + self.stop_summarization_timer(); + self.stop_warping_timer(); + self.stop_last_read_timer(); + if self.active_exchange_model.as_ref().is_some_and(|model| { + model + .conversation(ctx) + .is_some_and(|conversation| conversation.id() == conversation_id) + }) { + self.active_exchange_model = None; + } + self.latest_response_stream_id = None; + self.agent_message_bar.update(ctx, |_, ctx| ctx.notify()); + self.controller.update(ctx, |controller, ctx| { + controller.cancel_conversation_progress( + conversation_id, + CancellationReason::ManuallyCancelled, + ctx, + ); + }); + ctx.notify(); + } + pub fn notify_and_notify_children(&mut self, ctx: &mut ViewContext) { ctx.notify(); self.agent_message_bar.update(ctx, |_, ctx| ctx.notify()); @@ -516,6 +548,9 @@ impl BlocklistAIStatusBar { let conversation = history_model.conversation(&conversation_id); let exchange = conversation.and_then(|conversation| conversation.exchange_with_id(exchange_id)); + let conversation_is_in_progress = conversation + .as_ref() + .is_some_and(|conversation| conversation.status().is_in_progress()); if self.active_exchange_model.as_ref().is_none_or(|model| { model.exchange_id(ctx).is_none_or(|id| id != exchange_id) @@ -549,7 +584,11 @@ impl BlocklistAIStatusBar { }); self.is_summarization_cancel_dialog_open = false; self.stop_summarization_timer(); - self.start_warping_timer(ctx); + if conversation_is_in_progress { + self.start_warping_timer(ctx); + } else { + self.stop_warping_timer(); + } if FeatureFlag::AgentTips.is_enabled() { self.update_agent_tip(ctx); @@ -596,20 +635,8 @@ impl BlocklistAIStatusBar { AIBlockOutputStatus::Pending | AIBlockOutputStatus::Failed { .. } => (), } - // An exchange finishing does not necessarily mean the logical run is complete: tool calls - // can execute and append a follow-up exchange. Conversation status is the authoritative - // lifecycle signal, so preserve the timer and phrase while it remains in progress. - // Placed after the match so `model` (which borrows self.active_exchange_model) is no longer - // used, avoiding borrow conflicts with &mut self. if is_finished { - let conversation_is_in_progress = self - .active_exchange_model - .as_ref() - .and_then(|model| model.conversation(ctx)) - .is_some_and(|conversation| conversation.status().is_in_progress()); - if !conversation_is_in_progress { - self.stop_warping_timer(); - } + self.stop_warping_timer(); } ctx.notify(); @@ -661,11 +688,22 @@ impl BlocklistAIStatusBar { /// Cancels either the in-flight request stream or a pending/running action if present. /// If neither is found but the conversation is still in progress (e.g., a subagent is running), /// cancels the entire conversation's progress. - fn cancel_active_request_or_action(&mut self, ctx: &mut ViewContext) { - let Some(model) = self.active_exchange_model.as_ref() else { - return; - }; - if model.status(ctx).is_streaming() { + fn cancel_active_request_or_action( + &mut self, + conversation_id: Option, + ctx: &mut ViewContext, + ) { + let model_conversation_id = self + .active_exchange_model + .as_ref() + .and_then(|model| model.conversation_id(ctx)); + let conversation_id = conversation_id.or(model_conversation_id); + + if self + .active_exchange_model + .as_ref() + .is_some_and(|model| model.status(ctx).is_streaming()) + { if let Some(response_stream_id) = self.latest_response_stream_id.as_ref() { self.controller.update(ctx, |controller, ctx| { controller.cancel_request( @@ -674,49 +712,49 @@ impl BlocklistAIStatusBar { ctx, ); }); + return; } + } + + let Some(conversation_id) = conversation_id else { + return; + }; + let actions = self + .active_exchange_model + .as_ref() + .and_then(|model| model.status(ctx).output_to_render()) + .map(|output| { + output + .get() + .actions() + .map(|action| action.id.clone()) + .collect::>() + }); + if let Some(active_action_id) = self + .action_model + .as_ref(ctx) + .get_pending_or_running_action_id(ctx) + .filter(|id| actions.as_ref().is_some_and(|actions| actions.contains(id))) + .cloned() + { + self.action_model.update(ctx, |action_model, ctx| { + action_model.cancel_action_with_id( + conversation_id, + &active_action_id, + CancellationReason::ManuallyCancelled, + ctx, + ); + }); } else { - let Some(conversation_id) = model.conversation_id(ctx) else { - return; - }; - let Some(output) = model.status(ctx).output_to_render() else { - return; - }; - let actions = output - .get() - .actions() - .map(|action| action.id.clone()) - .collect::>(); - if let Some(active_action_id) = self - .action_model - .as_ref(ctx) - .get_pending_or_running_action_id(ctx) - .filter(|id| actions.contains(id)) - .cloned() - { - self.action_model.update(ctx, |action_model, ctx| { - action_model.cancel_action_with_id( - conversation_id, - &active_action_id, - CancellationReason::ManuallyCancelled, - ctx, - ); - }); - } else if model - .conversation(ctx) - .is_some_and(|c| c.status().is_in_progress()) - { - // No streaming request or pending action, but conversation is still in progress. - // This happens when a subagent (e.g., computer use or advice) is running. - // Cancel the entire conversation's progress. - self.controller.update(ctx, |controller, ctx| { - controller.cancel_conversation_progress( - conversation_id, - CancellationReason::ManuallyCancelled, - ctx, - ); - }); - } + // The exchange can be complete or absent while a provider run/subagent remains + // active. Cancel by conversation identity so neither UI model is authoritative. + self.controller.update(ctx, |controller, ctx| { + controller.cancel_conversation_progress( + conversation_id, + CancellationReason::ManuallyCancelled, + ctx, + ); + }); } } @@ -764,6 +802,7 @@ impl BlocklistAIStatusBar { self.warping_start_time.get_or_insert_with(Instant::now); self.warping_message .get_or_insert_with(random_load_output_message); + self.warping_message_ticks = 0; // Don't start a new timer if one is already running if self.warping_timer_handle.is_some() { return; @@ -775,6 +814,11 @@ impl BlocklistAIStatusBar { |me, _, ctx| { me.warping_timer_handle = None; if me.warping_start_time.is_some() { + me.warping_message_ticks = me.warping_message_ticks.saturating_add(1); + if me.warping_message_ticks >= 5 { + me.warping_message_ticks = 0; + me.warping_message = Some(random_load_output_message()); + } ctx.notify(); me.restart_warping_timer(ctx); } @@ -795,6 +839,11 @@ impl BlocklistAIStatusBar { |me, _, ctx| { me.warping_timer_handle = None; if me.warping_start_time.is_some() { + me.warping_message_ticks = me.warping_message_ticks.saturating_add(1); + if me.warping_message_ticks >= 5 { + me.warping_message_ticks = 0; + me.warping_message = Some(random_load_output_message()); + } ctx.notify(); me.restart_warping_timer(ctx); } @@ -807,6 +856,7 @@ impl BlocklistAIStatusBar { fn stop_warping_timer(&mut self) { self.warping_start_time = None; self.warping_message = None; + self.warping_message_ticks = 0; if let Some(handle) = self.warping_timer_handle.take() { handle.abort(); } diff --git a/app/src/ai/blocklist/controller.rs b/app/src/ai/blocklist/controller.rs index 61eac0cc..1687b39c 100644 --- a/app/src/ai/blocklist/controller.rs +++ b/app/src/ai/blocklist/controller.rs @@ -7285,7 +7285,8 @@ impl BlocklistAIController { .try_cancel_stream(stream_id, reason, ctx) } - pub(super) fn has_active_provider_run(&self, conversation_id: AIConversationId) -> bool { + /// Returns whether the durable provider lifecycle still owns work for this conversation. + pub fn has_active_provider_run(&self, conversation_id: AIConversationId) -> bool { self.active_provider_runs.contains_key(&conversation_id) } @@ -7526,31 +7527,39 @@ impl BlocklistAIController { action_model.cancel_all_pending_actions(conversation_id, Some(reason), ctx); }); self.set_input_mode_for_cancellation(ctx); + } - // Force-cancel all streaming exchanges and set the conversation status to - // Cancelled. This handles the case where a query gets stuck (e.g. a subagent - // hangs or the stream ended unexpectedly without proper cleanup) and repeated - // Ctrl+C presses cannot resolve it. Without this, the conversation remains - // InProgress indefinitely, hiding the input box and blocking user interaction. - if !reason.is_follow_up_for_same_conversation() { - let history_model = BlocklistAIHistoryModel::handle(ctx); - if history_model - .as_ref(ctx) - .conversation(&conversation_id) - .is_some_and(|c| c.status().is_in_progress()) - { - let terminal_view_id = self.terminal_surface_id; - history_model.update(ctx, |history_model, ctx| { - if let Some(conversation) = history_model.conversation_mut(&conversation_id) - { - conversation.force_cancel_all_streaming_exchanges( - terminal_view_id, - reason, - ctx, - ); - } - }); - } + // Cancellation must immediately leave the conversation in a terminal UI state even when + // a provider run or response stream was found above. Those paths finish asynchronously; + // waiting for their callbacks leaves the input in "Steer the running agent" mode. + if !reason.is_follow_up_for_same_conversation() + && matches!( + reason.conversation_outcome(), + CancellationOutcome::Cancelled + ) + { + let history_model = BlocklistAIHistoryModel::handle(ctx); + if history_model + .as_ref(ctx) + .conversation(&conversation_id) + .is_some_and(|conversation| conversation.status().is_in_progress()) + { + let terminal_view_id = self.terminal_surface_id; + history_model.update(ctx, |history_model, ctx| { + if let Some(conversation) = history_model.conversation_mut(&conversation_id) { + conversation.force_cancel_all_streaming_exchanges( + terminal_view_id, + reason, + ctx, + ); + } + history_model.update_conversation_status( + terminal_view_id, + conversation_id, + ConversationStatus::Cancelled, + ctx, + ); + }); } } } diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index 2a98f089..e63f39ba 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -3839,7 +3839,7 @@ impl TerminalView { }); ctx.emit(Event::SummarizationCancelDialogToggled { is_open: *is_open }); } - BlocklistAIStatusBarEvent::Stop => me.ctrl_c(ctx), + BlocklistAIStatusBarEvent::Stop => me.cancel_active_conversation_via_status_bar(ctx), }); if let Some(ambient_agent_view_model) = ambient_agent_view_model.as_ref() { ctx.subscribe_to_model(ambient_agent_view_model, |me, _, event, ctx| { @@ -8679,6 +8679,10 @@ impl TerminalView { .ai_controller .as_ref(ctx) .has_active_stream_for_conversation(conversation_id, ctx); + let had_active_provider_run = self + .ai_controller + .as_ref(ctx) + .has_active_provider_run(conversation_id); self.ai_controller.update(ctx, |controller, ctx| { controller.cancel_conversation_progress( @@ -8688,6 +8692,12 @@ impl TerminalView { ); }); + if !had_active_stream && !had_active_provider_run { + log::debug!( + "Stop agent requested for {conversation_id:?}, but no provider run or response stream was visible" + ); + } + let visible_conversation_id = self .agent_view_controller .as_ref(ctx) @@ -8767,8 +8777,13 @@ impl TerminalView { active_block.is_active_and_long_running() && active_block.ai_conversation_id() == Some(conversation_id) }; + let provider_run_is_active = self + .ai_controller + .as_ref(ctx) + .has_active_provider_run(conversation_id); - (conversation_has_progress || command_is_monitored).then_some(conversation_id) + (conversation_has_progress || command_is_monitored || provider_run_is_active) + .then_some(conversation_id) } fn user_write_ctrl_c_to_pty(&mut self, ctx: &mut ViewContext) { @@ -9081,13 +9096,19 @@ impl TerminalView { .active_conversation(self.view_id) .map(|conversation| conversation.id()) }); - let status_bar = self.input.as_ref(ctx).agent_status_bar().clone(); - status_bar.update(ctx, |status_bar, ctx| { - if let Some(conversation_id) = conversation_id { - status_bar.handle_ctrl_c_for_conversation(conversation_id, ctx); - } else { - status_bar.handle_ctrl_c(ctx); - } + let Some(conversation_id) = conversation_id else { + log::debug!("Stop agent requested, but AgentView has no active conversation identity"); + return; + }; + // The status bar model can be stale or absent after the latest exchange completes. Clear + // its activity UI immediately, then cancel through the controller's authoritative identity. + self.input.update(ctx, |input, ctx| { + input.agent_status_bar().update(ctx, |status_bar, ctx| { + status_bar.cancel_conversation_for_terminal(conversation_id, ctx); + }); + // The cancellation synchronously updates conversation status in the history model; + // refresh the placeholder after that transition so it cannot remain on "Steer...". + input.set_zero_state_hint_text(ctx); }); } diff --git a/app/src/terminal/view_tests.rs b/app/src/terminal/view_tests.rs index da5b30c0..2e65b13a 100644 --- a/app/src/terminal/view_tests.rs +++ b/app/src/terminal/view_tests.rs @@ -30,6 +30,7 @@ use crate::ai::blocklist::agent_view::{ ExitAgentViewError, }; use crate::ai::blocklist::block::cli_controller::UserTakeOverReason; +use crate::ai::blocklist::block::status_bar::BlocklistAIStatusBarAction; use crate::ai::blocklist::{ BlocklistAIHistoryEvent, BlocklistAIHistoryModel, InputConfig, InputType, ResponseStream, ResponseStreamId, @@ -5646,6 +5647,58 @@ fn ctrl_c_after_stop_takeover_cancels_conversation() { }) } +#[test] +fn status_bar_stop_cancels_conversation_after_exchange_finishes() { + App::test((), |mut app| async move { + initialize_app_for_terminal_view(&mut app); + FeatureFlag::AgentView.set_enabled(true); + let terminal = add_window_with_terminal(&mut app, None); + + let conversation_id = terminal.update(&mut app, |view, ctx| { + let conversation_id = + BlocklistAIHistoryModel::handle(ctx).update(ctx, |history, ctx| { + history.start_new_conversation(view.view_id, false, false, false, ctx) + }); + let stream_id = ResponseStreamId::new_for_test(); + BlocklistAIHistoryModel::handle(ctx).update(ctx, |history, ctx| { + history + .conversation_mut(&conversation_id) + .expect("conversation should exist") + .append_reassigned_exchange( + &stream_id, + exchange_with_inputs(vec![]), + view.view_id, + ctx, + ) + .expect("exchange should append"); + }); + let stream = ctx.add_model(|_| ResponseStream::new_for_test(stream_id.clone())); + view.ai_controller.update(ctx, |controller, ctx| { + controller.register_mock_stream_for_test(stream_id, conversation_id, stream, ctx); + }); + conversation_id + }); + + terminal.update(&mut app, |view, ctx| { + view.input.update(ctx, |input, ctx| { + input.agent_status_bar().update(ctx, |status_bar, ctx| { + status_bar.handle_action(&BlocklistAIStatusBarAction::Stop, ctx); + }); + }); + }); + + terminal.read(&app, |_, ctx| { + assert_eq!( + BlocklistAIHistoryModel::as_ref(ctx) + .conversation(&conversation_id) + .expect("conversation should exist") + .status(), + &ConversationStatus::Cancelled + ); + }); + }) +} + #[test] fn ctrl_c_after_transfer_takeover_does_not_cancel_conversation() { App::test((), |mut app| async move {