From a3a09dfc9b90b0a37e175b3c8e6f14336adeec8c Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Tue, 5 May 2026 14:41:43 -0300 Subject: [PATCH] fix(tui): external editor expansion for same-size large pastes (#21190) ## Why We found this while reviewing #21091, but confirmed it is not introduced by that PR: the order-sensitive `current_text_with_pending()` replacement loop already existed, and `main` already allowed active same-size large pastes to use prefix-overlapping labels such as `[Pasted Content N chars]` and `[Pasted Content N chars] #2`. #21091 fixes placeholder numbering after a draft is cleared, so a fresh same-size paste can reuse the base label. This PR fixes a different path: when a draft already contains multiple active same-size large pastes, the placeholders can overlap by prefix, for example `[Pasted Content N chars]` and `[Pasted Content N chars] #2`. That overlap breaks `current_text_with_pending()` when the composer materializes the draft text for the external editor. Replacing the base placeholder first can partially rewrite the `#2` placeholder, leaving the external editor seeded with corrupted text instead of both paste payloads. | Before | After | |---|---| | CleanShot 2026-05-05 at 10 18 09 | CleanShot 2026-05-05 at 10 20 31 | | CleanShot 2026-05-05 at 10 18 57 | CleanShot 2026-05-05 at 10 21 31 | ## What Changed - Changed `current_text_with_pending()` to expand pending pastes through the existing element-range based `expand_pending_pastes()` helper instead of global string replacement. - Added a regression test with two different same-length large pastes to ensure both overlapping placeholders expand to their original payloads. ## How to Test 1. Start Codex TUI. 2. Paste a large string, for example 1004 `A` characters. ```shell perl -e 'print "A" x 1004' | pbcopy ``` 3. Paste a second large string with the same length, for example 1004 `B` characters. ```shell perl -e 'print "B" x 1004' | pbcopy ``` 4. Open the external editor from the composer. 5. Confirm the editor is seeded with the full `A...` payload followed by the full `B...` payload, with no literal `#2` left behind. Targeted tests: - `cargo test -p codex-tui current_text_with_pending_expands_overlapping_placeholders` - `just argument-comment-lint-from-source -p codex-tui` I also ran `cargo test -p codex-tui`; it reached the full crate suite but failed two unrelated local status tests because this machine's `/etc/codex/requirements.toml` rejects `DangerFullAccess`. --- codex-rs/tui/src/bottom_pane/chat_composer.rs | 38 ++++++++++++++++--- 1 file changed, 33 insertions(+), 5 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 09fdaedf6..32e8c092f 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -1129,12 +1129,13 @@ impl ChatComposer { } pub(crate) fn current_text_with_pending(&self) -> String { - let mut text = self.current_text(); - for (placeholder, actual) in &self.pending_pastes { - if text.contains(placeholder) { - text = text.replace(placeholder, actual); - } + let text = self.current_text(); + if self.pending_pastes.is_empty() { + return text; } + + let (text, _) = + Self::expand_pending_pastes(&text, self.current_text_elements(), &self.pending_pastes); text } @@ -10225,6 +10226,33 @@ mod tests { ); } + #[test] + fn current_text_with_pending_expands_overlapping_placeholders() { + let (tx, _rx) = unbounded_channel::(); + 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, + ); + + let first_paste = "a".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4); + let second_paste = "b".repeat(LARGE_PASTE_CHAR_THRESHOLD + 4); + let base = format!("[Pasted Content {} chars]", first_paste.chars().count()); + let second = format!("{base} #2"); + + composer.handle_paste(first_paste.clone()); + composer.handle_paste(second_paste.clone()); + + assert_eq!(composer.current_text(), format!("{base}{second}")); + assert_eq!( + composer.current_text_with_pending(), + format!("{first_paste}{second_paste}") + ); + } + #[test] fn apply_external_edit_limits_duplicates_to_occurrences() { let (tx, _rx) = unbounded_channel::();