diff --git a/codex-rs/tui/src/tui.rs b/codex-rs/tui/src/tui.rs index bc316fd0b2..a0ac709af6 100644 --- a/codex-rs/tui/src/tui.rs +++ b/codex-rs/tui/src/tui.rs @@ -45,6 +45,7 @@ use crate::notifications::detect_backend; use crate::terminal_hyperlinks::HyperlinkLine; use crate::terminal_hyperlinks::plain_hyperlink_lines; use crate::tui::event_stream::EventBroker; +use crate::tui::event_stream::InitialInputPolicy; use crate::tui::event_stream::TuiEventStream; #[cfg(unix)] use crate::tui::job_control::SuspendContext; @@ -64,6 +65,7 @@ pub(crate) mod test_support; /// Target frame interval for UI redraw scheduling. pub(crate) const TARGET_FRAME_INTERVAL: Duration = frame_rate_limiter::MIN_FRAME_INTERVAL; +pub(crate) const STARTUP_INPUT_QUIET_PERIOD: Duration = Duration::from_secs(/*secs*/ 1); /// A type alias for the terminal type used in this application pub type Terminal = CustomTerminal>; @@ -77,9 +79,9 @@ pub(crate) struct InitializedTerminal { pub(crate) use startup::PreparedTerminal; use startup::StartupInputBuffer; +use startup::StartupInputHandoff; pub(crate) use startup::abandon_prepared_terminal; use startup::capture_startup_input; -pub(crate) use startup::discard_terminal_input; pub(super) fn flush_terminal_input_buffer() { startup::flush_terminal_input_buffer(); @@ -108,6 +110,11 @@ impl Drop for Tui { mod tests { use std::io::Write as _; + use crossterm::event::KeyCode; + use crossterm::event::KeyEvent; + use crossterm::event::KeyModifiers; + use tokio_stream::StreamExt; + use super::StartupInputBuffer; use super::clear_for_viewport_change; use super::should_emit_notification; @@ -154,6 +161,28 @@ mod tests { Ok(()) } + #[tokio::test(flavor = "current_thread")] + async fn startup_interrupt_survives_text_extraction() -> std::io::Result<()> { + let mut tui = make_test_tui()?; + let mut startup_input = StartupInputBuffer::default(); + startup_input.handle_probe_input(b"draft\x03"); + tui.startup_input = Some(startup_input); + + assert_eq!( + tui.take_startup_text_with_capture(|_| Ok(()))?.as_deref(), + Some("draft") + ); + let mut events = tui.startup_event_stream(); + assert!(matches!( + events.next().await, + Some(super::TuiEvent::Key(key)) if key == KeyEvent::new( + KeyCode::Char('c'), + KeyModifiers::CONTROL, + ) + )); + Ok(()) + } + #[test] fn always_notification_condition_emits_when_focused() { assert!(should_emit_notification( @@ -643,31 +672,43 @@ impl Tui { /// A screen that requests events becomes the new input owner, so text captured before the TUI /// was ready must not survive through it and later appear in the composer. pub fn event_stream(&mut self) -> Pin + Send + 'static>> { - let discard_initial_input = self.claim_startup_input(); - self.build_event_stream(discard_initial_input) + let startup_input = self.claim_startup_input(); + self.build_event_stream(InitialInputPolicy::DiscardAll, startup_input) } pub(crate) fn startup_event_stream( &mut self, ) -> Pin + Send + 'static>> { - self.claim_startup_input(); - self.build_event_stream(/*discard_initial_input*/ true) + let startup_input = self.claim_startup_input(); + self.build_event_stream(InitialInputPolicy::PreserveText, startup_input) } - fn claim_startup_input(&mut self) -> bool { - if self.startup_input_active { - self.startup_input = None; - discard_terminal_input(); - self.startup_input_active = false; - true - } else { - false + fn claim_startup_input(&mut self) -> StartupInputHandoff { + if !self.startup_input_active { + return StartupInputHandoff::default(); } + self.startup_input_active = false; + let startup_input = self.startup_input.take().unwrap_or_default().into_handoff(); + #[cfg(unix)] + if startup_input.suspend_requested { + self.event_broker.pause_events(); + let suspend_result = self.suspend_context.suspend(&self.alt_screen_active); + self.event_broker.resume_events(); + if let Err(err) = suspend_result { + tracing::warn!( + event = "tui_startup_suspend_failed", + error = %err, + "failed to suspend TUI process during startup" + ); + } + } + startup_input } fn build_event_stream( &self, - discard_initial_input: bool, + initial_input_policy: InitialInputPolicy, + startup_input: StartupInputHandoff, ) -> Pin + Send + 'static>> { #[cfg(unix)] let stream = TuiEventStream::new( @@ -683,11 +724,16 @@ impl Tui { self.draw_tx.subscribe(), self.terminal_focused.clone(), ); - let stream = if discard_initial_input { - stream.discarding_initial_input() - } else { - stream + let start_quiet = match initial_input_policy { + InitialInputPolicy::DiscardAll => startup_input.had_input, + InitialInputPolicy::PreserveText => startup_input.suppress_repeats, }; + let stream = stream.filtering_initial_input( + initial_input_policy, + start_quiet, + startup_input.interrupt_requested, + startup_input.suspend_requested, + ); Box::pin(stream) } @@ -705,9 +751,10 @@ impl Tui { let mut input = self.startup_input.take().unwrap_or_default(); capture(&mut input)?; - // Keep ownership until `event_stream()` performs the final flush immediately before the - // crossterm stream takes over stdin. - Ok(input.into_text()) + // Keep action/control state until `event_stream()` hands input to crossterm. + let text = input.take_text(); + self.startup_input = Some(input); + Ok(text) } /// Enter alternate screen and expand the viewport to full terminal size, saving the current diff --git a/codex-rs/tui/src/tui/event_stream.rs b/codex-rs/tui/src/tui/event_stream.rs index add8d77f04..42b078b5eb 100644 --- a/codex-rs/tui/src/tui/event_stream.rs +++ b/codex-rs/tui/src/tui/event_stream.rs @@ -17,6 +17,7 @@ //! //! See https://ratatui.rs/recipes/apps/spawn-vim/ and https://www.reddit.com/r/rust/comments/1f3o33u/myterious_crossterm_input_after_running_vim for more details. +use std::future::Future; use std::pin::Pin; use std::sync::Arc; use std::sync::Mutex; @@ -26,18 +27,71 @@ use std::task::Context; use std::task::Poll; use crossterm::event::Event; +use crossterm::event::KeyCode; +use crossterm::event::KeyEvent; +use crossterm::event::KeyEventKind; +use crossterm::event::KeyModifiers; use tokio::sync::broadcast; use tokio::sync::watch; +use tokio::time::Instant; +use tokio::time::Sleep; use tokio_stream::Stream; use tokio_stream::wrappers::BroadcastStream; use tokio_stream::wrappers::WatchStream; use tokio_stream::wrappers::errors::BroadcastStreamRecvError; +use super::STARTUP_INPUT_QUIET_PERIOD; use super::TuiEvent; /// Result type produced by an event source. pub type EventResult = std::io::Result; +#[derive(Clone, Copy)] +pub(super) enum InitialInputPolicy { + DiscardAll, + PreserveText, +} + +struct InitialInputFilter { + policy: InitialInputPolicy, + drain_ready: bool, + quiet_timer: Option>>, + interrupt_forwarded: bool, + suspend_forwarded: bool, +} + +impl InitialInputFilter { + fn new( + policy: InitialInputPolicy, + start_quiet: bool, + interrupt_forwarded: bool, + suspend_forwarded: bool, + ) -> Self { + let quiet_timer = start_quiet.then(|| { + Box::pin(tokio::time::sleep_until( + Instant::now() + STARTUP_INPUT_QUIET_PERIOD, + )) + }); + Self { + policy, + drain_ready: !start_quiet, + quiet_timer, + interrupt_forwarded, + suspend_forwarded, + } + } + + fn reset_quiet_timer(&mut self) { + let deadline = Instant::now() + STARTUP_INPUT_QUIET_PERIOD; + if let Some(timer) = &mut self.quiet_timer { + timer.as_mut().reset(deadline); + } else { + self.quiet_timer = Some(Box::pin(tokio::time::sleep_until(deadline))); + } + self.drain_ready = false; + } +} + /// Abstraction over a source of terminal events. Allows swapping in a fake for tests. /// Value in production is [`CrosstermEventSource`]. pub trait EventSource: Send + 'static { @@ -142,7 +196,8 @@ pub struct TuiEventStream, terminal_focused: Arc, poll_draw_first: bool, - discard_initial_input: bool, + initial_input_filter: Option, + pending_interrupt: bool, #[cfg(unix)] suspend_context: crate::tui::job_control::SuspendContext, #[cfg(unix)] @@ -164,7 +219,8 @@ impl TuiEventStream { resume_stream, terminal_focused, poll_draw_first: false, - discard_initial_input: false, + initial_input_filter: None, + pending_interrupt: false, #[cfg(unix)] suspend_context, #[cfg(unix)] @@ -172,17 +228,88 @@ impl TuiEventStream { } } - pub fn discarding_initial_input(mut self) -> Self { - self.discard_initial_input = true; + pub(super) fn filtering_initial_input( + mut self, + policy: InitialInputPolicy, + start_quiet: bool, + pending_interrupt: bool, + suspend_forwarded: bool, + ) -> Self { + self.initial_input_filter = Some(InitialInputFilter::new( + policy, + start_quiet, + pending_interrupt, + suspend_forwarded, + )); + self.pending_interrupt = pending_interrupt; self } + fn should_forward_initial_event(&mut self, event: &Event) -> bool { + let Some(filter) = &mut self.initial_input_filter else { + return true; + }; + match event { + Event::Key(key_event) => { + filter.reset_quiet_timer(); + if is_interrupt(*key_event) { + return if filter.interrupt_forwarded { + false + } else { + filter.interrupt_forwarded = true; + true + }; + } + #[cfg(unix)] + if crate::tui::job_control::SUSPEND_KEY.is_press(*key_event) { + return if filter.suspend_forwarded { + false + } else { + filter.suspend_forwarded = true; + true + }; + } + matches!(filter.policy, InitialInputPolicy::PreserveText) + && is_text_input(*key_event) + } + Event::Paste(_) => { + filter.reset_quiet_timer(); + matches!(filter.policy, InitialInputPolicy::PreserveText) + } + Event::Resize(_, _) | Event::FocusGained | Event::FocusLost => true, + _ => false, + } + } + + fn poll_initial_input_filter(&mut self, cx: &mut Context<'_>) { + let Some(filter) = &mut self.initial_input_filter else { + return; + }; + if filter.drain_ready { + self.initial_input_filter = None; + return; + } + let Some(timer) = &mut filter.quiet_timer else { + self.initial_input_filter = None; + return; + }; + if timer.as_mut().poll(cx).is_ready() { + self.initial_input_filter = None; + } + } + /// Poll the shared crossterm stream for the next mapped `TuiEvent`. /// /// This skips events we don't use (mouse events, etc.) and keeps polling until it yields /// a mapped event, hits `Pending`, or sees EOF/error. When the broker is paused, it drops /// the underlying stream and returns `Pending` to fully release stdin. pub fn poll_crossterm_event(&mut self, cx: &mut Context<'_>) -> Poll> { + if std::mem::take(&mut self.pending_interrupt) { + return Poll::Ready(Some(TuiEvent::Key(KeyEvent::new( + KeyCode::Char('c'), + KeyModifiers::CONTROL, + )))); + } // Some crossterm events map to None (e.g. FocusLost, mouse); loop so we keep polling // until we return a mapped event, hit Pending, or see EOF/error. loop { @@ -217,7 +344,7 @@ impl TuiEventStream { Poll::Ready(Some(())) => continue, Poll::Ready(None) => return Poll::Ready(None), Poll::Pending => { - self.discard_initial_input = false; + self.poll_initial_input_filter(cx); return Poll::Pending; } } @@ -225,10 +352,13 @@ impl TuiEventStream { } }; - if self.discard_initial_input { + let Some(event) = poll_result else { + continue; + }; + if !self.should_forward_initial_event(&event) { continue; } - if let Some(mapped) = poll_result.and_then(|event| self.map_crossterm_event(event)) { + if let Some(mapped) = self.map_crossterm_event(event) { return Poll::Ready(Some(mapped)); } } @@ -282,6 +412,25 @@ impl TuiEventStream { } } +fn is_interrupt(key_event: KeyEvent) -> bool { + matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) + && key_event.code == KeyCode::Char('c') + && key_event.modifiers.contains(KeyModifiers::CONTROL) +} + +fn is_text_input(key_event: KeyEvent) -> bool { + matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) + && key_event + .modifiers + .difference(KeyModifiers::SHIFT) + .is_empty() + && match key_event.code { + KeyCode::Char(ch) => !ch.is_control(), + KeyCode::Backspace => true, + _ => false, + } +} + impl Unpin for TuiEventStream {} impl Stream for TuiEventStream { @@ -391,10 +540,13 @@ mod tests { ) } - fn make_stream_discarding_initial_input( + fn make_stream_filtering_initial_input( broker: Arc>, draw_rx: broadcast::Receiver<()>, terminal_focused: Arc, + policy: InitialInputPolicy, + start_quiet: bool, + pending_interrupt: bool, ) -> TuiEventStream { TuiEventStream::new( broker, @@ -405,7 +557,12 @@ mod tests { #[cfg(unix)] Arc::new(AtomicBool::new(false)), ) - .discarding_initial_input() + .filtering_initial_input( + policy, + start_quiet, + pending_interrupt, + /*suspend_forwarded*/ false, + ) } type SetupState = ( @@ -447,29 +604,108 @@ mod tests { } } - #[tokio::test(flavor = "current_thread")] - async fn initial_input_handoff_discards_only_events_already_ready() { + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn initial_input_handoff_waits_for_legacy_repeats_to_settle() { let (broker, handle, _draw_tx, draw_rx, terminal_focused) = setup(); - let mut stream = make_stream_discarding_initial_input(broker, draw_rx, terminal_focused); + let mut stream = make_stream_filtering_initial_input( + broker, + draw_rx, + terminal_focused, + InitialInputPolicy::DiscardAll, + /*start_quiet*/ false, + /*pending_interrupt*/ false, + ); handle.send(Ok(Event::Key(KeyEvent::new( KeyCode::Enter, KeyModifiers::NONE, )))); assert!( - timeout(Duration::from_millis(10), stream.next()) + timeout(Duration::from_nanos(1), stream.next()) + .await + .is_err() + ); + tokio::time::advance(STARTUP_INPUT_QUIET_PERIOD / 2).await; + handle.send(Ok(Event::Key(KeyEvent::new( + KeyCode::Enter, + KeyModifiers::NONE, + )))); + assert!( + timeout(Duration::from_nanos(1), stream.next()) + .await + .is_err() + ); + tokio::time::advance(STARTUP_INPUT_QUIET_PERIOD).await; + assert!( + timeout(Duration::from_nanos(1), stream.next()) .await .is_err() ); let expected = KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE); handle.send(Ok(Event::Key(expected))); - match timeout(Duration::from_secs(1), stream.next()).await { + match timeout(Duration::from_nanos(1), stream.next()).await { Ok(Some(TuiEvent::Key(actual))) => assert_eq!(actual, expected), other => panic!("expected post-handoff key event, saw {other:?}"), } } + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn startup_handoff_preserves_text_while_suppressing_actions() { + let (broker, handle, _draw_tx, draw_rx, terminal_focused) = setup(); + let mut stream = make_stream_filtering_initial_input( + broker, + draw_rx, + terminal_focused, + InitialInputPolicy::PreserveText, + /*start_quiet*/ true, + /*pending_interrupt*/ false, + ); + + let key = KeyEvent::new(KeyCode::Char('a'), KeyModifiers::NONE); + handle.send(Ok(Event::Key(key))); + assert!(matches!(stream.next().await, Some(TuiEvent::Key(actual)) if actual == key)); + + handle.send(Ok(Event::Paste("b\nc".to_string()))); + assert!(matches!(stream.next().await, Some(TuiEvent::Paste(text)) if text == "b\nc")); + + let backspace = KeyEvent::new(KeyCode::Backspace, KeyModifiers::NONE); + handle.send(Ok(Event::Key(backspace))); + assert!(matches!(stream.next().await, Some(TuiEvent::Key(actual)) if actual == backspace)); + + handle.send(Ok(Event::Key(KeyEvent::new( + KeyCode::Enter, + KeyModifiers::NONE, + )))); + assert!( + timeout(Duration::from_nanos(1), stream.next()) + .await + .is_err() + ); + } + + #[tokio::test(flavor = "current_thread", start_paused = true)] + async fn startup_handoff_forwards_interrupt_only_once() { + let (broker, handle, _draw_tx, draw_rx, terminal_focused) = setup(); + let mut stream = make_stream_filtering_initial_input( + broker, + draw_rx, + terminal_focused, + InitialInputPolicy::DiscardAll, + /*start_quiet*/ true, + /*pending_interrupt*/ true, + ); + + let interrupt = KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL); + assert!(matches!(stream.next().await, Some(TuiEvent::Key(actual)) if actual == interrupt)); + handle.send(Ok(Event::Key(interrupt))); + assert!( + timeout(Duration::from_nanos(1), stream.next()) + .await + .is_err() + ); + } + #[tokio::test(flavor = "current_thread")] async fn draw_and_key_events_yield_both() { let (broker, handle, draw_tx, draw_rx, terminal_focused) = setup(); diff --git a/codex-rs/tui/src/tui/startup.rs b/codex-rs/tui/src/tui/startup.rs index 867a6faca7..6124d695e4 100644 --- a/codex-rs/tui/src/tui/startup.rs +++ b/codex-rs/tui/src/tui/startup.rs @@ -41,6 +41,17 @@ pub(crate) struct StartupInputBuffer { text: String, char_count: usize, pending_plain_whitespace: String, + suppressed_action: bool, + interrupt_requested: bool, + suspend_requested: bool, +} + +#[derive(Default)] +pub(super) struct StartupInputHandoff { + pub(super) had_input: bool, + pub(super) suppress_repeats: bool, + pub(super) interrupt_requested: bool, + pub(super) suspend_requested: bool, } impl StartupInputBuffer { @@ -52,15 +63,33 @@ impl StartupInputBuffer { kind: KeyEventKind::Press | KeyEventKind::Repeat, .. }) => { + if code == KeyCode::Char('c') && modifiers.contains(KeyModifiers::CONTROL) { + self.interrupt_requested = true; + return; + } + if code == KeyCode::Char('z') && modifiers.contains(KeyModifiers::CONTROL) { + self.suspend_requested = true; + return; + } if modifiers.difference(KeyModifiers::SHIFT).is_empty() { match code { KeyCode::Char(ch) if !ch.is_control() => self.push_plain_char(ch), KeyCode::Backspace => self.pop_char(), - KeyCode::Enter => self.push_pending_plain_whitespace('\n'), - KeyCode::Tab => self.push_pending_plain_whitespace('\t'), - _ => self.pending_plain_whitespace.clear(), + KeyCode::Enter => { + self.suppressed_action = true; + self.push_pending_plain_whitespace('\n'); + } + KeyCode::Tab => { + self.suppressed_action = true; + self.push_pending_plain_whitespace('\t'); + } + _ => { + self.suppressed_action = true; + self.pending_plain_whitespace.clear(); + } } } else { + self.suppressed_action = true; self.pending_plain_whitespace.clear(); } } @@ -129,14 +158,23 @@ impl StartupInputBuffer { while let Some(ch) = chars.next() { match ch { '\u{8}' | '\u{7f}' => self.pop_char(), + '\u{3}' => self.interrupt_requested = true, + '\u{1a}' => self.suspend_requested = true, '\r' => { + self.suppressed_action = true; if chars.peek() == Some(&'\n') { chars.next(); } self.push_pending_plain_whitespace('\n'); } - '\n' => self.push_pending_plain_whitespace('\n'), - '\t' => self.push_pending_plain_whitespace('\t'), + '\n' => { + self.suppressed_action = true; + self.push_pending_plain_whitespace('\n'); + } + '\t' => { + self.suppressed_action = true; + self.push_pending_plain_whitespace('\t'); + } ch if !ch.is_control() => self.push_plain_char(ch), _ => {} } @@ -157,8 +195,25 @@ impl StartupInputBuffer { } } - pub(super) fn into_text(self) -> Option { - (!self.text.is_empty()).then_some(self.text) + pub(super) fn take_text(&mut self) -> Option { + self.char_count = 0; + (!self.text.is_empty()).then(|| std::mem::take(&mut self.text)) + } + + pub(super) fn into_handoff(self) -> StartupInputHandoff { + let had_input = self.char_count > 0 + || !self.pending_plain_whitespace.is_empty() + || self.suppressed_action + || self.interrupt_requested + || self.suspend_requested; + StartupInputHandoff { + had_input, + suppress_repeats: self.suppressed_action + || self.interrupt_requested + || self.suspend_requested, + interrupt_requested: self.interrupt_requested, + suspend_requested: self.suspend_requested, + } } } diff --git a/codex-rs/tui/src/tui/startup_tests.rs b/codex-rs/tui/src/tui/startup_tests.rs index bd04b35dfc..cf187b04f1 100644 --- a/codex-rs/tui/src/tui/startup_tests.rs +++ b/codex-rs/tui/src/tui/startup_tests.rs @@ -28,7 +28,10 @@ fn startup_input_keeps_text_without_replaying_actions() { input.handle_event(event); } - assert_eq!(input.into_text(), Some("hello\n\tworld".to_string())); + assert_eq!(input.take_text(), Some("hello\n\tworld".to_string())); + let handoff = input.into_handoff(); + assert!(handoff.interrupt_requested); + assert!(handoff.suppress_repeats); } #[test] @@ -45,7 +48,7 @@ fn startup_input_preserves_internal_plain_whitespace_but_drops_trailing_actions( input.handle_event(Event::Key(KeyEvent::new(code, KeyModifiers::NONE))); } - assert_eq!(input.into_text(), Some("a\nb\tc".to_string())); + assert_eq!(input.take_text(), Some("a\nb\tc".to_string())); } #[test] @@ -58,7 +61,7 @@ fn startup_probe_input_preserves_internal_plain_whitespace_across_phases() { ))); input.handle_probe_input(b"\t"); - assert_eq!(input.into_text(), Some("a\nb".to_string())); + assert_eq!(input.take_text(), Some("a\nb".to_string())); } #[cfg(unix)] @@ -69,7 +72,7 @@ fn startup_probe_preserves_bracketed_paste_whitespace() { b"a\r\n\t".to_vec(), )]); - assert_eq!(input.into_text(), Some("a\n\t".to_string())); + assert_eq!(input.take_text(), Some("a\n\t".to_string())); } #[test] @@ -77,7 +80,7 @@ fn startup_input_is_bounded() { let mut input = StartupInputBuffer::default(); input.handle_event(Event::Paste("x".repeat(MAX_STARTUP_INPUT_CHARS + 1))); - assert_eq!(input.into_text(), Some("x".repeat(MAX_STARTUP_INPUT_CHARS))); + assert_eq!(input.take_text(), Some("x".repeat(MAX_STARTUP_INPUT_CHARS))); } #[test] @@ -91,5 +94,30 @@ fn startup_input_applies_edits_across_capture_phases() { KeyModifiers::NONE, ))); - assert_eq!(input.into_text(), Some("drap".to_string())); + assert_eq!(input.take_text(), Some("drap".to_string())); +} + +#[test] +fn startup_probe_controls_survive_text_extraction() { + let mut input = StartupInputBuffer::default(); + input.handle_probe_input(b"draft\x03\x1a"); + + assert_eq!(input.take_text(), Some("draft".to_string())); + let handoff = input.into_handoff(); + assert!(handoff.interrupt_requested); + assert!(handoff.suspend_requested); +} + +#[cfg(unix)] +#[test] +fn startup_paste_control_bytes_are_not_replayed_as_actions() { + let mut input = StartupInputBuffer::default(); + input.handle_startup_probe_input(&[crate::terminal_probe::StartupInput::Paste( + b"a\x03\x1ab".to_vec(), + )]); + + assert_eq!(input.take_text(), Some("ab".to_string())); + let handoff = input.into_handoff(); + assert!(!handoff.interrupt_requested); + assert!(!handoff.suspend_requested); }