Skip to content

Commit 316352b

Browse files
Fix apply_patch rename move path resolution (#5486)
Fixes #5485. Fixed rename hunks so `apply_patch` resolves the destination path using the verifier’s effective cwd, ensuring patches that run under `cd <worktree> && apply_patch` stay inside the worktree. Added a regression test (`test_apply_patch_resolves_move_path_with_effective_cwd`) that reproduced the old behavior (dest path resolved in the main repo) and now passes. Related to #5483. Co-authored-by: Eric Traut <[email protected]>
1 parent f8b30af commit 316352b

File tree

1 file changed

+48
-1
lines changed
  • codex-rs/apply-patch/src

1 file changed

+48
-1
lines changed

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

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp
288288
path,
289289
ApplyPatchFileChange::Update {
290290
unified_diff,
291-
move_path: move_path.map(|p| cwd.join(p)),
291+
move_path: move_path.map(|p| effective_cwd.join(p)),
292292
new_content: contents,
293293
},
294294
);
@@ -1603,6 +1603,53 @@ g
16031603
);
16041604
}
16051605

1606+
#[test]
1607+
fn test_apply_patch_resolves_move_path_with_effective_cwd() {
1608+
let session_dir = tempdir().unwrap();
1609+
let worktree_rel = "alt";
1610+
let worktree_dir = session_dir.path().join(worktree_rel);
1611+
fs::create_dir_all(&worktree_dir).unwrap();
1612+
1613+
let source_name = "old.txt";
1614+
let dest_name = "renamed.txt";
1615+
let source_path = worktree_dir.join(source_name);
1616+
fs::write(&source_path, "before\n").unwrap();
1617+
1618+
let patch = wrap_patch(&format!(
1619+
r#"*** Update File: {source_name}
1620+
*** Move to: {dest_name}
1621+
@@
1622+
-before
1623+
+after"#
1624+
));
1625+
1626+
let shell_script = format!("cd {worktree_rel} && apply_patch <<'PATCH'\n{patch}\nPATCH");
1627+
let argv = vec!["bash".into(), "-lc".into(), shell_script];
1628+
1629+
let result = maybe_parse_apply_patch_verified(&argv, session_dir.path());
1630+
let action = match result {
1631+
MaybeApplyPatchVerified::Body(action) => action,
1632+
other => panic!("expected verified body, got {other:?}"),
1633+
};
1634+
1635+
assert_eq!(action.cwd, worktree_dir);
1636+
1637+
let change = action
1638+
.changes()
1639+
.get(&worktree_dir.join(source_name))
1640+
.expect("source file change present");
1641+
1642+
match change {
1643+
ApplyPatchFileChange::Update { move_path, .. } => {
1644+
assert_eq!(
1645+
move_path.as_deref(),
1646+
Some(worktree_dir.join(dest_name).as_path())
1647+
);
1648+
}
1649+
other => panic!("expected update change, got {other:?}"),
1650+
}
1651+
}
1652+
16061653
#[test]
16071654
fn test_apply_patch_fails_on_write_error() {
16081655
let dir = tempdir().unwrap();

0 commit comments

Comments
 (0)