From 2808a9c348ee90a6fc94aee1570dd3fdf2c0b021 Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Wed, 9 Sep 2026 22:38:52 +0000 Subject: [PATCH] Clear pending TUI questions when accepting a new prompt (#44328) ## Why Unanswered questions from the previous prompt remain visible after a new follow-up. Clear them when the user moves on to new work. ## What changed - Clear pending questions when a model prompt is submitted or queued, and when a review starts. Clear again on queued prompt delivery to remove questions that arrived while waiting. - Retain seen question IDs so replay cannot restore cleared questions. - Track question answers separately from prompts through queues, retries, thread restoration, and reconnects so delivering an answer preserves other pending questions. - Preserve questions for local commands and rejected submissions. ## Testing Add regression tests for immediate and queued follow-ups, replay suppression, slash prompts, reviews, rejected input, and answer preservation across delivery and recovery. Add a snapshot of the composer after questions are cleared. GitOrigin-RevId: 76353c6ee098c1505678ac43b1dc6391ea9fa738 --- codex-rs/tui/src/app/thread_routing.rs | 1 + .../src/bottom_pane/async_questions/state.rs | 11 + codex-rs/tui/src/bottom_pane/questions.rs | 7 + codex-rs/tui/src/chatwidget.rs | 6 + .../tui/src/chatwidget/backend_banners.rs | 6 +- codex-rs/tui/src/chatwidget/constructor.rs | 1 + codex-rs/tui/src/chatwidget/input_flow.rs | 89 +++++- codex-rs/tui/src/chatwidget/input_queue.rs | 5 + codex-rs/tui/src/chatwidget/input_restore.rs | 66 +++- .../tui/src/chatwidget/input_submission.rs | 30 +- codex-rs/tui/src/chatwidget/questions.rs | 9 +- codex-rs/tui/src/chatwidget/reconnect.rs | 5 +- codex-rs/tui/src/chatwidget/review_popups.rs | 4 + codex-rs/tui/src/chatwidget/slash_dispatch.rs | 2 +- .../chatwidget/tests/composer_submission.rs | 3 + codex-rs/tui/src/chatwidget/tests/helpers.rs | 1 + .../src/chatwidget/tests/questions_tests.rs | 290 +++++++++++++++++- .../tui/src/chatwidget/tests/review_mode.rs | 2 + ...tests__questions_cleared_by_follow_up.snap | 5 + codex-rs/tui/src/chatwidget/user_messages.rs | 11 + 20 files changed, 528 insertions(+), 26 deletions(-) create mode 100644 codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__questions_cleared_by_follow_up.snap diff --git a/codex-rs/tui/src/app/thread_routing.rs b/codex-rs/tui/src/app/thread_routing.rs index 8f206c0605..5254d18af8 100644 --- a/codex-rs/tui/src/app/thread_routing.rs +++ b/codex-rs/tui/src/app/thread_routing.rs @@ -917,6 +917,7 @@ impl App { let store = Arc::clone(&self.ensure_thread_channel(review_thread_id).store); let mut store = store.lock().await; store.set_active_turn_id(response.turn.id); + self.chat_widget.on_review_started(); Ok(true) } AppCommand::CleanBackgroundTerminals => { diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state.rs b/codex-rs/tui/src/bottom_pane/async_questions/state.rs index d7796c7549..0a4a82effa 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state.rs @@ -147,6 +147,17 @@ impl AsyncQuestions { self.composer.reset_vim_mode(); } + pub(crate) fn clear_pending(&mut self) { + // Keep seen_ids so replay cannot restore questions from the previous prompt. + self.state.pending.clear(); + self.state.current_idx = 0; + self.expanded = false; + self.submission = None; + self.visible_options.set((0, 0)); + self.restore_current_draft(); + self.composer.reset_vim_mode(); + } + pub(crate) fn capture(&mut self) -> QuestionState { self.composer.cancel_history_search(); self.save_current_draft(); diff --git a/codex-rs/tui/src/bottom_pane/questions.rs b/codex-rs/tui/src/bottom_pane/questions.rs index 1d375baa12..7645406e6a 100644 --- a/codex-rs/tui/src/bottom_pane/questions.rs +++ b/codex-rs/tui/src/bottom_pane/questions.rs @@ -13,6 +13,13 @@ impl BottomPane { self.request_redraw(); } + pub(crate) fn clear_pending_questions(&mut self) { + if let Some(questions) = &mut self.questions { + questions.clear_pending(); + self.request_redraw(); + } + } + fn question_editor(&mut self) -> &mut AsyncQuestions { self.questions.get_or_insert_with(|| { let mut questions = AsyncQuestions::new( diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 0ccb35e6e6..4e34536699 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -471,6 +471,7 @@ use self::user_messages::UserMessageDisplay; #[cfg(test)] use self::user_messages::UserMessageHistoryOverride; use self::user_messages::UserMessageHistoryRecord; +use self::user_messages::UserMessageSource; use self::user_messages::app_server_text_elements; pub(crate) use self::user_messages::create_initial_user_message; pub(crate) use self::user_messages::mention_bindings_from_user_inputs; @@ -724,6 +725,7 @@ pub(crate) struct ChatWidget { suppress_initial_user_message_submit: bool, input_queue: InputQueueState, safety_buffering_prompt: Option, + safety_buffering_source: UserMessageSource, /// Main chat-surface bindings resolved from `tui.keymap.chat`. chat_keymap: ChatKeymap, permission_shortcut_pending: bool, @@ -1847,10 +1849,14 @@ impl ChatWidget { match &self.codex_op_target { CodexOpTarget::Direct(codex_op_tx) => { crate::session_log::log_outbound_op(&op); + let is_review = op.is_review(); if let Err(e) = codex_op_tx.send(op) { tracing::error!("failed to submit op: {e}"); return false; } + if is_review { + self.on_review_started(); + } } CodexOpTarget::AppEvent => { self.app_event_tx.send(AppEvent::CodexOp(op)); diff --git a/codex-rs/tui/src/chatwidget/backend_banners.rs b/codex-rs/tui/src/chatwidget/backend_banners.rs index 98192e8d57..6f2fac4cd0 100644 --- a/codex-rs/tui/src/chatwidget/backend_banners.rs +++ b/codex-rs/tui/src/chatwidget/backend_banners.rs @@ -6,6 +6,7 @@ //! Its entry notice is shared across chats until the backend confirms ordinary usage has recovered. use super::ChatWidget; +use super::QueuedUserMessage; use super::luna_reserve_return::ReserveReturnModel; use crate::app_command::AppCommand; use crate::backend_banners::BackendBanner; @@ -221,7 +222,10 @@ impl ChatWidget { self.finalize_turn(); self.input_queue .queued_user_messages - .push_front(prompt.into()); + .push_front(QueuedUserMessage { + source: self.safety_buffering_source, + ..QueuedUserMessage::from(prompt) + }); self.input_queue .queued_user_message_history_records .push_front(super::UserMessageHistoryRecord::UserMessageText); diff --git a/codex-rs/tui/src/chatwidget/constructor.rs b/codex-rs/tui/src/chatwidget/constructor.rs index 7be8282a2e..e0be308b80 100644 --- a/codex-rs/tui/src/chatwidget/constructor.rs +++ b/codex-rs/tui/src/chatwidget/constructor.rs @@ -226,6 +226,7 @@ impl ChatWidget { interrupted_turn_notice_mode: InterruptedTurnNoticeMode::Default, input_queue: InputQueueState::default(), safety_buffering_prompt: None, + safety_buffering_source: UserMessageSource::Prompt, chat_keymap, permission_shortcut_pending: false, queued_message_edit_hint_binding, diff --git a/codex-rs/tui/src/chatwidget/input_flow.rs b/codex-rs/tui/src/chatwidget/input_flow.rs index 97d3e59401..d6ad607744 100644 --- a/codex-rs/tui/src/chatwidget/input_flow.rs +++ b/codex-rs/tui/src/chatwidget/input_flow.rs @@ -5,6 +5,9 @@ //! follow-ups, and restoring draft state across interrupts or thread switches. use super::*; +use crate::bottom_pane::prompt_args::parse_slash_name; +use crate::bottom_pane::slash_commands::SlashCommandItem; +use crate::bottom_pane::slash_commands::find_slash_command; impl ChatWidget { pub(crate) fn set_parent_owned_thread(&mut self) { @@ -95,8 +98,13 @@ impl ChatWidget { } } - pub(super) fn queue_user_message(&mut self, user_message: UserMessage) { - self.queue_user_message_with_options(user_message, QueuedInputAction::Plain, Vec::new()); + pub(super) fn queue_user_message(&mut self, user_message: UserMessage) -> bool { + self.queue_user_message_with_options_and_source( + user_message, + QueuedInputAction::Plain, + Vec::new(), + UserMessageSource::Prompt, + ) } pub(crate) fn set_queue_submissions_until_session_configured(&mut self, queue: bool) { @@ -109,31 +117,82 @@ impl ChatWidget { user_message: UserMessage, action: QueuedInputAction, pending_pastes: Vec<(String, String)>, - ) { + ) -> bool { + self.queue_user_message_with_options_and_source( + user_message, + action, + pending_pastes, + UserMessageSource::Prompt, + ) + } + + pub(super) fn queue_user_message_with_options_and_source( + &mut self, + user_message: UserMessage, + action: QueuedInputAction, + pending_pastes: Vec<(String, String)>, + source: UserMessageSource, + ) -> bool { if self.has_misalignment_policy_violation() { - return; + return false; } let should_run_now = self.is_session_configured() && !self.is_user_turn_pending_or_running() && !self.input_queue.suppress_queue_autosend && !self.input_queue.rate_limit_recovery_pending; if !should_run_now || action != QueuedInputAction::Plain { + let queued_slash_prompt = action == QueuedInputAction::ParseSlash + && parse_slash_name(&user_message.text).is_none_or(|(name, args, _)| { + if name.contains('/') { + return true; + } + !args.trim().is_empty() + && find_slash_command( + name, + self.builtin_command_flags(), + &self.current_model_service_tier_commands(), + ) + .is_some_and(|command| { + !command.supports_inline_args() + || matches!( + command, + SlashCommandItem::Builtin( + SlashCommand::Plan | SlashCommand::Review + ) + ) + }) + }); + let model_prompt = source == UserMessageSource::Prompt + && (action == QueuedInputAction::Literal + || action == QueuedInputAction::Plain && !user_message.text.starts_with('!') + || queued_slash_prompt); self.input_queue .queued_user_messages .push_back(QueuedUserMessage { user_message, action, pending_pastes, + source, }); self.input_queue .queued_user_message_history_records .push_back(UserMessageHistoryRecord::UserMessageText); self.refresh_pending_input_preview(); + if model_prompt && !should_run_now { + self.bottom_pane.clear_pending_questions(); + } if should_run_now { self.maybe_send_next_queued_input(); } + true } else { - self.submit_user_message(user_message); + self.submit_user_message_with_history_and_shell_escape_policy( + user_message, + UserMessageHistoryRecord::UserMessageText, + ShellEscapePolicy::Allow, + source, + ) + .0 } } @@ -160,16 +219,22 @@ impl ChatWidget { }; match queued_message.action { QueuedInputAction::Plain => { - submitted_follow_up = self.submit_user_message_with_history_record( - queued_message.into_user_message(), - history_record, - ); + let source = queued_message.source; + submitted_follow_up = self + .submit_user_message_with_history_and_shell_escape_policy( + queued_message.into_user_message(), + history_record, + ShellEscapePolicy::Allow, + source, + ) + .0; break; } QueuedInputAction::Literal => { let QueuedUserMessage { user_message, pending_pastes, + source, .. } = queued_message; let mut restored_pending_pastes = self.bottom_pane.composer_pending_pastes(); @@ -196,11 +261,13 @@ impl ChatWidget { ); } submitted_follow_up = self - .submit_user_message_with_shell_escape_policy( + .submit_user_message_with_history_and_shell_escape_policy( user_message, + history_record, ShellEscapePolicy::Disallow, + source, ) - .is_some(); + .0; if !submitted_follow_up { restored_pending_pastes.extend(pending_pastes); self.bottom_pane diff --git a/codex-rs/tui/src/chatwidget/input_queue.rs b/codex-rs/tui/src/chatwidget/input_queue.rs index 48a0916c0a..e4de46d51f 100644 --- a/codex-rs/tui/src/chatwidget/input_queue.rs +++ b/codex-rs/tui/src/chatwidget/input_queue.rs @@ -9,6 +9,7 @@ use super::PendingSteer; use super::QueuedUserMessage; use super::UserMessage; use super::UserMessageHistoryRecord; +use super::UserMessageSource; use super::user_message_preview_text; #[derive(Debug, Default, PartialEq, Eq)] @@ -31,6 +32,8 @@ pub(super) struct InputQueueState { pub(super) user_turn_pending_start: bool, /// User messages that tried to steer a non-regular turn and must be retried first. pub(super) rejected_steers_queue: VecDeque, + /// The origin of each rejected steer, kept in lockstep with its message. + pub(super) rejected_steer_sources: VecDeque, /// History records for rejected steers. Slash commands such as `/goal` can /// render history that differs from the text submitted to core, so this stays /// in lockstep with `rejected_steers_queue`, with missing entries treated as @@ -58,6 +61,7 @@ impl InputQueueState { self.queued_user_message_history_records.clear(); self.user_turn_pending_start = false; self.rejected_steers_queue.clear(); + self.rejected_steer_sources.clear(); self.rejected_steer_history_records.clear(); self.pending_steers.clear(); self.submit_pending_steers_after_interrupt = false; @@ -119,6 +123,7 @@ mod tests { client_id: "test-submission".to_string(), user_message: UserMessage::from("pending"), history_record: UserMessageHistoryRecord::UserMessageText, + source: UserMessageSource::Prompt, compare_key: crate::chatwidget::user_messages::PendingSteerCompareKey { message: "pending".to_string(), image_count: 0, diff --git a/codex-rs/tui/src/chatwidget/input_restore.rs b/codex-rs/tui/src/chatwidget/input_restore.rs index e3a35cf663..f078146d2a 100644 --- a/codex-rs/tui/src/chatwidget/input_restore.rs +++ b/codex-rs/tui/src/chatwidget/input_restore.rs @@ -162,6 +162,21 @@ impl ChatWidget { .rejected_steers_queue .drain(..) .collect::>(); + let sources = self + .input_queue + .rejected_steer_sources + .drain(..) + .collect::>(); + let source = if !rejected_messages.is_empty() + && sources.len() == rejected_messages.len() + && sources + .iter() + .all(|source| *source == UserMessageSource::QuestionAnswer) + { + UserMessageSource::QuestionAnswer + } else { + UserMessageSource::Prompt + }; let mut history_records = self .input_queue .rejected_steer_history_records @@ -177,7 +192,13 @@ impl ChatWidget { .zip(history_records) .collect::>(), ); - Some((QueuedUserMessage::from(message), history_record)) + Some(( + QueuedUserMessage { + source, + ..QueuedUserMessage::from(message) + }, + history_record, + )) } } @@ -201,6 +222,7 @@ impl ChatWidget { )) } else { let user_message = self.input_queue.rejected_steers_queue.pop_back()?; + self.input_queue.rejected_steer_sources.pop_back(); self.input_queue.recovered_queue &= self.input_queue.has_queued_follow_up_messages() || !self.input_queue.pending_steers.is_empty(); let history_record = self @@ -225,6 +247,9 @@ impl ChatWidget { self.input_queue .rejected_steers_queue .push_back(pending_steer.user_message); + self.input_queue + .rejected_steer_sources + .push_back(pending_steer.source); self.input_queue .rejected_steer_history_records .push_back(pending_steer.history_record); @@ -263,12 +288,28 @@ impl ChatWidget { .input_queue .pending_steers .drain(..) - .map(|pending| (pending.user_message, pending.history_record)) .collect::>(); if !pending_steers.is_empty() { - let (user_message, history_record) = - merge_user_messages_with_history_record(pending_steers); - self.submit_user_message_with_history_record(user_message, history_record); + let source = if pending_steers + .iter() + .all(|pending| pending.source == UserMessageSource::QuestionAnswer) + { + UserMessageSource::QuestionAnswer + } else { + UserMessageSource::Prompt + }; + let (user_message, history_record) = merge_user_messages_with_history_record( + pending_steers + .into_iter() + .map(|pending| (pending.user_message, pending.history_record)) + .collect(), + ); + self.submit_user_message_with_history_and_shell_escape_policy( + user_message, + history_record, + ShellEscapePolicy::Allow, + source, + ); } else if let Some(combined) = self.drain_pending_messages_for_restore() { self.restore_composer_state(combined); } @@ -306,6 +347,7 @@ impl ChatWidget { .rejected_steers_queue .drain(..) .collect::>(); + self.input_queue.rejected_steer_sources.clear(); let mut rejected_history_records = self .input_queue .rejected_steer_history_records @@ -455,8 +497,10 @@ impl ChatWidget { .map(crate::bottom_pane::AsyncQuestions::capture), composer: composer.has_content().then_some(composer), safety_buffering_prompt: self.safety_buffering_prompt.clone(), + safety_buffering_source: self.safety_buffering_source, pending_steers: self.input_queue.pending_steers.clone(), rejected_steers_queue: self.input_queue.rejected_steers_queue.clone(), + rejected_steer_sources: self.input_queue.rejected_steer_sources.clone(), rejected_steer_history_records: self.input_queue.rejected_steer_history_records.clone(), queued_user_messages: self.input_queue.queued_user_messages.clone(), queued_user_message_history_records: self @@ -489,6 +533,7 @@ impl ChatWidget { self.current_collaboration_mode = input_state.current_collaboration_mode; self.active_collaboration_mask = input_state.active_collaboration_mask; self.safety_buffering_prompt = input_state.safety_buffering_prompt; + self.safety_buffering_source = input_state.safety_buffering_source; self.turn_lifecycle.restore_running( preserve_in_flight_turn && input_state.agent_turn_running, Instant::now(), @@ -509,11 +554,19 @@ impl ChatWidget { } else { self.input_queue.pending_steers.clear(); for pending in pending_steers.into_iter().rev() { - queued_user_messages.push_front(pending.user_message.into()); + queued_user_messages.push_front(QueuedUserMessage { + source: pending.source, + ..QueuedUserMessage::from(pending.user_message) + }); queued_user_message_history_records.push_front(pending.history_record); } } self.input_queue.rejected_steers_queue = input_state.rejected_steers_queue; + self.input_queue.rejected_steer_sources = input_state.rejected_steer_sources; + self.input_queue.rejected_steer_sources.resize( + self.input_queue.rejected_steers_queue.len(), + UserMessageSource::Prompt, + ); self.input_queue.rejected_steer_history_records = input_state.rejected_steer_history_records; self.input_queue.rejected_steer_history_records.resize( @@ -531,6 +584,7 @@ impl ChatWidget { self.turn_lifecycle .restore_running(/*running*/ false, Instant::now()); self.safety_buffering_prompt = None; + self.safety_buffering_source = UserMessageSource::Prompt; self.input_queue.clear(); self.restore_composer_state(Default::default()); } diff --git a/codex-rs/tui/src/chatwidget/input_submission.rs b/codex-rs/tui/src/chatwidget/input_submission.rs index b0ba5c7d1e..fdfe3f6ab6 100644 --- a/codex-rs/tui/src/chatwidget/input_submission.rs +++ b/codex-rs/tui/src/chatwidget/input_submission.rs @@ -90,6 +90,7 @@ impl ChatWidget { user_message, history_record, ShellEscapePolicy::Allow, + UserMessageSource::Prompt, ) .0 } @@ -103,6 +104,7 @@ impl ChatWidget { user_message, UserMessageHistoryRecord::UserMessageText, shell_escape_policy, + UserMessageSource::Prompt, ) .1 } @@ -112,29 +114,48 @@ impl ChatWidget { user_message: UserMessage, history_record: UserMessageHistoryRecord, shell_escape_policy: ShellEscapePolicy, + source: UserMessageSource, ) -> (bool, Option) { if self.has_misalignment_policy_violation() { return (false, None); } if self.input_queue.rate_limit_recovery_pending { + let model_prompt = source == UserMessageSource::Prompt + && (shell_escape_policy == ShellEscapePolicy::Disallow + || !user_message.text.starts_with('!')); self.input_queue .queued_user_messages - .push_back(QueuedUserMessage::from(user_message)); + .push_back(QueuedUserMessage { + source, + ..QueuedUserMessage::from(user_message) + }); self.input_queue .queued_user_message_history_records .push_back(history_record); self.refresh_pending_input_preview(); + if model_prompt { + self.bottom_pane.clear_pending_questions(); + } return (true, None); } if !self.is_session_configured() { + let model_prompt = source == UserMessageSource::Prompt + && (shell_escape_policy == ShellEscapePolicy::Disallow + || !user_message.text.starts_with('!')); tracing::warn!("cannot submit user message before session is configured; queueing"); self.input_queue .queued_user_messages - .push_front(QueuedUserMessage::from(user_message)); + .push_front(QueuedUserMessage { + source, + ..QueuedUserMessage::from(user_message) + }); self.input_queue .queued_user_message_history_records .push_front(history_record); self.refresh_pending_input_preview(); + if model_prompt { + self.bottom_pane.clear_pending_questions(); + } return (true, None); } if user_message.text.is_empty() @@ -358,6 +379,7 @@ impl ChatWidget { mention_bindings: mention_bindings.clone(), }, history_record: history_record.clone(), + source, compare_key: Self::pending_steer_compare_key_from_items(&items), }); let personality = self @@ -409,6 +431,9 @@ impl ChatWidget { if !self.submit_op(op.clone()) { return (false, None); } + if source == UserMessageSource::Prompt { + self.bottom_pane.clear_pending_questions(); + } self.dismiss_backend_banner_for_new_turn(); self.note_realtime_typed_input(&submitted_message.text); if render_in_history { @@ -456,6 +481,7 @@ impl ChatWidget { if render_in_history { self.safety_buffering_prompt = Some(submitted_message.clone()); + self.safety_buffering_source = source; if !render_before_submit { self.on_user_message_display(user_message_display_for_history( submitted_message, diff --git a/codex-rs/tui/src/chatwidget/questions.rs b/codex-rs/tui/src/chatwidget/questions.rs index ab3a9587c6..5313d25372 100644 --- a/codex-rs/tui/src/chatwidget/questions.rs +++ b/codex-rs/tui/src/chatwidget/questions.rs @@ -117,13 +117,18 @@ impl ChatWidget { && !self.turn_lifecycle.agent_turn_running || self.only_user_shell_commands_running() { - self.queue_user_message(UserMessage::from(text)); - true + self.queue_user_message_with_options_and_source( + UserMessage::from(text), + QueuedInputAction::Plain, + Vec::new(), + UserMessageSource::QuestionAnswer, + ) } else { self.submit_user_message_with_history_and_shell_escape_policy( UserMessage::from(text), UserMessageHistoryRecord::UserMessageText, ShellEscapePolicy::Disallow, + UserMessageSource::QuestionAnswer, ) .0 }; diff --git a/codex-rs/tui/src/chatwidget/reconnect.rs b/codex-rs/tui/src/chatwidget/reconnect.rs index 3d88597f4b..0e94051794 100644 --- a/codex-rs/tui/src/chatwidget/reconnect.rs +++ b/codex-rs/tui/src/chatwidget/reconnect.rs @@ -41,7 +41,10 @@ impl ChatWidget { if input.user_turn_pending_start && let Some(prompt) = input.safety_buffering_prompt.take() { - input.queued_user_messages.push_front(prompt.into()); + input.queued_user_messages.push_front(QueuedUserMessage { + source: input.safety_buffering_source, + ..QueuedUserMessage::from(prompt) + }); input .queued_user_message_history_records .push_front(UserMessageHistoryRecord::UserMessageText); diff --git a/codex-rs/tui/src/chatwidget/review_popups.rs b/codex-rs/tui/src/chatwidget/review_popups.rs index 846434d46f..7185e735b6 100644 --- a/codex-rs/tui/src/chatwidget/review_popups.rs +++ b/codex-rs/tui/src/chatwidget/review_popups.rs @@ -3,6 +3,10 @@ use super::*; impl ChatWidget { + pub(crate) fn on_review_started(&mut self) { + self.bottom_pane.clear_pending_questions(); + } + pub(crate) fn open_review_popup(&mut self) { let mut items: Vec = Vec::new(); diff --git a/codex-rs/tui/src/chatwidget/slash_dispatch.rs b/codex-rs/tui/src/chatwidget/slash_dispatch.rs index d9580abd57..1ee962f045 100644 --- a/codex-rs/tui/src/chatwidget/slash_dispatch.rs +++ b/codex-rs/tui/src/chatwidget/slash_dispatch.rs @@ -1126,7 +1126,7 @@ impl ChatWidget { self.queued_command_drain_result(cmd) } - fn builtin_command_flags(&self) -> BuiltinCommandFlags { + pub(super) fn builtin_command_flags(&self) -> BuiltinCommandFlags { #[cfg(target_os = "windows")] let allow_elevate_sandbox = { let windows_sandbox_level = crate::windows_sandbox::level_from_config(&self.config); diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index af912ae51f..7d184d880d 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -352,6 +352,7 @@ async fn parent_owned_thread_preserves_queued_input_before_draining() { user_message: UserMessage::from("keep this queued prompt"), action: QueuedInputAction::Plain, pending_pastes: vec![("[Image 1]".to_string(), "pasted contents".to_string())], + source: UserMessageSource::Prompt, }; let history_record = UserMessageHistoryRecord::UserMessageText; chat.input_queue @@ -1762,11 +1763,13 @@ async fn restore_thread_input_state_applies_running_state_policy() { ..Default::default() }), safety_buffering_prompt: Some(UserMessage::from("buffered prompt")), + safety_buffering_source: UserMessageSource::Prompt, pending_steers: VecDeque::from([PendingSteer { history_record: pending_history.clone(), ..pending_steer("submitted to the interrupted turn") }]), rejected_steers_queue: VecDeque::new(), + rejected_steer_sources: VecDeque::new(), rejected_steer_history_records: VecDeque::new(), queued_user_messages: VecDeque::from([UserMessage::from("already queued").into()]), queued_user_message_history_records: VecDeque::from([queued_history.clone()]), diff --git a/codex-rs/tui/src/chatwidget/tests/helpers.rs b/codex-rs/tui/src/chatwidget/tests/helpers.rs index 8ad907f2c9..3b54b8cfcb 100644 --- a/codex-rs/tui/src/chatwidget/tests/helpers.rs +++ b/codex-rs/tui/src/chatwidget/tests/helpers.rs @@ -994,6 +994,7 @@ pub(super) fn pending_steer(text: &str) -> PendingSteer { client_id: "test-submission".to_string(), user_message: UserMessage::from(text), history_record: UserMessageHistoryRecord::UserMessageText, + source: UserMessageSource::Prompt, compare_key: PendingSteerCompareKey { message: text.to_string(), image_count: 0, diff --git a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs index 77dd754a72..81e84ba8da 100644 --- a/codex-rs/tui/src/chatwidget/tests/questions_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/questions_tests.rs @@ -58,6 +58,291 @@ async fn accepted_question_answer_uses_existing_delivery_and_keeps_main_draft() } } +#[tokio::test] +async fn ordinary_follow_up_clears_unanswered_questions_after_accepted_input() { + for queued in [false, true] { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.show_welcome_banner = false; + chat.add_async_questions("old", &questions()); + if queued { + chat.on_task_started(); + } + chat.bottom_pane + .set_composer_text("New prompt".into(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::from(if queued { + KeyCode::Tab + } else { + KeyCode::Enter + })); + + assert_eq!(question_count(&chat), 0); + if queued { + assert_eq!( + chat.input_queue.queued_user_messages.front().unwrap().text, + "New prompt" + ); + assert!(op_rx.try_recv().is_err()); + } else { + assert_answer(op_rx.try_recv().unwrap(), "New prompt"); + insta::assert_snapshot!( + "questions_cleared_by_follow_up", + render_bottom_popup(&chat, /*width*/ 80) + .lines() + .next() + .unwrap() + ); + } + chat.add_async_questions("old", &questions()); + assert_eq!(question_count(&chat), 0); + chat.add_async_questions("new", &[question("New question?", /*options*/ None)]); + assert_eq!(question_count(&chat), 1); + } +} + +#[tokio::test] +async fn local_shell_command_preserves_unanswered_questions() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.bottom_pane + .set_composer_text("!echo hi".into(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert_eq!(question_count(&chat), 2); + assert_matches!( + op_rx.try_recv(), + Ok(Op::RunUserShellCommand { command }) if command == "echo hi" + ); +} + +#[tokio::test] +async fn queued_prompt_clears_questions_arriving_after_enqueue_when_it_starts() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.on_task_started(); + chat.add_async_questions("old", &questions()); + chat.bottom_pane + .set_composer_text("New prompt".into(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::from(KeyCode::Tab)); + assert_eq!(question_count(&chat), 0); + + chat.add_async_questions("late", &questions()); + assert_eq!(question_count(&chat), 2); + chat.turn_lifecycle.finish(); + chat.update_task_running_state(); + assert!(chat.maybe_send_next_queued_input()); + assert_answer(op_rx.try_recv().unwrap(), "New prompt"); + assert_eq!(question_count(&chat), 0); + chat.add_async_questions("late", &questions()); + assert_eq!(question_count(&chat), 0); +} + +#[tokio::test] +async fn queued_question_answer_preserves_other_questions_on_delivery() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.input_queue.suppress_queue_autosend = true; + chat.add_async_questions("old", &questions()); + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + chat.bottom_pane.handle_paste("answer".into()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_eq!(question_count(&chat), 1); + + chat.input_queue.suppress_queue_autosend = false; + assert!(chat.maybe_send_next_queued_input()); + assert_answer(op_rx.try_recv().unwrap(), "> Which way?\n\nanswer"); + assert_eq!(question_count(&chat), 1); +} + +#[tokio::test] +async fn recovered_question_answers_preserve_other_questions() { + for restore_thread in [false, true] { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.on_task_started(); + chat.add_async_questions("old", &questions()); + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + chat.bottom_pane.handle_paste("answer".into()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_answer(op_rx.try_recv().unwrap(), "> Which way?\n\nanswer"); + assert_eq!(question_count(&chat), 1); + chat.add_async_questions("late", &[question("Late?", /*options*/ None)]); + + if restore_thread { + let state = chat.capture_thread_input_state().unwrap(); + chat.restore_thread_input_state( + Some(state), + ThreadInputStateRestoreMode { + preserve_in_flight_turn: false, + }, + ); + } else { + assert!(chat.enqueue_rejected_steer()); + chat.turn_lifecycle.finish(); + chat.update_task_running_state(); + } + assert!(chat.maybe_send_next_queued_input()); + assert_answer(op_rx.try_recv().unwrap(), "> Which way?\n\nanswer"); + assert_eq!(question_count(&chat), 2); + } +} + +#[tokio::test] +async fn slash_and_mode_prompts_clear_unanswered_questions_after_delivery() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.handle_composer_input_result( + InputResult::CommandWithArgs(SlashCommand::Plan, "New plan".into(), Vec::new()), + /*had_modal_or_popup*/ false, + ); + assert_answer(op_rx.try_recv().unwrap(), "New plan"); + assert_eq!(question_count(&chat), 0); + + chat.input_queue.user_turn_pending_start = false; + chat.add_async_questions("next", &questions()); + let mode = collaboration_modes::default_mask(chat.model_catalog.as_ref()).unwrap(); + chat.submit_user_message_with_mode("Implement".to_string(), mode); + assert_answer(op_rx.try_recv().unwrap(), "Implement"); + assert_eq!(question_count(&chat), 0); +} + +#[tokio::test] +async fn accepted_review_clears_questions_but_rejected_review_preserves_them() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.handle_composer_input_result( + InputResult::CommandWithArgs(SlashCommand::Review, "check this".into(), Vec::new()), + /*had_modal_or_popup*/ false, + ); + assert_matches!( + op_rx.try_recv(), + Ok(Op::Review { + target: ReviewTarget::Custom { instructions } + }) if instructions == "check this" + ); + assert_eq!(question_count(&chat), 0); + + chat.add_async_questions("new", &questions()); + chat.blocks_direct_input = true; + chat.handle_composer_input_result( + InputResult::CommandWithArgs(SlashCommand::Review, "try again".into(), Vec::new()), + /*had_modal_or_popup*/ false, + ); + assert_eq!(question_count(&chat), 2); + assert!(op_rx.try_recv().is_err()); +} + +#[tokio::test] +async fn queued_model_slash_prompt_clears_questions_but_local_slash_command_does_not() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.on_task_started(); + chat.add_async_questions("old", &questions()); + for text in ["/model", "/goal clear"] { + chat.handle_composer_input_result( + InputResult::Queued { + text: text.into(), + text_elements: Vec::new(), + action: QueuedInputAction::ParseSlash, + pending_pastes: Vec::new(), + }, + /*had_modal_or_popup*/ false, + ); + assert_eq!(question_count(&chat), 2); + } + chat.handle_composer_input_result( + InputResult::Queued { + text: "/plan New work".into(), + text_elements: Vec::new(), + action: QueuedInputAction::ParseSlash, + pending_pastes: Vec::new(), + }, + /*had_modal_or_popup*/ false, + ); + assert_eq!(question_count(&chat), 0); + assert!(op_rx.try_recv().is_err()); + + chat.add_async_questions("new", &questions()); + chat.handle_composer_input_result( + InputResult::Queued { + text: "/diff explain the change".into(), + text_elements: Vec::new(), + action: QueuedInputAction::ParseSlash, + pending_pastes: Vec::new(), + }, + /*had_modal_or_popup*/ false, + ); + assert_eq!(question_count(&chat), 0); +} + +#[tokio::test] +async fn rejected_synchronous_literal_prompt_keeps_questions() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.set_model(""); + chat.handle_composer_input_result( + InputResult::Queued { + text: "literal prompt".into(), + text_elements: Vec::new(), + action: QueuedInputAction::Literal, + pending_pastes: Vec::new(), + }, + /*had_modal_or_popup*/ false, + ); + assert_eq!(question_count(&chat), 2); + assert!(op_rx.try_recv().is_err()); +} + +#[tokio::test] +async fn reconnect_preserves_buffered_question_answer_source() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + chat.bottom_pane.handle_paste("answer".into()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + assert_answer(op_rx.try_recv().unwrap(), "> Which way?\n\nanswer"); + assert_eq!(question_count(&chat), 1); + chat.pause_for_disconnect(); + let state = chat.capture_thread_input_state(); + + let (mut restored, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + restored.thread_id = Some(ThreadId::new()); + restored.restore_reconnected_input(state); + assert_eq!( + restored + .input_queue + .queued_user_messages + .front() + .unwrap() + .source, + UserMessageSource::QuestionAnswer, + ); + // Recovered input stays paused until the user elects to retry it. + restored.input_queue.recovered_queue = false; + assert!(restored.maybe_send_next_queued_input()); + assert_answer(op_rx.try_recv().unwrap(), "> Which way?\n\nanswer"); + assert_eq!(question_count(&restored), 1); +} + +#[tokio::test] +async fn blocked_follow_up_keeps_unanswered_questions() { + let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.add_async_questions("old", &questions()); + chat.set_model(""); + chat.bottom_pane + .set_composer_text("Blocked prompt".into(), Vec::new(), Vec::new()); + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + + assert_eq!(question_count(&chat), 2); + assert!(op_rx.try_recv().is_err()); +} + #[tokio::test] async fn disconnected_questions_remain_editable_without_sending() { let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await; @@ -376,7 +661,7 @@ async fn question_key_repeats_do_not_consume_another_question() { } #[tokio::test] -async fn question_queue_pop_becomes_an_ordinary_composer_draft() { +async fn question_queue_pop_becomes_an_ordinary_composer_draft_and_clears_questions_on_send() { let (mut chat, _rx, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; for text in ["older", "newer"] { chat.input_queue @@ -402,7 +687,8 @@ async fn question_queue_pop_becomes_an_ordinary_composer_draft() { chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); assert_answer(ops.try_recv().unwrap(), "older"); chat.handle_key_event(forward); - assert!(chat.bottom_pane.questions.as_ref().unwrap().expanded); + assert_eq!(question_count(&chat), 0); + assert!(!chat.bottom_pane.questions.as_ref().unwrap().expanded); assert!( !chat .bottom_pane diff --git a/codex-rs/tui/src/chatwidget/tests/review_mode.rs b/codex-rs/tui/src/chatwidget/tests/review_mode.rs index e5b643baa1..f947b364d2 100644 --- a/codex-rs/tui/src/chatwidget/tests/review_mode.rs +++ b/codex-rs/tui/src/chatwidget/tests/review_mode.rs @@ -379,8 +379,10 @@ async fn restore_thread_input_state_restores_pending_steers_without_downgrading_ questions: None, composer: None, safety_buffering_prompt: None, + safety_buffering_source: UserMessageSource::Prompt, pending_steers: VecDeque::from([expected_pending.clone()]), rejected_steers_queue, + rejected_steer_sources: VecDeque::new(), rejected_steer_history_records: VecDeque::new(), queued_user_messages, queued_user_message_history_records: VecDeque::new(), diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__questions_cleared_by_follow_up.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__questions_cleared_by_follow_up.snap new file mode 100644 index 0000000000..5fc368856d --- /dev/null +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__questions_tests__questions_cleared_by_follow_up.snap @@ -0,0 +1,5 @@ +--- +source: tui/src/chatwidget/tests/questions_tests.rs +expression: "render_bottom_popup(&chat, 80).lines().next().unwrap()" +--- +› Ask Codex to do anything diff --git a/codex-rs/tui/src/chatwidget/user_messages.rs b/codex-rs/tui/src/chatwidget/user_messages.rs index 01112aaf74..59dec1f6d2 100644 --- a/codex-rs/tui/src/chatwidget/user_messages.rs +++ b/codex-rs/tui/src/chatwidget/user_messages.rs @@ -58,11 +58,18 @@ pub(super) enum ShellEscapePolicy { Disallow, } +#[derive(Clone, Copy, Debug, Eq, PartialEq)] +pub(super) enum UserMessageSource { + Prompt, + QuestionAnswer, +} + #[derive(Debug, Clone, PartialEq)] pub(super) struct QueuedUserMessage { pub(super) user_message: UserMessage, pub(super) action: QueuedInputAction, pub(super) pending_pastes: Vec<(String, String)>, + pub(super) source: UserMessageSource, } impl QueuedUserMessage { @@ -71,6 +78,7 @@ impl QueuedUserMessage { user_message, action, pending_pastes: Vec::new(), + source: UserMessageSource::Prompt, } } @@ -125,8 +133,10 @@ pub(crate) struct ThreadInputState { pub(crate) questions: Option, pub(super) composer: Option, pub(super) safety_buffering_prompt: Option, + pub(super) safety_buffering_source: UserMessageSource, pub(crate) pending_steers: VecDeque, pub(super) rejected_steers_queue: VecDeque, + pub(super) rejected_steer_sources: VecDeque, pub(super) rejected_steer_history_records: VecDeque, pub(super) queued_user_messages: VecDeque, pub(super) queued_user_message_history_records: VecDeque, @@ -176,6 +186,7 @@ pub(crate) struct PendingSteer { pub(crate) client_id: String, pub(super) user_message: UserMessage, pub(super) history_record: UserMessageHistoryRecord, + pub(super) source: UserMessageSource, pub(super) compare_key: PendingSteerCompareKey, }