Dismiss viewed TUI warnings when closing the viewer (#48205)

## 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
This commit is contained in:
Felipe Coury
2026-09-25 20:26:19 +00:00
committed by copyberry
parent dd9a7101d9
commit 7703f8b354
15 changed files with 294 additions and 26 deletions
+10
View File
@@ -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);
+3
View File
@@ -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();
@@ -25,4 +25,4 @@ Earlier transcript row 22
esc back · ctrl+o copy · ←/→ warning · ↓ scroll
esc dismiss & close · ctrl+o copy · ←/→ warning · ↓ scroll
@@ -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
+119 -7
View File
@@ -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::<String>();
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::<String>()
.trim_end()
.to_string()
})
.collect::<Vec<_>>()
.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::<String>();
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, _) =
+5
View File
@@ -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<crate::history_cell::WarningEntry>,
},
/// Export all current-thread history to the selected destination.
ExportTranscript {
+8 -5
View File
@@ -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());
}
@@ -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
+3 -1
View File
@@ -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<WarningEntry>) {
pub(crate) fn show_warnings(&mut self, entries: Vec<WarningEntry>, transcript: Arc<()>) {
self.warnings_view = Some(warnings_view::WarningsView::new(
entries,
transcript,
self.keymap.clone(),
self.app_event_tx.clone(),
));
+23 -3
View File
@@ -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<WarningEntry>,
transcript: Arc<()>,
current: usize,
visited: Vec<Cell<bool>>,
offset: Cell<usize>,
page_size: Cell<usize>,
max_offset: Cell<usize>,
@@ -30,11 +34,14 @@ pub(super) struct WarningsView {
impl WarningsView {
pub(super) fn new(
entries: Vec<WarningEntry>,
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) {
@@ -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
@@ -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(),
},
]
);
}
+1 -1
View File
@@ -579,7 +579,7 @@ pub(crate) struct ChatWidget {
backend_banner_notice_model: Option<String>,
// Remember the account's Reserve entry notice across chats and transient banner refreshes.
luna_reserve_notice_account_id: Option<String>,
warning_display_state: WarningDisplayState,
pub(crate) warning_display_state: WarningDisplayState,
rate_limit_switch_prompt: RateLimitSwitchPromptState,
add_credits_nudge_email_in_flight: Option<rate_limits::PendingCreditsNudge>,
adaptive_chunking: AdaptiveChunkingPolicy,
@@ -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
+38 -4
View File
@@ -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<String>,
fallback_model_metadata_slugs: HashSet<String>,
/// Only hide an unchanged diagnostic: an identity can acquire more details later.
pub(crate) dismissed: BTreeMap<WarningId, String>,
/// 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<Vec<Arc<dyn HistoryCell>>>,
}
impl WarningDisplayState {
fn visible_entries(&self, cells: &[Arc<dyn HistoryCell>]) -> Vec<WarningEntry> {
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<dyn HistoryCell>]) {
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<dyn HistoryCell>]) {
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(