adjust apply_patch replacement behavior

Normalize existing-target replacement before persisting new content.

Add focused regression coverage for the updated write-path behavior and keep the change scoped to the relevant integration boundary.

Co-authored-by: Codex <noreply@openai.com>

codex-rs/apply-patch/src/lib.rs

@@ -397,7 +397,7 @@ async fn apply_hunks_to_files(
                     read_optional_file_text_for_delta(&path_abs, fs, sandbox, &mut delta.exact)
                         .await;
                 try_write!(
-                    write_file_with_missing_parent_retry(
+                    replace_file_with_missing_parent_retry(
                         fs,
                         &path_abs,
                         contents.clone().into_bytes(),
@@ -466,7 +466,7 @@ async fn apply_hunks_to_files(
                         read_optional_file_text_for_delta(&dest_abs, fs, sandbox, &mut delta.exact)
                             .await;
                     try_write!(
-                        write_file_with_missing_parent_retry(
+                        replace_file_with_missing_parent_retry(
                             fs,
                             &dest_abs,
                             new_contents.clone().into_bytes(),
@@ -522,12 +522,13 @@ async fn apply_hunks_to_files(
                     modified.push(affected_path);
                 } else {
                     try_write!(
-                        fs.write_file(&path_abs, new_contents.clone().into_bytes(), sandbox)
-                            .await
-                            .with_context(|| format!(
-                                "Failed to write file {}",
-                                path_abs.display()
-                            ))
+                        replace_file_with_missing_parent_retry(
+                            fs,
+                            &path_abs,
+                            new_contents.clone().into_bytes(),
+                            sandbox,
+                        )
+                        .await
                     );
                     delta.changes.push(AppliedPatchChange {
                         path: path_abs.into_path_buf(),
@@ -645,6 +646,45 @@ async fn write_file_with_missing_parent_retry(
     }
 }
 
+async fn replace_file_with_missing_parent_retry(
+    fs: &dyn ExecutorFileSystem,
+    path_abs: &AbsolutePathBuf,
+    contents: Vec<u8>,
+    sandbox: Option<&FileSystemSandboxContext>,
+) -> anyhow::Result<()> {
+    match fs.get_metadata(path_abs, sandbox).await {
+        Ok(metadata) => {
+            if metadata.is_directory {
+                return Err(io::Error::new(
+                    io::ErrorKind::InvalidInput,
+                    "path is a directory",
+                ))
+                .with_context(|| format!("Failed to write file {}", path_abs.display()));
+            }
+            if metadata.is_symlink {
+                return write_file_with_missing_parent_retry(fs, path_abs, contents, sandbox).await;
+            }
+            fs.remove(
+                path_abs,
+                RemoveOptions {
+                    recursive: false,
+                    force: false,
+                },
+                sandbox,
+            )
+            .await
+            .with_context(|| format!("Failed to replace file {}", path_abs.display()))?;
+        }
+        Err(error) if error.kind() == io::ErrorKind::NotFound => {}
+        Err(error) => {
+            return Err(error)
+                .with_context(|| format!("Failed to inspect file {}", path_abs.display()));
+        }
+    }
+
+    write_file_with_missing_parent_retry(fs, path_abs, contents, sandbox).await
+}
+
 struct AppliedPatch {
     original_contents: String,
     new_contents: String,

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

@@ -205,6 +205,52 @@ fn test_apply_patch_cli_add_overwrites_existing_file() -> anyhow::Result<()> {
     Ok(())
 }
 
+#[test]
+fn test_apply_patch_cli_add_replaces_existing_hard_link() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let workspace = tmp.path().join("workspace");
+    let outside = tmp.path().join("outside.txt");
+    let link = workspace.join("hard-link.txt");
+    fs::create_dir_all(&workspace)?;
+    fs::write(&outside, "outside\n")?;
+    fs::hard_link(&outside, &link)?;
+
+    run_apply_patch_in_dir(
+        &workspace,
+        "*** Begin Patch\n*** Add File: hard-link.txt\n+workspace replacement\n*** End Patch",
+    )?
+    .success()
+    .stdout("Success. Updated the following files:\nA hard-link.txt\n");
+
+    assert_eq!(fs::read_to_string(&outside)?, "outside\n");
+    assert_eq!(fs::read_to_string(&link)?, "workspace replacement\n");
+
+    Ok(())
+}
+
+#[test]
+fn test_apply_patch_cli_update_replaces_existing_hard_link() -> anyhow::Result<()> {
+    let tmp = tempdir()?;
+    let workspace = tmp.path().join("workspace");
+    let outside = tmp.path().join("outside.txt");
+    let link = workspace.join("hard-link.txt");
+    fs::create_dir_all(&workspace)?;
+    fs::write(&outside, "outside\n")?;
+    fs::hard_link(&outside, &link)?;
+
+    run_apply_patch_in_dir(
+        &workspace,
+        "*** Begin Patch\n*** Update File: hard-link.txt\n@@\n-outside\n+workspace replacement\n*** End Patch",
+    )?
+    .success()
+    .stdout("Success. Updated the following files:\nM hard-link.txt\n");
+
+    assert_eq!(fs::read_to_string(&outside)?, "outside\n");
+    assert_eq!(fs::read_to_string(&link)?, "workspace replacement\n");
+
+    Ok(())
+}
+
 #[test]
 fn test_apply_patch_cli_delete_directory_fails() -> anyhow::Result<()> {
     let tmp = tempdir()?;

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

@@ -796,7 +796,7 @@ async fn apply_patch_cli_does_not_write_through_symlink_escape_outside_workspace
 #[test_case(ApplyPatchModelOutput::Shell ; "shell")]
 #[test_case(ApplyPatchModelOutput::ShellViaHeredoc ; "shell_heredoc")]
 #[test_case(ApplyPatchModelOutput::ShellCommandViaHeredoc ; "shell_command_heredoc")]
-async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace(
+async fn apply_patch_cli_replaces_existing_hard_link_outside_workspace(
     model_output: ApplyPatchModelOutput,
 ) -> Result<()> {
     skip_if_no_network!(Ok(()));
@@ -872,24 +872,24 @@ async fn apply_patch_cli_preserves_existing_hard_link_outside_workspace(
 
     assert!(
         out.contains("Success. Updated the following files:"),
-        "apply_patch should intentionally allow updates through existing hard links; tool output: {out}"
+        "apply_patch should update the workspace path without mutating the outside hard-link target; tool output: {out}"
     );
     assert_eq!(
         std::fs::read_to_string(&outside_file)?,
-        "updated through existing hard link\n",
-        "apply_patch intentionally preserves existing hard-link semantics; the outside path observes the shared inode update"
+        "original outside content\n",
+        "apply_patch must not mutate the outside hard-link target"
     );
     assert_eq!(
         std::fs::read_to_string(&link_path)?,
         "updated through existing hard link\n",
-        "apply_patch intentionally preserves existing hard-link semantics; the workspace path observes the same update"
+        "apply_patch should still update the workspace path"
     );
 
     std::fs::write(&outside_file, "post-apply outside write\n")?;
     assert_eq!(
         std::fs::read_to_string(&link_path)?,
-        "post-apply outside write\n",
-        "apply_patch must not unlink or replace an existing hard link; later writes through either path should still be visible"
+        "updated through existing hard link\n",
+        "apply_patch should replace the workspace hard-link entry instead of preserving the shared inode"
     );
 
     Ok(())