From 57a38c1bebc77a04565b5a8ff08110f62f9f6e5d Mon Sep 17 00:00:00 2001 From: Ian MacLeod Date: Thu, 1 Oct 2026 17:43:29 +0000 Subject: [PATCH] Preserve question context in recovered TUI answer drafts (#50052) ## Why When a turn ends, unsent question answers are recovered into the main composer. Recovering only the answers loses which question each answer belongs to. ## What changed Prepend each recovered answer with its question title as a Markdown blockquote. Separate questions, answers, and existing composer text with blank lines. Sanitize question titles and limit them to 512 bytes at a UTF-8 character boundary, adding an ellipsis when truncated. Keep answer text untruncated. ## Testing Add coverage for bounded titles, multibyte characters, control characters, and multiline question-and-answer submission. Update recovery assertions and snapshots for turn completion, failure, interruption, large pastes, and Vim undo. GitOrigin-RevId: 141dacfb64bcad5fcabc5be217d2340ef7de1eb5 --- .../src/bottom_pane/async_questions/state.rs | 35 ++++++++- .../async_questions/state_tests.rs | 26 ++++++- codex-rs/tui/src/bottom_pane/chat_composer.rs | 5 +- .../bottom_pane/chat_composer/inline_input.rs | 2 +- .../chat_composer/vim_history_tests.rs | 6 +- codex-rs/tui/src/bottom_pane/questions.rs | 2 +- ...__recovered_answer_cancels_vim_search.snap | 3 +- .../tests/question_turn_end_tests.rs | 76 ++++++++++++++----- ..._tests__question_turn_end_large_draft.snap | 11 ++- ..._question_turn_end_recovered_composer.snap | 9 ++- 10 files changed, 139 insertions(+), 36 deletions(-) diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state.rs b/codex-rs/tui/src/bottom_pane/async_questions/state.rs index 666405debd..96c4c2536b 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state.rs @@ -2,8 +2,13 @@ //! Message IDs survive removal so replay cannot reopen an answered or skipped question. use super::*; +use crate::history_cell::sanitize_user_text; use codex_context_fragments::AnsweredQuestion; use codex_context_fragments::ContextualUserFragment; +use codex_utils_string::take_bytes_at_char_boundary; + +// Match the model-authored title budget used by AnsweredQuestion. +const MAX_RECOVERED_QUESTION_TITLE_BYTES: usize = 512; impl AsyncQuestions { pub(crate) fn append(&mut self, message_id: &str, questions: &[AsyncUserInputQuestion]) { @@ -206,15 +211,14 @@ impl AsyncQuestions { self.composer.reset_vim_mode(); } - /// Recover unsent typed answers before clearing all pending questions. + /// Recover unsent typed answers with their quoted questions before clearing pending questions. pub(crate) fn take_pending_drafts(&mut self) -> Vec { self.save_current_draft(); let drafts = self .state .pending .iter() - .map(|question| question.draft.text_with_pending().trim().to_string()) - .filter(|text| !text.is_empty()) + .filter_map(PendingQuestion::recovered_draft) .collect(); self.clear_pending(); drafts @@ -247,3 +251,28 @@ impl AsyncQuestions { self.resolve_answers(&answered_ids.into_iter().collect::>()); } } + +impl PendingQuestion { + fn recovered_draft(&self) -> Option { + let answer = self.draft.text_with_pending(); + let answer = answer.trim(); + if answer.is_empty() { + return None; + } + + let title = &self.question.title; + let prefix = take_bytes_at_char_boundary(title, MAX_RECOVERED_QUESTION_TITLE_BYTES); + let sanitized = sanitize_user_text(prefix.into()); + let mut quoted = textwrap::indent(&sanitized, "> "); + if quoted.ends_with('\n') { + quoted.pop(); + } + if quoted.is_empty() { + quoted.push_str("> "); + } + if prefix.len() < title.len() { + quoted.push('…'); + } + Some(format!("{quoted}\n\n{answer}")) + } +} diff --git a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs index 9b4ada31f7..dca4e202b2 100644 --- a/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs +++ b/codex-rs/tui/src/bottom_pane/async_questions/state_tests.rs @@ -30,6 +30,7 @@ fn editor() -> AsyncQuestions { #[test] fn taking_pending_drafts_preserves_order_expands_pastes_and_skips_blank_drafts() { let mut editor = editor(); + editor.state.pending[0].question.title = "First\n\n".into(); editor.set_expanded(/*expanded*/ true); let large_answer = "typed answer ".repeat(/*n*/ 200); editor.handle_paste(large_answer.clone()); @@ -40,12 +41,35 @@ fn taking_pending_drafts_preserves_order_expands_pastes_and_skips_blank_drafts() assert_eq!( editor.take_pending_drafts(), - vec![large_answer.trim().to_string(), "other answer".to_string()] + vec![ + format!("> First\n>\n\n{}", large_answer.trim()), + "> Second\n\nother answer".to_string() + ] ); assert_eq!((editor.unanswered_count(), editor.expanded), (0, false)); assert!(editor.submission.is_none()); } +#[test] +fn taking_pending_drafts_bounds_question_titles_without_truncating_answers() { + for (character, retained_count) in [("a", 512), ("界", 170), ("\x07", 0)] { + let mut editor = editor(); + editor.state.pending[0].question.title = + character.repeat(codex_protocol::user_input::MAX_USER_INPUT_TEXT_CHARS + 1); + let answer = "typed answer ".repeat(/*n*/ 200); + editor.handle_paste(answer.clone()); + + assert_eq!( + editor.take_pending_drafts(), + vec![format!( + "> {}…\n\n{}", + character.repeat(retained_count), + answer.trim(), + )] + ); + } +} + #[test] fn replay_preserves_drafts_and_does_not_reopen_handled_questions() { let mut original = editor(); diff --git a/codex-rs/tui/src/bottom_pane/chat_composer.rs b/codex-rs/tui/src/bottom_pane/chat_composer.rs index 8921eaa66c..c19a99dd58 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer.rs @@ -96,8 +96,9 @@ //! # Question Draft Recovery //! //! Live terminal turns recover typed, unsubmitted question answers into the main composer. -//! The append flushes buffered input, dismisses unused sparkle eligibility, and adds a newline -//! after existing text. The separator and recovered answer form one Vim edit; large answers +//! Each recovered answer follows its original question as a Markdown blockquote, with blank lines +//! separating questions, answers, and existing text. The append flushes buffered input and dismisses +//! unused sparkle eligibility. The separator and recovered answer form one Vim edit; large answers //! use atomic paste placeholders backed by their original text. //! Recovery escapes a shell or slash command prefix before appending answers, keeping the //! combined draft editable and its later submission literal. Shell mode's separate `!` becomes 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 d9f4f1fe01..48f7adff59 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 @@ -46,7 +46,7 @@ impl ChatComposer { } self.move_cursor_to_end(); if !self.current_text().is_empty() { - self.insert_str("\n"); + self.insert_str("\n\n"); } let char_count = drafts.chars().count(); if char_count > LARGE_PASTE_CHAR_THRESHOLD { diff --git a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs index 729f6ad2b0..f3a47b6754 100644 --- a/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs +++ b/codex-rs/tui/src/bottom_pane/chat_composer/vim_history_tests.rs @@ -104,7 +104,7 @@ fn recovered_answers_undo_as_one_vim_edit() { composer.current_text_with_pending(), composer.draft.textarea.vim_mode_label(), ), - (format!("{edited}\n{answer}"), Some(mode)) + (format!("{edited}\n\n{answer}"), Some(mode)) ); escape(&mut composer); let after = composer.draft_snapshot(); @@ -127,7 +127,7 @@ fn recovered_answers_undo_as_one_vim_edit() { composer.draft.textarea.vim_query().is_some(), composer.draft.textarea.is_vim_operator_pending(), ), - (format!("existing draft\n{answer}"), false, false) + (format!("existing draft\n\n{answer}"), false, false) ); let after = composer.draft_snapshot(); keys(&mut composer, "u"); @@ -151,7 +151,7 @@ fn recovered_answers_undo_as_one_vim_edit() { escape(&mut composer); assert_eq!( composer.current_text_with_pending(), - format!("bc\n{answer}") + format!("bc\n\n{answer}") ); let after = composer.draft_snapshot(); keys(&mut composer, "u"); diff --git a/codex-rs/tui/src/bottom_pane/questions.rs b/codex-rs/tui/src/bottom_pane/questions.rs index 0251efa896..132c5ea82e 100644 --- a/codex-rs/tui/src/bottom_pane/questions.rs +++ b/codex-rs/tui/src/bottom_pane/questions.rs @@ -59,7 +59,7 @@ impl BottomPane { return; } self.composer - .edit_stored_draft(|composer| composer.append_recovered_drafts(&drafts.join("\n"))); + .edit_stored_draft(|composer| composer.append_recovered_drafts(&drafts.join("\n\n"))); self.request_redraw(); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__recovered_answer_cancels_vim_search.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__recovered_answer_cancels_vim_search.snap index fe46d2b6eb..ff5b3fc3b6 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__recovered_answer_cancels_vim_search.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__chat_composer__tests__recovered_answer_cancels_vim_search.snap @@ -1,13 +1,14 @@ --- source: tui/src/bottom_pane/chat_composer.rs +assertion_line: 5289 expression: terminal.backend() --- " " "› existing draft " +" " " answer " " " " " " " " " -" " " 100% context left | Vim: Normal " diff --git a/codex-rs/tui/src/chatwidget/tests/question_turn_end_tests.rs b/codex-rs/tui/src/chatwidget/tests/question_turn_end_tests.rs index 6bb7392f7f..660a76a2fc 100644 --- a/codex-rs/tui/src/chatwidget/tests/question_turn_end_tests.rs +++ b/codex-rs/tui/src/chatwidget/tests/question_turn_end_tests.rs @@ -5,11 +5,11 @@ use crate::bottom_pane::QueuedInputAction; use codex_protocol::items::AsyncUserInputQuestion; use pretty_assertions::assert_eq; -fn questions(chat: &mut ChatWidget, message_id: &str) { +fn questions(chat: &mut ChatWidget, message_id: &str, title: &str) { chat.add_async_questions( message_id, &[AsyncUserInputQuestion { - title: "Which way?".into(), + title: title.into(), options: None, }], ); @@ -94,8 +94,8 @@ async fn question_turn_end_appends_open_drafts_in_order_once() { Vec::new(), ); chat.bottom_pane.set_composer_cursor(/*cursor*/ 0); - questions(&mut chat, "first"); - questions(&mut chat, "second"); + questions(&mut chat, "first", "Which style?"); + questions(&mut chat, "second", "What budget?"); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); chat.bottom_pane.handle_paste(" Over-ear ".into()); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); @@ -103,6 +103,8 @@ async fn question_turn_end_appends_open_drafts_in_order_once() { chat.handle_key_event(KeyEvent::new(KeyCode::Char('r'), KeyModifiers::CONTROL)); chat.handle_key_event(KeyEvent::from(KeyCode::Char('e'))); + let recovered = + "$tool existing draft\n\n> Which style?\n\nOver-ear\n\n> What budget?\n\nUnder 250"; handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None); assert_eq!( ( @@ -110,11 +112,7 @@ async fn question_turn_end_appends_open_drafts_in_order_once() { chat.bottom_pane.composer_text_elements(), chat.bottom_pane.question_editor().unanswered_count() ), - ( - "$tool existing draft\nOver-ear\nUnder 250".into(), - vec![element], - 0 - ) + (recovered.into(), vec![element], 0) ); assert!(ops.try_recv().is_err()); insta::assert_snapshot!( @@ -123,19 +121,16 @@ async fn question_turn_end_appends_open_drafts_in_order_once() { ); handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None); - assert_recovered_draft(&mut chat, "$tool existing draft\nOver-ear\nUnder 250"); + assert_recovered_draft(&mut chat, recovered); - questions(&mut chat, "large"); + questions(&mut chat, "large", "Which way?"); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); let large_answer = "long answer ".repeat(/*n*/ 200); chat.bottom_pane.handle_paste(large_answer.clone()); handle_turn_completed(&mut chat, "ended-turn", /*duration_ms*/ None); assert_eq!( chat.bottom_pane.composer_text_with_pending(), - format!( - "$tool existing draft\nOver-ear\nUnder 250\n{}", - large_answer.trim() - ) + format!("{recovered}\n\n> Which way?\n\n{}", large_answer.trim()) ); assert_eq!(chat.bottom_pane.composer_pending_pastes().len(), 1); insta::assert_snapshot!( @@ -151,7 +146,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure() chat.thread_id = Some(ThreadId::new()); chat.bottom_pane.set_disable_paste_burst(/*disabled*/ false); handle_turn_started(&mut chat, "turn"); - questions(&mut chat, "question"); + questions(&mut chat, "question", "Which way?"); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); // Leave typing buffered, then collapse before completion. for ch in "answer".chars() { @@ -182,7 +177,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure() .composer .unwrap() .text, - "main\nanswer" + "main\n\n> Which way?\n\nanswer" ); } chat.handle_server_notification( @@ -205,7 +200,7 @@ async fn question_turn_end_recovers_collapsed_drafts_on_completion_and_failure() assert_eq!(chat.bottom_pane.composer_text(), "earlier prompt"); assert!(!chat.bottom_pane.no_modal_or_popup_active()); chat.handle_key_event(KeyEvent::from(KeyCode::Esc)); - assert_recovered_draft(&mut chat, "main\nanswer"); + assert_recovered_draft(&mut chat, "main\n\n> Which way?\n\nanswer"); assert!(ops.try_recv().is_err()); } } @@ -227,7 +222,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() { chat.bottom_pane.set_disable_paste_burst(/*disabled*/ false); chat.bottom_pane .set_composer_text("main draft".into(), Vec::new(), Vec::new()); - questions(&mut chat, "question"); + questions(&mut chat, "question", "Which way?"); chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); chat.bottom_pane.handle_paste("answer".into()); chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); @@ -266,7 +261,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() { .composer .unwrap() .text, - "pending steer\nqueued prompt\nmain draft\nanswer" + "pending steer\nqueued prompt\nmain draft\n\n> Which way?\n\nanswer" ); // A query miss and unsuccessful Enter must retain both the search and the answers. chat.handle_key_event(KeyEvent::from(KeyCode::Char('z'))); @@ -290,7 +285,7 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() { } else { assert_recovered_draft( &mut chat, - &format!("pending steer\nqueued prompt\n{main_draft}\nanswer"), + &format!("pending steer\nqueued prompt\n{main_draft}\n\n> Which way?\n\nanswer"), ); } assert!(!chat.has_queued_follow_up_messages()); @@ -298,6 +293,45 @@ async fn question_turn_end_recovers_after_interruption_restores_queued_input() { } } +#[tokio::test] +async fn question_turn_end_keeps_multiline_question_and_answer_as_text() { + let (mut chat, _rx, mut ops) = make_chatwidget_manual(/*model_override*/ None).await; + chat.thread_id = Some(ThreadId::new()); + handle_turn_started(&mut chat, "turn"); + questions( + &mut chat, + "question", + "First line\r\n\r\n\x1b[31mLast\r line?\x1b[0m\x07", + ); + chat.handle_key_event(KeyEvent::new(KeyCode::Up, KeyModifiers::ALT)); + chat.bottom_pane + .handle_paste(" First answer line\n\nLast answer line ".into()); + chat.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::ALT)); + + handle_turn_completed(&mut chat, "turn", /*duration_ms*/ None); + let recovered = "> First line\n>\n> Last line?\n\nFirst answer line\n\nLast answer line"; + assert_eq!( + ( + chat.bottom_pane.composer_text_with_pending(), + chat.bottom_pane.question_editor().unanswered_count(), + ), + (recovered.to_string(), 0), + ); + assert!(ops.try_recv().is_err()); + + chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); + let Op::UserTurn { items, .. } = ops.try_recv().unwrap() else { + panic!("user turn") + }; + assert_eq!( + items, + vec![UserInput::Text { + text: recovered.into(), + text_elements: Vec::new(), + }] + ); +} + fn assert_recovered_draft(chat: &mut ChatWidget, text: &str) { assert_eq!( ( diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_large_draft.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_large_draft.snap index 208cc508f3..d9ffa813a3 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_large_draft.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_large_draft.snap @@ -1,11 +1,18 @@ --- source: tui/src/chatwidget/tests/question_turn_end_tests.rs -assertion_line: 86 +assertion_line: 73 expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))" --- › $tool existing draft + + > Which style? + Over-ear + + > What budget? + Under 250 - [Pasted Content 2399 chars] + + [Pasted Content 2413 chars] GPT-5.6-Sol default · /tmp/project diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_recovered_composer.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_recovered_composer.snap index 6288a8931f..07eb53c3c3 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_recovered_composer.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__question_turn_end_tests__question_turn_end_recovered_composer.snap @@ -1,9 +1,16 @@ --- source: tui/src/chatwidget/tests/question_turn_end_tests.rs -expression: "render_bottom_popup(&chat, 80)" +assertion_line: 55 +expression: "normalize_snapshot_paths(render_bottom_popup(&chat, 80))" --- › $tool existing draft + + > Which style? + Over-ear + + > What budget? + Under 250 GPT-5.6-Sol default · /tmp/project