From c9f7c88f3d90a15c1a382c3660c27db470ea15e0 Mon Sep 17 00:00:00 2001 From: Michael Bolin Date: Tue, 28 Apr 2026 18:09:20 -0700 Subject: [PATCH] fix: restore live event submit path for apply patch tests (#20108) ## Summary This fixes the CI regression introduced by [#20040](https://github.com/openai/codex/pull/20040). That PR migrated several `apply_patch_cli` tests from direct `codex.submit(Op::UserTurn { ... })` calls to `harness.submit(...)`. `harness.submit()` waits for `TurnComplete` before returning, which drains the same event stream that these tests use to assert `TurnDiff`, `PatchApplyUpdated`, and related live events. The regressed tests then timed out waiting for events that had already been consumed. This change restores a no-wait submit path for the event-observing `apply_patch_cli` tests so they can watch the turn stream directly again. ## What Changed - added a local `submit_without_wait(...)` helper in `codex-rs/core/tests/suite/apply_patch_cli.rs` - switched the `apply_patch_cli` tests that assert live turn events back to that helper - left the profile-backed `harness.submit(...)` migration in place for tests that only care about final filesystem or tool output state ## Why macOS Looked Green In the failing run [25084487331](https://github.com/openai/codex/actions/runs/25084487331), `//codex-rs/core:core-all-test` was cached on macOS, so the regressed tests were not rerun there. The Linux GNU, Linux MUSL, and Windows Bazel jobs reran the target and exposed the failure. ## Verification - `cargo test -p codex-core apply_patch_ -- --nocapture` - previously failing local cases now pass again: - `apply_patch_cli_move_without_content_change_has_no_turn_diff` - `apply_patch_turn_diff_for_rename_with_content_change` - `apply_patch_aggregates_diff_across_multiple_tool_calls` --- codex-rs/core/tests/suite/apply_patch_cli.rs | 47 ++++++++++++++++---- 1 file changed, 39 insertions(+), 8 deletions(-) diff --git a/codex-rs/core/tests/suite/apply_patch_cli.rs b/codex-rs/core/tests/suite/apply_patch_cli.rs index 2d931efb3..44c3aff16 100644 --- a/codex-rs/core/tests/suite/apply_patch_cli.rs +++ b/codex-rs/core/tests/suite/apply_patch_cli.rs @@ -15,7 +15,11 @@ use std::time::Duration; use codex_features::Feature; use codex_protocol::models::PermissionProfile; use codex_protocol::permissions::NetworkSandboxPolicy; +use codex_protocol::protocol::AskForApproval; use codex_protocol::protocol::EventMsg; +use codex_protocol::protocol::Op; +use codex_protocol::protocol::SandboxPolicy; +use codex_protocol::user_input::UserInput; #[cfg(target_os = "linux")] use codex_sandboxing::landlock::CODEX_LINUX_SANDBOX_ARG0; use core_test_support::assert_regex_match; @@ -57,6 +61,33 @@ async fn apply_patch_harness_with( Box::pin(TestCodexHarness::with_remote_aware_builder(builder)).await } +async fn submit_without_wait(harness: &TestCodexHarness, prompt: &str) -> Result<()> { + let test = harness.test(); + let session_model = test.session_configured.model.clone(); + test.codex + .submit(Op::UserTurn { + environments: None, + items: vec![UserInput::Text { + text: prompt.into(), + text_elements: Vec::new(), + }], + final_output_json_schema: None, + cwd: harness.cwd().to_path_buf(), + approval_policy: AskForApproval::Never, + approvals_reviewer: None, + sandbox_policy: SandboxPolicy::DangerFullAccess, + permission_profile: None, + model: session_model, + effort: None, + summary: None, + service_tier: None, + collaboration_mode: None, + personality: None, + }) + .await?; + Ok(()) +} + fn restrictive_workspace_write_profile() -> PermissionProfile { PermissionProfile::workspace_write_with( &[], @@ -361,7 +392,7 @@ async fn apply_patch_cli_move_without_content_change_has_no_turn_diff( let call_id = "apply-move-no-change"; mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; - harness.submit("rename without content change").await?; + submit_without_wait(&harness, "rename without content change").await?; let mut saw_turn_diff = false; wait_for_event(&codex, |event| match event { @@ -969,7 +1000,7 @@ async fn apply_patch_custom_tool_streaming_emits_updated_changes() -> Result<()> ) .await; - harness.submit("create streamed file").await?; + submit_without_wait(&harness, "create streamed file").await?; let mut updates = Vec::new(); wait_for_event(&codex, |event| match event { @@ -1047,7 +1078,7 @@ async fn apply_patch_shell_command_heredoc_with_cd_emits_turn_diff() -> Result<( ]; mount_sse_sequence(harness.server(), bodies).await; - harness.submit("apply via shell heredoc with cd").await?; + submit_without_wait(&harness, "apply via shell heredoc with cd").await?; let mut saw_turn_diff = None; let mut saw_patch_begin = false; @@ -1112,7 +1143,7 @@ async fn apply_patch_shell_command_failure_propagates_error_and_skips_diff() -> ]; mount_sse_sequence(harness.server(), bodies).await; - harness.submit("apply patch via shell").await?; + submit_without_wait(&harness, "apply patch via shell").await?; let mut saw_turn_diff = false; wait_for_event(&codex, |event| match event { @@ -1248,7 +1279,7 @@ async fn apply_patch_emits_turn_diff_event_with_unified_diff( let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch\n"); mount_apply_patch(&harness, call_id, patch.as_str(), "ok", model_output).await; - harness.submit("emit diff").await?; + submit_without_wait(&harness, "emit diff").await?; let mut saw_turn_diff = None; wait_for_event(&codex, |event| match event { @@ -1296,7 +1327,7 @@ async fn apply_patch_turn_diff_for_rename_with_content_change( let patch = "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"; mount_apply_patch(&harness, call_id, patch, "ok", model_output).await; - harness.submit("rename with change").await?; + submit_without_wait(&harness, "rename with change").await?; let mut last_diff: Option = None; wait_for_event(&codex, |event| match event { @@ -1353,7 +1384,7 @@ async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> ]); mount_sse_sequence(harness.server(), vec![s1, s2, s3]).await; - harness.submit("aggregate diffs").await?; + submit_without_wait(&harness, "aggregate diffs").await?; let mut last_diff: Option = None; wait_for_event(&codex, |event| match event { @@ -1410,7 +1441,7 @@ async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result ]; mount_sse_sequence(harness.server(), responses).await; - harness.submit("apply patch twice with failure").await?; + submit_without_wait(&harness, "apply patch twice with failure").await?; let mut last_diff: Option = None; wait_for_event_with_timeout(