diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 428c676b7f..ab75ec4d4d 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -195,6 +195,7 @@ use super::skill_popup::SkillPopup; use super::slash_commands::BuiltinCommandFlags; use super::slash_commands::ServiceTierCommand; use super::slash_commands::SlashCommandItem; +use super::task_activity::TaskActivity; use crate::bottom_pane::paste_burst::FlushResult; use crate::key_hint::KeyBindingListExt; use crate::keymap::EditorKeymap; @@ -359,7 +360,7 @@ pub(crate) struct ChatComposer { frame_requester: Option, attachments: AttachmentState, placeholder_text: String, - is_task_running: bool, + task_activity: TaskActivity, queue_submissions: bool, /// Slash-command draft staged for local recall after application-level dispatch. /// @@ -527,7 +528,7 @@ impl ChatComposer { frame_requester: None, attachments: AttachmentState::default(), placeholder_text, - is_task_running: false, + task_activity: TaskActivity::default(), queue_submissions: false, pending_slash_command_history: None, skills: None, @@ -1763,7 +1764,7 @@ impl ChatComposer { return (InputResult::None, true); } if key_event.code == KeyCode::Esc { - let next_mode = esc_hint_mode(self.footer.mode, self.is_task_running); + let next_mode = esc_hint_mode(self.footer.mode, self.task_activity.is_busy()); if next_mode != self.footer.mode { self.footer.mode = next_mode; return (InputResult::None, true); @@ -2904,10 +2905,13 @@ impl ChatComposer { self.record_pending_slash_command_history(); return Some(InputResult::None); } - let preserve_review_draft = self.is_task_running - && matches!(command, SlashCommandItem::Builtin(SlashCommand::Review)); + let defer_draft_clear = self.task_activity.is_busy() + && matches!( + command, + SlashCommandItem::Builtin(cmd) if cmd.available_during_background_task_only() + ); self.stage_slash_command_history(&command); - if !preserve_review_draft { + if !defer_draft_clear { self.draft.textarea.set_text_clearing_elements(""); self.draft.is_bash_mode = false; } @@ -2972,7 +2976,14 @@ impl ChatComposer { } fn reject_slash_command_if_unavailable(&self, command: &SlashCommandItem) -> bool { - if !self.is_task_running || command.available_during_task() { + let command_available = if self.task_activity.foreground_task_running() { + command.available_during_task() + } else if self.task_activity.mcp_startup_running() { + command.available_during_mcp_startup() + } else { + true + }; + if command_available { return false; } let message = format!( @@ -3098,7 +3109,7 @@ impl ChatComposer { } if key_event.code == KeyCode::Esc { if self.is_empty() { - let next_mode = esc_hint_mode(self.footer.mode, self.is_task_running); + let next_mode = esc_hint_mode(self.footer.mode, self.task_activity.is_busy()); if next_mode != self.footer.mode { self.footer.mode = next_mode; return (InputResult::None, true); @@ -3108,9 +3119,11 @@ impl ChatComposer { self.footer.mode = reset_mode_after_activity(self.footer.mode); } if self.queue_keys.is_pressed(key_event) - && (self.is_task_running || self.queue_submissions || !self.is_bang_shell_command()) + && (self.task_activity.is_busy() + || self.queue_submissions + || !self.is_bang_shell_command()) { - return self.handle_submission(self.is_task_running || self.queue_submissions); + return self.handle_submission(self.task_activity.is_busy() || self.queue_submissions); } if self.submit_keys.is_pressed(key_event) { @@ -3434,7 +3447,7 @@ impl ChatComposer { mode, esc_backtrack_hint: self.footer.esc_backtrack_hint, use_shift_enter_hint: self.footer.use_shift_enter_hint, - is_task_running: self.is_task_running, + is_task_running: self.task_activity.is_busy(), queue_submissions: self.queue_submissions, quit_shortcut_key: self.footer.quit_shortcut_key, collaboration_modes_enabled: self.collaboration_modes_enabled, @@ -3887,8 +3900,13 @@ impl ChatComposer { self.footer.flash = None; } + #[cfg(test)] pub fn set_task_running(&mut self, running: bool) { - self.is_task_running = running; + self.task_activity.set_foreground_task_running(running); + } + + pub(super) fn set_task_activity(&mut self, task_activity: TaskActivity) { + self.task_activity = task_activity; } pub(crate) fn set_queue_submissions(&mut self, queue_submissions: bool) { @@ -3908,7 +3926,7 @@ impl ChatComposer { pub(crate) fn set_esc_backtrack_hint(&mut self, show: bool) { self.footer.esc_backtrack_hint = show; if show { - self.footer.mode = esc_hint_mode(self.footer.mode, self.is_task_running); + self.footer.mode = esc_hint_mode(self.footer.mode, self.task_activity.is_busy()); } else { self.footer.mode = reset_mode_after_activity(self.footer.mode); } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs b/codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs index 37e10d048d..2ee0fde599 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs @@ -215,7 +215,7 @@ impl ChatComposer { return (InputResult::None, true); } if key_event.code == KeyCode::Esc { - let next_mode = esc_hint_mode(self.footer.mode, self.is_task_running); + let next_mode = esc_hint_mode(self.footer.mode, self.task_activity.is_busy()); if next_mode != self.footer.mode { self.footer.mode = next_mode; return (InputResult::None, true); @@ -301,7 +301,7 @@ impl ChatComposer { return (InputResult::None, true); } } - if self.is_task_running { + if self.task_activity.is_busy() { return self.handle_submission(/*should_queue*/ true); } (InputResult::None, true) @@ -360,9 +360,12 @@ impl ChatComposer { } self.stage_selected_slash_command_history(&sel); - let preserve_review_draft = self.is_task_running - && matches!(sel, CommandItem::Builtin(SlashCommand::Review)); - if !preserve_review_draft { + let defer_draft_clear = self.task_activity.is_busy() + && matches!( + sel, + CommandItem::Builtin(cmd) if cmd.available_during_background_task_only() + ); + if !defer_draft_clear { self.draft.textarea.set_text_clearing_elements(""); self.draft.is_bash_mode = false; } diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index ad30dedf5a..bee2dd77f7 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -59,6 +59,7 @@ mod request_user_input; mod status_line_setup; mod status_line_style; mod status_surface_preview; +mod task_activity; mod title_setup; pub(crate) use action_required_title::ACTION_REQUIRED_PREVIEW_PREFIX; pub(crate) use action_required_title::build_action_required_title_text; @@ -73,6 +74,7 @@ pub(crate) use mcp_server_elicitation::McpServerElicitationFormRequest; pub(crate) use mcp_server_elicitation::McpServerElicitationOverlay; pub(crate) use request_user_input::RequestUserInputOverlay; pub(crate) use status_line_style::status_line_from_segments; +use task_activity::TaskActivity; mod bottom_pane_view; #[derive(Clone, Debug, PartialEq, Eq)] @@ -222,7 +224,7 @@ pub(crate) struct BottomPane { has_input_focus: bool, enhanced_keys_supported: bool, disable_paste_burst: bool, - is_task_running: bool, + task_activity: TaskActivity, esc_backtrack_hint: bool, animations_enabled: bool, @@ -287,7 +289,7 @@ impl BottomPane { has_input_focus, enhanced_keys_supported, disable_paste_burst, - is_task_running: false, + task_activity: TaskActivity::default(), status: None, unified_exec_footer: UnifiedExecFooter::new(), pending_input_preview: PendingInputPreview::new(), @@ -990,9 +992,22 @@ impl BottomPane { // esc_backtrack_hint_visible removed; hints are controlled internally. pub fn set_task_running(&mut self, running: bool) { - let was_running = self.is_task_running; - self.is_task_running = running; - self.composer.set_task_running(running); + let mut task_activity = self.task_activity; + task_activity.set_foreground_task_running(running); + self.set_task_activity(task_activity); + } + + pub(crate) fn set_mcp_startup_running(&mut self, running: bool) { + let mut task_activity = self.task_activity; + task_activity.set_mcp_startup_running(running); + self.set_task_activity(task_activity); + } + + fn set_task_activity(&mut self, task_activity: TaskActivity) { + let was_running = self.task_activity.is_busy(); + let running = task_activity.is_busy(); + self.task_activity = task_activity; + self.composer.set_task_activity(task_activity); if running { if !was_running { @@ -1294,7 +1309,15 @@ impl BottomPane { } pub(crate) fn is_task_running(&self) -> bool { - self.is_task_running + self.task_activity.is_busy() + } + + pub(crate) fn is_foreground_task_running(&self) -> bool { + self.task_activity.foreground_task_running() + } + + pub(crate) fn is_mcp_startup_running(&self) -> bool { + self.task_activity.mcp_startup_running() } pub(crate) fn should_interrupt_running_task(&self, key_event: KeyEvent) -> bool { @@ -1306,7 +1329,7 @@ impl BottomPane { .is_some_and(|(name, _, _)| name == "agent"); self.keymap.chat.interrupt_turn.is_pressed(key_event) - && self.is_task_running + && self.task_activity.is_busy() && !(is_agent_command && key_event.code == KeyCode::Esc) && self.no_modal_or_popup_active() && !self.composer_should_handle_vim_insert_escape(key_event) @@ -1323,7 +1346,7 @@ impl BottomPane { } pub(crate) fn active_view_will_interrupt_turn_on_key_event(&self, key_event: KeyEvent) -> bool { - self.is_task_running + self.task_activity.is_busy() && self .active_view() .is_some_and(|view| view.will_interrupt_turn_on_key_event(key_event)) @@ -1338,7 +1361,7 @@ impl BottomPane { /// overlays or popups and not running a task. This is the safe context to /// use Esc-Esc for backtracking from the main view. pub(crate) fn is_normal_backtrack_mode(&self) -> bool { - !self.is_task_running && self.view_stack.is_empty() && !self.composer.popup_active() + !self.task_activity.is_busy() && self.view_stack.is_empty() && !self.composer.popup_active() } /// Return true when no popups or modal views are active, regardless of task state. diff --git a/codex-rs/tui/src/bottom_pane/slash_commands.rs b/codex-rs/tui/src/bottom_pane/slash_commands.rs index 6ca1ba93d1..7e96ec66e7 100644 --- a/codex-rs/tui/src/bottom_pane/slash_commands.rs +++ b/codex-rs/tui/src/bottom_pane/slash_commands.rs @@ -51,6 +51,13 @@ impl SlashCommandItem { Self::ServiceTier(_) => true, } } + + pub(crate) fn available_during_mcp_startup(&self) -> bool { + match self { + Self::Builtin(cmd) => cmd.available_during_mcp_startup(), + Self::ServiceTier(_) => true, + } + } } #[derive(Clone, Copy, Debug, Default)] diff --git a/codex-rs/tui/src/bottom_pane/task_activity.rs b/codex-rs/tui/src/bottom_pane/task_activity.rs new file mode 100644 index 0000000000..16a52c6199 --- /dev/null +++ b/codex-rs/tui/src/bottom_pane/task_activity.rs @@ -0,0 +1,27 @@ +#[derive(Clone, Copy, Debug, Default, PartialEq, Eq)] +pub(crate) struct TaskActivity { + foreground_task_running: bool, + mcp_startup_running: bool, +} + +impl TaskActivity { + pub(crate) fn is_busy(self) -> bool { + self.foreground_task_running || self.mcp_startup_running + } + + pub(crate) fn foreground_task_running(self) -> bool { + self.foreground_task_running + } + + pub(crate) fn mcp_startup_running(self) -> bool { + self.mcp_startup_running + } + + pub(crate) fn set_foreground_task_running(&mut self, running: bool) { + self.foreground_task_running = running; + } + + pub(crate) fn set_mcp_startup_running(&mut self, running: bool) { + self.mcp_startup_running = running; + } +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index a44d88be47..47e7ad61a4 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -16,11 +16,9 @@ //! output is time-dependent so the overlay can refresh its cached tail without rebuilding it on //! every draw. //! -//! The bottom pane exposes a single "task running" indicator that drives the spinner and interrupt -//! hints. This module treats that indicator as derived UI-busy state: it is set while an agent turn -//! is in progress and while MCP server startup is in progress. Those lifecycles are tracked -//! independently (`agent_turn_running` and `mcp_startup_status`) and synchronized via -//! `update_task_running_state`. +//! The bottom pane tracks foreground task activity separately from background MCP startup while +//! rendering both through the same busy indicator. This lets command availability distinguish +//! work that blocks a new foreground task from startup that can continue behind it. //! //! For preamble-capable models, assistant output may include commentary before //! the final answer. During streaming we hide the status row to avoid duplicate @@ -596,8 +594,6 @@ pub(crate) struct ChatWidget { /// as "running" while this is populated, even if no agent turn is currently /// executing. mcp_startup_status: Option>, - /// Whether the active MCP startup round began while no other task was running. - mcp_startup_started_while_idle: bool, /// Expected MCP servers for the current startup round, seeded from enabled local config. mcp_startup_expected_servers: Option>, /// After startup settles, ignore stale updates until enough notifications confirm a new round. @@ -608,8 +604,6 @@ pub(crate) struct ChatWidget { mcp_startup_pending_next_round: HashMap, /// Tracks whether the buffered next round has seen any `Starting` update yet. mcp_startup_pending_next_round_saw_starting: bool, - /// Ignore startup notifications while a review that skipped MCP startup is active. - mcp_startup_updates_suppressed_for_review: bool, connectors: ConnectorsState, ide_context: IdeContextState, plugins_cache: PluginsCacheState, @@ -1244,11 +1238,10 @@ impl ChatWidget { } fn enter_review_mode_with_hint(&mut self, hint: String, from_replay: bool) { - self.skip_mcp_startup_for_review(); if self.review.pre_review_token_info.is_none() { self.review.pre_review_token_info = Some(self.token_info.clone()); } - if !from_replay && !self.bottom_pane.is_task_running() { + if !from_replay { self.bottom_pane.set_task_running(/*running*/ true); } self.review.is_review_mode = true; @@ -1257,19 +1250,11 @@ impl ChatWidget { self.request_redraw(); } - fn exit_review_mode_after_item(&mut self, render_source: ThreadItemRenderSource) { + fn exit_review_mode_after_item(&mut self) { self.flush_answer_stream_with_separator(); self.flush_interrupt_queue(); self.flush_active_cell(); self.review.is_review_mode = false; - self.mcp_startup_updates_suppressed_for_review = false; - if render_source.is_replay() { - self.mcp_startup_ignore_updates_until_next_start = false; - self.mcp_startup_allow_terminal_only_next_round = false; - self.mcp_startup_pending_next_round.clear(); - self.mcp_startup_pending_next_round_saw_starting = false; - self.mcp_startup_started_while_idle = false; - } self.restore_pre_review_token_info(); self.add_to_history(history_cell::new_review_status_line( "<< Code review finished >>".to_string(), @@ -1849,9 +1834,9 @@ impl ChatWidget { pub(crate) fn prepare_local_op_submission(&mut self, op: &AppCommand) { if op.is_review() && !self.turn_lifecycle.agent_turn_running { - self.skip_mcp_startup_for_review(); - if !self.bottom_pane.is_task_running() { - self.bottom_pane.set_task_running(/*running*/ true); + self.bottom_pane.set_task_running(/*running*/ true); + if self.status_header_is_mcp_startup_owned() { + self.set_status_header(String::from("Working")); } } if let AppCommand::Interrupt { behavior } = op diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index b08f948f2f..7fb6fe54e9 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -158,13 +158,11 @@ impl ChatWidget { task_complete_pending: false, unified_exec_processes: Vec::new(), mcp_startup_status: None, - mcp_startup_started_while_idle: false, mcp_startup_expected_servers: None, mcp_startup_ignore_updates_until_next_start: false, mcp_startup_allow_terminal_only_next_round: false, mcp_startup_pending_next_round: HashMap::new(), mcp_startup_pending_next_round_saw_starting: false, - mcp_startup_updates_suppressed_for_review: false, connectors: ConnectorsState::default(), ide_context: IdeContextState::default(), plugins_cache: PluginsCacheState::default(), diff --git a/codex-rs/tui/src/chatwidget/input_restore.rs b/codex-rs/tui/src/chatwidget/input_restore.rs index bcc4e956a7..8e4c85301c 100644 --- a/codex-rs/tui/src/chatwidget/input_restore.rs +++ b/codex-rs/tui/src/chatwidget/input_restore.rs @@ -439,7 +439,7 @@ impl ChatWidget { } self.turn_lifecycle .restore_running(self.turn_lifecycle.agent_turn_running, Instant::now()); - self.update_task_running_state(); + self.update_foreground_task_running_state(); if restored_task_running && !self.bottom_pane.is_task_running() { self.bottom_pane.set_task_running(/*running*/ true); self.refresh_status_surfaces(); diff --git a/codex-rs/tui/src/chatwidget/mcp_startup.rs b/codex-rs/tui/src/chatwidget/mcp_startup.rs index 92e2bae2ee..2d19a760a4 100644 --- a/codex-rs/tui/src/chatwidget/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/mcp_startup.rs @@ -38,11 +38,6 @@ impl ChatWidget { status: McpStartupStatus, complete_when_settled: bool, ) { - if self.mcp_startup_updates_suppressed_for_review { - return; - } - - let mcp_startup_was_inactive = self.mcp_startup_status.is_none(); let mut activated_pending_round = false; let startup_status = if self.mcp_startup_ignore_updates_until_next_start { // Ignore-mode buffers the next plausible round so stale post-finish @@ -107,11 +102,8 @@ impl ChatWidget { } } } - if mcp_startup_was_inactive { - self.mcp_startup_started_while_idle = !self.bottom_pane.is_task_running(); - } self.mcp_startup_status = Some(startup_status); - self.update_task_running_state(); + self.update_mcp_startup_running_state(); // App-server-backed startup completes when every expected server has // reported a non-Starting status. Lag handling can force an earlier @@ -142,46 +134,65 @@ impl ChatWidget { self.finish_mcp_startup(failed, cancelled); return; } - if let Some(current) = &self.mcp_startup_status { - // Otherwise keep the status header focused on the remaining - // in-progress servers for the active round. - let total = current.len(); - let mut starting: Vec<_> = current - .iter() - .filter_map(|(name, state)| { - if matches!(state, McpStartupStatus::Starting) { - Some(name) - } else { - None - } - }) - .collect(); - starting.sort(); - if let Some(first) = starting.first() { - let completed = total.saturating_sub(starting.len()); - let max_to_show = 3; - let mut to_show: Vec = starting - .iter() - .take(max_to_show) - .map(ToString::to_string) - .collect(); - if starting.len() > max_to_show { - to_show.push("…".to_string()); - } - let header = if total > 1 { - format!( - "{MCP_STARTUP_MULTI_HEADER_PREFIX} ({completed}/{total}): {}", - to_show.join(", ") - ) - } else { - format!("{MCP_STARTUP_SINGLE_HEADER_PREFIX} {first}") - }; - self.set_status_header(header); - } - } + self.refresh_mcp_startup_status_header(); self.request_redraw(); } + fn update_mcp_startup_running_state(&mut self) { + self.bottom_pane + .set_mcp_startup_running(self.mcp_startup_status.is_some()); + self.refresh_plan_mode_nudge(); + self.refresh_status_surfaces(); + } + + pub(super) fn refresh_mcp_startup_status_header(&mut self) { + if self.review.is_review_mode + || (self.bottom_pane.is_foreground_task_running() + && !self.turn_lifecycle.agent_turn_running) + { + return; + } + let Some(current) = &self.mcp_startup_status else { + return; + }; + + let total = current.len(); + let mut starting: Vec<_> = current + .iter() + .filter_map(|(name, state)| { + if matches!(state, McpStartupStatus::Starting) { + Some(name) + } else { + None + } + }) + .collect(); + starting.sort(); + let Some(first) = starting.first() else { + return; + }; + + let completed = total.saturating_sub(starting.len()); + let max_to_show = 3; + let mut to_show: Vec = starting + .iter() + .take(max_to_show) + .map(ToString::to_string) + .collect(); + if starting.len() > max_to_show { + to_show.push("…".to_string()); + } + let header = if total > 1 { + format!( + "{MCP_STARTUP_MULTI_HEADER_PREFIX} ({completed}/{total}): {}", + to_show.join(", ") + ) + } else { + format!("{MCP_STARTUP_SINGLE_HEADER_PREFIX} {first}") + }; + self.set_status_header(header); + } + pub(crate) fn set_mcp_startup_expected_servers(&mut self, server_names: I) where I: IntoIterator, @@ -206,12 +217,11 @@ impl ChatWidget { let mcp_startup_owned_status = self.status_header_is_mcp_startup_owned(); self.mcp_startup_status = None; - self.mcp_startup_started_while_idle = false; self.mcp_startup_ignore_updates_until_next_start = true; self.mcp_startup_allow_terminal_only_next_round = false; self.mcp_startup_pending_next_round.clear(); self.mcp_startup_pending_next_round_saw_starting = false; - self.update_task_running_state(); + self.update_mcp_startup_running_state(); if self.bottom_pane.is_task_running() && mcp_startup_owned_status { self.restore_reasoning_status_header(); } @@ -219,30 +229,7 @@ impl ChatWidget { self.request_redraw(); } - pub(super) fn skip_mcp_startup_for_review(&mut self) { - self.mcp_startup_updates_suppressed_for_review = true; - self.mcp_startup_ignore_updates_until_next_start = true; - self.mcp_startup_allow_terminal_only_next_round = false; - self.mcp_startup_pending_next_round.clear(); - self.mcp_startup_pending_next_round_saw_starting = false; - self.mcp_startup_started_while_idle = false; - if self.mcp_startup_status.is_none() { - return; - } - - let mcp_startup_owned_status = self.status_header_is_mcp_startup_owned(); - self.mcp_startup_status = None; - if mcp_startup_owned_status { - self.set_status_header(String::from("Working")); - } - self.request_redraw(); - } - pub(crate) fn finish_mcp_startup_after_lag(&mut self) { - if self.mcp_startup_updates_suppressed_for_review { - return; - } - if self.mcp_startup_ignore_updates_until_next_start { if self.mcp_startup_pending_next_round.is_empty() { self.mcp_startup_pending_next_round_saw_starting = false; diff --git a/codex-rs/tui/src/chatwidget/replay.rs b/codex-rs/tui/src/chatwidget/replay.rs index 1a0a1ec90c..3ab5b3486b 100644 --- a/codex-rs/tui/src/chatwidget/replay.rs +++ b/codex-rs/tui/src/chatwidget/replay.rs @@ -165,7 +165,7 @@ impl ChatWidget { } } ThreadItem::ExitedReviewMode { .. } => { - self.exit_review_mode_after_item(render_source); + self.exit_review_mode_after_item(); } ThreadItem::ContextCompaction { .. } => { self.add_info_message("Context compacted".to_string(), /*hint*/ None); diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index 63fbd0e3f6..e3fdddf977 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -45,11 +45,11 @@ impl ChatWidget { /// that staged entry after dispatch so slash-command recall follows the same "submitted input" /// rule as normal text. pub(super) fn handle_slash_command_dispatch(&mut self, cmd: SlashCommand) { - let clear_review_draft = cmd == SlashCommand::Review - && (!self.active_side_conversation || cmd.available_in_side_conversation()) - && !self.slash_command_blocked_by_active_task(SlashCommand::Review); + let clear_deferred_draft = cmd.available_during_background_task_only() + && !self.active_side_conversation + && !self.slash_command_blocked_by_active_task(cmd); self.dispatch_command(cmd); - if clear_review_draft { + if clear_deferred_draft { self.bottom_pane .set_composer_text(String::new(), Vec::new(), Vec::new()); } @@ -139,16 +139,17 @@ impl ChatWidget { } fn slash_command_blocked_by_active_task(&self, cmd: SlashCommand) -> bool { - if cmd == SlashCommand::Review { - let mcp_startup_is_only_task = self.mcp_startup_started_while_idle - && self.mcp_startup_status.is_some() - && !self.turn_lifecycle.agent_turn_running - && !self.input_queue.user_turn_pending_start - && !self.has_queued_follow_up_messages() - && !self.review.is_review_mode; - return self.bottom_pane.is_task_running() && !mcp_startup_is_only_task; - } - (!cmd.available_during_task() && self.bottom_pane.is_task_running()) + let foreground_work_active = self.bottom_pane.is_foreground_task_running() + || self.input_queue.user_turn_pending_start + || self.has_queued_follow_up_messages(); + let blocked_by_task = if foreground_work_active { + !cmd.available_during_task() + } else if self.bottom_pane.is_mcp_startup_running() { + !cmd.available_during_mcp_startup() + } else { + false + }; + blocked_by_task || (cmd == SlashCommand::Resume && (self.input_queue.user_turn_pending_start || self.turn_lifecycle.agent_turn_running)) @@ -167,9 +168,7 @@ impl ChatWidget { cmd.command() ); self.add_to_history(history_cell::new_error_event(message)); - if cmd != SlashCommand::Review { - self.bottom_pane.drain_pending_submission_state(); - } + self.bottom_pane.drain_pending_submission_state(); self.request_redraw(); return; } diff --git a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs index 3058a6b853..7456046656 100644 --- a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs +++ b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs @@ -27,6 +27,44 @@ fn notify_mcp_status_error(chat: &mut ChatWidget, name: &str, error: &str) { ); } +fn submit_bare_review(chat: &mut ChatWidget) { + chat.bottom_pane + .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); +} + +fn submit_review_with_args( + chat: &mut ChatWidget, + op_rx: &mut tokio::sync::mpsc::UnboundedReceiver, + instructions: &str, +) { + chat.dispatch_command_with_args(SlashCommand::Review, instructions.to_string(), Vec::new()); + assert_matches!( + op_rx.try_recv(), + Ok(Op::Review { + target: ReviewTarget::Custom { + instructions: actual + } + }) if actual == instructions + ); +} + +fn assert_bare_review_rejected( + chat: &mut ChatWidget, + rx: &mut tokio::sync::mpsc::UnboundedReceiver, + expected_error: &str, +) { + submit_bare_review(chat); + let error = drain_insert_history(rx) + .into_iter() + .map(|lines| lines_to_single_string(&lines)) + .collect::(); + assert!(error.contains(expected_error), "unexpected error: {error}"); + assert_eq!(chat.bottom_pane.composer_text(), "/review"); +} + #[tokio::test] async fn mcp_startup_ignores_status_for_other_thread() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -74,11 +112,7 @@ async fn review_command_during_mcp_startup_opens_popup_snapshot() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); - chat.bottom_pane - .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + submit_bare_review(&mut chat); assert!(chat.bottom_pane.composer_text().is_empty()); let popup = render_bottom_popup(&chat, /*width*/ 80); @@ -86,39 +120,25 @@ async fn review_command_during_mcp_startup_opens_popup_snapshot() { } #[tokio::test] -async fn review_with_args_during_mcp_startup_skips_local_startup_round() { +async fn review_with_args_keeps_mcp_startup_in_background() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); - chat.dispatch_command_with_args( - SlashCommand::Review, - "check regressions".to_string(), - Vec::new(), - ); - - assert_matches!( - op_rx.try_recv(), - Ok(Op::Review { - target: ReviewTarget::Custom { instructions } - }) if instructions == "check regressions" - ); - assert!(chat.mcp_startup_status.is_none()); + submit_review_with_args(&mut chat, &mut op_rx, "check regressions"); + assert!(chat.mcp_startup_status.is_some()); + assert!(chat.bottom_pane.is_foreground_task_running()); + assert!(chat.bottom_pane.is_mcp_startup_running()); assert!(chat.bottom_pane.is_task_running()); assert_eq!(chat.status_state.current_status.header, "Working"); } #[tokio::test] -async fn skipped_mcp_startup_updates_do_not_reopen_or_finish_review_task() { +async fn background_mcp_startup_completion_does_not_finish_review_task() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_mcp_startup_expected_servers(["alpha".to_string(), "beta".to_string()]); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); - chat.dispatch_command_with_args( - SlashCommand::Review, - "check regressions".to_string(), - Vec::new(), - ); - assert_matches!(op_rx.try_recv(), Ok(Op::Review { .. })); + submit_review_with_args(&mut chat, &mut op_rx, "check regressions"); notify_mcp_status(&mut chat, "beta", McpServerStartupState::Starting); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Ready); @@ -136,16 +156,11 @@ async fn skipped_mcp_startup_updates_do_not_reopen_or_finish_review_task() { } #[tokio::test] -async fn review_before_first_mcp_update_suppresses_startup_round() { +async fn mcp_startup_can_complete_after_review_submission() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_mcp_startup_expected_servers(["alpha".to_string()]); - chat.dispatch_command_with_args( - SlashCommand::Review, - "check regressions".to_string(), - Vec::new(), - ); - assert_matches!(op_rx.try_recv(), Ok(Op::Review { .. })); + submit_review_with_args(&mut chat, &mut op_rx, "check regressions"); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Ready); @@ -156,18 +171,15 @@ async fn review_before_first_mcp_update_suppresses_startup_round() { } #[tokio::test] -async fn failed_review_setup_reenables_mcp_startup_updates() { +async fn failed_review_setup_restores_background_mcp_status() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_mcp_startup_expected_servers(["alpha".to_string()]); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); - chat.dispatch_command_with_args( - SlashCommand::Review, - "review unrelated branch".to_string(), - Vec::new(), - ); - assert_matches!(op_rx.try_recv(), Ok(Op::Review { .. })); - assert!(chat.mcp_startup_status.is_none()); + submit_review_with_args(&mut chat, &mut op_rx, "review unrelated branch"); + assert!(chat.mcp_startup_status.is_some()); + assert!(chat.bottom_pane.is_foreground_task_running()); + assert!(chat.bottom_pane.is_mcp_startup_running()); handle_error( &mut chat, @@ -178,19 +190,22 @@ async fn failed_review_setup_reenables_mcp_startup_updates() { assert!(chat.mcp_startup_status.is_some()); assert!(chat.bottom_pane.is_task_running()); + assert!(!chat.bottom_pane.is_foreground_task_running()); + assert!(chat.bottom_pane.is_mcp_startup_running()); + assert!( + chat.status_state + .current_status + .header + .starts_with("Booting MCP server") + ); } #[tokio::test] -async fn late_partial_startup_round_after_failed_review_setup_stays_ignored() { +async fn partial_mcp_startup_continues_after_failed_review_setup() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_mcp_startup_expected_servers(["alpha".to_string(), "beta".to_string()]); - chat.dispatch_command_with_args( - SlashCommand::Review, - "review unrelated branch".to_string(), - Vec::new(), - ); - assert_matches!(op_rx.try_recv(), Ok(Op::Review { .. })); + submit_review_with_args(&mut chat, &mut op_rx, "review unrelated branch"); handle_error( &mut chat, @@ -201,8 +216,9 @@ async fn late_partial_startup_round_after_failed_review_setup_stays_ignored() { notify_mcp_status(&mut chat, "beta", McpServerStartupState::Ready); - assert!(chat.mcp_startup_status.is_none()); - assert!(!chat.bottom_pane.is_task_running()); + assert!(chat.mcp_startup_status.is_some()); + assert!(chat.bottom_pane.is_task_running()); + assert!(chat.bottom_pane.is_mcp_startup_running()); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); notify_mcp_status(&mut chat, "beta", McpServerStartupState::Starting); @@ -212,7 +228,7 @@ async fn late_partial_startup_round_after_failed_review_setup_stays_ignored() { } #[tokio::test] -async fn replayed_active_review_rearms_mcp_startup_suppression() { +async fn replayed_active_review_keeps_mcp_startup_in_background() { let (mut chat, _rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.set_mcp_startup_expected_servers(["alpha".to_string()]); @@ -220,7 +236,9 @@ async fn replayed_active_review_rearms_mcp_startup_suppression() { notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); assert!(chat.review.is_review_mode); - assert!(chat.mcp_startup_status.is_none()); + assert!(chat.mcp_startup_status.is_some()); + assert!(chat.bottom_pane.is_mcp_startup_running()); + assert!(!chat.bottom_pane.is_foreground_task_running()); handle_exited_review_mode(&mut chat); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); @@ -249,17 +267,12 @@ async fn completed_replayed_review_does_not_hide_fresh_mcp_round() { } #[tokio::test] -async fn lag_recovery_during_review_does_not_promote_skipped_terminal_round() { +async fn lag_recovery_after_failed_review_tracks_background_mcp_failure() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; chat.show_welcome_banner = false; chat.set_mcp_startup_expected_servers(["alpha".to_string(), "beta".to_string()]); - chat.dispatch_command_with_args( - SlashCommand::Review, - "review unrelated branch".to_string(), - Vec::new(), - ); - assert_matches!(op_rx.try_recv(), Ok(Op::Review { .. })); + submit_review_with_args(&mut chat, &mut op_rx, "review unrelated branch"); chat.finish_mcp_startup_after_lag(); handle_error( @@ -272,11 +285,15 @@ async fn lag_recovery_during_review_does_not_promote_skipped_terminal_round() { notify_mcp_status_error( &mut chat, "alpha", - "MCP client for `alpha` failed to start: stale failure", + "MCP client for `alpha` failed to start: handshake failed", ); notify_mcp_status(&mut chat, "beta", McpServerStartupState::Ready); - assert!(drain_insert_history(&mut rx).is_empty()); + let warning = drain_insert_history(&mut rx) + .into_iter() + .map(|lines| lines_to_single_string(&lines)) + .collect::(); + assert!(warning.contains("handshake failed")); assert!(chat.mcp_startup_status.is_none()); assert!(!chat.bottom_pane.is_task_running()); } @@ -286,18 +303,11 @@ async fn bare_review_submission_during_agent_turn_preserves_draft() { let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await; handle_turn_started(&mut chat, "turn-1"); - chat.bottom_pane - .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - - let error = drain_insert_history(&mut rx) - .into_iter() - .map(|lines| lines_to_single_string(&lines)) - .collect::(); - assert!(error.contains("'/review' is disabled while a task is in progress.")); - assert_eq!(chat.bottom_pane.composer_text(), "/review"); + assert_bare_review_rejected( + &mut chat, + &mut rx, + "'/review' is disabled while a task is in progress.", + ); } #[tokio::test] @@ -306,18 +316,11 @@ async fn queued_prompt_blocks_review_during_mcp_startup() { notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); chat.queue_user_message("queued prompt".into()); - chat.bottom_pane - .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - - let error = drain_insert_history(&mut rx) - .into_iter() - .map(|lines| lines_to_single_string(&lines)) - .collect::(); - assert!(error.contains("'/review' is disabled while a task is in progress.")); - assert_eq!(chat.bottom_pane.composer_text(), "/review"); + assert_bare_review_rejected( + &mut chat, + &mut rx, + "'/review' is disabled while a task is in progress.", + ); } #[tokio::test] @@ -328,18 +331,11 @@ async fn compact_task_blocks_review_when_mcp_startup_arrives() { assert_matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Compact))); notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); - chat.bottom_pane - .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - - let error = drain_insert_history(&mut rx) - .into_iter() - .map(|lines| lines_to_single_string(&lines)) - .collect::(); - assert!(error.contains("'/review' is disabled while a task is in progress.")); - assert_eq!(chat.bottom_pane.composer_text(), "/review"); + assert_bare_review_rejected( + &mut chat, + &mut rx, + "'/review' is disabled while a task is in progress.", + ); } #[tokio::test] @@ -348,18 +344,11 @@ async fn side_conversation_rejection_preserves_review_draft_during_mcp_startup() notify_mcp_status(&mut chat, "alpha", McpServerStartupState::Starting); chat.set_side_conversation_active(/*active*/ true); - chat.bottom_pane - .set_composer_text("/review".to_string(), Vec::new(), Vec::new()); - chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); - - let error = drain_insert_history(&mut rx) - .into_iter() - .map(|lines| lines_to_single_string(&lines)) - .collect::(); - assert!(error.contains("'/review' is unavailable in side conversations.")); - assert_eq!(chat.bottom_pane.composer_text(), "/review"); + assert_bare_review_rejected( + &mut chat, + &mut rx, + "'/review' is unavailable in side conversations.", + ); } #[tokio::test] diff --git a/codex-rs/tui/src/chatwidget/turn_runtime.rs b/codex-rs/tui/src/chatwidget/turn_runtime.rs index 1e67bf9454..445e07ec15 100644 --- a/codex-rs/tui/src/chatwidget/turn_runtime.rs +++ b/codex-rs/tui/src/chatwidget/turn_runtime.rs @@ -9,18 +9,15 @@ const SAFETY_ACCESS_BLOCK_PREFIX: &str = "Invalid prompt: we've limited access to this content for safety reasons."; impl ChatWidget { - /// Synchronize the bottom-pane "task running" indicator with the current lifecycles. - /// - /// The bottom pane only has one running flag, but this module treats it as a derived state of - /// the agent turn, review, and MCP startup lifecycles. - pub(super) fn update_task_running_state(&mut self) { - self.bottom_pane.set_task_running( - self.turn_lifecycle.agent_turn_running - || self.mcp_startup_status.is_some() - || self.review.is_review_mode, - ); + /// Synchronize foreground task activity without disturbing background MCP startup state. + pub(super) fn update_foreground_task_running_state(&mut self) { + self.bottom_pane + .set_task_running(self.turn_lifecycle.agent_turn_running || self.review.is_review_mode); self.refresh_plan_mode_nudge(); self.refresh_status_surfaces(); + if !self.bottom_pane.is_foreground_task_running() { + self.refresh_mcp_startup_status_header(); + } } pub(super) fn collect_runtime_metrics_delta(&mut self) { @@ -65,7 +62,7 @@ impl ChatWidget { self.bottom_pane.clear_quit_shortcut_hint(); self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; - self.update_task_running_state(); + self.update_foreground_task_running_state(); self.status_state.retry_status_header = None; self.clear_active_hook_cell(); self.status_state.pending_status_indicator_restore = false; @@ -173,7 +170,7 @@ impl ChatWidget { self.clear_active_hook_cell(); self.turn_lifecycle.finish(); self.clear_safety_buffering(); - self.update_task_running_state(); + self.update_foreground_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -317,7 +314,7 @@ impl ChatWidget { // Reset running state and clear streaming buffers. self.input_queue.user_turn_pending_start = false; self.turn_lifecycle.finish(); - self.update_task_running_state(); + self.update_foreground_task_running_state(); self.running_commands.clear(); self.suppressed_exec_calls.clear(); self.last_unified_wait = None; @@ -350,11 +347,6 @@ impl ChatWidget { pub(super) fn on_error(&mut self, message: String) { self.input_queue.submit_pending_steers_after_interrupt = false; - if !self.review.is_review_mode { - // Review setup errors do not emit ExitedReviewMode, so release any MCP startup - // suppression armed when the request was submitted. - self.mcp_startup_updates_suppressed_for_review = false; - } self.flush_answer_stream_with_separator(); self.finalize_turn(); self.add_to_history(history_cell::new_error_event(message)); diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index e5dfd312b2..15698b8f00 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/codex-rs/tui/src/slash_command.rs @@ -200,6 +200,7 @@ impl SlashCommand { | SlashCommand::Experimental | SlashCommand::Memories | SlashCommand::Import + | SlashCommand::Review | SlashCommand::Plan | SlashCommand::Clear | SlashCommand::Logout @@ -207,7 +208,6 @@ impl SlashCommand { | SlashCommand::MemoryUpdate => false, SlashCommand::Diff | SlashCommand::Resume - | SlashCommand::Review | SlashCommand::Model | SlashCommand::Personality | SlashCommand::Permissions @@ -243,6 +243,15 @@ impl SlashCommand { } } + /// Whether this command can run while MCP servers initialize in the background. + pub fn available_during_mcp_startup(self) -> bool { + self == SlashCommand::Review || self.available_during_task() + } + + pub fn available_during_background_task_only(self) -> bool { + self.available_during_mcp_startup() && !self.available_during_task() + } + fn is_visible(self) -> bool { match self { SlashCommand::SandboxReadRoot => cfg!(target_os = "windows"), @@ -295,7 +304,6 @@ mod tests { assert!(SlashCommand::Raw.available_in_side_conversation()); assert!(SlashCommand::Raw.supports_inline_args()); assert!(SlashCommand::App.available_during_task()); - assert!(SlashCommand::Review.available_during_task()); } #[test]