diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 9bfe443e23..b25d7ac023 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/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() { 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 0a1604e5e3..0208b8e73c 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 @@ -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 "); } } diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index e97f014730..5e17ad614c 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/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) => { diff --git a/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_during_mcp_startup.snap b/codex-rs/tui/src/chatwidget/snapshots/codex_tui__chatwidget__tests__review_during_mcp_startup.snap new file mode 100644 index 0000000000..a5906d4e94 --- /dev/null +++ b/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 diff --git a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs b/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs index bc3f249767..2f526949f7 100644 --- a/codex-rs/tui/src/chatwidget/tests/mcp_startup.rs +++ b/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::>(); + 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)); diff --git a/codex-rs/tui/src/slash_command.rs b/codex-rs/tui/src/slash_command.rs index 24b4af75cc..c15edac558 100644 --- a/codex-rs/tui/src/slash_command.rs +++ b/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 {