mirror of
https://github.com/pchuan98/codex.git
synced 2026-07-01 00:31:56 +08:00
fix(tui): show shutdown feedback on exit (#23323)
## Why Ctrl+C can take a noticeable amount of time to finish when the TUI is waiting for the app-server thread shutdown path to complete. Before this change, the UI could look like it had not accepted the shutdown request because the composer and cursor remained in their normal interactive state during that wait. This PR makes the accepted shutdown visible immediately. It does not add an artificial sleep or change the shutdown timeout; it only draws one final feedback frame before continuing through the existing shutdown flow. ## What Changed - On `ExitMode::ShutdownFirst`, the TUI now renders shutdown feedback before awaiting the existing thread shutdown future. - The bottom pane disables composer input, which hides the cursor through the existing disabled-input cursor path. - The composer shows `Shutting down...` as the disabled input hint and suppresses footer content so the shutdown acknowledgement is not competing with shortcut/status text. - The logout path uses the same feedback path before shutting down. ## How to Test 1. Start Codex from this branch. 2. Press `Ctrl+C` to request shutdown. 3. If shutdown takes long enough to observe, confirm the composer changes to `› Shutting down...`, the cursor disappears, and no footer hint is rendered below it. 4. Regression check: repeat with text already typed in the composer and confirm the visible row still switches to `Shutting down...` while the draft remains preserved internally until the process exits. Targeted tests: - `cargo test -p codex-tui shutdown_in_progress_disables_input_and_uses_hint_without_footer` - `cargo test -p codex-tui bottom_pane::footer::tests::` ## Local Validation Note `cargo test -p codex-tui` still aborts in `app::tests::discard_side_thread_removes_agent_navigation_entry` with a stack overflow. That same test also failed when run alone locally, and the failure appears unrelated to this shutdown feedback path.
This commit is contained in:
committed by
GitHub
Unverified
parent
d335b00212
commit
bb43044cba
+45
-24
@@ -1195,30 +1195,8 @@ See the Codex keymap documentation for supported actions and examples."
|
||||
}
|
||||
// Allow widgets to process any pending timers before rendering.
|
||||
self.chat_widget.pre_draw_tick();
|
||||
let desired_height =
|
||||
self.chat_widget.desired_height(tui.terminal.size()?.width);
|
||||
let mut rendered_area = Rect::default();
|
||||
if terminal_resize_reflow_enabled {
|
||||
tui.draw_with_resize_reflow(desired_height, |frame| {
|
||||
let area = frame.area();
|
||||
rendered_area = area;
|
||||
self.chat_widget.render(area, frame.buffer);
|
||||
if let Some((x, y)) = self.chat_widget.cursor_pos(area) {
|
||||
frame.set_cursor_style(self.chat_widget.cursor_style(area));
|
||||
frame.set_cursor_position((x, y));
|
||||
}
|
||||
})?;
|
||||
} else {
|
||||
tui.draw(desired_height, |frame| {
|
||||
let area = frame.area();
|
||||
rendered_area = area;
|
||||
self.chat_widget.render(area, frame.buffer);
|
||||
if let Some((x, y)) = self.chat_widget.cursor_pos(area) {
|
||||
frame.set_cursor_style(self.chat_widget.cursor_style(area));
|
||||
frame.set_cursor_position((x, y));
|
||||
}
|
||||
})?;
|
||||
}
|
||||
let rendered_area =
|
||||
self.render_chat_widget_frame(tui, terminal_resize_reflow_enabled)?;
|
||||
if self.chat_widget.ambient_pet_image_enabled() {
|
||||
let terminal_size = tui.terminal.size()?;
|
||||
let ambient_pet_area = Rect::new(
|
||||
@@ -1253,6 +1231,49 @@ See the Codex keymap documentation for supported actions and examples."
|
||||
}
|
||||
Ok(AppRunControl::Continue)
|
||||
}
|
||||
|
||||
pub(super) fn show_shutdown_feedback(&mut self, tui: &mut tui::Tui) -> Result<()> {
|
||||
self.disable_ambient_pet_before_shutdown(tui)?;
|
||||
self.chat_widget.show_shutdown_in_progress();
|
||||
let terminal_resize_reflow_enabled = self.terminal_resize_reflow_enabled();
|
||||
if terminal_resize_reflow_enabled {
|
||||
self.handle_draw_pre_render(tui)?;
|
||||
}
|
||||
self.chat_widget.pre_draw_tick();
|
||||
self.render_chat_widget_frame(tui, terminal_resize_reflow_enabled)?;
|
||||
Ok(())
|
||||
}
|
||||
|
||||
fn render_chat_widget_frame(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
terminal_resize_reflow_enabled: bool,
|
||||
) -> Result<Rect> {
|
||||
let desired_height = self.chat_widget.desired_height(tui.terminal.size()?.width);
|
||||
let mut rendered_area = Rect::default();
|
||||
if terminal_resize_reflow_enabled {
|
||||
tui.draw_with_resize_reflow(desired_height, |frame| {
|
||||
let area = frame.area();
|
||||
rendered_area = area;
|
||||
self.chat_widget.render(area, frame.buffer);
|
||||
if let Some((x, y)) = self.chat_widget.cursor_pos(area) {
|
||||
frame.set_cursor_style(self.chat_widget.cursor_style(area));
|
||||
frame.set_cursor_position((x, y));
|
||||
}
|
||||
})?;
|
||||
} else {
|
||||
tui.draw(desired_height, |frame| {
|
||||
let area = frame.area();
|
||||
rendered_area = area;
|
||||
self.chat_widget.render(area, frame.buffer);
|
||||
if let Some((x, y)) = self.chat_widget.cursor_pos(area) {
|
||||
frame.set_cursor_style(self.chat_widget.cursor_style(area));
|
||||
frame.set_cursor_position((x, y));
|
||||
}
|
||||
})?;
|
||||
}
|
||||
Ok(rendered_area)
|
||||
}
|
||||
}
|
||||
|
||||
impl Drop for App {
|
||||
|
||||
@@ -296,10 +296,14 @@ impl App {
|
||||
self.chat_widget.on_commit_tick();
|
||||
}
|
||||
AppEvent::Exit(mode) => {
|
||||
if mode == ExitMode::ShutdownFirst {
|
||||
self.show_shutdown_feedback(tui)?;
|
||||
}
|
||||
return Ok(self.handle_exit_mode(app_server, mode).await);
|
||||
}
|
||||
AppEvent::Logout => match app_server.logout_account().await {
|
||||
Ok(()) => {
|
||||
self.show_shutdown_feedback(tui)?;
|
||||
return Ok(self
|
||||
.handle_exit_mode(app_server, ExitMode::ShutdownFirst)
|
||||
.await);
|
||||
|
||||
@@ -3,6 +3,22 @@
|
||||
use super::*;
|
||||
|
||||
impl App {
|
||||
pub(super) fn disable_ambient_pet_before_shutdown(&mut self, tui: &mut tui::Tui) -> Result<()> {
|
||||
self.chat_widget.disable_ambient_pet_for_session();
|
||||
if let Err(clear_err) = tui.clear_ambient_pet_image() {
|
||||
match clear_err {
|
||||
crate::pets::PetImageRenderError::Terminal(err) => return Err(err.into()),
|
||||
crate::pets::PetImageRenderError::Asset(err) => {
|
||||
tracing::warn!(
|
||||
error = %err,
|
||||
"failed to clear ambient pet image before shutdown feedback"
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
pub(super) fn handle_ambient_pet_image_render_error(
|
||||
&mut self,
|
||||
tui: &mut tui::Tui,
|
||||
|
||||
@@ -4130,6 +4130,15 @@ impl ChatComposer {
|
||||
}
|
||||
}
|
||||
|
||||
pub(crate) fn show_shutdown_in_progress(&mut self) {
|
||||
self.set_input_enabled(/*enabled*/ false, Some("Shutting down...".to_string()));
|
||||
self.footer.quit_shortcut_expires_at = None;
|
||||
self.footer.mode = FooterMode::ComposerEmpty;
|
||||
self.footer.hint_override = Some(Vec::new());
|
||||
self.footer.plan_mode_nudge_visible = false;
|
||||
self.footer.flash = None;
|
||||
}
|
||||
|
||||
pub fn set_task_running(&mut self, running: bool) {
|
||||
self.is_task_running = running;
|
||||
}
|
||||
@@ -4627,36 +4636,38 @@ impl ChatComposer {
|
||||
|
||||
let mut state = self.draft.textarea_state.borrow_mut();
|
||||
let textarea_is_empty = self.draft.textarea.text().is_empty() && !self.draft.is_bash_mode;
|
||||
if let Some(mask_char) = mask_char {
|
||||
self.draft
|
||||
.textarea
|
||||
.render_ref_masked(textarea_rect, buf, &mut state, mask_char);
|
||||
} else {
|
||||
let highlight_ranges = self.history_search_highlight_ranges();
|
||||
if highlight_ranges.is_empty() {
|
||||
StatefulWidgetRef::render_ref(
|
||||
&(&self.draft.textarea),
|
||||
textarea_rect,
|
||||
buf,
|
||||
&mut state,
|
||||
);
|
||||
if self.draft.input_enabled {
|
||||
if let Some(mask_char) = mask_char {
|
||||
self.draft
|
||||
.textarea
|
||||
.render_ref_masked(textarea_rect, buf, &mut state, mask_char);
|
||||
} else {
|
||||
let highlight_style =
|
||||
Style::default().add_modifier(Modifier::REVERSED | Modifier::BOLD);
|
||||
let highlights = highlight_ranges
|
||||
.into_iter()
|
||||
.map(|range| (range, highlight_style))
|
||||
.collect::<Vec<_>>();
|
||||
self.draft.textarea.render_ref_styled_with_highlights(
|
||||
textarea_rect,
|
||||
buf,
|
||||
&mut state,
|
||||
Style::default(),
|
||||
&highlights,
|
||||
);
|
||||
let highlight_ranges = self.history_search_highlight_ranges();
|
||||
if highlight_ranges.is_empty() {
|
||||
StatefulWidgetRef::render_ref(
|
||||
&(&self.draft.textarea),
|
||||
textarea_rect,
|
||||
buf,
|
||||
&mut state,
|
||||
);
|
||||
} else {
|
||||
let highlight_style =
|
||||
Style::default().add_modifier(Modifier::REVERSED | Modifier::BOLD);
|
||||
let highlights = highlight_ranges
|
||||
.into_iter()
|
||||
.map(|range| (range, highlight_style))
|
||||
.collect::<Vec<_>>();
|
||||
self.draft.textarea.render_ref_styled_with_highlights(
|
||||
textarea_rect,
|
||||
buf,
|
||||
&mut state,
|
||||
Style::default(),
|
||||
&highlights,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
if textarea_is_empty {
|
||||
if !self.draft.input_enabled || textarea_is_empty {
|
||||
let text = if self.draft.input_enabled {
|
||||
self.placeholder_text.as_str().to_string()
|
||||
} else {
|
||||
@@ -10684,4 +10695,41 @@ mod tests {
|
||||
};
|
||||
assert_eq!(composer.cursor_pos(area), None);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn shutdown_in_progress_disables_input_and_uses_hint_without_footer() {
|
||||
use ratatui::Terminal;
|
||||
use ratatui::backend::TestBackend;
|
||||
|
||||
let (tx, _rx) = unbounded_channel::<AppEvent>();
|
||||
let sender = AppEventSender::new(tx);
|
||||
let mut composer = ChatComposer::new(
|
||||
/*has_input_focus*/ true,
|
||||
sender,
|
||||
/*enhanced_keys_supported*/ false,
|
||||
"Ask Codex to do anything".to_string(),
|
||||
/*disable_paste_burst*/ false,
|
||||
);
|
||||
|
||||
composer.set_text_content("hello".to_string(), Vec::new(), Vec::new());
|
||||
composer.show_shutdown_in_progress();
|
||||
|
||||
assert!(!composer.input_enabled());
|
||||
assert_eq!(composer.current_text(), "hello");
|
||||
assert_eq!(composer.custom_footer_height(), Some(0));
|
||||
|
||||
let area = Rect {
|
||||
x: 0,
|
||||
y: 0,
|
||||
width: 40,
|
||||
height: 5,
|
||||
};
|
||||
assert_eq!(composer.cursor_pos(area), None);
|
||||
|
||||
let mut terminal = Terminal::new(TestBackend::new(40, 5)).expect("terminal");
|
||||
terminal
|
||||
.draw(|f| composer.render(f.area(), f.buffer_mut()))
|
||||
.unwrap();
|
||||
insta::assert_snapshot!("shutdown_in_progress", terminal.backend());
|
||||
}
|
||||
}
|
||||
|
||||
@@ -789,6 +789,12 @@ impl BottomPane {
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
pub(crate) fn show_shutdown_in_progress(&mut self) {
|
||||
self.view_stack.clear();
|
||||
self.composer.show_shutdown_in_progress();
|
||||
self.request_redraw();
|
||||
}
|
||||
|
||||
pub(crate) fn clear_composer_for_ctrl_c(&mut self) {
|
||||
if let Some(text) = self.composer.clear_for_ctrl_c() {
|
||||
if let Some(thread_id) = self.thread_id {
|
||||
|
||||
+9
@@ -0,0 +1,9 @@
|
||||
---
|
||||
source: tui/src/bottom_pane/chat_composer.rs
|
||||
expression: terminal.backend()
|
||||
---
|
||||
" "
|
||||
"› Shutting down... "
|
||||
" "
|
||||
" "
|
||||
" "
|
||||
@@ -1277,6 +1277,10 @@ impl ChatWidget {
|
||||
.send(AppEvent::Exit(ExitMode::ShutdownFirst));
|
||||
}
|
||||
|
||||
pub(crate) fn show_shutdown_in_progress(&mut self) {
|
||||
self.bottom_pane.show_shutdown_in_progress();
|
||||
}
|
||||
|
||||
fn request_redraw(&mut self) {
|
||||
self.frame_requester.schedule_frame();
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user