Allow /review during MCP startup (#46493)

## Why

MCP startup alone should not prevent opening the review picker or submitting inline review instructions.

## What changed

- Defer `/review` availability checks and draft clearing to dispatch, where thread state is available.
- Reject live review commands while foreground work is running, pending, or queued, preserving the draft and attachments.
- Distinguish live and queued dispatch so a queued review does not clear a newer composer draft.

## Testing

Add regression tests for the review picker and inline instructions during MCP startup, draft and attachment preservation when review is blocked, and newer draft preservation when a queued review opens.

GitOrigin-RevId: 48962bfc26127ffa8dd157f15be512b78bf9b39b

codex-rs/tui/src/bottom_pane/chat_composer.rs

@@ -111,6 +111,8 @@
 //! and attachment pruning, and clears pending paste state on success.
 //! Slash commands with arguments (like `/plan` and `/review`) reuse the same preparation path so
 //! pasted content and text elements are preserved when extracting args.
+//! Commands requiring dispatch validation retain their draft and defer busy-state checks to
+//! `ChatWidget`, which has the thread state needed to accept or reject the command.
 //!
 //! # Parent-Owned Thread Mode
 //!
@@ -3359,8 +3361,11 @@ impl ChatComposer {
             return Some(InputResult::None);
         }
         self.stage_slash_command_history(&command);
-        self.draft.textarea.set_text_clearing_elements("");
-        self.draft.is_bash_mode = false;
+        if !matches!(command, SlashCommandItem::Builtin(cmd) if cmd.requires_dispatch_validation())
+        {
+            self.draft.textarea.set_text_clearing_elements("");
+            self.draft.is_bash_mode = false;
+        }
         Some(match command {
             SlashCommandItem::Builtin(cmd) => InputResult::Command(cmd),
             SlashCommandItem::ServiceTier(command) => InputResult::ServiceTierCommand(command),
@@ -3423,7 +3428,10 @@ impl ChatComposer {
     }
 
     fn reject_slash_command_if_unavailable(&self, command: &SlashCommandItem) -> bool {
-        if !self.is_task_running || command.available_during_task() {
+        if !self.is_task_running
+            || command.available_during_task()
+            || matches!(command, SlashCommandItem::Builtin(cmd) if cmd.requires_dispatch_validation())
+        {
             return false;
         }
         let message = format!(
@@ -9931,13 +9939,13 @@ mod tests {
         composer
             .draft
             .textarea
-            .set_text_clearing_elements("/review these changes");
+            .set_text_clearing_elements("/compact");
 
         let (result, _needs_redraw) =
             composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
 
         assert_eq!(InputResult::None, result);
-        assert_eq!("/review these changes", composer.draft.textarea.text());
+        assert_eq!("/compact", composer.draft.textarea.text());
 
         let mut found_error = false;
         while let Ok(event) = rx.try_recv() {

codex-rs/tui/src/bottom_pane/chat_composer/slash_input.rs

@@ -386,8 +386,11 @@ impl ChatComposer {
                     }
 
                     self.stage_selected_slash_command_history(&sel);
-                    self.draft.textarea.set_text_clearing_elements("");
-                    self.draft.is_bash_mode = false;
+                    if !matches!(sel, CommandItem::Builtin(cmd) if cmd.requires_dispatch_validation())
+                    {
+                        self.draft.textarea.set_text_clearing_elements("");
+                        self.draft.is_bash_mode = false;
+                    }
                     return (
                         match sel {
                             CommandItem::Builtin(cmd) => InputResult::Command(cmd),
@@ -682,6 +685,6 @@ mod tests {
             press(&mut composer, KeyCode::Enter),
             InputResult::Command(SlashCommand::Review)
         );
-        assert!(composer.draft.textarea.is_empty());
+        assert_eq!(composer.draft.textarea.text(), "/review ");
     }
 }

codex-rs/tui/src/chatwidget/slash_dispatch.rs

@@ -134,7 +134,11 @@ impl ChatWidget {
             .send(AppEvent::RawOutputModeChanged { enabled });
     }
 
-    fn slash_command_blocked_by_active_task(&self, cmd: SlashCommand) -> bool {
+    fn slash_command_blocked_by_active_task(
+        &self,
+        cmd: SlashCommand,
+        source: SlashCommandDispatchSource,
+    ) -> bool {
         (!cmd.available_during_task()
             && (self.turn_lifecycle.agent_turn_running
                 || self.review.is_review_mode
@@ -145,9 +149,21 @@ impl ChatWidget {
                 && (self.input_queue.user_turn_pending_start
                     || self.turn_lifecycle.agent_turn_running))
             || (cmd == SlashCommand::Export && self.input_queue.suppress_queue_autosend)
+            || (cmd == SlashCommand::Review
+                && source == SlashCommandDispatchSource::Live
+                && (self.is_user_turn_pending_or_running()
+                    || self.input_queue.has_queued_follow_up_messages()))
     }
 
     pub(super) fn dispatch_command(&mut self, cmd: SlashCommand) {
+        self.dispatch_command_from_source(cmd, SlashCommandDispatchSource::Live);
+    }
+
+    fn dispatch_command_from_source(
+        &mut self,
+        cmd: SlashCommand,
+        source: SlashCommandDispatchSource,
+    ) {
         if cmd != SlashCommand::Copy {
             self.transcript.last_status_copy_targets = None;
         }
@@ -157,13 +173,16 @@ impl ChatWidget {
         if !self.ensure_side_command_allowed_outside_review(cmd) {
             return;
         }
-        if self.slash_command_blocked_by_active_task(cmd) {
+        if self.slash_command_blocked_by_active_task(cmd, source) {
             let message = format!(
                 "'/{}' is disabled while a task is in progress.",
                 cmd.command()
             );
             self.add_to_history(history_cell::new_error_event(message));
-            self.bottom_pane.drain_pending_submission_state();
+            // Retain attachments when the composer has deferred consuming the draft.
+            if self.bottom_pane.composer_text().is_empty() {
+                self.bottom_pane.drain_pending_submission_state();
+            }
             self.request_redraw();
             return;
         }
@@ -302,6 +321,11 @@ impl ChatWidget {
                     .send(AppEvent::GenerateRecap { thread_id });
             }
             SlashCommand::Review => {
+                if source == SlashCommandDispatchSource::Live {
+                    self.bottom_pane
+                        .set_composer_text(String::new(), Vec::new(), Vec::new());
+                    self.bottom_pane.drain_pending_submission_state();
+                }
                 self.open_review_popup();
                 if self.mcp_startup_status.is_some() {
                     self.defer_input_until_settings_applied();
@@ -616,7 +640,7 @@ impl ChatWidget {
             self.dispatch_command(cmd);
             return;
         }
-        if self.slash_command_blocked_by_active_task(cmd) {
+        if self.slash_command_blocked_by_active_task(cmd, SlashCommandDispatchSource::Live) {
             let message = format!(
                 "'/{}' is disabled while a task is in progress.",
                 cmd.command()
@@ -1016,7 +1040,7 @@ impl ChatWidget {
             SlashCommand::Pets if !trimmed.is_empty() => {
                 self.select_pet_by_id(args);
             }
-            _ => self.dispatch_command(cmd),
+            _ => self.dispatch_command_from_source(cmd, source),
         }
         if source == SlashCommandDispatchSource::Live && cmd != SlashCommand::Goal {
             self.bottom_pane.drain_pending_submission_state();
@@ -1077,7 +1101,7 @@ impl ChatWidget {
         if rest.is_empty() {
             return match command {
                 SlashCommandItem::Builtin(cmd) => {
-                    self.dispatch_command(cmd);
+                    self.dispatch_command_from_source(cmd, SlashCommandDispatchSource::Queued);
                     self.queued_command_drain_result(cmd)
                 }
                 SlashCommandItem::ServiceTier(command) => {

codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_during_mcp_startup.snap

@@ -0,0 +1,12 @@
+---
+source: tui/src/chatwidget/tests/mcp_startup.rs
+expression: render_bottom_popup(&chat, 80)
+---
+  Select a review preset
+
+› 1. Review against a base branch  (PR Style)
+  2. Review uncommitted changes
+  3. Review a commit
+  4. Custom review instructions
+
+  Press enter to confirm or esc to go back

codex-rs/tui/src/chatwidget/tests/mcp_startup.rs

@@ -216,6 +216,109 @@ async fn mcp_startup_complete_does_not_clear_running_task() {
     assert_eq!(chat.status_state.current_status.header, "Working");
 }
 
+#[tokio::test]
+async fn review_opens_during_mcp_startup() {
+    for dismiss_completion in [false, true] {
+        let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
+        chat.show_welcome_banner = false;
+        chat.set_mcp_startup_expected_servers(["slow".to_string()]);
+        notify_mcp_status(&mut chat, "slow", McpServerStartupState::Starting);
+        chat.bottom_pane
+            .set_composer_text("/review".to_string(), Vec::new(), Vec::new());
+        chat.bottom_pane
+            .set_remote_image_urls(vec!["https://example.com/image.png".to_string()]);
+        if dismiss_completion {
+            chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
+        }
+
+        chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+
+        assert!(drain_insert_history(&mut rx).is_empty());
+        assert_eq!(chat.bottom_pane.composer_text(), "");
+        assert!(chat.bottom_pane.remote_image_urls().is_empty());
+        assert_chatwidget_snapshot!(
+            "review_during_mcp_startup",
+            render_bottom_popup(&chat, /*width*/ 80)
+        );
+    }
+}
+
+#[tokio::test]
+async fn inline_review_submits_during_mcp_startup() {
+    let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
+    chat.set_mcp_startup_expected_servers(["slow".to_string()]);
+    notify_mcp_status(&mut chat, "slow", McpServerStartupState::Starting);
+    chat.bottom_pane.set_composer_text(
+        "/review check regressions".to_string(),
+        Vec::new(),
+        Vec::new(),
+    );
+
+    chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+
+    assert_eq!(
+        op_rx.try_recv().expect("review command"),
+        Op::Review {
+            target: ReviewTarget::Custom {
+                instructions: "check regressions".to_string(),
+            }
+        }
+    );
+    assert!(drain_insert_history(&mut rx).is_empty());
+    assert_eq!(chat.bottom_pane.composer_text(), "");
+}
+
+#[tokio::test]
+async fn review_during_mcp_startup_preserves_draft_when_foreground_work_is_pending() {
+    for activity in ["turn", "review", "pending", "queued", "images"] {
+        for draft in ["/review", "/review check regressions"] {
+            let (mut chat, mut rx, mut op_rx) =
+                make_chatwidget_manual(/*model_override*/ None).await;
+            chat.set_mcp_startup_expected_servers(["slow".to_string()]);
+            notify_mcp_status(&mut chat, "slow", McpServerStartupState::Starting);
+            match activity {
+                "turn" => handle_turn_started(&mut chat, "turn-1"),
+                "review" => chat.review.is_review_mode = true,
+                "pending" => chat.input_queue.user_turn_pending_start = true,
+                "images" => chat.prepare_image_submission(
+                    "image prompt".into(),
+                    UserMessageHistoryRecord::UserMessageText,
+                    UserMessageSource::Prompt,
+                ),
+                "queued" => chat
+                    .input_queue
+                    .queued_user_messages
+                    .push_back(UserMessage::from("queued prompt".to_string()).into()),
+                _ => unreachable!(),
+            }
+            chat.bottom_pane
+                .set_composer_text(draft.to_string(), Vec::new(), Vec::new());
+            chat.bottom_pane
+                .set_remote_image_urls(vec!["https://example.com/image.png".to_string()]);
+
+            chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
+
+            assert_eq!(chat.bottom_pane.composer_text(), draft);
+            assert_eq!(
+                chat.bottom_pane.remote_image_urls(),
+                vec!["https://example.com/image.png".to_string()]
+            );
+            let errors = drain_insert_history(&mut rx)
+                .iter()
+                .map(|cell| lines_to_single_string(cell))
+                .collect::<Vec<_>>();
+            assert_eq!(
+                errors,
+                vec!["■ '/review' is disabled while a task is in progress.\n".to_string()]
+            );
+            assert!(
+                !std::iter::from_fn(|| op_rx.try_recv().ok())
+                    .any(|op| matches!(op, Op::Review { .. } | Op::UserTurn { .. }))
+            );
+        }
+    }
+}
+
 #[tokio::test]
 async fn pending_mcp_startup_does_not_block_queued_follow_up() {
     let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
@@ -288,7 +391,10 @@ async fn pending_mcp_startup_does_not_drain_follow_up_before_review_starts() {
             .set_composer_text(message.to_string(), Vec::new(), Vec::new());
         chat.handle_key_event(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE));
     }
+    chat.bottom_pane
+        .set_composer_text("new draft".to_string(), Vec::new(), Vec::new());
     handle_turn_completed(&mut chat, "turn-1", /*duration_ms*/ None);
+    assert_eq!(chat.bottom_pane.composer_text(), "new draft");
 
     chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
     chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));

codex-rs/tui/src/slash_command.rs

@@ -203,6 +203,12 @@ impl SlashCommand {
         )
     }
 
+    /// Whether dispatch needs thread state to validate this command before consuming its draft.
+    /// The composer must defer busy-state rejection and draft clearing for these commands.
+    pub(crate) fn requires_dispatch_validation(self) -> bool {
+        matches!(self, SlashCommand::Review)
+    }
+
     /// Whether this command can be run while a task is in progress.
     pub fn available_during_task(self) -> bool {
         match self {