openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-10-24 18:18:41 +00:00
Code Issues Packages Projects Releases Wiki Activity

Simplify review MCP isolation

Browse Source
This commit is contained in:
Charlie Marsh
2026-06-29 09:39:14 -04:00
parent 3f75f965da
commit f6e8c636d2
14 changed files with 307 additions and 271 deletions
Download Patch File Download Diff File Expand all files Collapse all files
codex-rs/tui/src/bottom_pane/chat_composer.rs
+31 -13
View File
@@ -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<FrameRequester>,
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);
}
codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs
+8 -5
View File
@@ -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;
}
codex-rs/tui/src/bottom_pane/mod.rs
+32 -9
View File
@@ -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.
codex-rs/tui/src/bottom_pane/slash_commands.rs
+7
View File
@@ -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)]
codex-rs/tui/src/bottom_pane/task_activity.rs
+27
View File
@@ -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;
}
}
codex-rs/tui/src/chatwidget.rs
+8 -23
View File
@@ -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<HashMap<String, McpStartupStatus>>,
/// 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<HashSet<String>>,
/// 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<String, McpStartupStatus>,
/// 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
codex-rs/tui/src/chatwidget/constructor.rs
-2
View File
@@ -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(),
codex-rs/tui/src/chatwidget/input_restore.rs
+1 -1
View File
@@ -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();
codex-rs/tui/src/chatwidget/mcp_startup.rs
+58 -71
View File
@@ -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<String> = 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<String> = 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<I>(&mut self, server_names: I)
where
I: IntoIterator<Item = String>,
@@ -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;
codex-rs/tui/src/chatwidget/replay.rs
+1 -1
View File
@@ -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);
codex-rs/tui/src/chatwidget/slash_dispatch.rs
+16 -17
View File
@@ -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;
}
codex-rs/tui/src/chatwidget/tests/mcp_startup.rs
+98 -109
View File
@@ -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<Op>,
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<AppEvent>,
expected_error: &str,
) {
submit_bare_review(chat);
let error = drain_insert_history(rx)
.into_iter()
.map(|lines| lines_to_single_string(&lines))
.collect::<String>();
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::<String>();
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::<String>();
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::<String>();
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::<String>();
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::<String>();
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]
codex-rs/tui/src/chatwidget/turn_runtime.rs
+10 -18
View File
@@ -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));
codex-rs/tui/src/slash_command.rs
+10 -2
View File
@@ -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]