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>
This commit is contained in:
Ahmed Ibrahim
2026-03-14 04:39:59 +00:00
parent 8bc3d489a3
commit e96d895c9f
2 changed files with 17 additions and 8 deletions

View File

@@ -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. |

View File

@@ -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);