From a4c8fc7a6b01ffc8965761d5da32fd8b33b1fb21 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Mon, 21 Sep 2026 16:17:16 +0000 Subject: [PATCH] Deduplicate deprecation notices in the TUI transcript (#47065) ## Why Global deprecation notices can be delivered again by hidden threads, creating duplicate transcript entries. ## What changed Skip incoming deprecation notices whose summary and details match a notice already in retained history. Preserve notices with different summaries or details, and allow notices to appear again after clearing history or rebuilding the transcript during a thread switch. ## Testing Add a regression test and transcript snapshot covering repeated live and replayed notifications, an open transcript overlay, distinct notice content, and delivery after clearing history or switching threads. GitOrigin-RevId: 9c0512f588dfb54f7934380119211b57b0df0ee4 --- codex-rs/tui/src/app/history_ui.rs | 14 +++ ...__deduplicated_deprecation_transcript.snap | 50 ++++++++++ .../src/app/tests/startup_warnings_tests.rs | 95 +++++++++++++++++++ codex-rs/tui/src/history_cell/notices.rs | 2 +- 4 files changed, 160 insertions(+), 1 deletion(-) create mode 100644 codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__deduplicated_deprecation_transcript.snap diff --git a/codex-rs/tui/src/app/history_ui.rs b/codex-rs/tui/src/app/history_ui.rs index 6f160c74b6..1aaf1143e5 100644 --- a/codex-rs/tui/src/app/history_ui.rs +++ b/codex-rs/tui/src/app/history_ui.rs @@ -23,6 +23,20 @@ pub(super) struct ThreadUsageStatusHistory { impl App { pub(super) fn insert_history_cell(&mut self, tui: &mut tui::Tui, cell: Box) { + // Global deprecations can be delivered again by hidden threads. Scope deduplication to + // retained history so clearing or rebuilding a transcript can show the notice again. + if let Some(notice) = cell + .as_any() + .downcast_ref::() + && self.transcript_cells.iter().any(|existing| { + existing + .as_any() + .downcast_ref::() + == Some(notice) + }) + { + return; + } if !crate::empty_state_animation::is_startup_cell(cell.as_ref()) { self.chat_widget .empty_state_animation diff --git a/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__deduplicated_deprecation_transcript.snap b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__deduplicated_deprecation_transcript.snap new file mode 100644 index 0000000000..edd3e5f0c7 --- /dev/null +++ b/codex-rs/tui/src/app/tests/snapshots/codex_tui__app__tests__startup_warnings_tests__deduplicated_deprecation_transcript.snap @@ -0,0 +1,50 @@ +--- +source: tui/src/app/tests/startup_warnings_tests.rs +expression: "format!(\"{buffer:?}\")" +--- +Buffer { + area: Rect { x: 0, y: 0, width: 80, height: 12 }, + content: [ + "/ T R A N S C R I P T / / / / / / / / / / / / / / / / / / / / / / / / / / / / / ", + "⚠ `transcript_v2` is deprecated. ", + "Remove it from your configuration. ", + " ", + " ", + "› Hello ", + " Please say hello. ", + " ", + "Ctrl+Space select ", + " ↑/↓ to scroll · pgup/pgdn to page · home/end to jump ", + " q close · f3 find · esc browse prompts ", + " ", + ], + styles: [ + x: 0, y: 0, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 0, y: 1, fg: Red, bg: Reset, underline: Reset, modifier: BOLD, + x: 2, y: 1, fg: Red, bg: Reset, underline: Reset, modifier: NONE, + x: 32, y: 1, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 34, y: 2, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD | DIM, + x: 2, y: 5, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 10, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 17, y: 8, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 1, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 4, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 17, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 26, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 37, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 45, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 53, y: 9, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + x: 0, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 1, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 2, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 11, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 13, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 21, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: BOLD, + x: 24, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: DIM, + x: 39, y: 10, fg: Reset, bg: Reset, underline: Reset, modifier: NONE, + ] +} diff --git a/codex-rs/tui/src/app/tests/startup_warnings_tests.rs b/codex-rs/tui/src/app/tests/startup_warnings_tests.rs index 26ba648168..db46bc0ee1 100644 --- a/codex-rs/tui/src/app/tests/startup_warnings_tests.rs +++ b/codex-rs/tui/src/app/tests/startup_warnings_tests.rs @@ -41,6 +41,101 @@ fn drain_history(app: &mut App, tui: &mut tui::Tui, events: &mut UnboundedReceiv } } +#[tokio::test] +async fn deprecation_delivery_deduplicates_retained_transcript() -> Result<()> { + let (mut app, mut events, _op_rx) = make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + tui.set_owned_screen(/*owned*/ false)?; + let notice = ServerNotification::DeprecationNotice( + codex_app_server_protocol::DeprecationNoticeNotification { + summary: "`transcript_v2` is deprecated.".into(), + details: Some("Remove it from your configuration.".into()), + }, + ); + app.chat_widget + .handle_server_notification(notice.clone(), /*replay_kind*/ None); + drain_history(&mut app, &mut tui, &mut events); + app.chat_widget.open_warnings(&app.transcript_cells); + app.chat_widget.handle_key_event(KeyCode::Esc.into()); + app.insert_history_cell( + &mut tui, + Box::new(UserHistoryCell { + message: "Hello\nPlease say hello.".into(), + text_elements: Vec::new(), + local_image_paths: Vec::new(), + remote_image_urls: Vec::new(), + spoken: false, + }), + ); + app.open_transcript_overlay(&mut tui); + // Deliver both duplicates before draining: insertion must not depend on frame timing. + for replay_kind in [None, Some(crate::chatwidget::ReplayKind::ThreadSnapshot)] { + app.chat_widget + .handle_server_notification(notice.clone(), replay_kind); + } + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 2); + assert_eq!(history_cell::warning_count(&app.transcript_cells), 1); + let area = Rect::new( + /*x*/ 0, /*y*/ 0, /*width*/ 80, /*height*/ 12, + ); + let mut buffer = Buffer::empty(area); + let Some(Overlay::Transcript(overlay)) = app.overlay.as_mut() else { + panic!("expected transcript") + }; + overlay.render(area, &mut buffer); + insta::assert_snapshot!("deduplicated_deprecation_transcript", format!("{buffer:?}")); + + for (summary, details) in [ + ("`transcript_v2` is deprecated.", None), + ("`transcript_v2` is deprecated.", Some("Updated guidance.")), + ("Another deprecated setting.", Some("Updated guidance.")), + ] { + app.chat_widget.handle_server_notification( + ServerNotification::DeprecationNotice( + codex_app_server_protocol::DeprecationNoticeNotification { + summary: summary.into(), + details: details.map(str::to_owned), + }, + ), + /*replay_kind*/ None, + ); + } + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 5); + assert_eq!(history_cell::warning_count(&app.transcript_cells), 2); + assert_eq!( + history_cell::warning_entries(&app.transcript_cells) + .into_iter() + .map(|entry| entry.details) + .collect::>(), + vec![ + "`transcript_v2` is deprecated.\nRemove it from your configuration.\n\n`transcript_v2` is deprecated.\nUpdated guidance.", + "Another deprecated setting.\nUpdated guidance.", + ], + ); + + app.reset_app_ui_state_after_clear(); + app.chat_widget + .handle_server_notification(notice.clone(), /*replay_kind*/ None); + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 1); + assert_eq!(history_cell::warning_count(&app.transcript_cells), 1); + + app.reset_for_thread_switch(&mut tui)?; + for replay_kind in [ + Some(crate::chatwidget::ReplayKind::ResumeInitialMessages), + None, + ] { + app.chat_widget + .handle_server_notification(notice.clone(), replay_kind); + } + drain_history(&mut app, &mut tui, &mut events); + assert_eq!(app.transcript_cells.len(), 1); + assert_eq!(history_cell::warning_count(&app.transcript_cells), 1); + Ok(()) +} + #[tokio::test] async fn startup_warnings_preserve_stream_repair_and_backtrack_selection() -> Result<()> { let mut app = crate::app::test_support::make_test_app().await; diff --git a/codex-rs/tui/src/history_cell/notices.rs b/codex-rs/tui/src/history_cell/notices.rs index a786340378..c5fa224172 100644 --- a/codex-rs/tui/src/history_cell/notices.rs +++ b/codex-rs/tui/src/history_cell/notices.rs @@ -252,7 +252,7 @@ impl HistoryCell for SafetyAccessBlockCell { } } -#[derive(Debug)] +#[derive(Debug, Eq, PartialEq)] pub(crate) struct DeprecationNoticeCell { summary: String, details: Option,