mirror of
https://github.com/openai/codex.git
synced 2026-11-05 20:10:33 +00:00
tui: resume queued replay after idle slash actions
Pause queued replay on local slash actions that need app-side updates or popup dismissal, then resume once the app event queue and bottom pane are idle. Add regression coverage for queued theme replay and popup-gated resume. Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
co-authored by
Codex
parent
bcb3555e70
commit
7ea03de12b
@@ -2410,6 +2410,13 @@ impl App {
|
||||
) {
|
||||
waiting_for_initial_session_configured = false;
|
||||
}
|
||||
// Some replayed slash commands pause queue draining until their app-side updates
|
||||
// or popup flows settle. Only resume once the app-event queue is fully drained so
|
||||
// multi-event commands (for example approvals updates) cannot interleave later
|
||||
// queued input.
|
||||
if app_event_rx.is_empty() {
|
||||
app.chat_widget.maybe_resume_queued_inputs_when_idle();
|
||||
}
|
||||
match control {
|
||||
AppRunControl::Continue => {}
|
||||
AppRunControl::Exit(reason) => break Ok(reason),
|
||||
@@ -2754,6 +2761,7 @@ impl App {
|
||||
self.chat_widget.handle_serialized_slash_command(draft);
|
||||
self.refresh_status_line();
|
||||
}
|
||||
AppEvent::BottomPaneViewCompleted => {}
|
||||
AppEvent::UpdateCollaborationMode(mask) => {
|
||||
self.chat_widget.set_collaboration_mask(mask);
|
||||
self.refresh_status_line();
|
||||
|
||||
@@ -190,6 +190,10 @@ pub(crate) enum AppEvent {
|
||||
/// draft is queued and replayed later through the same path as queued composer input.
|
||||
HandleSlashCommandDraft(UserMessage),
|
||||
|
||||
/// Notify the app that an interactive bottom-pane view finished, so queued replay can resume
|
||||
/// once the UI is idle again.
|
||||
BottomPaneViewCompleted,
|
||||
|
||||
/// Update the active collaboration mask in the running app and widget.
|
||||
UpdateCollaborationMode(CollaborationModeMask),
|
||||
|
||||
|
||||
@@ -1008,6 +1008,7 @@ impl BottomPane {
|
||||
fn on_active_view_complete(&mut self) {
|
||||
self.resume_status_timer_after_modal();
|
||||
self.set_composer_input_enabled(true, None);
|
||||
self.app_event_tx.send(AppEvent::BottomPaneViewCompleted);
|
||||
}
|
||||
|
||||
fn pause_status_timer_for_modal(&mut self) {
|
||||
|
||||
@@ -720,6 +720,7 @@ pub(crate) struct ChatWidget {
|
||||
// Set when commentary output completes; once stream queues go idle we restore the status row.
|
||||
pending_status_indicator_restore: bool,
|
||||
suppress_queue_autosend: bool,
|
||||
resume_queued_inputs_when_idle: bool,
|
||||
thread_id: Option<ThreadId>,
|
||||
thread_name: Option<String>,
|
||||
forked_from: Option<ThreadId>,
|
||||
@@ -877,6 +878,7 @@ impl ThreadComposerState {
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
enum QueueReplayControl {
|
||||
Continue,
|
||||
ResumeWhenIdle,
|
||||
Stop,
|
||||
}
|
||||
|
||||
@@ -3639,6 +3641,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
suppress_queue_autosend: false,
|
||||
resume_queued_inputs_when_idle: false,
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
@@ -3825,6 +3828,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
suppress_queue_autosend: false,
|
||||
resume_queued_inputs_when_idle: false,
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
@@ -4003,6 +4007,7 @@ impl ChatWidget {
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
suppress_queue_autosend: false,
|
||||
resume_queued_inputs_when_idle: false,
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
@@ -4322,9 +4327,11 @@ impl ChatWidget {
|
||||
///
|
||||
/// Live callers usually ignore the return value, but queued replay uses it to decide whether
|
||||
/// draining can continue after this command. `Continue` means the command only changed local
|
||||
/// UI/config state. `Stop` means it submitted or queued work, changed session/navigation
|
||||
/// state, or otherwise hit a boundary where queued draining must stop. Commands that require
|
||||
/// interactive UI are resolved before queueing and should not open that UI during replay.
|
||||
/// state synchronously inside `ChatWidget`. `ResumeWhenIdle` means queued replay should pause
|
||||
/// until app-side work or popup interaction finishes. `Stop` means it submitted or queued
|
||||
/// work, changed session/navigation state, or otherwise hit a boundary where queued draining
|
||||
/// must stop entirely. Commands that require interactive UI are resolved before queueing and
|
||||
/// should not open that UI during replay.
|
||||
fn dispatch_command(&mut self, cmd: SlashCommand) -> QueueReplayControl {
|
||||
if self.bottom_pane.is_task_running()
|
||||
&& !cmd.available_during_task()
|
||||
@@ -4931,7 +4938,7 @@ impl ChatWidget {
|
||||
self.app_event_tx.send(AppEvent::InsertHistoryCell(Box::new(
|
||||
history_cell::new_info_event(format!("Permissions updated to {label}"), None),
|
||||
)));
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::Agent | SlashCommand::MultiAgents => {
|
||||
match ThreadId::from_string(&args_message.text) {
|
||||
@@ -5001,7 +5008,7 @@ impl ChatWidget {
|
||||
}
|
||||
self.app_event_tx
|
||||
.send(AppEvent::UpdateFeatureFlags { updates });
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::Fast => match args_message.text.to_ascii_lowercase().as_str() {
|
||||
"on" => {
|
||||
@@ -5045,7 +5052,7 @@ impl ChatWidget {
|
||||
};
|
||||
self.app_event_tx
|
||||
.send(AppEvent::OpenFeedbackConsent { category });
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::Model => match Self::parse_model_selection_args(&args_message.text) {
|
||||
Ok((model, effort, scope)) => {
|
||||
@@ -5181,16 +5188,16 @@ impl ChatWidget {
|
||||
};
|
||||
self.app_event_tx
|
||||
.send(AppEvent::PersistRealtimeAudioDeviceSelection { kind, name });
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::Skills => match args_message.text.trim().to_ascii_lowercase().as_str() {
|
||||
"list" => {
|
||||
self.open_skills_list();
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
"manage" => {
|
||||
self.app_event_tx.send(AppEvent::OpenManageSkillsPopup);
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
_ => self.restore_invalid_inline_slash_command(
|
||||
cmd,
|
||||
@@ -5223,7 +5230,7 @@ impl ChatWidget {
|
||||
}
|
||||
};
|
||||
self.app_event_tx.send(AppEvent::StatusLineSetup { items });
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::Theme => {
|
||||
if crate::render::highlight::resolve_theme_by_name(
|
||||
@@ -5241,7 +5248,7 @@ impl ChatWidget {
|
||||
self.app_event_tx.send(AppEvent::SyntaxThemeSelected {
|
||||
name: args_message.text.trim().to_string(),
|
||||
});
|
||||
QueueReplayControl::Stop
|
||||
QueueReplayControl::ResumeWhenIdle
|
||||
}
|
||||
SlashCommand::New
|
||||
| SlashCommand::Fork
|
||||
@@ -5563,12 +5570,25 @@ impl ChatWidget {
|
||||
{
|
||||
return QueueReplayControl::Continue;
|
||||
}
|
||||
return QueueReplayControl::Stop;
|
||||
return replay_control;
|
||||
}
|
||||
let args_message = Self::slash_command_args_message_from_draft(draft, rest_offset);
|
||||
self.execute_slash_command_with_args(cmd, args_message)
|
||||
}
|
||||
|
||||
pub(crate) fn maybe_resume_queued_inputs_when_idle(&mut self) {
|
||||
if !self.resume_queued_inputs_when_idle
|
||||
|| self.suppress_queue_autosend
|
||||
|| self.bottom_pane.is_task_running()
|
||||
|| !self.bottom_pane.no_modal_or_popup_active()
|
||||
{
|
||||
return;
|
||||
}
|
||||
|
||||
self.resume_queued_inputs_when_idle = false;
|
||||
self.drain_queued_inputs_until_blocked();
|
||||
}
|
||||
|
||||
fn slash_command_args_message_from_draft(
|
||||
draft: UserMessage,
|
||||
rest_offset: usize,
|
||||
@@ -6627,6 +6647,7 @@ impl ChatWidget {
|
||||
if self.bottom_pane.is_task_running() {
|
||||
return;
|
||||
}
|
||||
let mut resume_when_idle = false;
|
||||
while !self.bottom_pane.is_task_running() {
|
||||
let Some(queued_message) = self.queued_user_messages.pop_front() else {
|
||||
break;
|
||||
@@ -6637,12 +6658,19 @@ impl ChatWidget {
|
||||
self.submit_user_message(queued_message);
|
||||
QueueReplayControl::Stop
|
||||
};
|
||||
if replay_control == QueueReplayControl::Stop
|
||||
if replay_control == QueueReplayControl::Stop {
|
||||
break;
|
||||
}
|
||||
if replay_control == QueueReplayControl::ResumeWhenIdle
|
||||
|| !self.bottom_pane.no_modal_or_popup_active()
|
||||
{
|
||||
resume_when_idle = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
self.resume_queued_inputs_when_idle = resume_when_idle
|
||||
&& !self.bottom_pane.is_task_running()
|
||||
&& !self.queued_user_messages.is_empty();
|
||||
self.refresh_pending_input_preview();
|
||||
}
|
||||
|
||||
|
||||
@@ -1893,6 +1893,7 @@ async fn make_chatwidget_manual(
|
||||
retry_status_header: None,
|
||||
pending_status_indicator_restore: false,
|
||||
suppress_queue_autosend: false,
|
||||
resume_queued_inputs_when_idle: false,
|
||||
thread_id: None,
|
||||
thread_name: None,
|
||||
forked_from: None,
|
||||
@@ -8137,6 +8138,7 @@ async fn feedback_selection_popup_emits_serialized_slash_draft() {
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::HandleSlashCommandDraft(UserMessage { text, .. })) if text == "/feedback bug"
|
||||
);
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::BottomPaneViewCompleted));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -8151,6 +8153,7 @@ async fn skills_menu_emits_serialized_slash_drafts() {
|
||||
Ok(AppEvent::HandleSlashCommandDraft(UserMessage { text, .. }))
|
||||
if text == "/skills list"
|
||||
);
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::BottomPaneViewCompleted));
|
||||
|
||||
chat.open_skills_menu();
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE));
|
||||
@@ -8161,6 +8164,7 @@ async fn skills_menu_emits_serialized_slash_drafts() {
|
||||
Ok(AppEvent::HandleSlashCommandDraft(UserMessage { text, .. }))
|
||||
if text == "/skills manage"
|
||||
);
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::BottomPaneViewCompleted));
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
@@ -11678,6 +11682,153 @@ async fn queued_bare_theme_command_restores_to_composer_instead_of_opening_popup
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_theme_selection_resumes_followup_after_idle_resume() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.handle_codex_event(Event {
|
||||
id: "configured".into(),
|
||||
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
|
||||
session_id: ThreadId::new(),
|
||||
forked_from_id: None,
|
||||
thread_name: None,
|
||||
model: "test-model".to_string(),
|
||||
model_provider_id: "test-provider".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
cwd: PathBuf::from("/home/user/project"),
|
||||
reasoning_effort: Some(ReasoningEffortConfig::default()),
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: None,
|
||||
}),
|
||||
});
|
||||
chat.on_task_started();
|
||||
|
||||
chat.handle_serialized_slash_command(UserMessage::from("/theme ansi".to_string()));
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("followup".to_string()));
|
||||
|
||||
chat.on_task_complete(None, false);
|
||||
|
||||
assert_eq!(
|
||||
chat.queued_user_message_texts(),
|
||||
vec!["followup".to_string()]
|
||||
);
|
||||
assert!(chat.resume_queued_inputs_when_idle);
|
||||
loop {
|
||||
match rx.try_recv() {
|
||||
Ok(AppEvent::SyntaxThemeSelected { name }) => {
|
||||
assert_eq!(name, "ansi");
|
||||
break;
|
||||
}
|
||||
Ok(_) => continue,
|
||||
Err(TryRecvError::Empty) => panic!("expected AppEvent::SyntaxThemeSelected"),
|
||||
Err(TryRecvError::Disconnected) => {
|
||||
panic!("expected AppEvent::SyntaxThemeSelected")
|
||||
}
|
||||
}
|
||||
}
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
|
||||
chat.maybe_resume_queued_inputs_when_idle();
|
||||
|
||||
match next_submit_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: "followup".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
),
|
||||
other => panic!("expected Op::UserTurn, got {other:?}"),
|
||||
}
|
||||
assert!(chat.queued_user_messages.is_empty());
|
||||
assert!(!chat.resume_queued_inputs_when_idle);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_followup_waits_for_popup_dismissal_before_idle_resume() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
chat.handle_codex_event(Event {
|
||||
id: "configured".into(),
|
||||
msg: EventMsg::SessionConfigured(SessionConfiguredEvent {
|
||||
session_id: ThreadId::new(),
|
||||
forked_from_id: None,
|
||||
thread_name: None,
|
||||
model: "test-model".to_string(),
|
||||
model_provider_id: "test-provider".to_string(),
|
||||
service_tier: None,
|
||||
approval_policy: AskForApproval::Never,
|
||||
sandbox_policy: SandboxPolicy::new_read_only_policy(),
|
||||
cwd: PathBuf::from("/home/user/project"),
|
||||
reasoning_effort: Some(ReasoningEffortConfig::default()),
|
||||
history_log_id: 0,
|
||||
history_entry_count: 0,
|
||||
initial_messages: None,
|
||||
network_proxy: None,
|
||||
rollout_path: None,
|
||||
}),
|
||||
});
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("/theme ansi".to_string()));
|
||||
chat.queued_user_messages
|
||||
.push_back(UserMessage::from("followup".to_string()));
|
||||
|
||||
chat.drain_queued_inputs_until_blocked();
|
||||
|
||||
assert_eq!(
|
||||
chat.queued_user_message_texts(),
|
||||
vec!["followup".to_string()]
|
||||
);
|
||||
assert!(chat.resume_queued_inputs_when_idle);
|
||||
loop {
|
||||
match rx.try_recv() {
|
||||
Ok(AppEvent::SyntaxThemeSelected { name }) => {
|
||||
assert_eq!(name, "ansi");
|
||||
break;
|
||||
}
|
||||
Ok(_) => continue,
|
||||
Err(TryRecvError::Empty) => panic!("expected AppEvent::SyntaxThemeSelected"),
|
||||
Err(TryRecvError::Disconnected) => {
|
||||
panic!("expected AppEvent::SyntaxThemeSelected")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
chat.open_feedback_consent(crate::app_event::FeedbackCategory::Bug);
|
||||
chat.maybe_resume_queued_inputs_when_idle();
|
||||
|
||||
assert_eq!(
|
||||
chat.queued_user_message_texts(),
|
||||
vec!["followup".to_string()]
|
||||
);
|
||||
assert_no_submit_op(&mut op_rx);
|
||||
|
||||
let _ = chat
|
||||
.bottom_pane
|
||||
.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
assert!(!chat.has_active_view());
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::BottomPaneViewCompleted));
|
||||
|
||||
chat.maybe_resume_queued_inputs_when_idle();
|
||||
|
||||
match next_submit_op(&mut op_rx) {
|
||||
Op::UserTurn { items, .. } => assert_eq!(
|
||||
items,
|
||||
vec![UserInput::Text {
|
||||
text: "followup".to_string(),
|
||||
text_elements: Vec::new(),
|
||||
}]
|
||||
),
|
||||
other => panic!("expected Op::UserTurn, got {other:?}"),
|
||||
}
|
||||
assert!(chat.queued_user_messages.is_empty());
|
||||
assert!(!chat.resume_queued_inputs_when_idle);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn queued_custom_review_selection_preserves_branch_like_instructions_after_turn_complete() {
|
||||
let (mut chat, _rx, mut op_rx) = make_chatwidget_manual(None).await;
|
||||
|
||||
Reference in New Issue
Block a user