From 13c9d91b0f6358c060ea899d4883f6687a4ec535 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Date: Sat, 14 Mar 2026 05:27:21 +0000 Subject: [PATCH] Avoid false matches in permissions popup helper Match the selected permissions preset by the row name prefix instead of a loose substring search so the Smart Approvals description cannot masquerade as Default during popup navigation. Co-authored-by: Codex --- .codex/flaky-test-triage.md | 1 + codex-rs/tui/src/chatwidget/tests.rs | 18 +++++++++++++++++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md index 16613b1e12..acd646b063 100644 --- a/.codex/flaky-test-triage.md +++ b/.codex/flaky-test-triage.md @@ -49,6 +49,7 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron - 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. - Replace the remaining Windows-only extra navigation in the full-access confirmation/history test, and use the same label-driven movement in the adjacent Smart Approvals popup tests. +- Follow-up on `87941e5d7`: the helper compared the entire selected row with `contains(label)`, which made `Smart Approvals` look like `Default` because its description text says "Same workspace-write permissions as Default...". Tighten the helper to match the selected preset name at the start of the row instead of any substring in the rendered line. - Rationale: commit `e96d895c9` still failed on both Windows `Tests` jobs in run `23080608881`. The remaining common pattern is Windows-only popup navigation that assumes a fixed number of `Up`/`Down` presses even though the Windows menu includes `Read Only` and selection wraps. ## Constraints diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index a08d13a16e..fbd7465afa 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -6615,10 +6615,26 @@ fn selected_permissions_popup_line(popup: &str) -> &str { }) } +fn selected_permissions_popup_name(popup: &str) -> &'static str { + selected_permissions_popup_line(popup) + .trim_start() + .strip_prefix('›') + .map(str::trim_start) + .and_then(|line| line.split_once(". ").map(|(_, rest)| rest)) + .and_then(|line| { + ["Read Only", "Default", "Smart Approvals", "Full Access"] + .into_iter() + .find(|label| line.starts_with(label)) + }) + .unwrap_or_else(|| { + panic!("expected permissions popup row to start with a preset label: {popup}") + }) +} + fn move_permissions_popup_selection_to(chat: &mut ChatWidget, label: &str, direction: KeyCode) { for _ in 0..4 { let popup = render_bottom_popup(chat, 120); - if selected_permissions_popup_line(&popup).contains(label) { + if selected_permissions_popup_name(&popup) == label { return; } chat.handle_key_event(KeyEvent::from(direction));