From 316352be943cfe740850fb5e6dd543911a755bc6 Mon Sep 17 00:00:00 2001 From: Shane Vitarana Date: Thu, 6 Nov 2025 20:02:09 -0500 Subject: [PATCH] Fix apply_patch rename move path resolution (#5486) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes https://github.com/openai/codex/issues/5485. Fixed rename hunks so `apply_patch` resolves the destination path using the verifier’s effective cwd, ensuring patches that run under `cd && 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 https://github.com/openai/codex/issues/5483. Co-authored-by: Eric Traut --- codex-rs/apply-patch/src/lib.rs | 49 ++++++++++++++++++++++++++++++++- 1 file changed, 48 insertions(+), 1 deletion(-) diff --git a/codex-rs/apply-patch/src/lib.rs b/codex-rs/apply-patch/src/lib.rs index 3d0052ed..ac2f4097 100644 --- a/codex-rs/apply-patch/src/lib.rs +++ b/codex-rs/apply-patch/src/lib.rs @@ -288,7 +288,7 @@ pub fn maybe_parse_apply_patch_verified(argv: &[String], cwd: &Path) -> MaybeApp path, ApplyPatchFileChange::Update { unified_diff, - move_path: move_path.map(|p| cwd.join(p)), + move_path: move_path.map(|p| effective_cwd.join(p)), new_content: contents, }, ); @@ -1603,6 +1603,53 @@ g ); } + #[test] + fn test_apply_patch_resolves_move_path_with_effective_cwd() { + let session_dir = tempdir().unwrap(); + let worktree_rel = "alt"; + let worktree_dir = session_dir.path().join(worktree_rel); + fs::create_dir_all(&worktree_dir).unwrap(); + + let source_name = "old.txt"; + let dest_name = "renamed.txt"; + let source_path = worktree_dir.join(source_name); + fs::write(&source_path, "before\n").unwrap(); + + let patch = wrap_patch(&format!( + r#"*** Update File: {source_name} +*** Move to: {dest_name} +@@ +-before ++after"# + )); + + let shell_script = format!("cd {worktree_rel} && apply_patch <<'PATCH'\n{patch}\nPATCH"); + let argv = vec!["bash".into(), "-lc".into(), shell_script]; + + let result = maybe_parse_apply_patch_verified(&argv, session_dir.path()); + let action = match result { + MaybeApplyPatchVerified::Body(action) => action, + other => panic!("expected verified body, got {other:?}"), + }; + + assert_eq!(action.cwd, worktree_dir); + + let change = action + .changes() + .get(&worktree_dir.join(source_name)) + .expect("source file change present"); + + match change { + ApplyPatchFileChange::Update { move_path, .. } => { + assert_eq!( + move_path.as_deref(), + Some(worktree_dir.join(dest_name).as_path()) + ); + } + other => panic!("expected update change, got {other:?}"), + } + } + #[test] fn test_apply_patch_fails_on_write_error() { let dir = tempdir().unwrap();