From 7703f8b35460f54df49918349fdcce956fe12ac5 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 25 Sep 2026 20:09:57 +0000 Subject: [PATCH] Dismiss viewed TUI warnings when closing the viewer (#48205) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why Closing the warnings viewer left reviewed diagnostics in the warning badge and subsequent visits, with no way to dismiss them. ## What changed - Dismiss only warning pages actually rendered when closing with `Esc`, `F2`, or `Ctrl+C`, and update the footer hint to “dismiss & close”. Unviewed warnings remain available. - Hide dismissed diagnostics from the badge and viewer while their details are unchanged. Updated details make a warning visible again. - Reset dismissals when clearing the transcript and ignore queued dismissals from an earlier transcript. - Cache warning counts until transcript cell identities or dismissal decisions change. ## Testing Add regression coverage for partial dismissal, navigation that skips rendering a page, changed warning details, and transcript resets with queued dismissals. Update snapshots for the close hint and remaining warning count. GitOrigin-RevId: 3ce88c62a7f0ff9de34062165051e22168d3db25 --- codex-rs/tui/src/app/event_dispatch.rs | 10 ++ codex-rs/tui/src/app/history_ui.rs | 3 + ...i__app__warnings_tests__warnings_page.snap | 2 +- ...ngs_tests__warnings_partial_dismissal.snap | 28 ++++ codex-rs/tui/src/app/warnings_tests.rs | 126 +++++++++++++++++- codex-rs/tui/src/app_event.rs | 5 + codex-rs/tui/src/bottom_pane/mod.rs | 13 +- ...warnings_view__tests__warnings_narrow.snap | 2 +- codex-rs/tui/src/bottom_pane/warnings.rs | 4 +- codex-rs/tui/src/bottom_pane/warnings_view.rs | 26 +++- .../src/bottom_pane/warnings_view_render.rs | 8 +- .../src/bottom_pane/warnings_view_tests.rs | 47 ++++++- codex-rs/tui/src/chatwidget.rs | 2 +- ..._app_server__sqlite_log_write_warning.snap | 2 +- codex-rs/tui/src/chatwidget/warnings.rs | 42 +++++- 15 files changed, 294 insertions(+), 26 deletions(-) create mode 100644 codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_partial_dismissal.snap diff --git a/codex-rs/tui/src/app/event_dispatch.rs b/codex-rs/tui/src/app/event_dispatch.rs index 7ea0e3b622..dcb8ac18bb 100644 --- a/codex-rs/tui/src/app/event_dispatch.rs +++ b/codex-rs/tui/src/app/event_dispatch.rs @@ -41,6 +41,7 @@ impl App { AppEvent::OpenDaemonMenu | AppEvent::OpenWarnings | AppEvent::CopyWarning(_) + | AppEvent::UpdateWarnings { .. } | AppEvent::CopySelection { .. } | AppEvent::ConfirmDaemonUpdate(_) | AppEvent::RunDaemonUpdate(_) @@ -347,6 +348,15 @@ impl App { } } AppEvent::OpenWarnings => self.chat_widget.open_warnings(&self.transcript_cells), + AppEvent::UpdateWarnings { transcript, dismissed } => { + if !Arc::ptr_eq(&transcript, &self.chat_widget.warning_display_state.transcript) { + return Ok(AppRunControl::Continue); + } + let state = &mut self.chat_widget.warning_display_state.dismissed; + state.extend(dismissed.into_iter().map(|entry| (entry.id, entry.details))); + self.chat_widget.warning_display_state.synced_cells = None; + tui.frame_requester().schedule_frame(); + } AppEvent::CopyWarning(text) => { let result = tui.copy_transcript_selection(&text, crate::clipboard_copy::CopyFormat::PlainText); self.chat_widget.show_selection_copy_result(result); diff --git a/codex-rs/tui/src/app/history_ui.rs b/codex-rs/tui/src/app/history_ui.rs index 1aaf1143e5..fe379b2a57 100644 --- a/codex-rs/tui/src/app/history_ui.rs +++ b/codex-rs/tui/src/app/history_ui.rs @@ -395,6 +395,9 @@ impl App { pub(super) fn reset_transcript_state_after_clear(&mut self) { self.overlay = None; self.transcript_cells.clear(); + self.chat_widget.warning_display_state.dismissed.clear(); + self.chat_widget.warning_display_state.transcript = Arc::default(); + self.chat_widget.warning_display_state.synced_cells = None; self.native_history = Default::default(); self.cancel_pending_key_chord(); self.transcript_view = Default::default(); diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_page.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_page.snap index 5366fc1466..ba52e2339f 100644 --- a/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_page.snap +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_page.snap @@ -25,4 +25,4 @@ Earlier transcript row 22 - esc back · ctrl+o copy · ←/→ warning · ↓ scroll + esc dismiss & close · ctrl+o copy · ←/→ warning · ↓ scroll diff --git a/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_partial_dismissal.snap b/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_partial_dismissal.snap new file mode 100644 index 0000000000..915b91463e --- /dev/null +++ b/codex-rs/tui/src/app/snapshots/codex_tui__app__warnings_tests__warnings_partial_dismissal.snap @@ -0,0 +1,28 @@ +--- +source: tui/src/app/warnings_tests.rs +expression: "crate::chatwidget::tests::helpers::normalize_snapshot_paths(screen)" +--- +Earlier transcript row 12 +Earlier transcript row 13 +Earlier transcript row 14 +Earlier transcript row 15 +Earlier transcript row 16 +Earlier transcript row 17 +Earlier transcript row 18 +Earlier transcript row 19 +Earlier transcript row 20 +Earlier transcript row 21 +Earlier transcript row 22 +Earlier transcript row 23 +Earlier transcript row 24 +Earlier transcript row 25 +Earlier transcript row 26 +Earlier transcript row 27 +Earlier transcript row 28 +Earlier transcript row 29 + ↓ Back to bottom · esc + +› draft + + GPT-5.6-Sol default · /tmp/project + esc latest ⚠ 1 warning · f2 to view diff --git a/codex-rs/tui/src/app/warnings_tests.rs b/codex-rs/tui/src/app/warnings_tests.rs index a5f9f39d08..0a2c2e8e16 100644 --- a/codex-rs/tui/src/app/warnings_tests.rs +++ b/codex-rs/tui/src/app/warnings_tests.rs @@ -64,8 +64,13 @@ async fn warnings_hide_and_restore_draft_and_freeze_until_reopened() -> Result<( app.handle_tui_event(&mut tui, &mut app_server, TuiEvent::Paste("xample".into())) .await?; assert!(app.chat_widget.keymap_contexts().is_warnings()); - app.chat_widget - .handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyCode::Esc.into()), + ) + .await?; + assert!(!app.chat_widget.keymap_contexts().is_warnings()); app.render_owned_transcript(&mut tui, size)?; assert_eq!( crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal), @@ -85,19 +90,63 @@ async fn warnings_hide_and_restore_draft_and_freeze_until_reopened() -> Result<( .map(ratatui::buffer::Cell::symbol) .collect::(); assert!(frozen.contains("1 of 1")); + // The first close's queued update may arrive while the frozen viewer is reopened. + let first_dismissal = std::iter::from_fn(|| events.try_recv().ok()) + .find(|event| matches!(event, AppEvent::UpdateWarnings { dismissed, .. } if !dismissed.is_empty())) + .expect("the first close queued a dismissal"); + app.handle_event(&mut tui, &mut app_server, first_dismissal) + .await?; app.chat_widget.handle_key_event(KeyCode::Esc.into()); assert_eq!(app.chat_widget.composer_text_with_pending(), "/m"); + let dismissal = std::iter::from_fn(|| events.try_recv().ok()) + .find(|event| matches!(event, AppEvent::UpdateWarnings { dismissed, .. } if !dismissed.is_empty())) + .expect("dismiss the viewed warning offline"); + app.handle_event(&mut tui, &mut app_server, dismissal) + .await?; + // The slash popup masks the passive footer; check the count with a plain draft. + app.chat_widget.apply_external_edit("draft".into()); + app.render_owned_transcript(&mut tui, size)?; + let screen = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal) + .content + .chunks(usize::from(size.width)) + .map(|row| { + row.iter() + .map(ratatui::buffer::Cell::symbol) + .collect::() + .trim_end() + .to_string() + }) + .collect::>() + .join("\n"); + insta::assert_snapshot!( + "warnings_partial_dismissal", + crate::chatwidget::tests::helpers::normalize_snapshot_paths(screen), + ); + app.chat_widget.apply_external_edit("/m".into()); app.chat_widget.open_warnings(&app.transcript_cells); - app.chat_widget.handle_key_event(KeyCode::Right.into()); app.render_owned_transcript(&mut tui, size)?; let reopened = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal) .content .iter() .map(ratatui::buffer::Cell::symbol) .collect::(); - assert!(reopened.contains("2 of 2")); + assert!(reopened.contains("1 of 1")); assert!(reopened.contains("Later warning")); - app.chat_widget.handle_key_event(KeyCode::Esc.into()); + app.handle_tui_event( + &mut tui, + &mut app_server, + TuiEvent::Key(KeyEvent::new(KeyCode::Char('c'), KeyModifiers::CONTROL)), + ) + .await?; + let dismissal = std::iter::from_fn(|| events.try_recv().ok()) + .find(|event| matches!(event, AppEvent::UpdateWarnings { dismissed, .. } if !dismissed.is_empty())) + .expect("Ctrl+C dismisses the viewed warning"); + app.handle_event(&mut tui, &mut app_server, dismissal) + .await?; + app.render_owned_transcript(&mut tui, size)?; + let after = crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal); + assert!(!after.content.iter().any(|cell| cell.symbol() == "⚠")); + assert_eq!(tui.terminal.last_known_cursor_pos, cursor); // An empty composer must not steal Escape for transcript backtracking. app.chat_widget.apply_external_edit(String::new()); @@ -167,8 +216,7 @@ async fn warnings_badge_and_pages_work_with_terminal_scrollback() -> Result<()> assert!(screen.contains(text)); } assert!(!screen.contains("fallback draft")); - app.chat_widget - .handle_key_event(KeyEvent::new(KeyCode::Esc, KeyModifiers::NONE)); + app.chat_widget.handle_key_event(KeyCode::Esc.into()); app.render_chat_widget_frame(&mut tui, size)?; assert_eq!( crate::custom_terminal::test_support::last_rendered_buffer(&tui.terminal), @@ -177,6 +225,70 @@ async fn warnings_badge_and_pages_work_with_terminal_scrollback() -> Result<()> Ok(()) } +#[tokio::test] +async fn warnings_changed_details_reappear_and_clear_resets_dismissal() -> Result<()> { + use crate::chatwidget::tests::helpers::render_bottom_popup; + use codex_app_server_protocol::McpServerStartupFailureReason; + + let (mut app, mut events, _ops) = crate::app::tests::make_test_app_with_channels().await; + let mut tui = crate::tui::test_support::make_test_tui()?; + let mut app_server = crate::start_embedded_app_server_for_picker(&app.config).await?; + let original = history_cell::StartupWarningsCell::mcp( + vec!["example MCP unavailable".into()], + vec!["example".into()], + /*failure_reason*/ None, + ); + app.insert_history_cell(&mut tui, Box::new(original.clone())); + app.chat_widget.open_warnings(&app.transcript_cells); + let before = render_bottom_popup(&app.chat_widget, /*width*/ 80); + // The ID stays the same. Sign-in instructions arrive while the old page is open. + app.insert_history_cell( + &mut tui, + Box::new(history_cell::StartupWarningsCell::mcp( + vec!["Login again".into()], + vec!["example".into()], + Some(McpServerStartupFailureReason::ReauthenticationRequired), + )), + ); + assert_eq!(render_bottom_popup(&app.chat_widget, /*width*/ 80), before); + app.chat_widget.handle_key_event(KeyCode::Esc.into()); + let dismissal = std::iter::from_fn(|| events.try_recv().ok()) + .find(|event| matches!(event, AppEvent::UpdateWarnings { dismissed, .. } if !dismissed.is_empty())) + .expect("old details dismissed"); + app.handle_event(&mut tui, &mut app_server, dismissal) + .await?; + app.chat_widget.open_warnings(&app.transcript_cells); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 80).contains("Sign-in required")); + app.chat_widget.handle_key_event(KeyCode::F(2).into()); + let dismissal = std::iter::from_fn(|| events.try_recv().ok()) + .find(|event| matches!(event, AppEvent::UpdateWarnings { dismissed, .. } if !dismissed.is_empty())) + .expect("updated details dismissed"); + app.handle_event(&mut tui, &mut app_server, dismissal) + .await?; + // Duplicate unchanged delivery must not restore the badge or page. + app.insert_history_cell(&mut tui, Box::new(original.clone())); + app.chat_widget.open_warnings(&app.transcript_cells); + assert!(render_bottom_popup(&app.chat_widget, /*width*/ 80).contains("No warnings")); + app.chat_widget.handle_key_event(KeyCode::Esc.into()); + app.reset_app_ui_state_after_clear(); + app.insert_history_cell(&mut tui, Box::new(original.clone())); + app.chat_widget.open_warnings(&app.transcript_cells); + assert_eq!(render_bottom_popup(&app.chat_widget, /*width*/ 80), before); + // Clear before the queued dismissal arrives, then deliver the same warning again. + app.chat_widget.handle_key_event(KeyCode::Esc.into()); + app.reset_app_ui_state_after_clear(); + app.insert_history_cell(&mut tui, Box::new(original)); + while let Ok(event) = events.try_recv() { + if matches!(event, AppEvent::UpdateWarnings { .. }) { + app.handle_event(&mut tui, &mut app_server, event).await?; + } + } + app.chat_widget.open_warnings(&app.transcript_cells); + assert_eq!(render_bottom_popup(&app.chat_widget, /*width*/ 80), before); + app_server.shutdown().await?; + Ok(()) +} + #[tokio::test] async fn warnings_keep_remapped_copy_keys_out_of_composer_handlers() { let (mut widget, _, mut rx, _) = diff --git a/codex-rs/tui/src/app_event.rs b/codex-rs/tui/src/app_event.rs index 784e763c14..410af851a0 100644 --- a/codex-rs/tui/src/app_event.rs +++ b/codex-rs/tui/src/app_event.rs @@ -448,6 +448,11 @@ pub(crate) enum AppEvent { OpenWarnings, /// Copy a diagnostic and acknowledge in the footer, without appending history. CopyWarning(String), + /// Dismiss the visited diagnostics for this transcript, using the frozen warning details. + UpdateWarnings { + transcript: Arc<()>, + dismissed: Vec, + }, /// Export all current-thread history to the selected destination. ExportTranscript { diff --git a/codex-rs/tui/src/bottom_pane/mod.rs b/codex-rs/tui/src/bottom_pane/mod.rs index f3382db2da..18c8f8ef3c 100644 --- a/codex-rs/tui/src/bottom_pane/mod.rs +++ b/codex-rs/tui/src/bottom_pane/mod.rs @@ -818,8 +818,9 @@ impl BottomPane { .warnings_view .as_mut() .is_some_and(|view| view.handle_key(key_event)) + && let Some(view) = self.warnings_view.take() { - self.warnings_view = None; + view.close(); } self.request_redraw(); return InputResult::None; @@ -941,7 +942,9 @@ impl BottomPane { /// quit/interrupt state machine and uses the result to decide what happens next. pub(crate) fn on_ctrl_c(&mut self) -> CancellationEvent { if self.warnings_active() { - self.warnings_view = None; + if let Some(view) = self.warnings_view.take() { + view.close(); + } self.request_redraw(); return CancellationEvent::Handled; } @@ -2851,14 +2854,14 @@ mod tests { ] { let mut pane = test_pane(tx.clone()); match source { - "warning open" => pane.show_warnings(Vec::new()), + "warning open" => pane.show_warnings(Vec::new(), Default::default()), "warning navigation" => { - pane.show_warnings(Vec::new()); + pane.show_warnings(Vec::new(), Default::default()); pane.last_composer_activity_at = None; pane.handle_key_event(KeyEvent::new(KeyCode::Down, KeyModifiers::NONE)); } "warning paste" => { - pane.show_warnings(Vec::new()); + pane.show_warnings(Vec::new(), Default::default()); pane.last_composer_activity_at = None; pane.handle_paste("query".into()); } diff --git a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__warnings_view__tests__warnings_narrow.snap b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__warnings_view__tests__warnings_narrow.snap index c6803ef938..75aca2fd6b 100644 --- a/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__warnings_view__tests__warnings_narrow.snap +++ b/codex-rs/tui/src/bottom_pane/snapshots/codex_tui__bottom_pane__warnings_view__tests__warnings_narrow.snap @@ -10,4 +10,4 @@ expression: "draw(&view, 40, 9)" Details retained in full, including remediation instructions. Details retained in full, including - esc back · ctrl+o copy · ←/→ warning + esc dismiss & close · ctrl+o copy diff --git a/codex-rs/tui/src/bottom_pane/warnings.rs b/codex-rs/tui/src/bottom_pane/warnings.rs index 101c610e44..522856b48e 100644 --- a/codex-rs/tui/src/bottom_pane/warnings.rs +++ b/codex-rs/tui/src/bottom_pane/warnings.rs @@ -2,11 +2,13 @@ use super::*; use crate::history_cell::WarningEntry; +use std::sync::Arc; impl BottomPane { - pub(crate) fn show_warnings(&mut self, entries: Vec) { + pub(crate) fn show_warnings(&mut self, entries: Vec, transcript: Arc<()>) { self.warnings_view = Some(warnings_view::WarningsView::new( entries, + transcript, self.keymap.clone(), self.app_event_tx.clone(), )); diff --git a/codex-rs/tui/src/bottom_pane/warnings_view.rs b/codex-rs/tui/src/bottom_pane/warnings_view.rs index f87ec127ba..164962edfb 100644 --- a/codex-rs/tui/src/bottom_pane/warnings_view.rs +++ b/codex-rs/tui/src/bottom_pane/warnings_view.rs @@ -1,4 +1,5 @@ //! A frozen set of warnings, shown one at a time without changing the retained draft. +//! On intentional close, dismiss only warnings whose pages were actually drawn. use crate::app_event::AppEvent; use crate::app_event_sender::AppEventSender; @@ -11,13 +12,16 @@ use crate::keymap::RuntimeKeymap; use crossterm::event::KeyEvent; use crossterm::event::KeyEventKind; use std::cell::Cell; +use std::sync::Arc; #[path = "warnings_view_render.rs"] mod render; pub(super) struct WarningsView { entries: Vec, + transcript: Arc<()>, current: usize, + visited: Vec>, offset: Cell, page_size: Cell, max_offset: Cell, @@ -30,11 +34,14 @@ pub(super) struct WarningsView { impl WarningsView { pub(super) fn new( entries: Vec, + transcript: Arc<()>, keymap: RuntimeKeymap, tx: AppEventSender, ) -> Self { Self { + visited: vec![Cell::new(/*value*/ false); entries.len()], entries, + transcript, current: 0, offset: Cell::new(/*value*/ 0), page_size: Cell::new(/*value*/ 1), @@ -54,6 +61,21 @@ impl WarningsView { KeymapContextSet::warnings() } + pub(super) fn close(self) { + let dismissed: Vec<_> = self + .entries + .into_iter() + .zip(self.visited) + .filter_map(|(entry, visited)| visited.get().then_some(entry)) + .collect(); + if !dismissed.is_empty() { + self.tx.send(AppEvent::UpdateWarnings { + transcript: self.transcript, + dismissed, + }); + } + } + /// Returns true when the user closes the viewer. No key is forwarded to the draft. pub(super) fn handle_key(&mut self, event: KeyEvent) -> bool { if event.kind == KeyEventKind::Release { @@ -64,9 +86,7 @@ impl WarningsView { .list .action_for(event) .filter(|action| *action != ListAction::Accept); - if action.is_none() && self.keymap.app.open_warnings.is_pressed(event) - || self.keymap.list.cancel.is_pressed(event) - { + if action.is_none() && self.keymap.app.open_warnings.is_pressed(event) { return true; } if action.is_none() && self.keymap.app.copy.is_pressed(event) { diff --git a/codex-rs/tui/src/bottom_pane/warnings_view_render.rs b/codex-rs/tui/src/bottom_pane/warnings_view_render.rs index c187fbc50a..da56e4f326 100644 --- a/codex-rs/tui/src/bottom_pane/warnings_view_render.rs +++ b/codex-rs/tui/src/bottom_pane/warnings_view_render.rs @@ -38,6 +38,12 @@ impl Renderable for WarningsView { }; Line::from(title.bold()).render(header, buf); let details = entry.map_or("No warnings", |entry| entry.details.as_str()); + if body.width > 0 + && body.height > 0 + && let Some(visited) = self.visited.get(self.current) + { + visited.set(/*val*/ true); + } let lines: Vec<_> = details .lines() .flat_map(|line| textwrap::wrap(line, usize::from(body.width.max(/*other*/ 1)))) @@ -70,7 +76,7 @@ impl Renderable for WarningsView { self.keymap .primary_hint(KeymapContext::List, "cancel") .map(crate::key_hint::ShortcutHint::display_label), - "back", + "dismiss & close", ), ( self.keymap diff --git a/codex-rs/tui/src/bottom_pane/warnings_view_tests.rs b/codex-rs/tui/src/bottom_pane/warnings_view_tests.rs index 4192fabe62..5225930e9c 100644 --- a/codex-rs/tui/src/bottom_pane/warnings_view_tests.rs +++ b/codex-rs/tui/src/bottom_pane/warnings_view_tests.rs @@ -5,6 +5,7 @@ use crate::history_cell::WarningId; use crate::render::renderable::Renderable; use crossterm::event::KeyCode; use crossterm::event::KeyModifiers; +use pretty_assertions::assert_eq; use ratatui::buffer::Buffer; use ratatui::layout::Rect; use tokio::sync::mpsc::unbounded_channel; @@ -52,6 +53,7 @@ fn warning_pages_copy_only_the_current_diagnostic() { let (tx, mut rx) = unbounded_channel(); let mut view = WarningsView::new( entries(), + Arc::default(), RuntimeKeymap::from_config(&toml::from_str("[list]\nmove_right = 'x'").unwrap()).unwrap(), AppEventSender::new(tx), ); @@ -77,7 +79,12 @@ fn warning_pages_wrap_and_scroll_long_diagnostics() { "Long diagnostic 日本語\n{}\nEND OF DIAGNOSTIC", "Details retained in full, including remediation instructions.\n".repeat(/*n*/ 20) ); - let mut view = WarningsView::new(data, RuntimeKeymap::defaults(), AppEventSender::new(tx)); + let mut view = WarningsView::new( + data, + Arc::default(), + RuntimeKeymap::defaults(), + AppEventSender::new(tx), + ); insta::assert_snapshot!("warnings_narrow", draw(&view, /*width*/ 40, /*height*/ 9)); key(&mut view, KeyCode::PageDown); assert!(!draw(&view, /*width*/ 40, /*height*/ 9).contains("Long diagnostic")); @@ -88,3 +95,41 @@ fn warning_pages_wrap_and_scroll_long_diagnostics() { key(&mut view, KeyCode::Left); assert!(draw(&view, /*width*/ 40, /*height*/ 9).contains("Long diagnostic")); } + +#[test] +fn warnings_dismiss_only_drawn_pages_after_navigation_skips_a_frame() { + let (tx, mut rx) = unbounded_channel(); + let mut data = entries(); + data.push(WarningEntry { + id: WarningId::Message("later".into()), + source: "Startup".into(), + details: "Not yet viewed".into(), + }); + let mut view = WarningsView::new( + data, + Arc::default(), + RuntimeKeymap::defaults(), + AppEventSender::new(tx), + ); + assert!(draw(&view, /*width*/ 80, /*height*/ 12).contains("1 of 3")); + assert!(!key(&mut view, KeyCode::Right)); + assert!(!key(&mut view, KeyCode::Right)); + assert!(draw(&view, /*width*/ 80, /*height*/ 12).contains("3 of 3")); + key(&mut view, KeyCode::Left); + assert!(key(&mut view, KeyCode::F(2))); + view.close(); + let Ok(AppEvent::UpdateWarnings { dismissed, .. }) = rx.try_recv() else { + panic!("expected warning decisions"); + }; + assert_eq!( + dismissed, + vec![ + entries()[0].clone(), + WarningEntry { + id: WarningId::Message("later".into()), + source: "Startup".into(), + details: "Not yet viewed".into(), + }, + ] + ); +} diff --git a/codex-rs/tui/src/chatwidget.rs b/codex-rs/tui/src/chatwidget.rs index 79db6b69bf..56f85b46f5 100644 --- a/codex-rs/tui/src/chatwidget.rs +++ b/codex-rs/tui/src/chatwidget.rs @@ -579,7 +579,7 @@ pub(crate) struct ChatWidget { backend_banner_notice_model: Option, // Remember the account's Reserve entry notice across chats and transient banner refreshes. luna_reserve_notice_account_id: Option, - warning_display_state: WarningDisplayState, + pub(crate) warning_display_state: WarningDisplayState, rate_limit_switch_prompt: RateLimitSwitchPromptState, add_credits_nudge_email_in_flight: Option, adaptive_chunking: AdaptiveChunkingPolicy, diff --git a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__sqlite_log_write_warning.snap b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__sqlite_log_write_warning.snap index 14f59f8ef9..b685bdf60a 100644 --- a/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__sqlite_log_write_warning.snap +++ b/codex-rs/tui/src/chatwidget/tests/snapshots/codex_tui__chatwidget__tests__app_server__sqlite_log_write_warning.snap @@ -13,4 +13,4 @@ expression: "super::helpers::render_bottom_popup(&chat, 80)" - esc back · ctrl+o copy · ←/→ warning · ↓ scroll + esc dismiss & close · ctrl+o copy · ←/→ warning · ↓ scroll diff --git a/codex-rs/tui/src/chatwidget/warnings.rs b/codex-rs/tui/src/chatwidget/warnings.rs index d6d9c20bed..c196780349 100644 --- a/codex-rs/tui/src/chatwidget/warnings.rs +++ b/codex-rs/tui/src/chatwidget/warnings.rs @@ -1,7 +1,10 @@ //! Deduplicate incoming diagnostics and project them into the warning footer and viewer. use crate::history_cell::HistoryCell; +use crate::history_cell::WarningEntry; +use crate::history_cell::WarningId; use crate::tui::TuiEvent; +use std::collections::BTreeMap; use std::collections::HashSet; use std::sync::Arc; @@ -10,16 +13,30 @@ const FALLBACK_MODEL_METADATA_WARNING_SUFFIX: &str = "` not found. Defaulting to fallback metadata; this can degrade performance and cause issues."; #[derive(Default)] -pub(super) struct WarningDisplayState { +pub(crate) struct WarningDisplayState { pub(super) count: usize, /// Completed resume history does not end startup; active work does. pub(super) startup_complete: bool, /// Initialization config warnings may also arrive as ordinary thread warnings. pub(super) startup_config_warnings: HashSet, fallback_model_metadata_slugs: HashSet, + /// Only hide an unchanged diagnostic: an identity can acquire more details later. + pub(crate) dismissed: BTreeMap, + /// Identity of the current transcript; queued decisions must not survive its reset. + pub(crate) transcript: Arc<()>, + /// Warning-bearing history cells are immutable. Avoid rebuilding diagnostic text until + /// the transcript's cell identities or the user's dismissal decisions change. + pub(crate) synced_cells: Option>>, } impl WarningDisplayState { + fn visible_entries(&self, cells: &[Arc]) -> Vec { + crate::history_cell::warning_entries(cells) + .into_iter() + .filter(|entry| self.dismissed.get(&entry.id) != Some(&entry.details)) + .collect() + } + pub(super) fn should_display(&mut self, message: &str) -> bool { !self.startup_config_warnings.contains(message) && fallback_model_metadata_warning_slug(message) @@ -36,12 +53,29 @@ fn fallback_model_metadata_warning_slug(message: &str) -> Option<&str> { impl super::ChatWidget { pub(crate) fn open_warnings(&mut self, cells: &[Arc]) { self.empty_state_animation.borrow_mut().pause_clock(); - self.bottom_pane - .show_warnings(crate::history_cell::warning_entries(cells)); + self.bottom_pane.show_warnings( + self.warning_display_state.visible_entries(cells), + Arc::clone(&self.warning_display_state.transcript), + ); } pub(crate) fn sync_warnings(&mut self, cells: &[Arc]) { - self.warning_display_state.count = crate::history_cell::warning_count(cells); + let state = &mut self.warning_display_state; + if state.synced_cells.as_ref().is_some_and(|synced| { + synced.len() == cells.len() + && synced + .iter() + .zip(cells) + .all(|(previous, current)| Arc::ptr_eq(previous, current)) + }) { + return; + } + state.count = if state.dismissed.is_empty() { + crate::history_cell::warning_count(cells) + } else { + state.visible_entries(cells).len() + }; + state.synced_cells = Some(cells.to_vec()); } pub(crate) fn handle_warning_event(