From 9955eacd22e2ddf3e2096e2baa7609c6c93b2f6a Mon Sep 17 00:00:00 2001
From: Michael Bolin <mbolin@openai.com>
Date: Wed, 22 Apr 2026 21:01:26 -0700
Subject: [PATCH] tui: fix approvals popup disabled shortcut test (#19072)

## Why

This regressed in #19063, which made `GuardianApproval` stable and
enabled by default. That adds an enabled `Auto-review` row to the
permissions popup, but `approvals_popup_navigation_skips_disabled` still
assumed the disabled `Full Access` row lived behind a hard-coded numeric
shortcut, so the test started selecting a different row and closing the
popup instead of verifying disabled-row behavior.

## What

- disable `GuardianApproval` in
`approvals_popup_navigation_skips_disabled` so the popup layout matches
the scenario the test is exercising
- choose the hidden numeric shortcut for the disabled `Full Access` row
by platform (`2` on non-Windows, `3` on Windows where `Read Only` is
shown) before asserting that selecting the disabled row leaves the popup
open

## Testing

- `cargo test -p codex-tui --lib
chatwidget::tests::permissions::approvals_popup_navigation_skips_disabled
-- --exact --nocapture`
- `cargo test -p codex-tui --lib chatwidget::tests::permissions --
--nocapture`
- `cargo test -p codex-tui`
---
 .../tui/src/chatwidget/tests/permissions.rs   | 20 ++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/codex-rs/tui/src/chatwidget/tests/permissions.rs b/codex-rs/tui/src/chatwidget/tests/permissions.rs
index f9fcd3179..6a3421fe3 100644
--- a/codex-rs/tui/src/chatwidget/tests/permissions.rs
+++ b/codex-rs/tui/src/chatwidget/tests/permissions.rs
@@ -189,6 +189,7 @@ async fn approvals_popup_shows_disabled_presets() {
 #[tokio::test]
 async fn approvals_popup_navigation_skips_disabled() {
     let (mut chat, mut rx, mut op_rx) = make_chatwidget_manual(/*model_override*/ None).await;
+    chat.set_feature_enabled(Feature::GuardianApproval, /*enabled*/ false);
 
     chat.config.permissions.approval_policy =
         Constrained::new(AskForApproval::OnRequest, |candidate| match candidate {
@@ -198,14 +199,23 @@ async fn approvals_popup_navigation_skips_disabled() {
         .expect("construct constrained approval policy");
     chat.open_approvals_popup();
 
-    // The approvals popup is the active bottom-pane view; drive navigation via chat handle_key_event.
-    // Start selected at idx 0 (enabled), move down twice; the disabled option should be skipped
-    // and selection should wrap back to idx 0 (also enabled).
+    // 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'
+    };
+
+    // 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));
 
-    // Press numeric shortcut for the disabled row (3 => idx 2); should not close or accept.
-    chat.handle_key_event(KeyEvent::from(KeyCode::Char('3')));
+    // Press the hidden numeric shortcut for the disabled Full Access 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;