chore: testing on apply_path (#5557)

codex-rs/Cargo.lock

@@ -1633,6 +1633,7 @@ dependencies = [
  "anyhow",
  "assert_cmd",
  "codex-core",
+ "codex-protocol",
  "notify",
  "regex-lite",
  "serde_json",

codex-rs/apply-patch/tests/suite/mod.rs

@@ -1 +1,3 @@
 mod cli;
+#[cfg(not(target_os = "windows"))]
+mod tool;

codex-rs/apply-patch/tests/suite/tool.rs

@@ -0,0 +1,257 @@
+use assert_cmd::Command;
+use pretty_assertions::assert_eq;
+use std::fs;
+use std::path::Path;
+use tempfile::tempdir;
+
+fn run_apply_patch_in_dir(dir: &Path, patch: &str) -> anyhow::Result<assert_cmd::assert::Assert> {
+    let mut cmd = Command::cargo_bin("apply_patch")?;
+    cmd.current_dir(dir);
+    Ok(cmd.arg(patch).assert())
+}
+
+fn apply_patch_command(dir: &Path) -> anyhow::Result<Command> {
+    let mut cmd = Command::cargo_bin("apply_patch")?;
+    cmd.current_dir(dir);
+    Ok(cmd)
+}
+
+#[test]
+fn test_apply_patch_cli_applies_multiple_operations() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let modify_path = tmp.path().join("modify.txt");
+    let delete_path = tmp.path().join("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";
+
+    run_apply_patch_in_dir(tmp.path(), patch)?.success().stdout(
+        "Success. Updated the following files:\nA nested/new.txt\nM modify.txt\nD delete.txt\n",
+    );
+
+    assert_eq!(
+        fs::read_to_string(tmp.path().join("nested/new.txt"))?,
+        "created\n"
+    );
+    assert_eq!(fs::read_to_string(&modify_path)?, "line1\nchanged\n");
+    assert!(!delete_path.exists());
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_applies_multiple_chunks() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let target_path = tmp.path().join("multi.txt");
+    fs::write(&target_path, "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";
+
+    run_apply_patch_in_dir(tmp.path(), patch)?
+        .success()
+        .stdout("Success. Updated the following files:\nM multi.txt\n");
+
+    assert_eq!(
+        fs::read_to_string(&target_path)?,
+        "line1\nchanged2\nline3\nchanged4\n"
+    );
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_moves_file_to_new_directory() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let original_path = tmp.path().join("old/name.txt");
+    let new_path = tmp.path().join("renamed/dir/name.txt");
+    fs::create_dir_all(original_path.parent().expect("parent should exist"))?;
+    fs::write(&original_path, "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";
+
+    run_apply_patch_in_dir(tmp.path(), patch)?
+        .success()
+        .stdout("Success. Updated the following files:\nM renamed/dir/name.txt\n");
+
+    assert!(!original_path.exists());
+    assert_eq!(fs::read_to_string(&new_path)?, "new content\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_rejects_empty_patch() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("No files were modified.\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_reports_missing_context() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let target_path = tmp.path().join("modify.txt");
+    fs::write(&target_path, "line1\nline2\n")?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Update File: modify.txt\n@@\n-missing\n+changed\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("Failed to find expected lines in modify.txt:\nmissing\n");
+    assert_eq!(fs::read_to_string(&target_path)?, "line1\nline2\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_rejects_missing_file_delete() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Delete File: missing.txt\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("Failed to delete file missing.txt\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_rejects_empty_update_hunk() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Update File: foo.txt\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("Invalid patch hunk on line 2: Update file hunk for path 'foo.txt' is empty\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_requires_existing_file_for_update() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr(
+            "Failed to read file to update missing.txt: No such file or directory (os error 2)\n",
+        );
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_move_overwrites_existing_destination() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let original_path = tmp.path().join("old/name.txt");
+    let destination = tmp.path().join("renamed/dir/name.txt");
+    fs::create_dir_all(original_path.parent().expect("parent should exist"))?;
+    fs::create_dir_all(destination.parent().expect("parent should exist"))?;
+    fs::write(&original_path, "from\n")?;
+    fs::write(&destination, "existing\n")?;
+
+    run_apply_patch_in_dir(
+        tmp.path(),
+        "*** Begin Patch\n*** Update File: old/name.txt\n*** Move to: renamed/dir/name.txt\n@@\n-from\n+new\n*** End Patch",
+    )?
+    .success()
+    .stdout("Success. Updated the following files:\nM renamed/dir/name.txt\n");
+
+    assert!(!original_path.exists());
+    assert_eq!(fs::read_to_string(&destination)?, "new\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_add_overwrites_existing_file() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let path = tmp.path().join("duplicate.txt");
+    fs::write(&path, "old content\n")?;
+
+    run_apply_patch_in_dir(
+        tmp.path(),
+        "*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch",
+    )?
+    .success()
+    .stdout("Success. Updated the following files:\nA duplicate.txt\n");
+
+    assert_eq!(fs::read_to_string(&path)?, "new content\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_delete_directory_fails() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    fs::create_dir(tmp.path().join("dir"))?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Delete File: dir\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("Failed to delete file dir\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_rejects_invalid_hunk_header() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Frobnicate File: foo\n*** End Patch")
+        .assert()
+        .failure()
+        .stderr("Invalid patch hunk on line 2: '*** Frobnicate File: foo' is not a valid hunk header. Valid hunk headers: '*** Add File: {path}', '*** Delete File: {path}', '*** Update File: {path}'\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_updates_file_appends_trailing_newline() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let target_path = tmp.path().join("no_newline.txt");
+    fs::write(&target_path, "no newline at end")?;
+
+    run_apply_patch_in_dir(
+        tmp.path(),
+        "*** Begin Patch\n*** Update File: no_newline.txt\n@@\n-no newline at end\n+first line\n+second line\n*** End Patch",
+    )?
+    .success()
+    .stdout("Success. Updated the following files:\nM no_newline.txt\n");
+
+    let contents = fs::read_to_string(&target_path)?;
+    assert!(contents.ends_with('\n'));
+    assert_eq!(contents, "first line\nsecond line\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_failure_after_partial_success_leaves_changes() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let new_file = tmp.path().join("created.txt");
+
+    apply_patch_command(tmp.path())?
+        .arg("*** Begin Patch\n*** Add File: created.txt\n+hello\n*** Update File: missing.txt\n@@\n-old\n+new\n*** End Patch")
+        .assert()
+        .failure()
+        .stdout("")
+        .stderr("Failed to read file to update missing.txt: No such file or directory (os error 2)\n");
+
+    assert_eq!(fs::read_to_string(&new_file)?, "hello\n");
+
+    Ok(())
+}

codex-rs/core/src/tools/handlers/apply_patch.rs

@@ -100,7 +100,7 @@ impl ToolHandler for ApplyPatchHandler {
 
                         let req = ApplyPatchRequest {
                             patch: apply.action.patch.clone(),
-                            cwd,
+                            cwd: apply.action.cwd.clone(),
                             timeout_ms: None,
                             user_explicitly_approved: apply.user_explicitly_approved_this_action,
                             codex_exe: turn.codex_linux_sandbox_exe.clone(),

codex-rs/core/src/tools/handlers/shell.rs

@@ -154,7 +154,7 @@ impl ShellHandler {
 
                         let req = ApplyPatchRequest {
                             patch: apply.action.patch.clone(),
-                            cwd: exec_params.cwd.clone(),
+                            cwd: apply.action.cwd.clone(),
                             timeout_ms: exec_params.timeout_ms,
                             user_explicitly_approved: apply.user_explicitly_approved_this_action,
                             codex_exe: turn.codex_linux_sandbox_exe.clone(),

codex-rs/core/tests/common/Cargo.toml

@@ -10,6 +10,7 @@ path = "lib.rs"
 anyhow = { workspace = true }
 assert_cmd = { workspace = true }
 codex-core = { workspace = true }
+codex-protocol = { workspace = true }
 notify = { workspace = true }
 regex-lite = { workspace = true }
 serde_json = { workspace = true }

codex-rs/core/tests/common/test_codex.rs

@@ -1,17 +1,30 @@
 use std::mem::swap;
+use std::path::Path;
 use std::path::PathBuf;
 use std::sync::Arc;
 
+use anyhow::Result;
 use codex_core::CodexAuth;
 use codex_core::CodexConversation;
 use codex_core::ConversationManager;
 use codex_core::ModelProviderInfo;
 use codex_core::built_in_model_providers;
 use codex_core::config::Config;
+use codex_core::features::Feature;
+use codex_core::protocol::AskForApproval;
+use codex_core::protocol::EventMsg;
+use codex_core::protocol::Op;
+use codex_core::protocol::SandboxPolicy;
 use codex_core::protocol::SessionConfiguredEvent;
+use codex_protocol::config_types::ReasoningSummary;
+use codex_protocol::user_input::UserInput;
+use serde_json::Value;
 use tempfile::TempDir;
+use wiremock::MockServer;
 
 use crate::load_default_config_for_test;
+use crate::responses::start_mock_server;
+use crate::wait_for_event;
 
 type ConfigMutator = dyn FnOnce(&mut Config) + Send;
 
@@ -96,6 +109,12 @@ impl TestCodexBuilder {
             mutator(&mut config);
         }
 
+        if config.include_apply_patch_tool {
+            config.features.enable(Feature::ApplyPatchFreeform);
+        } else {
+            config.features.disable(Feature::ApplyPatchFreeform);
+        }
+
         Ok((config, cwd))
     }
 }
@@ -107,6 +126,139 @@ pub struct TestCodex {
     pub session_configured: SessionConfiguredEvent,
 }
 
+impl TestCodex {
+    pub fn cwd_path(&self) -> &Path {
+        self.cwd.path()
+    }
+
+    pub fn workspace_path(&self, rel: impl AsRef<Path>) -> PathBuf {
+        self.cwd_path().join(rel)
+    }
+
+    pub async fn submit_turn(&self, prompt: &str) -> Result<()> {
+        self.submit_turn_with_policy(prompt, SandboxPolicy::DangerFullAccess)
+            .await
+    }
+
+    pub async fn submit_turn_with_policy(
+        &self,
+        prompt: &str,
+        sandbox_policy: SandboxPolicy,
+    ) -> Result<()> {
+        let session_model = self.session_configured.model.clone();
+        self.codex
+            .submit(Op::UserTurn {
+                items: vec![UserInput::Text {
+                    text: prompt.into(),
+                }],
+                final_output_json_schema: None,
+                cwd: self.cwd.path().to_path_buf(),
+                approval_policy: AskForApproval::Never,
+                sandbox_policy,
+                model: session_model,
+                effort: None,
+                summary: ReasoningSummary::Auto,
+            })
+            .await?;
+
+        wait_for_event(&self.codex, |event| {
+            matches!(event, EventMsg::TaskComplete(_))
+        })
+        .await;
+        Ok(())
+    }
+}
+
+pub struct TestCodexHarness {
+    server: MockServer,
+    test: TestCodex,
+}
+
+impl TestCodexHarness {
+    pub async fn new() -> Result<Self> {
+        Self::with_builder(test_codex()).await
+    }
+
+    pub async fn with_config(mutator: impl FnOnce(&mut Config) + Send + 'static) -> Result<Self> {
+        Self::with_builder(test_codex().with_config(mutator)).await
+    }
+
+    pub async fn with_builder(mut builder: TestCodexBuilder) -> Result<Self> {
+        let server = start_mock_server().await;
+        let test = builder.build(&server).await?;
+        Ok(Self { server, test })
+    }
+
+    pub fn server(&self) -> &MockServer {
+        &self.server
+    }
+
+    pub fn test(&self) -> &TestCodex {
+        &self.test
+    }
+
+    pub fn cwd(&self) -> &Path {
+        self.test.cwd_path()
+    }
+
+    pub fn path(&self, rel: impl AsRef<Path>) -> PathBuf {
+        self.test.workspace_path(rel)
+    }
+
+    pub async fn submit(&self, prompt: &str) -> Result<()> {
+        self.test.submit_turn(prompt).await
+    }
+
+    pub async fn submit_with_policy(
+        &self,
+        prompt: &str,
+        sandbox_policy: SandboxPolicy,
+    ) -> Result<()> {
+        self.test
+            .submit_turn_with_policy(prompt, sandbox_policy)
+            .await
+    }
+
+    pub async fn request_bodies(&self) -> Vec<Value> {
+        self.server
+            .received_requests()
+            .await
+            .expect("requests")
+            .into_iter()
+            .map(|req| serde_json::from_slice(&req.body).expect("request body json"))
+            .collect()
+    }
+
+    pub async fn function_call_output_value(&self, call_id: &str) -> Value {
+        let bodies = self.request_bodies().await;
+        function_call_output(&bodies, call_id).clone()
+    }
+
+    pub async fn function_call_stdout(&self, call_id: &str) -> String {
+        self.function_call_output_value(call_id)
+            .await
+            .get("output")
+            .and_then(Value::as_str)
+            .expect("output string")
+            .to_string()
+    }
+}
+
+fn function_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("function_call_output")
+                    && item.get("call_id").and_then(Value::as_str) == Some(call_id)
+                {
+                    return item;
+                }
+            }
+        }
+    }
+    panic!("function_call_output {call_id} not found");
+}
+
 pub fn test_codex() -> TestCodexBuilder {
     TestCodexBuilder {
         config_mutators: vec![],

codex-rs/core/tests/suite/mod.rs

@@ -3,6 +3,8 @@
 #[cfg(not(target_os = "windows"))]
 mod abort_tasks;
 #[cfg(not(target_os = "windows"))]
+mod apply_patch_cli;
+#[cfg(not(target_os = "windows"))]
 mod approvals;
 mod cli_stream;
 mod client;