From fbbf90e2fb9881922642738ced64335782602014 Mon Sep 17 00:00:00 2001 From: Ryan Ward Date: Thu, 6 Aug 2026 16:17:38 -0500 Subject: [PATCH] Prevent duplicate command monitor timers --- app/src/terminal/view.rs | 25 ++++++++++++++++++++++++ app/src/terminal/view_tests.rs | 21 ++++++++++++++++++++ plans/long-running-command-side-agent.md | 7 ++++--- 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index 7c46969d..4dbb731e 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -734,6 +734,13 @@ const COMMAND_AUTO_MONITOR_DELAY: Duration = Duration::from_secs(3); const COMMAND_MONITOR_RETRY_INTERVAL: Duration = Duration::from_millis(500); const COMMAND_MONITOR_FORCE_REFRESH_RETRIES: u8 = 20; +fn register_command_monitor_start( + pending_starts: &mut HashSet, + block_id: &BlockId, +) -> bool { + pending_starts.insert(block_id.clone()) +} + #[derive(Default)] pub struct ControlMasterErrorBannerState { /// Whether or not the control master error banner is currently visible to @@ -2668,6 +2675,10 @@ pub struct TerminalView { /// long-running threshold. Reset when the active command starts and finishes. did_notify_long_running: bool, + /// Blocks with an automatic monitor-start timer already scheduled. Keeping this registry on + /// the terminal view prevents duplicate command-start events from creating competing timers. + pending_command_monitor_starts: HashSet, + /// This field is an "&&" combination of two other pieces of state: /// 1. Whether this View (or one of its children) is the focused View. /// 2. Whether this View's window is the active window. @@ -4347,6 +4358,7 @@ impl TerminalView { view_id: ctx.view_id(), current_state: TerminalViewStateChange::default(), did_notify_long_running: false, + pending_command_monitor_starts: HashSet::new(), is_focused_and_active: true, current_prompt, model_event_sender, @@ -6646,6 +6658,7 @@ impl TerminalView { conversation_id, initial_requested_command_action_id, } => { + self.pending_command_monitor_starts.remove(block_id); let subagent_view = ctx.add_typed_action_view(|ctx| { CLISubagentView::new( block_id.clone(), @@ -6755,6 +6768,7 @@ impl TerminalView { conversation_id, .. } => { + self.pending_command_monitor_starts.remove(block_id); self.cli_subagent_views.remove(block_id); // The command ended — drop any LRC-scoped auto-queue override so the @@ -7398,6 +7412,10 @@ impl TerminalView { } fn schedule_command_monitor_start(&mut self, block_id: BlockId, ctx: &mut ViewContext) { + if !register_command_monitor_start(&mut self.pending_command_monitor_starts, &block_id) { + return; + } + self.schedule_command_monitor_start_after( block_id, COMMAND_AUTO_MONITOR_DELAY, @@ -7439,10 +7457,13 @@ impl TerminalView { ctx, ); } else if waiting_for_threshold { + me.pending_command_monitor_starts.remove(&block_id); log::warn!( "Command block {block_id:?} never reached the long-running threshold; \ automatic command monitoring was not started" ); + } else { + me.pending_command_monitor_starts.remove(&block_id); } return; } @@ -7463,6 +7484,7 @@ impl TerminalView { ctx, ); } else { + me.pending_command_monitor_starts.remove(&block_id); log::warn!( "Could not find the pending shell action for long-running block \ {block_id:?}; automatic command monitoring was not started" @@ -11719,6 +11741,7 @@ impl TerminalView { ctx.request_user_attention(); } ModelEvent::Exit { reason } => { + self.pending_command_monitor_starts.clear(); if !self.manual_pty_shutdown_requested { if let Some(conversation_id) = self.maybe_send_agent_exited_shell_telemetry(ctx) { @@ -11764,6 +11787,8 @@ impl TerminalView { } } ModelEvent::BlockCompleted(block_completed_event) => { + self.pending_command_monitor_starts + .remove(&block_completed_event.block_id); record_trace_event!("command_execution:block_completed"); end_trace_after_next!("window:redraw:end"); let block_completed_event_clone = block_completed_event.clone(); diff --git a/app/src/terminal/view_tests.rs b/app/src/terminal/view_tests.rs index b1e0a198..8e784c65 100644 --- a/app/src/terminal/view_tests.rs +++ b/app/src/terminal/view_tests.rs @@ -106,6 +106,27 @@ fn has_pending_user_query_block(view: &TerminalView) -> bool { }) } +#[test] +fn automatic_command_monitor_start_is_deduplicated_per_block() { + let mut pending_starts = HashSet::new(); + let block_id = BlockId::new(); + + assert!(super::register_command_monitor_start( + &mut pending_starts, + &block_id + )); + assert!(!super::register_command_monitor_start( + &mut pending_starts, + &block_id + )); + + pending_starts.remove(&block_id); + assert!(super::register_command_monitor_start( + &mut pending_starts, + &block_id + )); +} + #[test] fn agent_view_lifecycle_updates_input_mode() { App::test((), |mut app| async move { diff --git a/plans/long-running-command-side-agent.md b/plans/long-running-command-side-agent.md index 2d34f4db..ba5ef6bc 100644 --- a/plans/long-running-command-side-agent.md +++ b/plans/long-running-command-side-agent.md @@ -168,11 +168,12 @@ Files: Tasks: -- Add a terminal-view-scoped monitor registry keyed by command/block ID. +- [x] Add a terminal-view-scoped monitor registry keyed by command/block ID. - Replace the current one-off delayed input-lock check with a monitor-start event or extend the existing event with a long-running transition. - Capture command metadata and snapshots without holding `TerminalModel` locks across async work. -- Add cancellation/completion cleanup paths. -- Add focused unit tests for timer race cases and duplicate monitor prevention. +- [x] Add cancellation/completion cleanup paths. +- [ ] Add focused unit tests for timer race cases. +- [x] Add deterministic unit coverage for duplicate monitor registration and cleanup. ### Phase 3: Side-agent conversation creation