diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md index ab8cd36347..14fa61db7c 100644 --- a/.codex/flaky-test-triage.md +++ b/.codex/flaky-test-triage.md @@ -46,9 +46,9 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron ## Current Fix In Progress -- Harden the `SessionConfigured` Smart Approvals popup tests in `codex-rs/tui/src/chatwidget/tests.rs`. -- Assert that `Smart Approvals (current)` is the selected row, not merely present somewhere in the popup body. -- Rationale: the same popup cluster was already showing selection-state brittleness on Windows CI. These two tests were still only checking rendered text and would miss a cursor-placement regression. +- Narrow the permissions-popup selection helper in `codex-rs/tui/src/chatwidget/tests.rs`. +- Match only the selected permission preset row (`Default`, `Smart Approvals`, or `Full Access`) instead of the first rendered line that contains `›`. +- Rationale: commit `fc98d21ad` failed in `Bazel (experimental)` across macOS and Linux while only TUI permission-popup assertions had changed. The helper introduced in that commit was too broad and could bind to a non-permission line, making the assertions brittle even when the popup state was correct. ## Constraints @@ -64,3 +64,6 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron | `60f44b4d7` | PR bootstrap | partial pass | PR opened and non-Rust checks passed after rebasing to current `main`, but `rust-ci` skipped because only the tracking doc changed. This commit does not count toward the five full-suite green commits. | | `b9c655ad4` | First full-suite flaky-test fix | full pass | Full PR CI passed on run `23078933382`, including `Tests — windows-x64 - x86_64-pc-windows-msvc` and `Tests — windows-arm64 - aarch64-pc-windows-msvc`. The approvals matrix write-file command now uses deterministic Python I/O instead of shell redirection. This is pass 1 of 5. | | `5dbb9c004` | Second full-suite flaky-test fix | full pass | Full PR CI passed on run `23079410130`, including both Windows test jobs. `permissions_selection_can_disable_smart_approvals` now seeds Smart Approvals mode explicitly and asserts the popup selection before and after navigation. This is pass 2 of 5. | +| `dc8d5d46d` | Harden history-cell permission selection assertions | superseded | `rust-ci` kept running, but `Bazel (experimental)` was cancelled by workflow concurrency after later commits landed on the PR branch. This SHA cannot be counted until its cancelled Bazel run is backfilled. | +| `1b6e21ccc` | Pin permission history snapshots to concrete presets | superseded | Same state as `dc8d5d46d`: `Bazel (experimental)` was cancelled by PR-level workflow concurrency, so this SHA is not countable yet. | +| `fc98d21ad` | Select Smart Approvals in session-configured popup tests | failed | `Bazel (experimental)` failed on run `23079852646` across macOS and Linux. Investigation narrowed the likely issue to the overly broad `selected_popup_line()` helper introduced in this commit. | diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 8214f3cb14..39d57b3650 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -6600,11 +6600,18 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String { lines.join("\n") } -fn selected_popup_line(popup: &str) -> &str { +fn selected_permissions_popup_line(popup: &str) -> &str { popup .lines() - .find(|line| line.contains('›')) - .expect("expected popup to have a selected row") + .find(|line| { + line.contains('›') + && (line.contains("Default") + || line.contains("Smart Approvals") + || line.contains("Full Access")) + }) + .unwrap_or_else(|| { + panic!("expected permissions popup to have a selected preset row: {popup}") + }) } #[tokio::test] @@ -8338,13 +8345,13 @@ async fn permissions_selection_emits_history_cell_when_selection_changes() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("(current)"), + selected_permissions_popup_line(&popup).contains("(current)"), "expected permissions popup to open with the current preset selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); let popup = render_bottom_popup(&chat, 120); assert!( - !selected_popup_line(&popup).contains("(current)"), + !selected_permissions_popup_line(&popup).contains("(current)"), "expected moving down to change the selected preset before confirmation: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -8375,7 +8382,7 @@ async fn permissions_selection_history_snapshot_after_mode_switch() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("(current)"), + selected_permissions_popup_line(&popup).contains("(current)"), "expected permissions popup to open with the current preset selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); @@ -8383,7 +8390,7 @@ async fn permissions_selection_history_snapshot_after_mode_switch() { chat.handle_key_event(KeyEvent::from(KeyCode::Down)); let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("Full Access"), + selected_permissions_popup_line(&popup).contains("Full Access"), "expected navigation to land on Full Access before confirmation: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -8419,7 +8426,7 @@ async fn permissions_selection_history_snapshot_full_access_to_default() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("Full Access (current)"), + selected_permissions_popup_line(&popup).contains("Full Access (current)"), "expected permissions popup to open with Full Access selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Up)); @@ -8428,7 +8435,7 @@ async fn permissions_selection_history_snapshot_full_access_to_default() { } let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("Default"), + selected_permissions_popup_line(&popup).contains("Default"), "expected navigation to land on Default before confirmation: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -8471,7 +8478,7 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("(current)"), + selected_permissions_popup_line(&popup).contains("(current)"), "expected permissions popup to open with the current preset selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -8579,7 +8586,7 @@ async fn permissions_selection_marks_smart_approvals_current_after_session_confi let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("Smart Approvals (current)"), + selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"), "expected SessionConfigured sync to select Smart Approvals in the popup: {popup}" ); } @@ -8633,7 +8640,7 @@ async fn permissions_selection_marks_smart_approvals_current_with_custom_workspa let popup = render_bottom_popup(&chat, 120); assert!( - selected_popup_line(&popup).contains("Smart Approvals (current)"), + selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"), "expected custom workspace-write details to keep Smart Approvals selected: {popup}" ); }