clone/codex
1
0
Fork 0
mirror of https://github.com/openai/codex.git synced 2026-04-24 06:35:50 +00:00
Code Issues Packages Projects Releases Wiki Activity

Preserve apply_patch summary paths

Browse Source
This commit is contained in:
pakrym-oai
2026-04-07 13:10:01 -07:00
parent ec19515a6e
commit 69758d8238
4 changed files with 41 additions and 71 deletions
Download Patch File Download Diff File Expand all files Collapse all files

17
codex-rs/apply-patch/src/lib.rs
View File

@@ -243,7 +243,8 @@ pub async fn apply_hunks(
/// Applies each parsed patch hunk to the filesystem.
/// Returns an error if any of the changes could not be applied.
/// Tracks file paths affected by applying a patch.
/// Tracks file paths affected by applying a patch, preserving the path spelling
/// from the patch for user-facing summaries.
pub struct AffectedPaths {
pub added: Vec<PathBuf>,
pub modified: Vec<PathBuf>,
@@ -265,6 +266,7 @@ async fn apply_hunks_to_files(
let mut modified: Vec<PathBuf> = Vec::new();
let mut deleted: Vec<PathBuf> = Vec::new();
for hunk in hunks {
let affected_path = hunk.path().to_path_buf();
let path_abs = hunk.resolve_path(cwd);
match hunk {
Hunk::AddFile { contents, .. } => {
@@ -281,7 +283,7 @@ async fn apply_hunks_to_files(
fs.write_file(&path_abs, contents.clone().into_bytes())
.await
.with_context(|| format!("Failed to write file {}", path_abs.display()))?;
added.push(path_abs.into_path_buf());
added.push(affected_path);
}
Hunk::DeleteFile { .. } => {
let result: io::Result<()> = async {
@@ -303,7 +305,7 @@ async fn apply_hunks_to_files(
}
.await;
result.with_context(|| format!("Failed to delete file {}", path_abs.display()))?;
deleted.push(path_abs.into_path_buf());
deleted.push(affected_path);
}
Hunk::UpdateFile {
move_path, chunks, ..
@@ -349,12 +351,12 @@ async fn apply_hunks_to_files(
result.with_context(|| {
format!("Failed to remove original {}", path_abs.display())
})?;
modified.push(dest_abs.into_path_buf());
modified.push(affected_path);
} else {
fs.write_file(&path_abs, new_contents.into_bytes())
.await
.with_context(|| format!("Failed to write file {}", path_abs.display()))?;
modified.push(path_abs.into_path_buf());
modified.push(affected_path);
}
}
}
@@ -690,12 +692,9 @@ mod tests {
assert_eq!(
String::from_utf8(stdout).unwrap(),
format!(
"Success. Updated the following files:\nA {}\nA {}\nM {}\nM {}\nD {}\nD {}\n",
relative_add.display(),
"Success. Updated the following files:\nA relative-add.txt\nA {}\nM relative-update.txt\nM {}\nD relative-delete.txt\nD {}\n",
absolute_add.display(),
relative_update.display(),
absolute_update.display(),
relative_delete.display(),
absolute_delete.display(),
)
);

24
codex-rs/apply-patch/src/parser.rs
View File

@@ -26,6 +26,7 @@ use crate::ApplyPatchArgs;
use codex_utils_absolute_path::AbsolutePathBuf;
#[cfg(test)]
use codex_utils_absolute_path::test_support::PathBufExt;
use std::path::Path;
use std::path::PathBuf;
use thiserror::Error;
@@ -79,10 +80,27 @@ pub enum Hunk {
impl Hunk {
pub fn resolve_path(&self, cwd: &AbsolutePathBuf) -> AbsolutePathBuf {
let path = match self {
Hunk::UpdateFile { path, .. } => path,
Hunk::AddFile { .. } | Hunk::DeleteFile { .. } => self.path(),
};
AbsolutePathBuf::resolve_path_against_base(path, cwd)
}
/// Returns the path affected by this hunk, using the move destination for rename hunks.
pub fn path(&self) -> &Path {
match self {
Hunk::AddFile { path, .. } => AbsolutePathBuf::resolve_path_against_base(path, cwd),
Hunk::DeleteFile { path } => AbsolutePathBuf::resolve_path_against_base(path, cwd),
Hunk::UpdateFile { path, .. } => AbsolutePathBuf::resolve_path_against_base(path, cwd),
Hunk::AddFile { path, .. } => path,
Hunk::DeleteFile { path } => path,
Hunk::UpdateFile {
move_path: Some(path),
..
} => path,
Hunk::UpdateFile {
path,
move_path: None,
..
} => path,
}
}
}

27
codex-rs/apply-patch/tests/suite/cli.rs
View File

@@ -1,6 +1,5 @@
use assert_cmd::Command;
use std::fs;
use std::path::PathBuf;
use tempfile::tempdir;
fn apply_patch_command() -> anyhow::Result<Command> {
@@ -9,16 +8,11 @@ fn apply_patch_command() -> anyhow::Result<Command> {
)?))
}
fn resolved_temp_path(tmp: &tempfile::TempDir, path: &str) -> anyhow::Result<PathBuf> {
Ok(tmp.path().canonicalize()?.join(path))
}
#[test]
fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> {
let tmp = tempdir()?;
let file = "cli_test.txt";
let absolute_path = tmp.path().join(file);
let expected_path = resolved_temp_path(&tmp, file)?;
// 1) Add a file
let add_patch = format!(
@@ -32,10 +26,7 @@ fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> {
.current_dir(tmp.path())
.assert()
.success()
.stdout(format!(
"Success. Updated the following files:\nA {}\n",
expected_path.display()
));
.stdout(format!("Success. Updated the following files:\nA {file}\n"));
assert_eq!(fs::read_to_string(&absolute_path)?, "hello\n");
// 2) Update the file
@@ -52,10 +43,7 @@ fn test_apply_patch_cli_add_and_update() -> anyhow::Result<()> {
.current_dir(tmp.path())
.assert()
.success()
.stdout(format!(
"Success. Updated the following files:\nM {}\n",
expected_path.display()
));
.stdout(format!("Success. Updated the following files:\nM {file}\n"));
assert_eq!(fs::read_to_string(&absolute_path)?, "world\n");
Ok(())
@@ -66,7 +54,6 @@ fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> {
let tmp = tempdir()?;
let file = "cli_test_stdin.txt";
let absolute_path = tmp.path().join(file);
let expected_path = resolved_temp_path(&tmp, file)?;
// 1) Add a file via stdin
let add_patch = format!(
@@ -80,10 +67,7 @@ fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> {
.write_stdin(add_patch)
.assert()
.success()
.stdout(format!(
"Success. Updated the following files:\nA {}\n",
expected_path.display()
));
.stdout(format!("Success. Updated the following files:\nA {file}\n"));
assert_eq!(fs::read_to_string(&absolute_path)?, "hello\n");
// 2) Update the file via stdin
@@ -100,10 +84,7 @@ fn test_apply_patch_cli_stdin_add_and_update() -> anyhow::Result<()> {
.write_stdin(update_patch)
.assert()
.success()
.stdout(format!(
"Success. Updated the following files:\nM {}\n",
expected_path.display()
));
.stdout(format!("Success. Updated the following files:\nM {file}\n"));
assert_eq!(fs::read_to_string(&absolute_path)?, "world\n");
Ok(())

44
codex-rs/apply-patch/tests/suite/tool.rs
View File

@@ -27,23 +27,15 @@ fn test_apply_patch_cli_applies_multiple_operations() -> anyhow::Result<()> {
let add_path = tmp.path().join("nested/new.txt");
let modify_path = tmp.path().join("modify.txt");
let delete_path = tmp.path().join("delete.txt");
let expected_add_path = resolved_under(tmp.path(), "nested/new.txt")?;
let expected_modify_path = resolved_under(tmp.path(), "modify.txt")?;
let expected_delete_path = resolved_under(tmp.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";
run_apply_patch_in_dir(tmp.path(), patch)?
.success()
.stdout(format!(
"Success. Updated the following files:\nA {}\nM {}\nD {}\n",
expected_add_path.display(),
expected_modify_path.display(),
expected_delete_path.display()
));
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(add_path)?, "created\n");
assert_eq!(fs::read_to_string(&modify_path)?, "line1\nchanged\n");
@@ -56,17 +48,13 @@ fn test_apply_patch_cli_applies_multiple_operations() -> anyhow::Result<()> {
fn test_apply_patch_cli_applies_multiple_chunks() -> anyhow::Result<()> {
let tmp = tempdir()?;
let target_path = tmp.path().join("multi.txt");
let expected_target_path = resolved_under(tmp.path(), "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(format!(
"Success. Updated the following files:\nM {}\n",
expected_target_path.display()
));
.stdout("Success. Updated the following files:\nM multi.txt\n");
assert_eq!(
fs::read_to_string(&target_path)?,
@@ -81,7 +69,6 @@ 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");
let expected_new_path = resolved_under(tmp.path(), "renamed/dir/name.txt")?;
fs::create_dir_all(original_path.parent().expect("parent should exist"))?;
fs::write(&original_path, "old content\n")?;
@@ -89,10 +76,7 @@ fn test_apply_patch_cli_moves_file_to_new_directory() -> anyhow::Result<()> {
run_apply_patch_in_dir(tmp.path(), patch)?
.success()
.stdout(format!(
"Success. Updated the following files:\nM {}\n",
expected_new_path.display()
));
.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");
@@ -185,7 +169,6 @@ 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");
let expected_destination = resolved_under(tmp.path(), "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")?;
@@ -196,10 +179,7 @@ fn test_apply_patch_cli_move_overwrites_existing_destination() -> anyhow::Result
"*** 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(format!(
"Success. Updated the following files:\nM {}\n",
expected_destination.display()
));
.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");
@@ -211,7 +191,6 @@ fn test_apply_patch_cli_move_overwrites_existing_destination() -> anyhow::Result
fn test_apply_patch_cli_add_overwrites_existing_file() -> anyhow::Result<()> {
let tmp = tempdir()?;
let path = tmp.path().join("duplicate.txt");
let expected_path = resolved_under(tmp.path(), "duplicate.txt")?;
fs::write(&path, "old content\n")?;
run_apply_patch_in_dir(
@@ -219,10 +198,7 @@ fn test_apply_patch_cli_add_overwrites_existing_file() -> anyhow::Result<()> {
"*** Begin Patch\n*** Add File: duplicate.txt\n+new content\n*** End Patch",
)?
.success()
.stdout(format!(
"Success. Updated the following files:\nA {}\n",
expected_path.display()
));
.stdout("Success. Updated the following files:\nA duplicate.txt\n");
assert_eq!(fs::read_to_string(&path)?, "new content\n");
@@ -265,7 +241,6 @@ fn test_apply_patch_cli_rejects_invalid_hunk_header() -> anyhow::Result<()> {
fn test_apply_patch_cli_updates_file_appends_trailing_newline() -> anyhow::Result<()> {
let tmp = tempdir()?;
let target_path = tmp.path().join("no_newline.txt");
let expected_target_path = resolved_under(tmp.path(), "no_newline.txt")?;
fs::write(&target_path, "no newline at end")?;
run_apply_patch_in_dir(
@@ -273,10 +248,7 @@ fn test_apply_patch_cli_updates_file_appends_trailing_newline() -> anyhow::Resul
"*** 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(format!(
"Success. Updated the following files:\nM {}\n",
expected_target_path.display()
));
.stdout("Success. Updated the following files:\nM no_newline.txt\n");
let contents = fs::read_to_string(&target_path)?;
assert!(contents.ends_with('\n'));