From 4a55646a02a0336c46c3fb4cd78aea35b966b513 Mon Sep 17 00:00:00 2001 From: Dylan Hurd Date: Thu, 30 Oct 2025 10:40:48 -0700 Subject: [PATCH] chore: testing on freeform apply_patch (#5952) ## Summary Duplicates the tests in `apply_patch_cli.rs`, but tests the freeform apply_patch tool as opposed to the function call path. The good news is that all the tests pass with zero logical tests, with the exception of the heredoc, which doesn't really make sense in the freeform tool context anyway. @jif-oai since you wrote the original tests in #5557, I'd love your opinion on the right way to DRY these test cases between the two. Happy to set up a more sophisticated harness, but didn't want to go down the rabbit hole until we agreed on the right pattern ## Testing - [x] These are tests --- codex-rs/core/tests/common/test_codex.rs | 24 + .../core/tests/suite/apply_patch_freeform.rs | 1001 +++++++++++++++++ codex-rs/core/tests/suite/mod.rs | 2 + 3 files changed, 1027 insertions(+) create mode 100644 codex-rs/core/tests/suite/apply_patch_freeform.rs diff --git a/codex-rs/core/tests/common/test_codex.rs b/codex-rs/core/tests/common/test_codex.rs index 6d1fda5a..0f9fdaae 100644 --- a/codex-rs/core/tests/common/test_codex.rs +++ b/codex-rs/core/tests/common/test_codex.rs @@ -240,6 +240,30 @@ impl TestCodexHarness { .expect("output string") .to_string() } + + pub async fn custom_tool_call_output(&self, call_id: &str) -> String { + let bodies = self.request_bodies().await; + custom_tool_call_output(&bodies, call_id) + .get("output") + .and_then(Value::as_str) + .expect("output string") + .to_string() + } +} + +fn custom_tool_call_output<'a>(bodies: &'a [Value], call_id: &str) -> &'a Value { + for body in bodies { + if let Some(items) = body.get("input").and_then(Value::as_array) { + for item in items { + if item.get("type").and_then(Value::as_str) == Some("custom_tool_call_output") + && item.get("call_id").and_then(Value::as_str) == Some(call_id) + { + return item; + } + } + } + } + panic!("custom_tool_call_output {call_id} not found"); } fn function_call_output<'a>(bodies: &'a [Value], call_id: &str) -> &'a Value { diff --git a/codex-rs/core/tests/suite/apply_patch_freeform.rs b/codex-rs/core/tests/suite/apply_patch_freeform.rs new file mode 100644 index 00000000..72c405eb --- /dev/null +++ b/codex-rs/core/tests/suite/apply_patch_freeform.rs @@ -0,0 +1,1001 @@ +#![allow(clippy::expect_used)] + +use anyhow::Result; +use core_test_support::responses::ev_apply_patch_custom_tool_call; +use pretty_assertions::assert_eq; +use std::fs; + +use codex_core::config::Config; +use codex_core::features::Feature; +use codex_core::model_family::find_family_for_model; +use codex_core::protocol::AskForApproval; +use codex_core::protocol::EventMsg; +use codex_core::protocol::Op; +use codex_core::protocol::SandboxPolicy; +use codex_protocol::config_types::ReasoningSummary; +use codex_protocol::user_input::UserInput; +use core_test_support::assert_regex_match; +use core_test_support::responses::ev_assistant_message; +use core_test_support::responses::ev_completed; +use core_test_support::responses::ev_response_created; +use core_test_support::responses::mount_sse_sequence; +use core_test_support::responses::sse; +use core_test_support::skip_if_no_network; +use core_test_support::test_codex::TestCodexHarness; +use core_test_support::wait_for_event; + +async fn apply_patch_harness() -> Result { + apply_patch_harness_with(|_| {}).await +} + +async fn apply_patch_harness_with( + configure: impl FnOnce(&mut Config) + Send + 'static, +) -> Result { + TestCodexHarness::with_config(|config| { + config.include_apply_patch_tool = true; + configure(config); + }) + .await +} + +async fn mount_apply_patch( + harness: &TestCodexHarness, + call_id: &str, + patch: &str, + assistant_msg: &str, +) { + mount_sse_sequence( + harness.server(), + freeform_apply_patch_responses(call_id, patch, assistant_msg), + ) + .await; +} + +fn freeform_apply_patch_responses(call_id: &str, patch: &str, assistant_msg: &str) -> Vec { + vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call_id, patch), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", assistant_msg), + ev_completed("resp-2"), + ]), + ] +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_multiple_operations_integration() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness_with(|config| { + config.model = "gpt-5".to_string(); + config.model_family = find_family_for_model("gpt-5").expect("gpt-5 is valid"); + }) + .await?; + + // Seed workspace state + let modify_path = harness.path("modify.txt"); + let delete_path = harness.path("delete.txt"); + fs::write(&modify_path, "line1\nline2\n")?; + fs::write(&delete_path, "obsolete\n")?; + + let patch = "*** Begin Patch\n*** Add File: nested/new.txt\n+created\n*** Delete File: delete.txt\n*** Update File: modify.txt\n@@\n-line2\n+changed\n*** End Patch"; + + let call_id = "apply-multi-ops"; + mount_apply_patch(&harness, call_id, patch, "done").await; + + harness.submit("please apply multi-ops patch").await?; + + let out = harness.custom_tool_call_output(call_id).await; + + let expected = r"(?s)^Exit code: 0 +Wall time: [0-9]+(?:\.[0-9]+)? seconds +Output: +Success. Updated the following files: +A nested/new.txt +M modify.txt +D delete.txt +?$"; + assert_regex_match(expected, &out); + + assert_eq!( + fs::read_to_string(harness.path("nested/new.txt"))?, + "created\n" + ); + assert_eq!(fs::read_to_string(&modify_path)?, "line1\nchanged\n"); + assert!(!delete_path.exists()); + + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_multiple_chunks() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("multi.txt"); + fs::write(&target, "line1\nline2\nline3\nline4\n")?; + + let patch = "*** Begin Patch\n*** Update File: multi.txt\n@@\n-line2\n+changed2\n@@\n-line4\n+changed4\n*** End Patch"; + let call_id = "apply-multi-chunks"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply multi-chunk patch").await?; + + assert_eq!( + fs::read_to_string(&target)?, + "line1\nchanged2\nline3\nchanged4\n" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_moves_file_to_new_directory() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let original = harness.path("old/name.txt"); + let new_path = harness.path("renamed/dir/name.txt"); + fs::create_dir_all(original.parent().expect("parent"))?; + fs::write(&original, "old content\n")?; + + let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-old content\n+new content\n*** End Patch"; + let call_id = "apply-move"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply move patch").await?; + + assert!(!original.exists()); + assert_eq!(fs::read_to_string(&new_path)?, "new content\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_updates_file_appends_trailing_newline() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("no_newline.txt"); + fs::write(&target, "no newline at end")?; + + let patch = "*** Begin Patch\n*** Update File: no_newline.txt\n@@\n-no newline at end\n+first line\n+second line\n*** End Patch"; + let call_id = "apply-append-nl"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply newline patch").await?; + + let contents = fs::read_to_string(&target)?; + assert!(contents.ends_with('\n')); + assert_eq!(contents, "first line\nsecond line\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_insert_only_hunk_modifies_file() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("insert_only.txt"); + fs::write(&target, "alpha\nomega\n")?; + + let patch = "*** Begin Patch\n*** Update File: insert_only.txt\n@@\n alpha\n+beta\n omega\n*** End Patch"; + let call_id = "apply-insert-only"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("insert lines via apply_patch").await?; + + assert_eq!(fs::read_to_string(&target)?, "alpha\nbeta\nomega\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_move_overwrites_existing_destination() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let original = harness.path("old/name.txt"); + let destination = harness.path("renamed/dir/name.txt"); + fs::create_dir_all(original.parent().expect("parent"))?; + fs::create_dir_all(destination.parent().expect("parent"))?; + fs::write(&original, "from\n")?; + fs::write(&destination, "existing\n")?; + + let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-from\n+new\n*** End Patch"; + let call_id = "apply-move-overwrite"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply move overwrite patch").await?; + + assert!(!original.exists()); + assert_eq!(fs::read_to_string(&destination)?, "new\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_move_without_content_change_has_no_turn_diff() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + let original = harness.path("old/name.txt"); + let destination = harness.path("renamed/name.txt"); + fs::create_dir_all(original.parent().expect("parent should exist"))?; + fs::write(&original, "same\n")?; + + let patch = "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/name.txt\n@@\n same\n*** End Patch"; + let call_id = "apply-move-no-change"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "rename without content change".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut saw_turn_diff = false; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(_) => { + saw_turn_diff = true; + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + assert!(!saw_turn_diff, "pure rename should not emit a turn diff"); + assert!(!original.exists()); + assert_eq!(fs::read_to_string(&destination)?, "same\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_add_overwrites_existing_file() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let path = harness.path("duplicate.txt"); + fs::write(&path, "old content\n")?; + + let patch = "*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch"; + let call_id = "apply-add-overwrite"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply add overwrite patch").await?; + + assert_eq!(fs::read_to_string(&path)?, "new content\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_rejects_invalid_hunk_header() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let patch = "*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch"; + let call_id = "apply-invalid-header"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply invalid header patch").await?; + + let out = harness.custom_tool_call_output(call_id).await; + + assert!( + out.contains("apply_patch verification failed"), + "expected verification failure message" + ); + assert!( + out.contains("is not a valid hunk header"), + "expected parse diagnostics in output: {out:?}" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_reports_missing_context() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("modify.txt"); + fs::write(&target, "line1\nline2\n")?; + + let patch = + "*** Begin Patch\n*** Update File: modify.txt\n@@\n-missing\n+changed\n*** End Patch"; + let call_id = "apply-missing-context"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply missing context patch").await?; + + let out = harness.custom_tool_call_output(call_id).await; + + assert!( + out.contains("apply_patch verification failed"), + "expected verification failure message" + ); + assert!(out.contains("Failed to find expected lines in")); + assert_eq!(fs::read_to_string(&target)?, "line1\nline2\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_reports_missing_target_file() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let patch = "*** Begin Patch\n*** Update File: missing.txt\n@@\n-nope\n+better\n*** End Patch"; + let call_id = "apply-missing-file"; + mount_apply_patch(&harness, call_id, patch, "fail").await; + + harness.submit("attempt to update a missing file").await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!( + out.contains("apply_patch verification failed"), + "expected verification failure message" + ); + assert!( + out.contains("Failed to read file to update"), + "expected missing file diagnostics: {out}" + ); + assert!( + out.contains("missing.txt"), + "expected missing file path in diagnostics: {out}" + ); + assert!(!harness.path("missing.txt").exists()); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_delete_missing_file_reports_error() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let patch = "*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch"; + let call_id = "apply-delete-missing"; + mount_apply_patch(&harness, call_id, patch, "fail").await; + + harness.submit("attempt to delete missing file").await?; + + let out = harness.custom_tool_call_output(call_id).await; + + assert!( + out.contains("apply_patch verification failed"), + "expected verification failure message: {out}" + ); + assert!( + out.contains("Failed to read"), + "missing delete diagnostics should mention read failure: {out}" + ); + assert!( + out.contains("missing.txt"), + "missing delete diagnostics should surface target path: {out}" + ); + assert!(!harness.path("missing.txt").exists()); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_rejects_empty_patch() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let patch = "*** Begin Patch\n*** End Patch"; + let call_id = "apply-empty"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply empty patch").await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!( + out.contains("patch rejected: empty patch"), + "expected rejection for empty patch: {out}" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_delete_directory_reports_verification_error() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + fs::create_dir(harness.path("dir"))?; + + let patch = "*** Begin Patch\n*** Delete File: dir\n*** End Patch"; + let call_id = "apply-delete-dir"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("delete a directory via apply_patch").await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!(out.contains("apply_patch verification failed")); + assert!(out.contains("Failed to read")); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_rejects_path_traversal_outside_workspace() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let escape_path = harness + .test() + .cwd + .path() + .parent() + .expect("cwd should have parent") + .join("escape.txt"); + let _ = fs::remove_file(&escape_path); + + let patch = "*** Begin Patch\n*** Add File: ../escape.txt\n+outside\n*** End Patch"; + let call_id = "apply-path-traversal"; + mount_apply_patch(&harness, call_id, patch, "fail").await; + + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + harness + .submit_with_policy( + "attempt to escape workspace via apply_patch", + sandbox_policy, + ) + .await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!( + out.contains( + "patch rejected: writing outside of the project; rejected by user approval settings" + ), + "expected rejection message for path traversal: {out}" + ); + assert!( + !escape_path.exists(), + "path traversal should be rejected; tool output: {out}" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_rejects_move_path_traversal_outside_workspace() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let escape_path = harness + .test() + .cwd + .path() + .parent() + .expect("cwd should have parent") + .join("escape-move.txt"); + let _ = fs::remove_file(&escape_path); + + let source = harness.path("stay.txt"); + fs::write(&source, "from\n")?; + + let patch = "*** Begin Patch\n*** Update File: stay.txt\n*** Move to: ../escape-move.txt\n@@\n-from\n+to\n*** End Patch"; + let call_id = "apply-move-traversal"; + mount_apply_patch(&harness, call_id, patch, "fail").await; + + let sandbox_policy = SandboxPolicy::WorkspaceWrite { + writable_roots: vec![], + network_access: false, + exclude_tmpdir_env_var: true, + exclude_slash_tmp: true, + }; + harness + .submit_with_policy("attempt move traversal via apply_patch", sandbox_policy) + .await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!( + out.contains( + "patch rejected: writing outside of the project; rejected by user approval settings" + ), + "expected rejection message for path traversal: {out}" + ); + assert!( + !escape_path.exists(), + "move path traversal should be rejected; tool output: {out}" + ); + assert_eq!(fs::read_to_string(&source)?, "from\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_verification_failure_has_no_side_effects() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness_with(|config| { + config.features.enable(Feature::ApplyPatchFreeform); + }) + .await?; + + // Compose a patch that would create a file, then fail verification on an update. + let call_id = "apply-partial-no-side-effects"; + let patch = "*** Begin Patch\n*** Add File: created.txt\n+hello\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch"; + + mount_apply_patch(&harness, call_id, patch, "failed").await; + + harness.submit("attempt partial apply patch").await?; + + let created = harness.path("created.txt"); + assert!( + !created.exists(), + "verification failure should prevent any filesystem changes" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_shell_failure_propagates_error_and_skips_diff() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness_with(|config| { + config.model = "gpt-5".to_string(); + config.model_family = find_family_for_model("gpt-5").expect("gpt-5 is valid"); + }) + .await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + let target = cwd.path().join("invalid.txt"); + fs::write(&target, "ok\n")?; + + let patch = + "*** Begin Patch\n*** Update File: invalid.txt\n@@\n-nope\n+changed\n*** End Patch\n"; + let call_id = "shell-apply-failure"; + let bodies = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call_id, patch), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "fail"), + ev_completed("resp-2"), + ]), + ]; + mount_sse_sequence(harness.server(), bodies).await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "apply patch via shell".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut saw_turn_diff = false; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(_) => { + saw_turn_diff = true; + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + assert!( + !saw_turn_diff, + "turn diff should not be emitted when shell apply_patch fails verification" + ); + + let out = harness.custom_tool_call_output(call_id).await; + assert!( + out.contains("apply_patch verification failed"), + "expected verification failure message" + ); + assert!( + out.contains("Failed to find expected lines in"), + "expected failure diagnostics: {out}" + ); + assert!( + out.contains("invalid.txt"), + "expected file path in output: {out}" + ); + assert_eq!(fs::read_to_string(&target)?, "ok\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_function_accepts_lenient_heredoc_wrapped_patch() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let file_name = "lenient.txt"; + let patch_inner = + format!("*** Begin Patch\n*** Add File: {file_name}\n+lenient\n*** End Patch\n"); + let wrapped = format!("<<'EOF'\n{patch_inner}EOF\n"); + let call_id = "apply-lenient"; + mount_apply_patch(&harness, call_id, &wrapped, "ok").await; + + harness.submit("apply lenient heredoc patch").await?; + + let new_file = harness.path(file_name); + assert_eq!(fs::read_to_string(new_file)?, "lenient\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_end_of_file_anchor() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("tail.txt"); + fs::write(&target, "alpha\nlast\n")?; + + let patch = "*** Begin Patch\n*** Update File: tail.txt\n@@\n-last\n+end\n*** End of File\n*** End Patch"; + let call_id = "apply-eof"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply EOF-anchored patch").await?; + assert_eq!(fs::read_to_string(&target)?, "alpha\nend\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_cli_missing_second_chunk_context_rejected() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("two_chunks.txt"); + fs::write(&target, "a\nb\nc\nd\n")?; + + // First chunk has @@, second chunk intentionally omits @@ to trigger parse error. + let patch = + "*** Begin Patch\n*** Update File: two_chunks.txt\n@@\n-b\n+B\n\n-d\n+D\n*** End Patch"; + let call_id = "apply-missing-ctx-2nd"; + mount_apply_patch(&harness, call_id, patch, "fail").await; + + harness.submit("apply missing context second chunk").await?; + + let out = harness.custom_tool_call_output(call_id).await; + assert!(out.contains("apply_patch verification failed")); + assert!( + out.contains("Failed to find expected lines in"), + "expected hunk context diagnostics: {out}" + ); + // Original file unchanged on failure + assert_eq!(fs::read_to_string(&target)?, "a\nb\nc\nd\n"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_emits_turn_diff_event_with_unified_diff() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + let call_id = "apply-diff-event"; + let file = "udiff.txt"; + let patch = format!("*** Begin Patch\n*** Add File: {file}\n+hello\n*** End Patch\n"); + let first = sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call_id, &patch), + ev_completed("resp-1"), + ]); + let second = sse(vec![ + ev_assistant_message("msg-1", "ok"), + ev_completed("resp-2"), + ]); + mount_sse_sequence(harness.server(), vec![first, second]).await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "emit diff".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut saw_turn_diff = None; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(ev) => { + saw_turn_diff = Some(ev.unified_diff.clone()); + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + let diff = saw_turn_diff.expect("expected TurnDiff event"); + // Basic markers of a unified diff with file addition + assert!(diff.contains("diff --git"), "diff header missing: {diff:?}"); + assert!(diff.contains("--- /dev/null") || diff.contains("--- a/")); + assert!(diff.contains("+++ b/")); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_turn_diff_for_rename_with_content_change() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + // Seed original file + let old = cwd.path().join("old.txt"); + fs::write(&old, "old\n")?; + + // Patch: update + move + let call_id = "apply-rename-change"; + let patch = "*** Begin Patch\n*** Update File: old.txt\n*** Move to: new.txt\n@@\n-old\n+new\n*** End Patch"; + let first = sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call_id, patch), + ev_completed("resp-1"), + ]); + let second = sse(vec![ + ev_assistant_message("msg-1", "ok"), + ev_completed("resp-2"), + ]); + mount_sse_sequence(harness.server(), vec![first, second]).await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "rename with change".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut last_diff: Option = None; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(ev) => { + last_diff = Some(ev.unified_diff.clone()); + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + let diff = last_diff.expect("expected TurnDiff event after rename"); + // Basic checks: shows old -> new, and the content delta + assert!(diff.contains("old.txt"), "diff missing old path: {diff:?}"); + assert!(diff.contains("new.txt"), "diff missing new path: {diff:?}"); + assert!(diff.contains("--- a/"), "missing old header"); + assert!(diff.contains("+++ b/"), "missing new header"); + assert!(diff.contains("-old\n"), "missing removal line"); + assert!(diff.contains("+new\n"), "missing addition line"); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_aggregates_diff_across_multiple_tool_calls() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + let call1 = "agg-1"; + let call2 = "agg-2"; + let patch1 = "*** Begin Patch\n*** Add File: agg/a.txt\n+v1\n*** End Patch"; + let patch2 = "*** Begin Patch\n*** Update File: agg/a.txt\n@@\n-v1\n+v2\n*** Add File: agg/b.txt\n+B\n*** End Patch"; + + let s1 = sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call1, patch1), + ev_completed("resp-1"), + ]); + let s2 = sse(vec![ + ev_response_created("resp-2"), + ev_apply_patch_custom_tool_call(call2, patch2), + ev_completed("resp-2"), + ]); + let s3 = sse(vec![ + ev_assistant_message("msg-1", "ok"), + ev_completed("resp-3"), + ]); + mount_sse_sequence(harness.server(), vec![s1, s2, s3]).await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "aggregate diffs".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut last_diff: Option = None; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(ev) => { + last_diff = Some(ev.unified_diff.clone()); + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + let diff = last_diff.expect("expected TurnDiff after two patches"); + assert!(diff.contains("agg/a.txt"), "diff missing a.txt"); + assert!(diff.contains("agg/b.txt"), "diff missing b.txt"); + // Final content reflects v2 for a.txt + assert!(diff.contains("+v2\n") || diff.contains("v2\n")); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_aggregates_diff_preserves_success_after_failure() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + let test = harness.test(); + let codex = test.codex.clone(); + let cwd = test.cwd.clone(); + + let call_success = "agg-success"; + let call_failure = "agg-failure"; + let patch_success = "*** Begin Patch\n*** Add File: partial/success.txt\n+ok\n*** End Patch"; + let patch_failure = + "*** Begin Patch\n*** Update File: partial/success.txt\n@@\n-missing\n+new\n*** End Patch"; + + let responses = vec![ + sse(vec![ + ev_response_created("resp-1"), + ev_apply_patch_custom_tool_call(call_success, patch_success), + ev_completed("resp-1"), + ]), + sse(vec![ + ev_response_created("resp-2"), + ev_apply_patch_custom_tool_call(call_failure, patch_failure), + ev_completed("resp-2"), + ]), + sse(vec![ + ev_assistant_message("msg-1", "failed"), + ev_completed("resp-3"), + ]), + ]; + mount_sse_sequence(harness.server(), responses).await; + + let model = test.session_configured.model.clone(); + codex + .submit(Op::UserTurn { + items: vec![UserInput::Text { + text: "apply patch twice with failure".into(), + }], + final_output_json_schema: None, + cwd: cwd.path().to_path_buf(), + approval_policy: AskForApproval::Never, + sandbox_policy: SandboxPolicy::DangerFullAccess, + model, + effort: None, + summary: ReasoningSummary::Auto, + }) + .await?; + + let mut last_diff: Option = None; + wait_for_event(&codex, |event| match event { + EventMsg::TurnDiff(ev) => { + last_diff = Some(ev.unified_diff.clone()); + false + } + EventMsg::TaskComplete(_) => true, + _ => false, + }) + .await; + + let diff = last_diff.expect("expected TurnDiff after failed patch"); + assert!( + diff.contains("partial/success.txt"), + "diff should still include the successful addition: {diff}" + ); + assert!( + diff.contains("+ok"), + "diff should include contents from successful patch: {diff}" + ); + + let failure_out = harness.custom_tool_call_output(call_failure).await; + assert!( + failure_out.contains("apply_patch verification failed"), + "expected verification failure output: {failure_out}" + ); + assert!( + failure_out.contains("Failed to find expected lines in"), + "expected missing context diagnostics: {failure_out}" + ); + + assert_eq!( + fs::read_to_string(cwd.path().join("partial/success.txt"))?, + "ok\n" + ); + Ok(()) +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn apply_patch_change_context_disambiguates_target() -> Result<()> { + skip_if_no_network!(Ok(())); + + let harness = apply_patch_harness().await?; + + let target = harness.path("multi_ctx.txt"); + fs::write(&target, "fn a\nx=10\ny=2\nfn b\nx=10\ny=20\n")?; + + let patch = + "*** Begin Patch\n*** Update File: multi_ctx.txt\n@@ fn b\n-x=10\n+x=11\n*** End Patch"; + let call_id = "apply-ctx"; + mount_apply_patch(&harness, call_id, patch, "ok").await; + + harness.submit("apply with change_context").await?; + + let contents = fs::read_to_string(&target)?; + assert_eq!(contents, "fn a\nx=10\ny=2\nfn b\nx=11\ny=20\n"); + Ok(()) +} diff --git a/codex-rs/core/tests/suite/mod.rs b/codex-rs/core/tests/suite/mod.rs index 5eca79b5..915e8a65 100644 --- a/codex-rs/core/tests/suite/mod.rs +++ b/codex-rs/core/tests/suite/mod.rs @@ -5,6 +5,8 @@ mod abort_tasks; #[cfg(not(target_os = "windows"))] mod apply_patch_cli; #[cfg(not(target_os = "windows"))] +mod apply_patch_freeform; +#[cfg(not(target_os = "windows"))] mod approvals; mod cli_stream; mod client;