Prevent duplicate command monitor timers
This commit is contained in:
@@ -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_RETRY_INTERVAL: Duration = Duration::from_millis(500);
|
||||||
const COMMAND_MONITOR_FORCE_REFRESH_RETRIES: u8 = 20;
|
const COMMAND_MONITOR_FORCE_REFRESH_RETRIES: u8 = 20;
|
||||||
|
|
||||||
|
fn register_command_monitor_start(
|
||||||
|
pending_starts: &mut HashSet<BlockId>,
|
||||||
|
block_id: &BlockId,
|
||||||
|
) -> bool {
|
||||||
|
pending_starts.insert(block_id.clone())
|
||||||
|
}
|
||||||
|
|
||||||
#[derive(Default)]
|
#[derive(Default)]
|
||||||
pub struct ControlMasterErrorBannerState {
|
pub struct ControlMasterErrorBannerState {
|
||||||
/// Whether or not the control master error banner is currently visible to
|
/// 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.
|
/// long-running threshold. Reset when the active command starts and finishes.
|
||||||
did_notify_long_running: bool,
|
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<BlockId>,
|
||||||
|
|
||||||
/// This field is an "&&" combination of two other pieces of state:
|
/// This field is an "&&" combination of two other pieces of state:
|
||||||
/// 1. Whether this View (or one of its children) is the focused View.
|
/// 1. Whether this View (or one of its children) is the focused View.
|
||||||
/// 2. Whether this View's window is the active window.
|
/// 2. Whether this View's window is the active window.
|
||||||
@@ -4347,6 +4358,7 @@ impl TerminalView {
|
|||||||
view_id: ctx.view_id(),
|
view_id: ctx.view_id(),
|
||||||
current_state: TerminalViewStateChange::default(),
|
current_state: TerminalViewStateChange::default(),
|
||||||
did_notify_long_running: false,
|
did_notify_long_running: false,
|
||||||
|
pending_command_monitor_starts: HashSet::new(),
|
||||||
is_focused_and_active: true,
|
is_focused_and_active: true,
|
||||||
current_prompt,
|
current_prompt,
|
||||||
model_event_sender,
|
model_event_sender,
|
||||||
@@ -6646,6 +6658,7 @@ impl TerminalView {
|
|||||||
conversation_id,
|
conversation_id,
|
||||||
initial_requested_command_action_id,
|
initial_requested_command_action_id,
|
||||||
} => {
|
} => {
|
||||||
|
self.pending_command_monitor_starts.remove(block_id);
|
||||||
let subagent_view = ctx.add_typed_action_view(|ctx| {
|
let subagent_view = ctx.add_typed_action_view(|ctx| {
|
||||||
CLISubagentView::new(
|
CLISubagentView::new(
|
||||||
block_id.clone(),
|
block_id.clone(),
|
||||||
@@ -6755,6 +6768,7 @@ impl TerminalView {
|
|||||||
conversation_id,
|
conversation_id,
|
||||||
..
|
..
|
||||||
} => {
|
} => {
|
||||||
|
self.pending_command_monitor_starts.remove(block_id);
|
||||||
self.cli_subagent_views.remove(block_id);
|
self.cli_subagent_views.remove(block_id);
|
||||||
|
|
||||||
// The command ended — drop any LRC-scoped auto-queue override so the
|
// 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<Self>) {
|
fn schedule_command_monitor_start(&mut self, block_id: BlockId, ctx: &mut ViewContext<Self>) {
|
||||||
|
if !register_command_monitor_start(&mut self.pending_command_monitor_starts, &block_id) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
self.schedule_command_monitor_start_after(
|
self.schedule_command_monitor_start_after(
|
||||||
block_id,
|
block_id,
|
||||||
COMMAND_AUTO_MONITOR_DELAY,
|
COMMAND_AUTO_MONITOR_DELAY,
|
||||||
@@ -7439,10 +7457,13 @@ impl TerminalView {
|
|||||||
ctx,
|
ctx,
|
||||||
);
|
);
|
||||||
} else if waiting_for_threshold {
|
} else if waiting_for_threshold {
|
||||||
|
me.pending_command_monitor_starts.remove(&block_id);
|
||||||
log::warn!(
|
log::warn!(
|
||||||
"Command block {block_id:?} never reached the long-running threshold; \
|
"Command block {block_id:?} never reached the long-running threshold; \
|
||||||
automatic command monitoring was not started"
|
automatic command monitoring was not started"
|
||||||
);
|
);
|
||||||
|
} else {
|
||||||
|
me.pending_command_monitor_starts.remove(&block_id);
|
||||||
}
|
}
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
@@ -7463,6 +7484,7 @@ impl TerminalView {
|
|||||||
ctx,
|
ctx,
|
||||||
);
|
);
|
||||||
} else {
|
} else {
|
||||||
|
me.pending_command_monitor_starts.remove(&block_id);
|
||||||
log::warn!(
|
log::warn!(
|
||||||
"Could not find the pending shell action for long-running block \
|
"Could not find the pending shell action for long-running block \
|
||||||
{block_id:?}; automatic command monitoring was not started"
|
{block_id:?}; automatic command monitoring was not started"
|
||||||
@@ -11719,6 +11741,7 @@ impl TerminalView {
|
|||||||
ctx.request_user_attention();
|
ctx.request_user_attention();
|
||||||
}
|
}
|
||||||
ModelEvent::Exit { reason } => {
|
ModelEvent::Exit { reason } => {
|
||||||
|
self.pending_command_monitor_starts.clear();
|
||||||
if !self.manual_pty_shutdown_requested {
|
if !self.manual_pty_shutdown_requested {
|
||||||
if let Some(conversation_id) = self.maybe_send_agent_exited_shell_telemetry(ctx)
|
if let Some(conversation_id) = self.maybe_send_agent_exited_shell_telemetry(ctx)
|
||||||
{
|
{
|
||||||
@@ -11764,6 +11787,8 @@ impl TerminalView {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
ModelEvent::BlockCompleted(block_completed_event) => {
|
ModelEvent::BlockCompleted(block_completed_event) => {
|
||||||
|
self.pending_command_monitor_starts
|
||||||
|
.remove(&block_completed_event.block_id);
|
||||||
record_trace_event!("command_execution:block_completed");
|
record_trace_event!("command_execution:block_completed");
|
||||||
end_trace_after_next!("window:redraw:end");
|
end_trace_after_next!("window:redraw:end");
|
||||||
let block_completed_event_clone = block_completed_event.clone();
|
let block_completed_event_clone = block_completed_event.clone();
|
||||||
|
|||||||
@@ -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]
|
#[test]
|
||||||
fn agent_view_lifecycle_updates_input_mode() {
|
fn agent_view_lifecycle_updates_input_mode() {
|
||||||
App::test((), |mut app| async move {
|
App::test((), |mut app| async move {
|
||||||
|
|||||||
@@ -168,11 +168,12 @@ Files:
|
|||||||
|
|
||||||
Tasks:
|
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.
|
- 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.
|
- Capture command metadata and snapshots without holding `TerminalModel` locks across async work.
|
||||||
- Add cancellation/completion cleanup paths.
|
- [x] Add cancellation/completion cleanup paths.
|
||||||
- Add focused unit tests for timer race cases and duplicate monitor prevention.
|
- [ ] 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
|
### Phase 3: Side-agent conversation creation
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user