Fix agent cancellation and monitor UI
This commit is contained in:
@@ -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<SpawnedFutureHandle>,
|
||||
|
||||
@@ -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<Self>) {
|
||||
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,9 +489,40 @@ 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.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>,
|
||||
) {
|
||||
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,
|
||||
@@ -498,7 +530,7 @@ impl BlocklistAIStatusBar {
|
||||
ctx,
|
||||
);
|
||||
});
|
||||
}
|
||||
ctx.notify();
|
||||
}
|
||||
|
||||
pub fn notify_and_notify_children(&mut self, ctx: &mut ViewContext<Self>) {
|
||||
@@ -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();
|
||||
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,21 +635,9 @@ 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();
|
||||
}
|
||||
}
|
||||
|
||||
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<Self>) {
|
||||
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<AIConversationId>,
|
||||
ctx: &mut ViewContext<Self>,
|
||||
) {
|
||||
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,24 +712,29 @@ impl BlocklistAIStatusBar {
|
||||
ctx,
|
||||
);
|
||||
});
|
||||
return;
|
||||
}
|
||||
} else {
|
||||
let Some(conversation_id) = model.conversation_id(ctx) else {
|
||||
}
|
||||
|
||||
let Some(conversation_id) = conversation_id else {
|
||||
return;
|
||||
};
|
||||
let Some(output) = model.status(ctx).output_to_render() else {
|
||||
return;
|
||||
};
|
||||
let actions = output
|
||||
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::<HashSet<_>>();
|
||||
.collect::<HashSet<_>>()
|
||||
});
|
||||
if let Some(active_action_id) = self
|
||||
.action_model
|
||||
.as_ref(ctx)
|
||||
.get_pending_or_running_action_id(ctx)
|
||||
.filter(|id| actions.contains(id))
|
||||
.filter(|id| actions.as_ref().is_some_and(|actions| actions.contains(id)))
|
||||
.cloned()
|
||||
{
|
||||
self.action_model.update(ctx, |action_model, ctx| {
|
||||
@@ -702,13 +745,9 @@ impl BlocklistAIStatusBar {
|
||||
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.
|
||||
} else {
|
||||
// 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,
|
||||
@@ -718,7 +757,6 @@ impl BlocklistAIStatusBar {
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Starts the periodic timer that updates the summarization UI while summarization is active.
|
||||
fn start_summarization_timer(&mut self, ctx: &mut ViewContext<Self>) {
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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,34 +7527,42 @@ 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() {
|
||||
// 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(|c| c.status().is_in_progress())
|
||||
.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)
|
||||
{
|
||||
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,
|
||||
);
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/// Finalizes a conversation as a terminal failure because an agent-issued
|
||||
/// command caused the shell process to exit (e.g. it ran `exit`, or ran a
|
||||
|
||||
@@ -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<Self>) {
|
||||
@@ -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);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user