mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
Pause active goals before TUI interrupts (#28813)
Fixes #28104. ## Summary Active `/goal` turns should leave the persisted goal paused whenever the TUI interrupts the running turn. The bug in #28104 showed this most visibly through `Esc`: some interrupt paths aborted the turn without updating the goal status, so the goal could remain active and continue automatically. This change makes `ChatWidget` pause an active goal before the TUI sends an interrupt from the status-row path, the pending-steer path, `Ctrl+C`, or a request-user-input overlay. The modal overlay now reports whether a key will interrupt the turn, which keeps modal `Esc` and `Ctrl+C` behavior aligned with the normal interrupt paths. ## Manual Testing Built the local CLI with `just codex --help`, then launched the local TUI with goals enabled. Started an active `/goal` turn and interrupted it with `Esc`, then resumed and repeated with `Ctrl+C`; both paths showed `Goal paused`, the interrupted-conversation message, and the `Goal paused (/goal resume)` footer. I also stopped the background terminal and exited the TUI cleanly after the run. I did not find a reliable standalone manual path to force the request-user-input overlay case, so that path is covered by the focused automated test.
This commit is contained in:
committed by
GitHub
Unverified
parent
5670360009
commit
07298a948c
@@ -67,6 +67,11 @@ pub(crate) trait BottomPaneView: Renderable {
|
||||
false
|
||||
}
|
||||
|
||||
/// Return true when this key event will interrupt the active agent turn.
|
||||
fn will_interrupt_turn_on_key_event(&self, _key_event: KeyEvent) -> bool {
|
||||
false
|
||||
}
|
||||
|
||||
/// Optional paste handler. Return true if the view modified its state and
|
||||
/// needs a redraw.
|
||||
fn handle_paste(&mut self, _pasted: String) -> bool {
|
||||
|
||||
@@ -616,21 +616,10 @@ impl BottomPane {
|
||||
self.request_redraw();
|
||||
InputResult::None
|
||||
} else {
|
||||
let is_agent_command = self
|
||||
.composer_text()
|
||||
.lines()
|
||||
.next()
|
||||
.and_then(parse_slash_name)
|
||||
.is_some_and(|(name, _, _)| name == "agent");
|
||||
|
||||
// If a task is running and a status line is visible, allow the
|
||||
// configured action to interrupt even while the composer has focus.
|
||||
// When a popup is active, prefer dismissing it over interrupting the task.
|
||||
if self.keymap.chat.interrupt_turn.is_pressed(key_event)
|
||||
&& self.is_task_running
|
||||
&& !(is_agent_command && key_event.code == KeyCode::Esc)
|
||||
&& !self.composer.popup_active()
|
||||
&& !self.composer_should_handle_vim_insert_escape(key_event)
|
||||
if self.should_interrupt_running_task(key_event)
|
||||
&& let Some(status) = &self.status
|
||||
{
|
||||
// Send Op::Interrupt
|
||||
@@ -1306,6 +1295,22 @@ impl BottomPane {
|
||||
self.is_task_running
|
||||
}
|
||||
|
||||
pub(crate) fn should_interrupt_running_task(&self, key_event: KeyEvent) -> bool {
|
||||
let is_agent_command = self
|
||||
.composer_text()
|
||||
.lines()
|
||||
.next()
|
||||
.and_then(parse_slash_name)
|
||||
.is_some_and(|(name, _, _)| name == "agent");
|
||||
|
||||
self.keymap.chat.interrupt_turn.is_pressed(key_event)
|
||||
&& self.is_task_running
|
||||
&& !(is_agent_command && key_event.code == KeyCode::Esc)
|
||||
&& self.no_modal_or_popup_active()
|
||||
&& !self.composer_should_handle_vim_insert_escape(key_event)
|
||||
&& self.status.is_some()
|
||||
}
|
||||
|
||||
pub(crate) fn terminal_title_requires_action(&self) -> bool {
|
||||
self.active_view()
|
||||
.is_some_and(bottom_pane_view::BottomPaneView::terminal_title_requires_action)
|
||||
@@ -1315,6 +1320,13 @@ impl BottomPane {
|
||||
!self.view_stack.is_empty()
|
||||
}
|
||||
|
||||
pub(crate) fn active_view_will_interrupt_turn_on_key_event(&self, key_event: KeyEvent) -> bool {
|
||||
self.is_task_running
|
||||
&& self
|
||||
.active_view()
|
||||
.is_some_and(|view| view.will_interrupt_turn_on_key_event(key_event))
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
pub(crate) fn active_view_id(&self) -> Option<&'static str> {
|
||||
self.view_stack.last().and_then(|view| view.view_id())
|
||||
|
||||
@@ -1183,6 +1183,21 @@ impl BottomPaneView for RequestUserInputOverlay {
|
||||
true
|
||||
}
|
||||
|
||||
fn will_interrupt_turn_on_key_event(&self, key_event: KeyEvent) -> bool {
|
||||
if KeyBinding::new(KeyCode::Char('c'), KeyModifiers::CONTROL).is_press(key_event) {
|
||||
return self.confirm_unanswered_active()
|
||||
|| !self.focus_is_notes()
|
||||
|| self.composer.current_text_with_pending().is_empty();
|
||||
}
|
||||
|
||||
key_event.kind != KeyEventKind::Release
|
||||
&& !self.confirm_unanswered_active()
|
||||
&& !(matches!(key_event.code, KeyCode::Esc)
|
||||
&& self.has_options()
|
||||
&& self.notes_ui_visible())
|
||||
&& self.interrupt_turn_keys.is_pressed(key_event)
|
||||
}
|
||||
|
||||
fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
if key_event.kind == KeyEventKind::Release {
|
||||
return;
|
||||
|
||||
@@ -17,7 +17,13 @@ impl ChatWidget {
|
||||
&& !key_hint::ctrl(KeyCode::Char('r')).is_press(key_event)
|
||||
&& !key_hint::ctrl(KeyCode::Char('u')).is_press(key_event)
|
||||
{
|
||||
let should_pause_active_goal = self
|
||||
.bottom_pane
|
||||
.active_view_will_interrupt_turn_on_key_event(key_event);
|
||||
self.bottom_pane.handle_key_event(key_event);
|
||||
if should_pause_active_goal {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
if self.bottom_pane.no_modal_or_popup_active() {
|
||||
self.maybe_send_next_queued_input();
|
||||
}
|
||||
@@ -133,7 +139,9 @@ impl ChatWidget {
|
||||
&& !self.should_handle_vim_insert_escape(key_event)
|
||||
{
|
||||
self.input_queue.submit_pending_steers_after_interrupt = true;
|
||||
if !self.submit_op(AppCommand::interrupt()) {
|
||||
if self.submit_op(AppCommand::interrupt()) {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
} else {
|
||||
self.input_queue.submit_pending_steers_after_interrupt = false;
|
||||
}
|
||||
return;
|
||||
@@ -165,7 +173,12 @@ impl ChatWidget {
|
||||
}
|
||||
_ => {
|
||||
let had_modal_or_popup = !self.bottom_pane.no_modal_or_popup_active();
|
||||
let should_pause_active_goal =
|
||||
self.bottom_pane.should_interrupt_running_task(key_event);
|
||||
let input_result = self.bottom_pane.handle_key_event(key_event);
|
||||
if should_pause_active_goal {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
self.handle_composer_input_result(input_result, had_modal_or_popup);
|
||||
}
|
||||
}
|
||||
@@ -360,6 +373,12 @@ impl ChatWidget {
|
||||
fn on_ctrl_c(&mut self) {
|
||||
let key = key_hint::ctrl(KeyCode::Char('c'));
|
||||
let modal_or_popup_active = !self.bottom_pane.no_modal_or_popup_active();
|
||||
let should_pause_active_goal = self
|
||||
.bottom_pane
|
||||
.active_view_will_interrupt_turn_on_key_event(KeyEvent::new(
|
||||
KeyCode::Char('c'),
|
||||
KeyModifiers::CONTROL,
|
||||
));
|
||||
if self.bottom_pane.on_ctrl_c() == CancellationEvent::Handled {
|
||||
if DOUBLE_PRESS_QUIT_SHORTCUT_ENABLED {
|
||||
if modal_or_popup_active {
|
||||
@@ -370,6 +389,9 @@ impl ChatWidget {
|
||||
self.arm_quit_shortcut(key);
|
||||
}
|
||||
}
|
||||
if should_pause_active_goal {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
@@ -378,8 +400,9 @@ impl ChatWidget {
|
||||
self.quit_shortcut_expires_at = None;
|
||||
self.quit_shortcut_key = None;
|
||||
self.bottom_pane.clear_quit_shortcut_hint();
|
||||
self.pause_active_goal_for_interrupt();
|
||||
self.submit_op(AppCommand::interrupt_and_restore_prompt_if_no_output());
|
||||
if self.submit_op(AppCommand::interrupt_and_restore_prompt_if_no_output()) {
|
||||
self.pause_active_goal_for_interrupt();
|
||||
}
|
||||
} else {
|
||||
self.request_quit_without_confirmation();
|
||||
}
|
||||
@@ -395,9 +418,10 @@ impl ChatWidget {
|
||||
|
||||
self.arm_quit_shortcut(key);
|
||||
|
||||
if self.is_cancellable_work_active() {
|
||||
if self.is_cancellable_work_active()
|
||||
&& self.submit_op(AppCommand::interrupt_and_restore_prompt_if_no_output())
|
||||
{
|
||||
self.pause_active_goal_for_interrupt();
|
||||
self.submit_op(AppCommand::interrupt_and_restore_prompt_if_no_output());
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
+11
@@ -0,0 +1,11 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/status_and_layout.rs
|
||||
expression: snapshot
|
||||
---
|
||||
" "
|
||||
"• Working (0s • esc to interrupt) "
|
||||
" "
|
||||
" "
|
||||
"› Ask Codex to do anything "
|
||||
" "
|
||||
" gpt-5.5 default · /tmp/project Goal paused (/goal resume) "
|
||||
+11
@@ -0,0 +1,11 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests/status_and_layout.rs
|
||||
expression: snapshot
|
||||
---
|
||||
" "
|
||||
"• Working (0s • esc to interrupt) "
|
||||
" "
|
||||
" "
|
||||
"› Ask Codex to do anything "
|
||||
" "
|
||||
" gpt-5.5 default · /tmp/project Goal paused (/goal resume) "
|
||||
@@ -1410,33 +1410,97 @@ async fn streaming_final_answer_keeps_task_running_state() {
|
||||
#[tokio::test]
|
||||
async fn ctrl_c_interrupt_pauses_active_goal_turn() {
|
||||
let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let thread_id = start_active_goal_turn(&mut chat);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
|
||||
next_interrupt_op(&mut op_rx);
|
||||
assert_goal_paused_event(&mut rx, thread_id);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn esc_interrupt_pauses_active_goal_turn() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
chat.show_welcome_banner = false;
|
||||
let thread_id = start_active_goal_turn(&mut chat);
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE));
|
||||
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt { .. })));
|
||||
assert_goal_paused_event(&mut rx, thread_id);
|
||||
|
||||
update_thread_goal(&mut chat, thread_id, AppThreadGoalStatus::Paused);
|
||||
let width = 80;
|
||||
let height = chat.desired_height(width);
|
||||
let mut terminal = ratatui::Terminal::new(TestBackend::new(width, height)).expect("terminal");
|
||||
terminal
|
||||
.draw(|f| chat.render(f.area(), f.buffer_mut()))
|
||||
.expect("draw goal paused footer");
|
||||
let snapshot = normalized_backend_snapshot(terminal.backend());
|
||||
#[cfg(target_os = "windows")]
|
||||
insta::with_settings!({ snapshot_suffix => "windows" }, {
|
||||
assert_chatwidget_snapshot!("esc_interrupt_goal_paused_footer", snapshot);
|
||||
});
|
||||
#[cfg(not(target_os = "windows"))]
|
||||
assert_chatwidget_snapshot!("esc_interrupt_goal_paused_footer", snapshot);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn request_user_input_interrupt_pauses_active_goal_turn() {
|
||||
for key_event in [
|
||||
KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE),
|
||||
KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL),
|
||||
] {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
|
||||
let thread_id = start_active_goal_turn(&mut chat);
|
||||
chat.handle_request_user_input_now(ToolRequestUserInputParams {
|
||||
thread_id: thread_id.to_string(),
|
||||
item_id: "call-1".to_string(),
|
||||
turn_id: "turn-1".to_string(),
|
||||
questions: Vec::new(),
|
||||
auto_resolution_ms: None,
|
||||
});
|
||||
|
||||
chat.handle_key_event(key_event);
|
||||
|
||||
assert_matches!(rx.try_recv(), Ok(AppEvent::CodexOp(Op::Interrupt { .. })));
|
||||
assert_goal_paused_event(&mut rx, thread_id);
|
||||
}
|
||||
}
|
||||
|
||||
fn start_active_goal_turn(chat: &mut ChatWidget) -> ThreadId {
|
||||
let thread_id = ThreadId::new();
|
||||
chat.set_feature_enabled(Feature::Goals, /*enabled*/ true);
|
||||
chat.thread_id = Some(thread_id);
|
||||
update_thread_goal(chat, thread_id, AppThreadGoalStatus::Active);
|
||||
chat.on_task_started();
|
||||
thread_id
|
||||
}
|
||||
|
||||
fn update_thread_goal(chat: &mut ChatWidget, thread_id: ThreadId, status: AppThreadGoalStatus) {
|
||||
let mut goal = test_thread_goal(
|
||||
codex_app_server_protocol::ThreadGoalStatus::Active,
|
||||
status,
|
||||
/*token_budget*/ Some(50_000),
|
||||
/*tokens_used*/ 40_000,
|
||||
);
|
||||
goal.thread_id = thread_id.to_string();
|
||||
let thread_id = thread_id.to_string();
|
||||
goal.thread_id = thread_id.clone();
|
||||
chat.handle_server_notification(
|
||||
ServerNotification::ThreadGoalUpdated(
|
||||
codex_app_server_protocol::ThreadGoalUpdatedNotification {
|
||||
thread_id: thread_id.to_string(),
|
||||
thread_id,
|
||||
turn_id: None,
|
||||
goal,
|
||||
},
|
||||
),
|
||||
/*replay_kind*/ None,
|
||||
);
|
||||
chat.on_task_started();
|
||||
}
|
||||
|
||||
chat.handle_key_event(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL));
|
||||
|
||||
match op_rx.try_recv() {
|
||||
Ok(Op::Interrupt { .. }) => {}
|
||||
other => panic!("expected Op::Interrupt, got {other:?}"),
|
||||
}
|
||||
fn assert_goal_paused_event(
|
||||
rx: &mut tokio::sync::mpsc::UnboundedReceiver<AppEvent>,
|
||||
thread_id: ThreadId,
|
||||
) {
|
||||
assert_matches!(
|
||||
rx.try_recv(),
|
||||
Ok(AppEvent::SetThreadGoalStatus {
|
||||
|
||||
Reference in New Issue
Block a user