From 8bc3d489a35157b372d4c759d5a2a4168535d928 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Date: Sat, 14 Mar 2026 04:11:56 +0000 Subject: [PATCH] Relax popup current-label assertions Match the selected permission preset label directly in the generic history tests instead of assuming the popup always renders a suffix. Co-authored-by: Codex --- .codex/flaky-test-triage.md | 7 ++++--- codex-rs/tui/src/chatwidget/tests.rs | 16 ++++++++-------- 2 files changed, 12 insertions(+), 11 deletions(-) diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md index 14fa61db7c..631882e1e8 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 -- 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. +- Drop generic `(current)` assumptions from the broad permission-selection history tests in `codex-rs/tui/src/chatwidget/tests.rs`. +- Assert the selected preset label (`Default` or `Full Access`) directly in contexts where popup snapshots show the `(current)` suffix is not guaranteed. +- Rationale: commit `a30e8e2ec` still failed in Linux `Bazel (experimental)` after narrowing the selected-row helper. Existing popup snapshots show `Default` selected without a `(current)` suffix on Unix, so those newer assertions were still stricter than the UI contract. ## Constraints @@ -67,3 +67,4 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron | `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. | +| `a30e8e2ec` | Narrow permissions popup selection helper | failed | `Bazel (experimental)` still failed on Linux in run `23080049381`. The helper narrowing was necessary, but Linux snapshots showed the generic `(current)` assertions were still too strict for the default/full-access permission history tests. | diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 39d57b3650..b1af49461b 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -8345,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_permissions_popup_line(&popup).contains("(current)"), - "expected permissions popup to open with the current preset selected: {popup}" + selected_permissions_popup_line(&popup).contains("Default"), + "expected permissions popup to open with Default selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); let popup = render_bottom_popup(&chat, 120); assert!( - !selected_permissions_popup_line(&popup).contains("(current)"), + !selected_permissions_popup_line(&popup).contains("Default"), "expected moving down to change the selected preset before confirmation: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); @@ -8382,8 +8382,8 @@ async fn permissions_selection_history_snapshot_after_mode_switch() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_permissions_popup_line(&popup).contains("(current)"), - "expected permissions popup to open with the current preset selected: {popup}" + selected_permissions_popup_line(&popup).contains("Default"), + "expected permissions popup to open with Default selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Down)); #[cfg(target_os = "windows")] @@ -8426,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_permissions_popup_line(&popup).contains("Full Access (current)"), + selected_permissions_popup_line(&popup).contains("Full Access"), "expected permissions popup to open with Full Access selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Up)); @@ -8478,8 +8478,8 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() { chat.open_permissions_popup(); let popup = render_bottom_popup(&chat, 120); assert!( - selected_permissions_popup_line(&popup).contains("(current)"), - "expected permissions popup to open with the current preset selected: {popup}" + selected_permissions_popup_line(&popup).contains("Default"), + "expected permissions popup to open with Default selected: {popup}" ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter));