From a3c38c0f0bd956f6b1e4493706e68d3839e92f17 Mon Sep 17 00:00:00 2001 From: Ryan Ward Date: Thu, 6 Aug 2026 16:22:07 -0500 Subject: [PATCH] Cover command monitor timer states --- app/src/terminal/view.rs | 106 ++++++++++++++--------- app/src/terminal/view_tests.rs | 20 +++++ plans/long-running-command-side-agent.md | 2 +- 3 files changed, 88 insertions(+), 40 deletions(-) diff --git a/app/src/terminal/view.rs b/app/src/terminal/view.rs index 4dbb731e..d9131424 100644 --- a/app/src/terminal/view.rs +++ b/app/src/terminal/view.rs @@ -741,6 +741,31 @@ fn register_command_monitor_start( pending_starts.insert(block_id.clone()) } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CommandMonitorStartStatus { + AlreadyMonitoring, + Ready, + WaitingForThreshold, + NoLongerEligible, +} + +fn command_monitor_start_status( + is_agent_monitoring: bool, + is_active_and_long_running: bool, + is_executing: bool, + is_command_grid_active: bool, +) -> CommandMonitorStartStatus { + if is_agent_monitoring { + CommandMonitorStartStatus::AlreadyMonitoring + } else if is_active_and_long_running { + CommandMonitorStartStatus::Ready + } else if is_executing || is_command_grid_active { + CommandMonitorStartStatus::WaitingForThreshold + } else { + CommandMonitorStartStatus::NoLongerEligible + } +} + #[derive(Default)] pub struct ControlMasterErrorBannerState { /// Whether or not the control master error banner is currently visible to @@ -7432,63 +7457,66 @@ impl TerminalView { ctx: &mut ViewContext, ) { ctx.spawn(Timer::after(delay), move |me, _, ctx| { - let (needs_monitor, waiting_for_threshold) = { + let status = { let model = me.model.lock(); let Some(block) = model.block_list().block_with_id(&block_id) else { return; }; - if block.is_agent_monitoring() { - (false, false) - } else if block.is_active_and_long_running() { - (true, false) - } else { - ( - false, - block.is_executing() || block.is_command_grid_active(), - ) - } + command_monitor_start_status( + block.is_agent_monitoring(), + block.is_active_and_long_running(), + block.is_executing(), + block.is_command_grid_active(), + ) }; - if !needs_monitor { - if waiting_for_threshold && remaining_force_refresh_retries > 0 { + + match status { + CommandMonitorStartStatus::WaitingForThreshold + if remaining_force_refresh_retries > 0 => + { me.schedule_command_monitor_start_after( block_id, COMMAND_MONITOR_RETRY_INTERVAL, remaining_force_refresh_retries - 1, ctx, ); - } else if waiting_for_threshold { + } + CommandMonitorStartStatus::WaitingForThreshold => { 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 { + } + CommandMonitorStartStatus::AlreadyMonitoring + | CommandMonitorStartStatus::NoLongerEligible => { me.pending_command_monitor_starts.remove(&block_id); } - return; - } - - let refresh_requested = me.cli_subagent_controller.update(ctx, |controller, ctx| { - controller.request_force_refresh(&block_id, ctx) - }); - if refresh_requested { - log::info!( - "Requested an immediate command snapshot to start monitoring block \ - {block_id:?}" - ); - } else if remaining_force_refresh_retries > 0 { - me.schedule_command_monitor_start_after( - block_id, - COMMAND_MONITOR_RETRY_INTERVAL, - remaining_force_refresh_retries - 1, - 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" - ); + CommandMonitorStartStatus::Ready => { + let refresh_requested = + me.cli_subagent_controller.update(ctx, |controller, ctx| { + controller.request_force_refresh(&block_id, ctx) + }); + if refresh_requested { + log::info!( + "Requested an immediate command snapshot to start monitoring block \ + {block_id:?}" + ); + } else if remaining_force_refresh_retries > 0 { + me.schedule_command_monitor_start_after( + block_id, + COMMAND_MONITOR_RETRY_INTERVAL, + remaining_force_refresh_retries - 1, + 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" + ); + } + } } }); } diff --git a/app/src/terminal/view_tests.rs b/app/src/terminal/view_tests.rs index 8e784c65..a9fc1512 100644 --- a/app/src/terminal/view_tests.rs +++ b/app/src/terminal/view_tests.rs @@ -127,6 +127,26 @@ fn automatic_command_monitor_start_is_deduplicated_per_block() { )); } +#[test] +fn command_monitor_start_status_handles_timer_races() { + assert_eq!( + super::command_monitor_start_status(true, true, true, true), + super::CommandMonitorStartStatus::AlreadyMonitoring + ); + assert_eq!( + super::command_monitor_start_status(false, false, true, false), + super::CommandMonitorStartStatus::WaitingForThreshold + ); + assert_eq!( + super::command_monitor_start_status(false, true, false, false), + super::CommandMonitorStartStatus::Ready + ); + assert_eq!( + super::command_monitor_start_status(false, false, false, false), + super::CommandMonitorStartStatus::NoLongerEligible + ); +} + #[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 ba5ef6bc..2a1ae479 100644 --- a/plans/long-running-command-side-agent.md +++ b/plans/long-running-command-side-agent.md @@ -172,7 +172,7 @@ Tasks: - 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. - [x] Add cancellation/completion cleanup paths. -- [ ] Add focused unit tests for timer race cases. +- [x] 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