From e96d895c9fcc03e10840855bc3a3f04870d1dc45 Mon Sep 17 00:00:00 2001
From: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com>
Date: Sat, 14 Mar 2026 04:39:59 +0000
Subject: [PATCH] Stabilize Windows permissions popup navigation

Handle the Windows-only Read Only preset in the generic helper and drive the permission history snapshot tests by the selected label instead of hard-coded key counts.

Co-authored-by: Codex <noreply@openai.com>
---
 .codex/flaky-test-triage.md          |  7 ++++---
 codex-rs/tui/src/chatwidget/tests.rs | 18 +++++++++++++-----
 2 files changed, 17 insertions(+), 8 deletions(-)

diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md
index 631882e1e8..960ad18c6c 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
 
-- 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.
+- Make the generic permissions-popup helper recognize `Read Only`, which is a real preset on Windows.
+- Replace Windows-specific hard-coded navigation counts with label-driven movement in the permission history snapshot tests.
+- Rationale: commit `8bc3d489a` fixed the cross-platform `(current)` suffix issue, but `rust-ci` still failed on both Windows test lanes. The remaining Windows-only difference is that the permissions popup includes `Read Only` and selection wraps, so hard-coded extra key presses can land on the wrong preset even when the product behavior is correct.
 
 ## Constraints
 
@@ -68,3 +68,4 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
 | `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. |
+| `8bc3d489a` | Relax popup current-label assertions | failed | `rust-ci` failed on run `23080137075` in both Windows test jobs after Linux and Bazel turned green. The remaining Windows-only issue is likely the `Read Only` preset and wrapping popup navigation in the generic permission history tests. |
diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs
index b1af49461b..f1bbcf0b35 100644
--- a/codex-rs/tui/src/chatwidget/tests.rs
+++ b/codex-rs/tui/src/chatwidget/tests.rs
@@ -6606,6 +6606,7 @@ fn selected_permissions_popup_line(popup: &str) -> &str {
         .find(|line| {
             line.contains('›')
                 && (line.contains("Default")
+                    || line.contains("Read Only")
                     || line.contains("Smart Approvals")
                     || line.contains("Full Access"))
         })
@@ -8385,9 +8386,13 @@ async fn permissions_selection_history_snapshot_after_mode_switch() {
         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")]
-    chat.handle_key_event(KeyEvent::from(KeyCode::Down));
+    for _ in 0..3 {
+        let popup = render_bottom_popup(&chat, 120);
+        if selected_permissions_popup_line(&popup).contains("Full Access") {
+            break;
+        }
+        chat.handle_key_event(KeyEvent::from(KeyCode::Down));
+    }
     let popup = render_bottom_popup(&chat, 120);
     assert!(
         selected_permissions_popup_line(&popup).contains("Full Access"),
@@ -8429,8 +8434,11 @@ async fn permissions_selection_history_snapshot_full_access_to_default() {
         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));
-    if popup.contains("Smart Approvals") {
+    for _ in 0..3 {
+        let popup = render_bottom_popup(&chat, 120);
+        if selected_permissions_popup_line(&popup).contains("Default") {
+            break;
+        }
         chat.handle_key_event(KeyEvent::from(KeyCode::Up));
     }
     let popup = render_bottom_popup(&chat, 120);