From b9c655ad46d02e10757038a7bc53e48231be52b6 Mon Sep 17 00:00:00 2001 From: Ahmed Ibrahim <219906144+aibrahim-oai@users.noreply.github.com> Date: Sat, 14 Mar 2026 03:00:42 +0000 Subject: [PATCH] Stabilize approval matrix write-file command Replace the approval-matrix shell redirection write path with deterministic Python file I/O and keep scenario-level diagnostics in CI while the flake is being verified. Co-authored-by: Codex --- .codex/flaky-test-triage.md | 9 ++++++++- codex-rs/core/tests/suite/approvals.rs | 13 ++++++++++++- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/.codex/flaky-test-triage.md b/.codex/flaky-test-triage.md index 649f71c0c9..b986a02bfd 100644 --- a/.codex/flaky-test-triage.md +++ b/.codex/flaky-test-triage.md @@ -38,6 +38,12 @@ 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 + +- 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. + ## Constraints - Do not run tests locally. @@ -49,4 +55,5 @@ Older failures also appeared on Linux, but the repeated cross-PR signal is stron | Commit | Purpose | PR CI result | Notes | | --- | --- | --- | --- | -| _pending_ | PR bootstrap | _pending_ | Tracking starts once the PR is open. | +| `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. | diff --git a/codex-rs/core/tests/suite/approvals.rs b/codex-rs/core/tests/suite/approvals.rs index 7a9dba038e..53978c1766 100644 --- a/codex-rs/core/tests/suite/approvals.rs +++ b/codex-rs/core/tests/suite/approvals.rs @@ -122,7 +122,11 @@ impl ActionKind { ActionKind::WriteFile { target, content } => { let (path, _) = target.resolve_for_patch(test); let _ = fs::remove_file(&path); - let command = format!("printf {content:?} > {path:?} && cat {path:?}"); + let path_str = path.display().to_string(); + let script = format!( + "from pathlib import Path; path = Path({path_str:?}); content = {content:?}; path.write_text(content, encoding='utf-8'); print(path.read_text(encoding='utf-8'), end='')", + ); + let command = format!("python3 -c {script:?}"); let event = shell_event(call_id, &command, 5_000, sandbox_permissions)?; Ok((event, Some(command))) } @@ -1611,6 +1615,9 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { .action .prepare(&test, &server, call_id, scenario.sandbox_permissions) .await?; + if let Some(command) = expected_command.as_deref() { + eprintln!("approval scenario {} command: {command}", scenario.name); + } let _ = mount_sse_once( &server, @@ -1692,6 +1699,10 @@ async fn run_scenario(scenario: &ScenarioSpec) -> Result<()> { let output_item = results_mock.single_request().function_call_output(call_id); let result = parse_result(&output_item); + eprintln!( + "approval scenario {} result: exit_code={:?} stdout={:?}", + scenario.name, result.exit_code, result.stdout + ); scenario.expectation.verify(&test, &result)?; Ok(())