openai/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-11-02 19:43:24 +00:00
Code Issues Packages Projects Releases Wiki Activity

Narrow permissions popup selection helper

Browse Source 
Restrict the selection helper to permission preset rows so popup assertions cannot bind to unrelated rendered lines.

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-03-14 04:07:03 +00:00
co-authored by Codex
parent fc98d21ad8
commit a30e8e2ecb
2 changed files with 25 additions and 15 deletions
Download Patch File Download Diff File Expand all files Collapse all files
.codex/flaky-test-triage.md
+6 -3
View File
@@ -46,9 +46,9 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
## Current Fix In Progress
- Harden the `SessionConfigured` Smart Approvals popup tests in `codex-rs/tui/src/chatwidget/tests.rs`.
- Assert that `Smart Approvals (current)` is the selected row, not merely present somewhere in the popup body.
- Rationale: the same popup cluster was already showing selection-state brittleness on Windows CI. These two tests were still only checking rendered text and would miss a cursor-placement regression.
- 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.
## Constraints
@@ -64,3 +64,6 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron
| `60f44b4d7` | PR bootstrap | partial pass | PR opened and non-Rust checks passed after rebasing to current `main`, but `rust-ci` skipped because only the tracking doc changed. This commit does not count toward the five full-suite green commits. |
| `b9c655ad4` | First full-suite flaky-test fix | full pass | Full PR CI passed on run `23078933382`, including `Tests — windows-x64 - x86_64-pc-windows-msvc` and `Tests — windows-arm64 - aarch64-pc-windows-msvc`. The approvals matrix write-file command now uses deterministic Python I/O instead of shell redirection. This is pass 1 of 5. |
| `5dbb9c004` | Second full-suite flaky-test fix | full pass | Full PR CI passed on run `23079410130`, including both Windows test jobs. `permissions_selection_can_disable_smart_approvals` now seeds Smart Approvals mode explicitly and asserts the popup selection before and after navigation. This is pass 2 of 5. |
| `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. |
codex-rs/tui/src/chatwidget/tests.rs
+19 -12
View File
@@ -6600,11 +6600,18 @@ fn render_bottom_popup(chat: &ChatWidget, width: u16) -> String {
lines.join("\n")
}
fn selected_popup_line(popup: &str) -> &str {
fn selected_permissions_popup_line(popup: &str) -> &str {
popup
.lines()
.find(|line| line.contains('›'))
.expect("expected popup to have a selected row")
.find(|line| {
line.contains('›')
&& (line.contains("Default")
|| line.contains("Smart Approvals")
|| line.contains("Full Access"))
})
.unwrap_or_else(|| {
panic!("expected permissions popup to have a selected preset row: {popup}")
})
}
#[tokio::test]
@@ -8338,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_popup_line(&popup).contains("(current)"),
selected_permissions_popup_line(&popup).contains("(current)"),
"expected permissions popup to open with the current preset selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
let popup = render_bottom_popup(&chat, 120);
assert!(
!selected_popup_line(&popup).contains("(current)"),
!selected_permissions_popup_line(&popup).contains("(current)"),
"expected moving down to change the selected preset before confirmation: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
@@ -8375,7 +8382,7 @@ async fn permissions_selection_history_snapshot_after_mode_switch() {
chat.open_permissions_popup();
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("(current)"),
selected_permissions_popup_line(&popup).contains("(current)"),
"expected permissions popup to open with the current preset selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
@@ -8383,7 +8390,7 @@ async fn permissions_selection_history_snapshot_after_mode_switch() {
chat.handle_key_event(KeyEvent::from(KeyCode::Down));
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("Full Access"),
selected_permissions_popup_line(&popup).contains("Full Access"),
"expected navigation to land on Full Access before confirmation: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
@@ -8419,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_popup_line(&popup).contains("Full Access (current)"),
selected_permissions_popup_line(&popup).contains("Full Access (current)"),
"expected permissions popup to open with Full Access selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Up));
@@ -8428,7 +8435,7 @@ async fn permissions_selection_history_snapshot_full_access_to_default() {
}
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("Default"),
selected_permissions_popup_line(&popup).contains("Default"),
"expected navigation to land on Default before confirmation: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
@@ -8471,7 +8478,7 @@ async fn permissions_selection_emits_history_cell_when_current_is_selected() {
chat.open_permissions_popup();
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("(current)"),
selected_permissions_popup_line(&popup).contains("(current)"),
"expected permissions popup to open with the current preset selected: {popup}"
);
chat.handle_key_event(KeyEvent::from(KeyCode::Enter));
@@ -8579,7 +8586,7 @@ async fn permissions_selection_marks_smart_approvals_current_after_session_confi
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("Smart Approvals (current)"),
selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"),
"expected SessionConfigured sync to select Smart Approvals in the popup: {popup}"
);
}
@@ -8633,7 +8640,7 @@ async fn permissions_selection_marks_smart_approvals_current_with_custom_workspa
let popup = render_bottom_popup(&chat, 120);
assert!(
selected_popup_line(&popup).contains("Smart Approvals (current)"),
selected_permissions_popup_line(&popup).contains("Smart Approvals (current)"),
"expected custom workspace-write details to keep Smart Approvals selected: {popup}"
);
}