mirror of
https://github.com/openai/codex.git
synced 2026-05-02 18:37:01 +00:00
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`
This commit is contained in:
@@ -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<String> = 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<String> = 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<String> = None;
|
||||
wait_for_event_with_timeout(
|
||||
|
||||
Reference in New Issue
Block a user