Stabilize Smart Approvals popup selection test

Co-authored-by: Codex <noreply@openai.com>
This commit is contained in:
Ahmed Ibrahim
2026-03-14 03:28:27 +00:00
parent b9c655ad46
commit 5dbb9c0048
2 changed files with 24 additions and 2 deletions

View File

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

View File

@@ -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::<Vec<_>>();