single control flow for both Esc and Ctrl+C (#2691)
Esc and Ctrl+C while a task is running should do the same thing. There were some cases where pressing Esc would leave a "stuck" widget in the history; this fixes that and cleans up the logic so there's just one path for interrupting the task. Also clean up some subtly mishandled key events (e.g. Ctrl+D would quit the app while an approval modal was showing if the textarea was empty). --------- Co-authored-by: Ahmed Ibrahim <aibrahim@openai.com>
This commit is contained in:
@@ -276,22 +276,6 @@ impl App {
|
||||
|
||||
async fn handle_key_event(&mut self, tui: &mut tui::Tui, key_event: KeyEvent) {
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('c'),
|
||||
modifiers: crossterm::event::KeyModifiers::CONTROL,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
} => {
|
||||
self.chat_widget.on_ctrl_c();
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('d'),
|
||||
modifiers: crossterm::event::KeyModifiers::CONTROL,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
} if self.chat_widget.composer_is_empty() => {
|
||||
self.app_event_tx.send(AppEvent::ExitRequest);
|
||||
}
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('t'),
|
||||
modifiers: crossterm::event::KeyModifiers::CONTROL,
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
use codex_core::protocol::TokenUsage;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEvent;
|
||||
use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use ratatui::buffer::Buffer;
|
||||
use ratatui::layout::Constraint;
|
||||
@@ -657,6 +658,15 @@ impl ChatComposer {
|
||||
/// Handle key event when no popup is visible.
|
||||
fn handle_key_event_without_popup(&mut self, key_event: KeyEvent) -> (InputResult, bool) {
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('d'),
|
||||
modifiers: crossterm::event::KeyModifiers::CONTROL,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
} if self.is_empty() => {
|
||||
self.app_event_tx.send(AppEvent::ExitRequest);
|
||||
(InputResult::None, true)
|
||||
}
|
||||
// -------------------------------------------------------------
|
||||
// History navigation (Up / Down) – only when the composer is not
|
||||
// empty or when the cursor is at the correct position, to avoid
|
||||
|
||||
@@ -244,6 +244,9 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
fn on_error(&mut self, message: String) {
|
||||
// Before emitting the error message, finalize the active exec as failed
|
||||
// so spinners are replaced with a red ✗ marker.
|
||||
self.finalize_active_exec_cell_as_failed();
|
||||
self.add_to_history(history_cell::new_error_event(message));
|
||||
self.bottom_pane.set_task_running(false);
|
||||
self.running_commands.clear();
|
||||
@@ -534,17 +537,7 @@ impl ChatWidget {
|
||||
ev.result,
|
||||
));
|
||||
}
|
||||
fn interrupt_running_task(&mut self) {
|
||||
if self.bottom_pane.is_task_running() {
|
||||
self.active_exec_cell = None;
|
||||
self.running_commands.clear();
|
||||
self.bottom_pane.clear_ctrl_c_quit_hint();
|
||||
self.submit_op(Op::Interrupt);
|
||||
self.bottom_pane.set_task_running(false);
|
||||
self.stream.clear_all();
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
|
||||
fn layout_areas(&self, area: Rect) -> [Rect; 2] {
|
||||
Layout::vertical([
|
||||
Constraint::Max(
|
||||
@@ -657,48 +650,57 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
pub(crate) fn handle_key_event(&mut self, key_event: KeyEvent) {
|
||||
if key_event.kind == KeyEventKind::Press {
|
||||
self.bottom_pane.clear_ctrl_c_quit_hint();
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::Char('c'),
|
||||
modifiers: crossterm::event::KeyModifiers::CONTROL,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
} => {
|
||||
self.on_ctrl_c();
|
||||
return;
|
||||
}
|
||||
other if other.kind == KeyEventKind::Press => {
|
||||
self.bottom_pane.clear_ctrl_c_quit_hint();
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
// Alt+Up: Edit the most recent queued user message (if any).
|
||||
if matches!(
|
||||
key_event,
|
||||
match key_event {
|
||||
KeyEvent {
|
||||
code: KeyCode::Up,
|
||||
modifiers: KeyModifiers::ALT,
|
||||
kind: KeyEventKind::Press,
|
||||
..
|
||||
}
|
||||
) && !self.queued_user_messages.is_empty()
|
||||
{
|
||||
// Prefer the most recently queued item.
|
||||
if let Some(user_message) = self.queued_user_messages.pop_back() {
|
||||
self.bottom_pane.set_composer_text(user_message.text);
|
||||
self.refresh_queued_user_messages();
|
||||
self.request_redraw();
|
||||
}
|
||||
return;
|
||||
}
|
||||
|
||||
match self.bottom_pane.handle_key_event(key_event) {
|
||||
InputResult::Submitted(text) => {
|
||||
// If a task is running, queue the user input to be sent after the turn completes.
|
||||
let user_message = UserMessage {
|
||||
text,
|
||||
image_paths: self.bottom_pane.take_recent_submission_images(),
|
||||
};
|
||||
if self.bottom_pane.is_task_running() {
|
||||
self.queued_user_messages.push_back(user_message);
|
||||
} if !self.queued_user_messages.is_empty() => {
|
||||
// Prefer the most recently queued item.
|
||||
if let Some(user_message) = self.queued_user_messages.pop_back() {
|
||||
self.bottom_pane.set_composer_text(user_message.text);
|
||||
self.refresh_queued_user_messages();
|
||||
} else {
|
||||
self.submit_user_message(user_message);
|
||||
self.request_redraw();
|
||||
}
|
||||
}
|
||||
InputResult::Command(cmd) => {
|
||||
self.dispatch_command(cmd);
|
||||
_ => {
|
||||
match self.bottom_pane.handle_key_event(key_event) {
|
||||
InputResult::Submitted(text) => {
|
||||
// If a task is running, queue the user input to be sent after the turn completes.
|
||||
let user_message = UserMessage {
|
||||
text,
|
||||
image_paths: self.bottom_pane.take_recent_submission_images(),
|
||||
};
|
||||
if self.bottom_pane.is_task_running() {
|
||||
self.queued_user_messages.push_back(user_message);
|
||||
self.refresh_queued_user_messages();
|
||||
} else {
|
||||
self.submit_user_message(user_message);
|
||||
}
|
||||
}
|
||||
InputResult::Command(cmd) => {
|
||||
self.dispatch_command(cmd);
|
||||
}
|
||||
InputResult::None => {}
|
||||
}
|
||||
}
|
||||
InputResult::None => {}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -948,6 +950,16 @@ impl ChatWidget {
|
||||
self.frame_requester.schedule_frame();
|
||||
}
|
||||
|
||||
/// Mark the active exec cell as failed (✗) and flush it into history.
|
||||
fn finalize_active_exec_cell_as_failed(&mut self) {
|
||||
if let Some(cell) = self.active_exec_cell.take() {
|
||||
let cell = cell.into_failed();
|
||||
// Insert finalized exec into history and keep grouping consistent.
|
||||
self.add_to_history(cell);
|
||||
self.last_history_was_exec = true;
|
||||
}
|
||||
}
|
||||
|
||||
// If idle and there are queued inputs, submit exactly one to start the next turn.
|
||||
fn maybe_send_next_queued_input(&mut self) {
|
||||
if self.bottom_pane.is_task_running() {
|
||||
@@ -1102,22 +1114,15 @@ impl ChatWidget {
|
||||
}
|
||||
|
||||
/// Handle Ctrl-C key press.
|
||||
/// Returns CancellationEvent::Handled if the event was consumed by the UI, or
|
||||
/// CancellationEvent::Ignored if the caller should handle it (e.g. exit).
|
||||
pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent {
|
||||
match self.bottom_pane.on_ctrl_c() {
|
||||
CancellationEvent::Handled => return CancellationEvent::Handled,
|
||||
CancellationEvent::Ignored => {}
|
||||
}
|
||||
if self.bottom_pane.is_task_running() {
|
||||
self.interrupt_running_task();
|
||||
CancellationEvent::Ignored
|
||||
} else if self.bottom_pane.ctrl_c_quit_hint_visible() {
|
||||
self.submit_op(Op::Shutdown);
|
||||
CancellationEvent::Handled
|
||||
} else {
|
||||
self.bottom_pane.show_ctrl_c_quit_hint();
|
||||
CancellationEvent::Ignored
|
||||
fn on_ctrl_c(&mut self) {
|
||||
if self.bottom_pane.on_ctrl_c() == CancellationEvent::Ignored {
|
||||
if self.bottom_pane.is_task_running() {
|
||||
self.submit_op(Op::Interrupt);
|
||||
} else if self.bottom_pane.ctrl_c_quit_hint_visible() {
|
||||
self.submit_op(Op::Shutdown);
|
||||
} else {
|
||||
self.bottom_pane.show_ctrl_c_quit_hint();
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,6 @@
|
||||
---
|
||||
source: tui/src/chatwidget/tests.rs
|
||||
expression: exec_blob
|
||||
---
|
||||
>_
|
||||
✗ ⌨️ sleep 1
|
||||
@@ -370,6 +370,48 @@ fn exec_history_cell_shows_working_then_failed() {
|
||||
);
|
||||
}
|
||||
|
||||
// Snapshot test: interrupting a running exec finalizes the active cell with a red ✗
|
||||
// marker (replacing the spinner) and flushes it into history.
|
||||
#[test]
|
||||
fn interrupt_exec_marks_failed_snapshot() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
|
||||
|
||||
// Begin a long-running command so we have an active exec cell with a spinner.
|
||||
chat.handle_codex_event(Event {
|
||||
id: "call-int".into(),
|
||||
msg: EventMsg::ExecCommandBegin(ExecCommandBeginEvent {
|
||||
call_id: "call-int".into(),
|
||||
command: vec!["bash".into(), "-lc".into(), "sleep 1".into()],
|
||||
cwd: std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")),
|
||||
parsed_cmd: vec![
|
||||
codex_core::parse_command::ParsedCommand::Unknown {
|
||||
cmd: "sleep 1".into(),
|
||||
}
|
||||
.into(),
|
||||
],
|
||||
}),
|
||||
});
|
||||
|
||||
// Simulate the task being aborted (as if ESC was pressed), which should
|
||||
// cause the active exec cell to be finalized as failed and flushed.
|
||||
chat.handle_codex_event(Event {
|
||||
id: "call-int".into(),
|
||||
msg: EventMsg::TurnAborted(codex_core::protocol::TurnAbortedEvent {
|
||||
reason: TurnAbortReason::Interrupted,
|
||||
}),
|
||||
});
|
||||
|
||||
let cells = drain_insert_history(&mut rx);
|
||||
assert!(
|
||||
!cells.is_empty(),
|
||||
"expected finalized exec cell to be inserted into history"
|
||||
);
|
||||
|
||||
// The first inserted cell should be the finalized exec; snapshot its text.
|
||||
let exec_blob = lines_to_single_string(&cells[0]);
|
||||
assert_snapshot!("interrupt_exec_marks_failed", exec_blob);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn exec_history_extends_previous_when_consecutive() {
|
||||
let (mut chat, mut rx, _op_rx) = make_chatwidget_manual();
|
||||
|
||||
@@ -175,6 +175,26 @@ impl WidgetRef for &ExecCell {
|
||||
}
|
||||
}
|
||||
|
||||
impl ExecCell {
|
||||
/// Convert an active exec cell into a failed, completed exec cell.
|
||||
/// Replaces the spinner with a red ✗ and sets a zero/elapsed duration.
|
||||
pub(crate) fn into_failed(mut self) -> ExecCell {
|
||||
let elapsed = self
|
||||
.start_time
|
||||
.map(|st| st.elapsed())
|
||||
.unwrap_or_else(|| Duration::from_millis(0));
|
||||
self.start_time = None;
|
||||
self.duration = Some(elapsed);
|
||||
self.output = Some(CommandOutput {
|
||||
exit_code: 1,
|
||||
stdout: String::new(),
|
||||
stderr: String::new(),
|
||||
formatted_output: String::new(),
|
||||
});
|
||||
self
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug)]
|
||||
struct CompletedMcpToolCallWithImageOutput {
|
||||
_image: DynamicImage,
|
||||
|
||||
Reference in New Issue
Block a user