diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 27e191d8e..f5c2bd269 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -33,7 +33,8 @@ //! burst detection for actual paste streams. //! //! The burst detector can also be disabled (`disable_paste_burst`), which bypasses the state -//! machine and treats the key stream as normal typing. +//! machine and treats the key stream as normal typing. When toggling from enabled → disabled, the +//! composer flushes/clears any in-flight burst state so it cannot leak into subsequent input. //! //! For the detailed burst state machine, see `codex-rs/tui/src/bottom_pane/paste_burst.rs`. //! For a narrative overview of the combined state machine, see `docs/tui-chat-composer.md`. @@ -379,16 +380,27 @@ impl ChatComposer { /// `disable_paste_burst` is an escape hatch for terminals/platforms where the burst heuristic /// is unwanted or has already been handled elsewhere. /// - /// When enabling the flag we clear the burst classification window so subsequent input cannot - /// be incorrectly grouped into a previous burst. + /// When transitioning from enabled → disabled, we "defuse" any in-flight burst state so it + /// cannot affect subsequent normal typing: /// - /// This does not flush any in-progress buffer; callers should avoid toggling this mid-burst - /// (or should flush first). + /// - First, flush any held/buffered text immediately via + /// [`PasteBurst::flush_before_modified_input`], and feed it through `handle_paste(String)`. + /// This preserves user input and routes it through the same integration path as explicit + /// pastes (large-paste placeholders, image-path detection, and popup sync). + /// - Then clear the burst timing and Enter-suppression window via + /// [`PasteBurst::clear_after_explicit_paste`]. + /// + /// We intentionally do not use `clear_window_after_non_char()` here: it clears timing state + /// without emitting any buffered text, which can leave a non-empty buffer unable to flush + /// later (because `flush_if_due()` relies on `last_plain_char_time` to time out). pub(crate) fn set_disable_paste_burst(&mut self, disabled: bool) { let was_disabled = self.disable_paste_burst; self.disable_paste_burst = disabled; if disabled && !was_disabled { - self.paste_burst.clear_window_after_non_char(); + if let Some(pasted) = self.paste_burst.flush_before_modified_input() { + self.handle_paste(pasted); + } + self.paste_burst.clear_after_explicit_paste(); } } @@ -799,6 +811,15 @@ impl ChatComposer { /// the cursor to a UTF-8 char boundary before slicing `textarea.text()`. #[inline] fn handle_non_ascii_char(&mut self, input: KeyEvent) -> (InputResult, bool) { + if self.disable_paste_burst { + // When burst detection is disabled, treat IME/non-ASCII input as normal typing. + // In particular, do not retro-capture or buffer already-inserted prefix text. + self.textarea.input(input); + let text_after = self.textarea.text(); + self.pending_pastes + .retain(|(placeholder, _)| text_after.contains(placeholder)); + return (InputResult::None, true); + } if let KeyEvent { code: KeyCode::Char(ch), .. @@ -1372,7 +1393,7 @@ impl ChatComposer { .next() .unwrap_or("") .starts_with('/'); - if self.paste_burst.is_active() && !in_slash_context { + if !self.disable_paste_burst && self.paste_burst.is_active() && !in_slash_context { let now = Instant::now(); if self.paste_burst.append_newline_if_active(now) { return (InputResult::None, true); @@ -1381,10 +1402,11 @@ impl ChatComposer { // During a paste-like burst, treat Enter/Ctrl+Shift+Q as a newline instead of submit. let now = Instant::now(); - if self - .paste_burst - .newline_should_insert_instead_of_submit(now) - && !in_slash_context + if !in_slash_context + && !self.disable_paste_burst + && self + .paste_burst + .newline_should_insert_instead_of_submit(now) { self.textarea.insert_str("\n"); self.paste_burst.extend_window(now); @@ -1580,6 +1602,7 @@ impl ChatComposer { // If we're capturing a burst and receive Enter, accumulate it instead of inserting. if matches!(input.code, KeyCode::Enter) + && !self.disable_paste_burst && self.paste_burst.is_active() && self.paste_burst.append_newline_if_active(now) { @@ -1598,7 +1621,7 @@ impl ChatComposer { } = input { let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); - if !has_ctrl_or_alt { + if !has_ctrl_or_alt && !self.disable_paste_burst { // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts, so avoid // holding the first char while still allowing burst detection for paste input. if !ch.is_ascii() { @@ -1644,6 +1667,17 @@ impl ChatComposer { } } + // Flush any buffered burst before applying a non-char input (arrow keys, etc). + // + // `clear_window_after_non_char()` clears `last_plain_char_time`. If we cleared that while + // `PasteBurst.buffer` is non-empty, `flush_if_due()` would no longer have a timestamp to + // time out against, and the buffered paste could remain stuck until another plain char + // arrives. + if !matches!(input.code, KeyCode::Char(_) | KeyCode::Enter) + && let Some(pasted) = self.paste_burst.flush_before_modified_input() + { + self.handle_paste(pasted); + } // Backspace at the start of an image placeholder should delete that placeholder (rather // than deleting content before it). Do this without scanning the full text by consulting // the textarea's element list. @@ -2921,6 +2955,104 @@ mod tests { assert_eq!(composer.textarea.text(), "hi\nthere"); } + /// Behavior: even if Enter suppression would normally be active for a burst, Enter should + /// still dispatch a built-in slash command when the first line begins with `/`. + #[test] + fn slash_context_enter_ignores_paste_burst_enter_suppression() { + use crate::slash_command::SlashCommand; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer.textarea.set_text("/diff"); + composer.textarea.set_cursor("/diff".len()); + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Command(SlashCommand::Diff))); + } + + /// Behavior: if a burst is buffering text and the user presses a non-char key, flush the + /// buffered burst *before* applying that key so the buffer cannot get stuck. + #[test] + fn non_char_key_flushes_active_burst_before_input() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active burst so we can deterministically buffer characters without relying on + // timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)); + assert!(composer.textarea.text().is_empty()); + assert!(composer.is_in_paste_burst()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "hi"); + assert_eq!(composer.textarea.cursor(), 1); + assert!(!composer.is_in_paste_burst()); + } + + /// Behavior: enabling `disable_paste_burst` flushes any held first character (flicker + /// suppression) and then inserts subsequent chars immediately without creating burst state. + #[test] + fn disable_paste_burst_flushes_pending_first_char_and_inserts_immediately() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // First ASCII char is normally held briefly. Flip the config mid-stream and ensure the + // held char is not dropped. + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE)); + assert!(composer.is_in_paste_burst()); + assert!(composer.textarea.text().is_empty()); + + composer.set_disable_paste_burst(true); + assert_eq!(composer.textarea.text(), "a"); + assert!(!composer.is_in_paste_burst()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "ab"); + assert!(!composer.is_in_paste_burst()); + } + /// Behavior: a small explicit paste inserts text directly (no placeholder), and the submitted /// text matches what is visible in the textarea. #[test] diff --git a/codex-rs/tui2/src/bottom_pane/chat_composer.rs b/codex-rs/tui2/src/bottom_pane/chat_composer.rs index df8016a16..b8648b1eb 100644 --- a/codex-rs/tui2/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui2/src/bottom_pane/chat_composer.rs @@ -32,7 +32,8 @@ //! burst detection for actual paste streams. //! //! The burst detector can also be disabled (`disable_paste_burst`), which bypasses the state -//! machine and treats the key stream as normal typing. +//! machine and treats the key stream as normal typing. When toggling from enabled → disabled, the +//! composer flushes/clears any in-flight burst state so it cannot leak into subsequent input. //! //! For the detailed burst state machine, see `codex-rs/tui2/src/bottom_pane/paste_burst.rs`. //! For a narrative overview of the combined state machine, see `docs/tui-chat-composer.md`. @@ -391,16 +392,27 @@ impl ChatComposer { /// `disable_paste_burst` is an escape hatch for terminals/platforms where the burst heuristic /// is unwanted or has already been handled elsewhere. /// - /// When enabling the flag we clear the burst classification window so subsequent input cannot - /// be incorrectly grouped into a previous burst. + /// When transitioning from enabled → disabled, we "defuse" any in-flight burst state so it + /// cannot affect subsequent normal typing: /// - /// This does not flush any in-progress buffer; callers should avoid toggling this mid-burst - /// (or should flush first). + /// - First, flush any held/buffered text immediately via + /// [`PasteBurst::flush_before_modified_input`], and feed it through `handle_paste(String)`. + /// This preserves user input and routes it through the same integration path as explicit + /// pastes (large-paste placeholders, image-path detection, and popup sync). + /// - Then clear the burst timing and Enter-suppression window via + /// [`PasteBurst::clear_after_explicit_paste`]. + /// + /// We intentionally do not use `clear_window_after_non_char()` here: it clears timing state + /// without emitting any buffered text, which can leave a non-empty buffer unable to flush + /// later (because `flush_if_due()` relies on `last_plain_char_time` to time out). pub(crate) fn set_disable_paste_burst(&mut self, disabled: bool) { let was_disabled = self.disable_paste_burst; self.disable_paste_burst = disabled; if disabled && !was_disabled { - self.paste_burst.clear_window_after_non_char(); + if let Some(pasted) = self.paste_burst.flush_before_modified_input() { + self.handle_paste(pasted); + } + self.paste_burst.clear_after_explicit_paste(); } } @@ -703,6 +715,7 @@ impl ChatComposer { } #[inline] + /// Clamp a cursor index to a UTF-8 char boundary. fn clamp_to_char_boundary(text: &str, pos: usize) -> usize { let mut p = pos.min(text.len()); if p < text.len() && !text.is_char_boundary(p) { @@ -732,6 +745,15 @@ impl ChatComposer { /// the cursor to a UTF-8 char boundary before slicing `textarea.text()`. #[inline] fn handle_non_ascii_char(&mut self, input: KeyEvent) -> (InputResult, bool) { + if self.disable_paste_burst { + // When burst detection is disabled, treat IME/non-ASCII input as normal typing. + // In particular, do not retro-capture or buffer already-inserted prefix text. + self.textarea.input(input); + let text_after = self.textarea.text(); + self.pending_pastes + .retain(|(placeholder, _)| text_after.contains(placeholder)); + return (InputResult::None, true); + } if let KeyEvent { code: KeyCode::Char(ch), .. @@ -1305,7 +1327,7 @@ impl ChatComposer { .next() .unwrap_or("") .starts_with('/'); - if self.paste_burst.is_active() && !in_slash_context { + if !self.disable_paste_burst && self.paste_burst.is_active() && !in_slash_context { let now = Instant::now(); if self.paste_burst.append_newline_if_active(now) { return (InputResult::None, true); @@ -1314,10 +1336,11 @@ impl ChatComposer { // During a paste-like burst, treat Enter/Ctrl+Shift+Q as a newline instead of submit. let now = Instant::now(); - if self - .paste_burst - .newline_should_insert_instead_of_submit(now) - && !in_slash_context + if !in_slash_context + && !self.disable_paste_burst + && self + .paste_burst + .newline_should_insert_instead_of_submit(now) { self.textarea.insert_str("\n"); self.paste_burst.extend_window(now); @@ -1519,6 +1542,7 @@ impl ChatComposer { // If we're capturing a burst and receive Enter, accumulate it instead of inserting. if matches!(input.code, KeyCode::Enter) + && !self.disable_paste_burst && self.paste_burst.is_active() && self.paste_burst.append_newline_if_active(now) { @@ -1537,7 +1561,7 @@ impl ChatComposer { } = input { let has_ctrl_or_alt = has_ctrl_or_alt(modifiers); - if !has_ctrl_or_alt { + if !has_ctrl_or_alt && !self.disable_paste_burst { // Non-ASCII characters (e.g., from IMEs) can arrive in quick bursts, so avoid // holding the first char while still allowing burst detection for paste input. if !ch.is_ascii() { @@ -1583,6 +1607,18 @@ impl ChatComposer { } } + // Flush any buffered burst before applying a non-char input (arrow keys, etc). + // + // `clear_window_after_non_char()` clears `last_plain_char_time`. If we cleared that while + // `PasteBurst.buffer` is non-empty, `flush_if_due()` would no longer have a timestamp to + // time out against, and the buffered paste could remain stuck until another plain char + // arrives. + if !matches!(input.code, KeyCode::Char(_) | KeyCode::Enter) + && let Some(pasted) = self.paste_burst.flush_before_modified_input() + { + self.handle_paste(pasted); + } + // Backspace at the start of an image placeholder should delete that placeholder (rather // than deleting content before it). Do this without scanning the full text by consulting // the textarea's element list. @@ -2896,6 +2932,136 @@ mod tests { assert_eq!(composer.textarea.text(), "hi\nthere"); } + /// Behavior: even if Enter suppression would normally be active for a burst, Enter should + /// still dispatch a built-in slash command when the first line begins with `/`. + #[test] + fn slash_context_enter_ignores_paste_burst_enter_suppression() { + use crate::slash_command::SlashCommand; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + composer.textarea.set_text("/diff"); + composer.textarea.set_cursor("/diff".len()); + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let (result, _) = + composer.handle_key_event(KeyEvent::new(KeyCode::Enter, KeyModifiers::NONE)); + assert!(matches!(result, InputResult::Command(SlashCommand::Diff))); + } + + /// Behavior: if a burst is buffering text and the user presses a non-char key, flush the + /// buffered burst *before* applying that key so the buffer cannot get stuck. + #[test] + fn non_char_key_flushes_active_burst_before_input() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active burst so we can deterministically buffer characters without relying on + // timing. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('h'), KeyModifiers::NONE)); + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE)); + assert!(composer.textarea.text().is_empty()); + assert!(composer.is_in_paste_burst()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Left, KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "hi"); + assert_eq!(composer.textarea.cursor(), 1); + assert!(!composer.is_in_paste_burst()); + } + + /// Behavior: enabling `disable_paste_burst` flushes any held first character (flicker + /// suppression) and then inserts subsequent chars immediately without creating burst state. + #[test] + fn disable_paste_burst_flushes_pending_first_char_and_inserts_immediately() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // First ASCII char is normally held briefly. Flip the config mid-stream and ensure the + // held char is not dropped. + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE)); + assert!(composer.is_in_paste_burst()); + assert!(composer.textarea.text().is_empty()); + + composer.set_disable_paste_burst(true); + assert_eq!(composer.textarea.text(), "a"); + assert!(!composer.is_in_paste_burst()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('b'), KeyModifiers::NONE)); + assert_eq!(composer.textarea.text(), "ab"); + assert!(!composer.is_in_paste_burst()); + } + + /// Behavior: when a burst is already active, a non-ASCII char should be captured into the + /// burst buffer via the `try_append_char_if_active` fast-path. + #[test] + fn non_ascii_appends_to_active_burst_buffer() { + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + + let (tx, _rx) = unbounded_channel::(); + let sender = AppEventSender::new(tx); + let mut composer = ChatComposer::new( + true, + sender, + false, + "Ask Codex to do anything".to_string(), + false, + ); + + // Force an active burst so the non-ASCII char takes the fast-path + // (try_append_char_if_active) into the burst buffer. + composer + .paste_burst + .begin_with_retro_grabbed(String::new(), Instant::now()); + + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('1'), KeyModifiers::NONE)); + let _ = composer.handle_key_event(KeyEvent::new(KeyCode::Char('あ'), KeyModifiers::NONE)); + + assert!(composer.textarea.text().is_empty()); + let _ = flush_after_paste_burst(&mut composer); + assert_eq!(composer.textarea.text(), "1あ"); + } + /// Behavior: a small explicit paste inserts text directly (no placeholder), and the submitted /// text matches what is visible in the textarea. #[test] diff --git a/docs/tui-chat-composer.md b/docs/tui-chat-composer.md index cc3beb17f..bda9b7ff7 100644 --- a/docs/tui-chat-composer.md +++ b/docs/tui-chat-composer.md @@ -92,9 +92,9 @@ When enabled: - The burst detector is bypassed for new input (no flicker suppression hold and no burst buffering decisions for incoming characters). - The key stream is treated as normal typing (including normal slash command behavior). -- Enabling the flag clears the burst classification window. In the current implementation it does - **not** flush or clear an already-buffered burst, so callers should avoid toggling this flag - mid-burst (or should flush first). +- Enabling the flag flushes any held/buffered burst text through the normal paste path + (`ChatComposer::handle_paste`) and then clears the burst timing and Enter-suppression windows so + transient burst state cannot leak into subsequent input. ### Enter handling