From 0389e80f07871a51e7b5a7b7e393139c38e5cbf0 Mon Sep 17 00:00:00 2001 From: Benjamin Carlsson Date: Fri, 6 Nov 2026 23:11:35 +0000 Subject: [PATCH] Refactor TUI clipboard and composer input handling (#60008) ## What changed - Extract helpers for image paste shortcuts, paste-burst ticks, restricted draft edits, and composer activity detection. - Thread `ShellPrefixHandling` through text insertion and pending-input flushing. - Return a configuration rebuild future that does not borrow the app or base configuration. ## Testing Add a regression test covering image paste while `!` is buffered: Enter preserves the draft during the paste burst, and subsequent submission includes the image and literal `!` without running a shell command. GitOrigin-RevId: fb9a3c8a70aed524c65022f2c0910ce2ad11d4d2 --- codex-rs/tui/src/app/config_persistence.rs | 5 +- .../bottom_pane/chat_composer/inline_input.rs | 9 +++- .../bottom_pane/chat_composer/paste_input.rs | 27 ++++++++-- .../bottom_pane/chat_composer/reconnect.rs | 43 ++++++++------- codex-rs/tui/src/bottom_pane/mod.rs | 25 +++++---- codex-rs/tui/src/chatwidget/clipboard.rs | 31 ++++++++++- codex-rs/tui/src/chatwidget/interaction.rs | 44 +++++---------- .../chatwidget/tests/composer_submission.rs | 53 +++++++++++++++++++ 8 files changed, 168 insertions(+), 69 deletions(-) diff --git a/codex-rs/tui/src/app/config_persistence.rs b/codex-rs/tui/src/app/config_persistence.rs index 8a04d72452..cb28791ff1 100644 --- a/codex-rs/tui/src/app/config_persistence.rs +++ b/codex-rs/tui/src/app/config_persistence.rs @@ -99,11 +99,11 @@ impl App { self.rebuild_config_for_cwd_from(&self.config, cwd).await } - pub(super) async fn rebuild_config_for_cwd_from( + pub(super) fn rebuild_config_for_cwd_from( &self, base: &Config, cwd: PathBuf, - ) -> Result { + ) -> impl std::future::Future> + use<> { let mut overrides = self.harness_overrides.clone(); overrides.cwd = Some(cwd.clone()); let cwd_display = cwd.display().to_string(); @@ -118,7 +118,6 @@ impl App { base.application_network_policy.clone(), format!("Failed to rebuild config for cwd {cwd_display}"), ) - .await } #[cfg(test)] diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs b/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs index 48f7adff59..69ce45d565 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/inline_input.rs @@ -119,8 +119,15 @@ impl ChatComposer { /// Integrate already-classified buffered typing before another answer takes over this editor. pub(crate) fn flush_pending_input(&mut self) { + self.flush_pending_input_with_shell_prefix(ShellPrefixHandling::Interpret); + } + + pub(in crate::bottom_pane) fn flush_pending_input_with_shell_prefix( + &mut self, + shell_prefix: ShellPrefixHandling, + ) { if let Some(text) = self.draft.paste_burst.flush_before_modified_input() { - self.apply_paste(text); + self.apply_paste_with_shell_prefix(text, shell_prefix); } self.draft.paste_burst.clear_after_explicit_paste(); } diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs b/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs index efb2a0bc85..251372a6ea 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/paste_input.rs @@ -13,10 +13,19 @@ use crate::bottom_pane::textarea::EditTarget; impl ChatComposer { pub(crate) fn insert_str(&mut self, text: &str) { - self.insert_str_at_target(self.draft.textarea.edit_target(), text); + self.insert_str_at_target( + self.draft.textarea.edit_target(), + text, + ShellPrefixHandling::Interpret, + ); } - fn insert_str_at_target(&mut self, target: EditTarget, text: &str) { + fn insert_str_at_target( + &mut self, + target: EditTarget, + text: &str, + shell_prefix: ShellPrefixHandling, + ) { if !text.is_empty() && self.sparkle.draft.get() == sparkle::SparkleDraft::Untouched { self.dismiss_sparkle(); } @@ -24,7 +33,9 @@ impl ChatComposer { let elements_before = target .replaces_text() .then(|| self.draft.textarea.element_payloads()); - let inserts_shell_prefix = target.start() == 0 && text.starts_with('!'); + let inserts_shell_prefix = matches!(shell_prefix, ShellPrefixHandling::Interpret) + && target.start() == 0 + && text.starts_with('!'); self.draft.textarea.insert_str_at_target(target, text); if let Some(elements_before) = elements_before { self.reconcile_deleted_elements(elements_before); @@ -153,6 +164,14 @@ impl ChatComposer { /// /// Composer edits clear paste-burst Enter suppression and sync popups. pub(super) fn apply_paste(&mut self, pasted: String) -> bool { + self.apply_paste_with_shell_prefix(pasted, ShellPrefixHandling::Interpret) + } + + pub(super) fn apply_paste_with_shell_prefix( + &mut self, + pasted: String, + shell_prefix: ShellPrefixHandling, + ) -> bool { let pasted = pasted.replace("\r\n", "\n").replace('\r', "\n"); let pasted = sanitize_user_text(pasted.into()); if self.history_search.is_some() { @@ -200,7 +219,7 @@ impl ChatComposer { let cursor = self.draft.textarea.cursor(); self.draft.textarea.insert_str_at(cursor, " "); } else { - self.insert_str_at_target(target, &pasted); + self.insert_str_at_target(target, &pasted, shell_prefix); } if is_blockquote { let textarea = &mut self.draft.textarea; diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs index 39be2c4609..1a31beb3d4 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/reconnect.rs @@ -27,9 +27,14 @@ impl ChatComposer { .next() .unwrap_or("") .starts_with('/')); - if !self.draft.disable_paste_burst - && self.draft.paste_burst.is_active() - && !in_slash_context + !in_slash_context && self.append_paste_newline(now) + } + + fn append_paste_newline(&mut self, now: Instant) -> bool { + if self.draft.disable_paste_burst { + return false; + } + if self.draft.paste_burst.is_active() && self .draft .paste_burst @@ -37,12 +42,10 @@ impl ChatComposer { { return true; } - if !in_slash_context - && !self.draft.disable_paste_burst - && self - .draft - .paste_burst - .newline_should_insert_instead_of_submit(now) + if self + .draft + .paste_burst + .newline_should_insert_instead_of_submit(now) { self.draft.textarea.insert_str("\n"); self.draft.paste_burst.extend_window(now); @@ -104,18 +107,22 @@ impl ChatComposer { if !matches!(key.code, KeyCode::Enter | KeyCode::Tab) && matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) { - // Null is sent internally to clean up on disconnect or expand a paste; it isn't an edit. - let before = if key.code == KeyCode::Null { - None - } else { - self.draft.textarea_state.get_mut().follow_cursor(); - self.before_sparkle_editor_key(key) - }; - let (result, _) = self.handle_input_basic(key); - self.after_sparkle_key(before, &result); + self.edit_restricted_draft(key); } InputResult::None } + + fn edit_restricted_draft(&mut self, key: KeyEvent) { + // Null is sent internally to clean up on disconnect or expand a paste. + let before = if key.code == KeyCode::Null { + None + } else { + self.draft.textarea_state.get_mut().follow_cursor(); + self.before_sparkle_editor_key(key) + }; + let (result, _) = self.handle_input_basic(key); + self.after_sparkle_key(before, &result); + } } #[cfg(test)] diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index 27d288a94b..006c777a9d 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -852,6 +852,19 @@ impl BottomPane { self.push_view(Box::new(modal)); } + fn is_composer_activity_key(key: KeyEvent) -> bool { + matches!(key.kind, KeyEventKind::Press | KeyEventKind::Repeat) + && !key_hint::has_ctrl_or_alt(key.modifiers) + && matches!( + key.code, + KeyCode::Char(_) + | KeyCode::Backspace + | KeyCode::Delete + | KeyCode::Enter + | KeyCode::Tab + ) + } + /// Preserve restricted drafts, allowing recovery commands only for connected unavailable threads. pub(crate) fn handle_restricted_key( &mut self, @@ -896,17 +909,7 @@ impl BottomPane { view.observe_key_event_while_covered(key_event, routed_key); } } - let records_composer_activity = - matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) - && !key_hint::has_ctrl_or_alt(key_event.modifiers) - && matches!( - key_event.code, - KeyCode::Char(_) - | KeyCode::Backspace - | KeyCode::Delete - | KeyCode::Enter - | KeyCode::Tab - ); + let records_composer_activity = Self::is_composer_activity_key(key_event); if self.warnings_active() { if matches!(key_event.kind, KeyEventKind::Press | KeyEventKind::Repeat) { self.record_composer_activity_at(Instant::now()); diff --git a/codex-rs/tui/src/chatwidget/clipboard.rs b/codex-rs/tui/src/chatwidget/clipboard.rs index 3d8bfa69d7..2754a109b1 100644 --- a/codex-rs/tui/src/chatwidget/clipboard.rs +++ b/codex-rs/tui/src/chatwidget/clipboard.rs @@ -4,7 +4,7 @@ use super::*; use crate::clipboard_copy::worker::CopyResult; -pub(super) const IMAGE_PASTE_REPEAT_WINDOW: Duration = Duration::from_millis(/*millis*/ 250); +const IMAGE_PASTE_REPEAT_WINDOW: Duration = Duration::from_millis(/*millis*/ 250); pub(super) enum PendingCopy { Message(u64, String), @@ -12,6 +12,35 @@ pub(super) enum PendingCopy { } impl ChatWidget { + /// Recognize image paste shortcuts, including presses consumed by repeat suppression. + pub(super) fn image_paste_key_action(&mut self, key: KeyEvent) -> Option { + let shift_insert = cfg!(windows) && key_hint::shift(KeyCode::Insert).is_press(key); + let image_paste = key.kind == KeyEventKind::Press + && ((matches!(key.code, KeyCode::Char(c) if c.eq_ignore_ascii_case(&'v')) + && key + .modifiers + .intersects(KeyModifiers::CONTROL | KeyModifiers::ALT)) + || shift_insert); + if !image_paste { + if matches!(key.kind, KeyEventKind::Press | KeyEventKind::Release) { + self.suppress_image_paste_until = Instant::now(); + } + return None; + } + if Instant::now() < self.suppress_image_paste_until { + self.suppress_image_paste_until = Instant::now() + IMAGE_PASTE_REPEAT_WINDOW; + return Some(KeyEventAction::None); + } + self.suppress_image_paste_until = Instant::now() + IMAGE_PASTE_REPEAT_WINDOW; + let text_fallback = (cfg!(windows) + && (key.modifiers == KeyModifiers::CONTROL || shift_insert)) + .then_some(key.code); + if text_fallback.is_some() { + self.bottom_pane.prepare_composer_clipboard_paste(); + } + Some(KeyEventAction::PasteImage { text_fallback }) + } + pub(crate) fn paste_image(&mut self) -> Result<(), crate::clipboard_paste::PasteImageError> { let result = paste_image_to_temp_png(); self.suppress_image_paste_until = Instant::now() + IMAGE_PASTE_REPEAT_WINDOW; diff --git a/codex-rs/tui/src/chatwidget/interaction.rs b/codex-rs/tui/src/chatwidget/interaction.rs index 2b4292875e..e35229c23f 100644 --- a/codex-rs/tui/src/chatwidget/interaction.rs +++ b/codex-rs/tui/src/chatwidget/interaction.rs @@ -263,39 +263,16 @@ impl ChatWidget { self.quit_shortcut_expires_at = None; self.quit_shortcut_key = None; } - key if key.kind == KeyEventKind::Press - && ((matches!(key.code, KeyCode::Char(c) if c.eq_ignore_ascii_case(&'v')) - && key - .modifiers - .intersects(KeyModifiers::CONTROL | KeyModifiers::ALT)) - || (cfg!(windows) && key_hint::shift(KeyCode::Insert).is_press(key))) => - { - if Instant::now() < self.suppress_image_paste_until { - self.suppress_image_paste_until = - Instant::now() + clipboard::IMAGE_PASTE_REPEAT_WINDOW; - return KeyEventAction::None; + _ => { + if let Some(action) = self.image_paste_key_action(key_event) { + return action; } - self.suppress_image_paste_until = - Instant::now() + clipboard::IMAGE_PASTE_REPEAT_WINDOW; - let text_fallback = (cfg!(windows) - && (key.modifiers == KeyModifiers::CONTROL - || key_hint::shift(KeyCode::Insert).is_press(key))) - .then_some(key.code); - if text_fallback.is_some() { - self.bottom_pane.prepare_composer_clipboard_paste(); + if key_event.kind == KeyEventKind::Press { + self.bottom_pane.clear_quit_shortcut_hint(); + self.quit_shortcut_expires_at = None; + self.quit_shortcut_key = None; } - return KeyEventAction::PasteImage { text_fallback }; } - other if other.kind == KeyEventKind::Press => { - self.suppress_image_paste_until = Instant::now(); - self.bottom_pane.clear_quit_shortcut_hint(); - self.quit_shortcut_expires_at = None; - self.quit_shortcut_key = None; - } - other if other.kind == KeyEventKind::Release => { - self.suppress_image_paste_until = Instant::now(); - } - _ => {} } if key_event.kind == KeyEventKind::Press @@ -627,7 +604,12 @@ impl ChatWidget { // Returns true if caller should skip rendering this frame (a future frame is scheduled). pub(crate) fn handle_paste_burst_tick(&mut self, frame_requester: FrameRequester) -> bool { - if self.bottom_pane.flush_paste_burst_if_due() { + let flushed = self.bottom_pane.flush_paste_burst_if_due(); + self.finish_paste_burst_tick(frame_requester, flushed) + } + + fn finish_paste_burst_tick(&mut self, frame_requester: FrameRequester, flushed: bool) -> bool { + if flushed { self.refresh_startup_recovery(); // A paste just flushed; request an immediate redraw and skip this frame. self.request_redraw(); diff --git a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs index daa34dc7ae..17374e1233 100644 --- a/codex-rs/tui/src/chatwidget/tests/composer_submission.rs +++ b/codex-rs/tui/src/chatwidget/tests/composer_submission.rs @@ -417,6 +417,59 @@ async fn parent_owned_thread_preserves_queued_input_before_draining() { assert_no_submit_op(&mut op_rx); } +#[tokio::test(start_paused = true)] +async fn local_clipboard_image_does_not_turn_a_held_bang_into_a_shell_command() { + let (mut chat, _events, mut operations) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + chat.bottom_pane.set_disable_paste_burst(/*disabled*/ false); + chat.handle_key_event(KeyCode::Char('!').into()); + assert_eq!(chat.bottom_pane.composer_text(), ""); + assert_eq!(chat.bottom_pane.composer_recovery_snapshot().text, "!"); + + let paste = chat.handle_key_event(KeyEvent::new(KeyCode::Char('v'), KeyModifiers::ALT)); + let KeyEventAction::PasteImage { .. } = paste else { + panic!("expected a clipboard image request"); + }; + let image_path = PathBuf::from("image.png"); + chat.attach_image(image_path.clone()); + chat.handle_key_event(KeyCode::Enter.into()); + + let draft = chat.bottom_pane.composer_recovery_snapshot(); + assert_eq!(draft.text, "[Image #1]!\n"); + assert_eq!(draft.shell_mode, Some(false)); + while let Ok(op) = operations.try_recv() { + assert!( + !matches!(op, Op::RunUserShellCommand { .. } | Op::UserTurn { .. }), + "{op:?}" + ); + } + + tokio::time::advance(Duration::from_secs(/*secs*/ 1)).await; + chat.handle_paste_burst_tick(FrameRequester::test_dummy()); + chat.handle_key_event(KeyCode::Enter.into()); + + let Op::UserTurn { items, .. } = next_submit_op(&mut operations) else { + panic!("expected submitted user turn"); + }; + let placeholder = "[Image #1]"; + assert_eq!( + items, + vec![ + UserInput::LocalImage { + path: image_path, + detail: None, + }, + UserInput::Text { + text: format!("{placeholder}!"), + text_elements: vec![ + TextElement::new((0..placeholder.len()).into(), Some(placeholder.to_string())) + .into(), + ], + }, + ] + ); +} + #[tokio::test] async fn submission_preserves_text_elements_and_local_images() { let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;