mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Allow resume and settings commands during tasks and MCP startup (#29154)
## Why The TUI treats both an active turn and MCP startup as a running task. That currently blocks `/resume` and several settings commands even though they do not compete with turn execution, which is especially frustrating when MCP startup is slow. Model, permissions, personality, and service-tier selections already update thread settings independently of the running turn. Other clients can send those updates mid-turn, while the current turn continues with its captured settings. Allowing the same updates from local slash commands makes the TUI consistent with that existing behavior. ## What changed - Allow `/resume` while a task is running. - Allow `/model`, `/permissions`, `/personality`, and service-tier commands such as `/fast` while a task is running. - Keep the existing behavior where the active turn uses its captured settings and updates apply to subsequent turns. - Exercise the commands under the busy state in the existing TUI tests and retain coverage for commands that should remain blocked. ## Behavior note Turn settings such as model selection and reasoning effort are captured when a turn starts. Changing them during an active turn affects the next turn, not the turn already in progress. The status bar updates immediately, so it may temporarily display the newly selected setting before that setting is actually in effect. ## Verification - Focused `codex-tui` tests for resume dispatch, settings popups, `/fast`, and disabled-command behavior. ## Related issues Closes #19015. Addresses the next-turn-safe model/reasoning switching portion of #14356; dedicated shortcuts and the proposed depth meter remain out of scope.
This commit is contained in:
committed by
GitHub
Unverified
parent
64bdeed9f7
commit
d667082322
@@ -892,6 +892,16 @@ impl App {
|
||||
self.sync_active_thread_personality_setting(app_server, personality)
|
||||
.await;
|
||||
}
|
||||
AppEvent::SettingsSelectionClosed => {
|
||||
self.app_event_tx.send(AppEvent::SettingsSelectionSettled);
|
||||
}
|
||||
AppEvent::SettingsSelectionSettled => {
|
||||
if self.chat_widget.no_modal_or_popup_active() {
|
||||
self.chat_widget
|
||||
.set_queue_autosend_suppressed(/*suppressed*/ false);
|
||||
self.chat_widget.maybe_send_next_queued_input();
|
||||
}
|
||||
}
|
||||
AppEvent::OpenReasoningPopup { model } => {
|
||||
self.chat_widget.open_reasoning_popup(model);
|
||||
}
|
||||
|
||||
@@ -694,6 +694,11 @@ pub(crate) enum AppEvent {
|
||||
/// Update the current personality in the running app and widget.
|
||||
UpdatePersonality(Personality),
|
||||
|
||||
/// Finish a settings selection after its preceding update events have been applied.
|
||||
SettingsSelectionClosed,
|
||||
/// Run after any nested settings events emitted while handling the close event.
|
||||
SettingsSelectionSettled,
|
||||
|
||||
/// Persist the selected model and reasoning effort to the appropriate config.
|
||||
PersistModelSelection {
|
||||
model: String,
|
||||
|
||||
@@ -48,7 +48,7 @@ impl SlashCommandItem {
|
||||
pub(crate) fn available_during_task(&self) -> bool {
|
||||
match self {
|
||||
Self::Builtin(cmd) => cmd.available_during_task(),
|
||||
Self::ServiceTier(_) => false,
|
||||
Self::ServiceTier(_) => true,
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -24,8 +24,9 @@ impl ChatWidget {
|
||||
{
|
||||
return;
|
||||
}
|
||||
let should_submit_now =
|
||||
self.is_session_configured() && !self.is_plan_streaming_in_tui();
|
||||
let should_submit_now = self.is_session_configured()
|
||||
&& !self.is_plan_streaming_in_tui()
|
||||
&& !self.input_queue.suppress_queue_autosend;
|
||||
if should_submit_now {
|
||||
if self.only_user_shell_commands_running()
|
||||
&& !user_message.text.starts_with('!')
|
||||
@@ -69,6 +70,20 @@ impl ChatWidget {
|
||||
self.refresh_plan_mode_nudge();
|
||||
}
|
||||
|
||||
pub(super) fn defer_input_until_settings_applied(&mut self) {
|
||||
if !self.bottom_pane.no_modal_or_popup_active() {
|
||||
self.input_queue.suppress_queue_autosend = true;
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn on_modal_or_popup_closed(&mut self) {
|
||||
if self.input_queue.suppress_queue_autosend {
|
||||
self.app_event_tx.send(AppEvent::SettingsSelectionClosed);
|
||||
} else {
|
||||
self.maybe_send_next_queued_input();
|
||||
}
|
||||
}
|
||||
|
||||
pub(super) fn queue_user_message(&mut self, user_message: UserMessage) {
|
||||
self.queue_user_message_with_options(user_message, QueuedInputAction::Plain, Vec::new());
|
||||
}
|
||||
@@ -84,7 +99,10 @@ impl ChatWidget {
|
||||
action: QueuedInputAction,
|
||||
pending_pastes: Vec<(String, String)>,
|
||||
) {
|
||||
if !self.is_session_configured() || self.is_user_turn_pending_or_running() {
|
||||
if !self.is_session_configured()
|
||||
|| self.is_user_turn_pending_or_running()
|
||||
|| self.input_queue.suppress_queue_autosend
|
||||
{
|
||||
self.input_queue
|
||||
.queued_user_messages
|
||||
.push_back(QueuedUserMessage {
|
||||
|
||||
@@ -25,7 +25,7 @@ impl ChatWidget {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
if self.bottom_pane.no_modal_or_popup_active() {
|
||||
self.maybe_send_next_queued_input();
|
||||
self.on_modal_or_popup_closed();
|
||||
}
|
||||
return;
|
||||
}
|
||||
@@ -392,6 +392,9 @@ impl ChatWidget {
|
||||
if should_pause_active_goal {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
if modal_or_popup_active && self.bottom_pane.no_modal_or_popup_active() {
|
||||
self.on_modal_or_popup_closed();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
|
||||
@@ -131,6 +131,13 @@ impl ChatWidget {
|
||||
.send(AppEvent::RawOutputModeChanged { enabled });
|
||||
}
|
||||
|
||||
fn slash_command_blocked_by_active_task(&self, cmd: SlashCommand) -> bool {
|
||||
(!cmd.available_during_task() && self.bottom_pane.is_task_running())
|
||||
|| (cmd == SlashCommand::Resume
|
||||
&& (self.input_queue.user_turn_pending_start
|
||||
|| self.turn_lifecycle.agent_turn_running))
|
||||
}
|
||||
|
||||
pub(super) fn dispatch_command(&mut self, cmd: SlashCommand) {
|
||||
if !self.ensure_slash_command_allowed_in_side_conversation(cmd) {
|
||||
return;
|
||||
@@ -138,7 +145,7 @@ impl ChatWidget {
|
||||
if !self.ensure_side_command_allowed_outside_review(cmd) {
|
||||
return;
|
||||
}
|
||||
if !cmd.available_during_task() && self.bottom_pane.is_task_running() {
|
||||
if self.slash_command_blocked_by_active_task(cmd) {
|
||||
let message = format!(
|
||||
"'/{}' is disabled while a task is in progress.",
|
||||
cmd.command()
|
||||
@@ -263,9 +270,11 @@ impl ChatWidget {
|
||||
}
|
||||
SlashCommand::Model => {
|
||||
self.open_model_popup();
|
||||
self.defer_input_until_settings_applied();
|
||||
}
|
||||
SlashCommand::Personality => {
|
||||
self.open_personality_popup();
|
||||
self.defer_input_until_settings_applied();
|
||||
}
|
||||
SlashCommand::Plan => {
|
||||
self.apply_plan_slash_command();
|
||||
@@ -293,6 +302,7 @@ impl ChatWidget {
|
||||
}
|
||||
SlashCommand::Permissions => {
|
||||
self.open_permissions_popup();
|
||||
self.defer_input_until_settings_applied();
|
||||
}
|
||||
SlashCommand::Vim => {
|
||||
self.toggle_vim_mode_and_notify();
|
||||
@@ -545,7 +555,7 @@ impl ChatWidget {
|
||||
self.dispatch_command(cmd);
|
||||
return;
|
||||
}
|
||||
if !cmd.available_during_task() && self.bottom_pane.is_task_running() {
|
||||
if self.slash_command_blocked_by_active_task(cmd) {
|
||||
let message = format!(
|
||||
"'/{}' is disabled while a task is in progress.",
|
||||
cmd.command()
|
||||
|
||||
+2
-2
@@ -1,5 +1,5 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
source: tui/src/chatwidget/tests/exec_flow.rs
|
||||
expression: blob
|
||||
---
|
||||
■ '/model' is disabled while a task is in progress.
|
||||
■ '/resume' is disabled while a task is in progress.
|
||||
|
||||
@@ -1069,10 +1069,10 @@ async fn user_message_during_user_shell_command_is_queued_not_steered() {
|
||||
async fn disabled_slash_command_while_task_running_snapshot() {
|
||||
// Build a chat widget and simulate an active task
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
|
||||
// Dispatch a command that is unavailable while a task runs (e.g., /model)
|
||||
chat.dispatch_command(SlashCommand::Model);
|
||||
// Resume remains available during MCP startup, but not while an agent turn is active.
|
||||
chat.dispatch_command(SlashCommand::Resume);
|
||||
|
||||
// Drain history and snapshot the rendered error line(s)
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
|
||||
@@ -337,8 +337,12 @@ async fn queued_bang_shell_waits_for_user_shell_completion_before_next_input() {
|
||||
assert!(chat.input_queue.queued_user_messages.is_empty());
|
||||
}
|
||||
|
||||
async fn assert_cancelled_queued_menu_drains_next_input(command: &str, expected_popup_text: &str) {
|
||||
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await;
|
||||
async fn assert_cancelled_queued_menu_drains_next_input(
|
||||
command: &str,
|
||||
expected_popup_text: &str,
|
||||
cancel_key: KeyEvent,
|
||||
) {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await;
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
|
||||
@@ -355,7 +359,14 @@ async fn assert_cancelled_queued_menu_drains_next_input(command: &str, expected_
|
||||
);
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
chat.handle_key_event(cancel_key);
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
assert!(
|
||||
std::iter::from_fn(|| rx.try_recv().ok())
|
||||
.any(|event| matches!(event, AppEvent::SettingsSelectionClosed))
|
||||
);
|
||||
chat.set_queue_autosend_suppressed(/*suppressed*/ false);
|
||||
chat.maybe_send_next_queued_input();
|
||||
|
||||
match next_submit_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
@@ -372,39 +383,65 @@ async fn assert_cancelled_queued_menu_drains_next_input(command: &str, expected_
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_slash_menu_cancel_drains_next_input() {
|
||||
assert_cancelled_queued_menu_drains_next_input("/model", "Select Model").await;
|
||||
assert_cancelled_queued_menu_drains_next_input("/permissions", "Update Model Permissions")
|
||||
.await;
|
||||
assert_cancelled_queued_menu_drains_next_input(
|
||||
"/model",
|
||||
"Select Model",
|
||||
KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE),
|
||||
)
|
||||
.await;
|
||||
assert_cancelled_queued_menu_drains_next_input(
|
||||
"/permissions",
|
||||
"Update Model Permissions",
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
)
|
||||
.await;
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_slash_menu_selection_drains_next_input() {
|
||||
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.2")).await;
|
||||
async fn queued_settings_selection_applies_before_next_input() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.3-codex")).await;
|
||||
chat.thread_id = Some(ThreadId::new());
|
||||
let mut preset = get_available_model(&chat, "gpt-5.4");
|
||||
preset.supported_reasoning_efforts.truncate(1);
|
||||
let selected_effort = preset.supported_reasoning_efforts[0].effort.clone();
|
||||
chat.model_catalog = std::sync::Arc::new(ModelCatalog::new(vec![preset]));
|
||||
handle_turn_started(&mut chat, "turn-1");
|
||||
|
||||
queue_composer_text_with_tab(&mut chat, "/permissions");
|
||||
queue_composer_text_with_tab(&mut chat, "/model");
|
||||
queue_composer_text_with_tab(&mut chat, "hello after selection");
|
||||
|
||||
complete_turn_with_message(&mut chat, "turn-1", Some("done"));
|
||||
|
||||
let popup = render_bottom_popup(&chat, /*width*/ 80);
|
||||
assert!(
|
||||
popup.contains("Update Model Permissions"),
|
||||
"expected permissions menu to open; popup:\n{popup}"
|
||||
popup.contains("Select Model and Effort"),
|
||||
"expected model menu to open; popup:\n{popup}"
|
||||
);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE));
|
||||
assert_matches!(op_rx.try_recv(), Err(TryRecvError::Empty));
|
||||
while let Ok(event) = rx.try_recv() {
|
||||
match event {
|
||||
AppEvent::OpenReasoningPopup { model } => chat.open_reasoning_popup(model),
|
||||
AppEvent::UpdateModel(model) => chat.set_model(&model),
|
||||
AppEvent::UpdateReasoningEffort(effort) => chat.set_reasoning_effort(effort),
|
||||
AppEvent::SettingsSelectionClosed => {
|
||||
chat.app_event_tx.send(AppEvent::SettingsSelectionSettled);
|
||||
}
|
||||
AppEvent::SettingsSelectionSettled if chat.no_modal_or_popup_active() => {
|
||||
chat.set_queue_autosend_suppressed(/*suppressed*/ false);
|
||||
chat.maybe_send_next_queued_input();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
}
|
||||
|
||||
match next_submit_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: "hello after selection".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
Op::UserTurn { model, effort, .. } => assert_eq!(
|
||||
(model, effort),
|
||||
("gpt-5.4".to_string(), Some(selected_effort))
|
||||
),
|
||||
other => panic!("expected queued message after permissions selection, got {other:?}"),
|
||||
other => panic!("expected queued message with updated model, got {other:?}"),
|
||||
}
|
||||
assert!(chat.input_queue.queued_user_messages.is_empty());
|
||||
}
|
||||
@@ -1595,7 +1632,7 @@ async fn unavailable_slash_command_is_available_from_local_recall() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
|
||||
submit_composer_text(&mut chat, "/model");
|
||||
submit_composer_text(&mut chat, "/review");
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
let rendered = cells
|
||||
@@ -1604,10 +1641,10 @@ async fn unavailable_slash_command_is_available_from_local_recall() {
|
||||
.collect::<Vec<_>>()
|
||||
.join("\n");
|
||||
assert!(
|
||||
rendered.contains("'/model' is disabled while a task is in progress."),
|
||||
rendered.contains("'/review' is disabled while a task is in progress."),
|
||||
"expected disabled-command message, got: {rendered:?}"
|
||||
);
|
||||
assert_eq!(recall_latest_after_clearing(&mut chat), "/model");
|
||||
assert_eq!(recall_latest_after_clearing(&mut chat), "/review");
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -2293,8 +2330,9 @@ async fn slash_memory_update_reports_stubbed_feature() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_resume_opens_picker() {
|
||||
async fn slash_resume_opens_picker_while_mcp_startup_is_running() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
|
||||
chat.dispatch_command(SlashCommand::Resume);
|
||||
|
||||
@@ -2350,8 +2388,9 @@ async fn slash_delete_confirmation_requests_current_thread_delete() {
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn slash_resume_with_arg_requests_named_session() {
|
||||
async fn slash_resume_with_arg_requests_named_session_while_mcp_startup_is_running() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
|
||||
chat.bottom_pane.set_composer_text(
|
||||
"/resume my-saved-thread".to_string(),
|
||||
@@ -2587,8 +2626,9 @@ async fn fast_slash_command_updates_and_persists_local_service_tier() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(Some("gpt-5.4")).await;
|
||||
set_fast_mode_test_catalog(&mut chat);
|
||||
chat.set_feature_enabled(Feature::FastMode, /*enabled*/ true);
|
||||
chat.bottom_pane.set_task_running(/*running*/ true);
|
||||
|
||||
chat.handle_service_tier_command_dispatch(fast_tier_command());
|
||||
submit_composer_text(&mut chat, "/fast");
|
||||
|
||||
let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::<Vec<_>>();
|
||||
assert!(
|
||||
|
||||
@@ -190,13 +190,9 @@ impl SlashCommand {
|
||||
SlashCommand::New
|
||||
| SlashCommand::Archive
|
||||
| SlashCommand::Delete
|
||||
| SlashCommand::Resume
|
||||
| SlashCommand::Fork
|
||||
| SlashCommand::Init
|
||||
| SlashCommand::Compact
|
||||
| SlashCommand::Model
|
||||
| SlashCommand::Personality
|
||||
| SlashCommand::Permissions
|
||||
| SlashCommand::Keymap
|
||||
| SlashCommand::Vim
|
||||
| SlashCommand::ElevateSandbox
|
||||
@@ -211,6 +207,10 @@ impl SlashCommand {
|
||||
| SlashCommand::MemoryDrop
|
||||
| SlashCommand::MemoryUpdate => false,
|
||||
SlashCommand::Diff
|
||||
| SlashCommand::Resume
|
||||
| SlashCommand::Model
|
||||
| SlashCommand::Personality
|
||||
| SlashCommand::Permissions
|
||||
| SlashCommand::Copy
|
||||
| SlashCommand::Raw
|
||||
| SlashCommand::Rename
|
||||
|
||||
Reference in New Issue
Block a user