From 5dbb9c004832eefa03a2d77b23448a71e4d82b06 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Date: Sat, 14 Mar 2026 03:28:27 +0000 Subject: [PATCH] Stabilize Smart Approvals popup selection test Co-authored-by: Codex --- .codex/flaky-test-triage.md | 10 ++++++++-- codex-rs/tui/src/chatwidget/tests.rs | 16 ++++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md index b986a02bfd..ab673178b9 100644 --- a/.codex/flaky-test-triage.md +++ b/.codex/flaky-test-triage.md @@ -38,12 +38,18 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron 1. Approval-related tests in `codex-rs/core/tests/suite/approvals.rs` still have timing-sensitive behavior, especially in cross-platform CI. 2. Windows-specific approval UI tests in `codex-rs/tui/src/chatwidget/tests.rs` may depend on partially implicit sandbox state and can fail intermittently on Windows runners. -## Current Fix In Progress +## First Fix Landed - Replace the approval-matrix write-file command from shell redirection (`printf > file && cat file`) with a deterministic `python3 -c` file write/readback command. - Keep targeted scenario diagnostics in the matrix so CI logs include the exact command, exit code, and stdout when a scenario fails again. - Rationale: the known `read_only_unless_trusted_requires_approval` flake was previously "fixed" by increasing timeout budget. This change removes shell-redirection timing sensitivity instead of stretching the timeout further. +## Current Fix In Progress + +- Harden `permissions_selection_can_disable_smart_approvals` in `codex-rs/tui/src/chatwidget/tests.rs`. +- Seed the popup into Smart Approvals mode explicitly, then assert the selected row before and after navigation instead of assuming the initial cursor position. +- Rationale: recent merged PR `#14645` fixed another Smart Approvals popup test that was implicitly relying on selection state. This remaining test still used the same brittle pattern. + ## Constraints - Do not run tests locally. @@ -56,4 +62,4 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron | Commit | Purpose | PR CI result | Notes | | --- | --- | --- | --- | | `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. | -| _pending_ | First full-suite flaky-test fix | _pending_ | Approvals matrix write-file command now uses deterministic Python I/O instead of shell redirection. | +| `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. | diff --git a/codex-rs/tui/src/chatwidget/tests.rs b/codex-rs/tui/src/chatwidget/tests.rs index 730aa99004..948c1fd0a2 100644 --- a/codex-rs/tui/src/chatwidget/tests.rs +++ b/codex-rs/tui/src/chatwidget/tests.rs @@ -8617,9 +8617,25 @@ async fn permissions_selection_can_disable_smart_approvals() { .sandbox_policy .set(SandboxPolicy::new_workspace_write_policy()) .expect("set sandbox policy"); + chat.set_approvals_reviewer(ApprovalsReviewer::GuardianSubagent); chat.open_permissions_popup(); + let popup = render_bottom_popup(&chat, 120); + assert!( + popup + .lines() + .any(|line| line.contains("Smart Approvals (current)") && line.contains('›')), + "expected permissions popup to open with Smart Approvals selected: {popup}" + ); + chat.handle_key_event(KeyEvent::from(KeyCode::Up)); + let popup = render_bottom_popup(&chat, 120); + assert!( + popup + .lines() + .any(|line| line.contains("Default") && line.contains('›')), + "expected one Up from Smart Approvals to select Default: {popup}" + ); chat.handle_key_event(KeyEvent::from(KeyCode::Enter)); let events = std::iter::from_fn(|| rx.try_recv().ok()).collect::>();