From cccc1b618ed2f4ddd2abdb2a22f5c83c7a958376 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Thu, 23 Apr 2026 13:21:35 -0700 Subject: [PATCH] Stabilize approvals popup disabled-row test (#19178) ## Summary The Windows Bazel job has been failing in `chatwidget::tests::permissions::approvals_popup_navigation_skips_disabled` because the test assumed a fixed approvals popup row order and shortcut for the disabled permissions option. The approvals popup can include platform-specific rows, so those assumptions made the test brittle. This updates the test to derive the disabled row shortcut from the rendered popup and assert navigation continues to skip disabled rows before checking that disabled numeric shortcuts do not close or accept the popup. --- .../tui/src/chatwidget/tests/permissions.rs | 61 ++++++++++++------- 1 file changed, 38 insertions(+), 23 deletions(-) diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs index 51c6a4df1..29f953452 100644 --- a/codex-rs/tui/src/chatwidget/tests/permissions.rs +++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs @@ -199,34 +199,49 @@ async fn approvals_popup_navigation_skips_disabled() { .expect("construct constrained approval policy"); chat.open_approvals_popup(); - // Keep the popup layout stable across platforms and feature defaults so - // the hidden numeric shortcut below still targets the disabled Full Access row. - let disabled_shortcut = if cfg!(target_os = "windows") { - '3' - } else { - '2' - }; + let popup = render_bottom_popup(&chat, /*width*/ 80); + let mut disabled_shortcut = None; + let mut row_number = 0; + for line in popup.lines() { + let row = line + .trim_start() + .strip_prefix('\u{203a}') + .unwrap_or_else(|| line.trim_start()) + .trim_start(); + let mut chars = row.chars(); + let has_numeric_shortcut = + chars.next().is_some_and(|ch| ch.is_ascii_digit()) && chars.next() == Some('.'); + if has_numeric_shortcut || row.contains("(disabled)") { + row_number += 1; + if row.contains("(disabled)") { + disabled_shortcut = char::from_digit(row_number, 10); + break; + } + } + } + let disabled_shortcut = disabled_shortcut + .unwrap_or_else(|| panic!("expected at least one disabled selection row: {popup}")); - // The approvals popup is the active bottom-pane view; drive navigation via - // chat.handle_key_event. Move through the menu so selection stays on an - // enabled preset even when disabled rows are skipped. - chat.handle_key_event(KeyEvent::from(KeyCode::Down)); - chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + for _ in 0..10 { + chat.handle_key_event(KeyEvent::from(KeyCode::Down)); + let popup = render_bottom_popup(&chat, /*width*/ 80); + let selected_disabled = popup + .lines() + .find(|line| line.trim_start().starts_with('\u{203a}')) + .expect("expected a selected selection row") + .contains("(disabled)"); + assert!( + !selected_disabled, + "navigation should skip disabled rows: {popup}" + ); + } - // Press the hidden numeric shortcut for the disabled Full Access row; it - // should not close the popup or accept the preset. + // Press the hidden numeric shortcut for a disabled row; it should not close + // the popup or accept the preset. chat.handle_key_event(KeyEvent::from(KeyCode::Char(disabled_shortcut))); // Ensure the popup remains open and no selection actions were sent. - let width = 80; - let height = chat.desired_height(width); - let mut terminal = - ratatui::Terminal::new(VT100Backend::new(width, height)).expect("create terminal"); - terminal.set_viewport_area(Rect::new(0, 0, width, height)); - terminal - .draw(|f| chat.render(f.area(), f.buffer_mut())) - .expect("render approvals popup after disabled selection"); - let screen = terminal.backend().vt100().screen().contents(); + let screen = render_bottom_popup(&chat, /*width*/ 80); assert!( screen.contains("Update Model Permissions"), "popup should remain open after selecting a disabled entry"