From 6e460f31cd89814d685df06a5464e0a1a6406318 Mon Sep 17 00:00:00 2001 From: Felipe Coury Date: Fri, 8 May 2026 14:13:46 -0300 Subject: [PATCH] feat(worktree): add move-all dirty policy --- codex-rs/cli/src/main.rs | 15 +++++ ...__tests__worktree_dirty_policy_prompt.snap | 8 ++- codex-rs/tui/src/worktree.rs | 41 +++++++++---- .../utils/cli/src/worktree_dirty_cli_arg.rs | 1 + codex-rs/worktree/src/dirty.rs | 57 ++++++++++++++++--- codex-rs/worktree/tests/git_backend.rs | 50 ++++++++++++++++ 6 files changed, 150 insertions(+), 22 deletions(-) diff --git a/codex-rs/cli/src/main.rs b/codex-rs/cli/src/main.rs index fa2cc62a52..faad516de1 100644 --- a/codex-rs/cli/src/main.rs +++ b/codex-rs/cli/src/main.rs @@ -789,6 +789,7 @@ fn dirty_policy_from_cli(arg: WorktreeDirtyCliArg) -> DirtyPolicy { WorktreeDirtyCliArg::Ignore => DirtyPolicy::Ignore, WorktreeDirtyCliArg::CopyTracked => DirtyPolicy::CopyTracked, WorktreeDirtyCliArg::CopyAll => DirtyPolicy::CopyAll, + WorktreeDirtyCliArg::MoveAll => DirtyPolicy::MoveAll, } } @@ -2418,6 +2419,20 @@ mod tests { ); } + #[test] + fn top_level_worktree_flags_parse_move_all_dirty_policy() { + let cli = MultitoolCli::try_parse_from([ + "codex", + "--worktree", + "parser-fix", + "--worktree-dirty", + "move-all", + ]) + .expect("worktree flags should parse"); + + assert_eq!(cli.interactive.worktree_dirty, WorktreeDirtyCliArg::MoveAll); + } + #[test] fn worktree_subcommand_parses() { let cli = MultitoolCli::try_parse_from(["codex", "worktree", "list", "--all", "--json"]) diff --git a/codex-rs/tui/src/snapshots/codex_tui__worktree__tests__worktree_dirty_policy_prompt.snap b/codex-rs/tui/src/snapshots/codex_tui__worktree__tests__worktree_dirty_policy_prompt.snap index 548fda3de9..afa9226de2 100644 --- a/codex-rs/tui/src/snapshots/codex_tui__worktree__tests__worktree_dirty_policy_prompt.snap +++ b/codex-rs/tui/src/snapshots/codex_tui__worktree__tests__worktree_dirty_policy_prompt.snap @@ -6,9 +6,11 @@ expression: "render_selection(dirty_policy_prompt_params(\"fcoury/demo\".to_stri Source checkout has uncommitted changes Choose what to carry into the new worktree. -› 1. Fail Cancel creation and leave the source checkout unchanged. - 2. Ignore Create from the requested base without copying local changes. +› 1. Move all Move tracked changes and untracked files; leave the source + checkout clean. + 2. Copy all Copy tracked changes and untracked files. 3. Copy tracked Copy staged and unstaged tracked changes. - 4. Copy all Copy tracked changes and untracked files. + 4. Ignore Create from the requested base without copying local changes. + 5. Fail Cancel creation and leave the source checkout unchanged. Press enter to confirm or esc to go back diff --git a/codex-rs/tui/src/worktree.rs b/codex-rs/tui/src/worktree.rs index 32a5554f5a..e01450f3d3 100644 --- a/codex-rs/tui/src/worktree.rs +++ b/codex-rs/tui/src/worktree.rs @@ -236,7 +236,11 @@ fn parse_dirty_policy(value: &str) -> Result { "ignore" => Ok(DirtyPolicy::Ignore), "copy-tracked" => Ok(DirtyPolicy::CopyTracked), "copy-all" => Ok(DirtyPolicy::CopyAll), - _ => Err("Dirty mode must be one of: fail, ignore, copy-tracked, copy-all.".to_string()), + "move-all" => Ok(DirtyPolicy::MoveAll), + _ => Err( + "Dirty mode must be one of: fail, ignore, copy-tracked, copy-all, move-all." + .to_string(), + ), } } @@ -479,14 +483,14 @@ pub(crate) fn dirty_policy_prompt_params( footer_hint: Some(standard_popup_hint_line()), items: vec![ item( - "Fail", - "Cancel creation and leave the source checkout unchanged.", - DirtyPolicy::Fail, + "Move all", + "Move tracked changes and untracked files; leave the source checkout clean.", + DirtyPolicy::MoveAll, ), item( - "Ignore", - "Create from the requested base without copying local changes.", - DirtyPolicy::Ignore, + "Copy all", + "Copy tracked changes and untracked files.", + DirtyPolicy::CopyAll, ), item( "Copy tracked", @@ -494,9 +498,14 @@ pub(crate) fn dirty_policy_prompt_params( DirtyPolicy::CopyTracked, ), item( - "Copy all", - "Copy tracked changes and untracked files.", - DirtyPolicy::CopyAll, + "Ignore", + "Create from the requested base without copying local changes.", + DirtyPolicy::Ignore, + ), + item( + "Fail", + "Cancel creation and leave the source checkout unchanged.", + DirtyPolicy::Fail, ), ], ..Default::default() @@ -622,6 +631,18 @@ mod tests { ); } + #[test] + fn parse_new_with_move_all_dirty_policy() { + assert_eq!( + parse_worktree_slash_args("new fcoury/demo --dirty move-all"), + Ok(WorktreeSlashAction::Create { + branch: "fcoury/demo".to_string(), + base_ref: /*base_ref*/ None, + dirty_policy: Some(DirtyPolicy::MoveAll), + }) + ); + } + #[test] fn parse_switch_aliases_move() { assert_eq!( diff --git a/codex-rs/utils/cli/src/worktree_dirty_cli_arg.rs b/codex-rs/utils/cli/src/worktree_dirty_cli_arg.rs index b7843957dd..eafd46a5dd 100644 --- a/codex-rs/utils/cli/src/worktree_dirty_cli_arg.rs +++ b/codex-rs/utils/cli/src/worktree_dirty_cli_arg.rs @@ -8,4 +8,5 @@ pub enum WorktreeDirtyCliArg { Ignore, CopyTracked, CopyAll, + MoveAll, } diff --git a/codex-rs/worktree/src/dirty.rs b/codex-rs/worktree/src/dirty.rs index 5d2a13bda4..8f2e00ef55 100644 --- a/codex-rs/worktree/src/dirty.rs +++ b/codex-rs/worktree/src/dirty.rs @@ -16,6 +16,7 @@ pub enum DirtyPolicy { Ignore, CopyTracked, CopyAll, + MoveAll, } #[derive(Debug, Clone, PartialEq, Eq, Default, Serialize, Deserialize)] @@ -61,14 +62,14 @@ pub fn validate_dirty_policy_before_create( DirtyPolicy::CopyTracked => { if state.has_untracked_files { Ok(vec![ - "untracked files were left in the source checkout; use --worktree-dirty copy-all to copy them" + "untracked files were left in the source checkout; use --worktree-dirty copy-all or move-all to carry them" .to_string(), ]) } else { Ok(Vec::new()) } } - DirtyPolicy::CopyAll => Ok(Vec::new()), + DirtyPolicy::CopyAll | DirtyPolicy::MoveAll => Ok(Vec::new()), } } @@ -90,12 +91,19 @@ pub fn apply_dirty_policy_after_create( copy_untracked_files(source_root, worktree_root)?; Ok(()) } + DirtyPolicy::MoveAll => { + let untracked_paths = untracked_paths(source_root)?; + apply_tracked_diff(source_root, worktree_root)?; + copy_untracked_files_at_paths(source_root, worktree_root, &untracked_paths)?; + clean_source_after_move(source_root, &untracked_paths)?; + Ok(()) + } } } fn bail_for_dirty_source() -> Result { anyhow::bail!( - "source checkout has uncommitted changes; use --worktree-dirty ignore, copy-tracked, or copy-all" + "source checkout has uncommitted changes; use --worktree-dirty ignore, copy-tracked, copy-all, or move-all" ); } @@ -119,18 +127,35 @@ fn apply_tracked_diff(source_root: &Path, worktree_root: &Path) -> Result<()> { } fn copy_untracked_files(source_root: &Path, worktree_root: &Path) -> Result<()> { + let paths = untracked_paths(source_root)?; + copy_untracked_files_at_paths(source_root, worktree_root, &paths) +} + +fn untracked_paths(source_root: &Path) -> Result> { let output = git::bytes( source_root, &["ls-files", "--others", "--exclude-standard", "-z"], )?; - for raw_path in output + output .split(|byte| *byte == 0) .filter(|path| !path.is_empty()) - { - let relative_path = PathBuf::from(String::from_utf8_lossy(raw_path).into_owned()); - ensure_safe_relative_path(&relative_path)?; - let source = source_root.join(&relative_path); - let destination = worktree_root.join(&relative_path); + .map(|raw_path| { + let relative_path = PathBuf::from(String::from_utf8_lossy(raw_path).into_owned()); + ensure_safe_relative_path(&relative_path)?; + Ok(relative_path) + }) + .collect() +} + +fn copy_untracked_files_at_paths( + source_root: &Path, + worktree_root: &Path, + paths: &[PathBuf], +) -> Result<()> { + for relative_path in paths { + ensure_safe_relative_path(relative_path)?; + let source = source_root.join(relative_path); + let destination = worktree_root.join(relative_path); let metadata = fs::symlink_metadata(&source)?; if let Some(parent) = destination.parent() { fs::create_dir_all(parent)?; @@ -145,6 +170,20 @@ fn copy_untracked_files(source_root: &Path, worktree_root: &Path) -> Result<()> Ok(()) } +fn clean_source_after_move(source_root: &Path, untracked_paths: &[PathBuf]) -> Result<()> { + git::status(source_root, &["reset", "--hard", "HEAD"]) + .context("failed to clean tracked changes from source checkout after move")?; + for relative_path in untracked_paths { + fs::remove_file(source_root.join(relative_path)).with_context(|| { + format!( + "failed to remove moved untracked path {} from source checkout", + relative_path.display() + ) + })?; + } + Ok(()) +} + fn ensure_safe_relative_path(path: &Path) -> Result<()> { if path.is_absolute() { anyhow::bail!( diff --git a/codex-rs/worktree/tests/git_backend.rs b/codex-rs/worktree/tests/git_backend.rs index 473108c487..637fa0c939 100644 --- a/codex-rs/worktree/tests/git_backend.rs +++ b/codex-rs/worktree/tests/git_backend.rs @@ -156,6 +156,56 @@ fn copy_tracked_preserves_staged_and_unstaged_diffs() -> anyhow::Result<()> { Ok(()) } +#[test] +fn move_all_transfers_dirty_state_and_cleans_source_checkout() -> anyhow::Result<()> { + let fixture = GitFixture::new()?; + fs::write(fixture.repo.path().join(".gitignore"), "ignored.txt\n")?; + run_git(fixture.repo.path(), &["add", ".gitignore"])?; + run_git(fixture.repo.path(), &["commit", "-m", "ignore fixture"])?; + fs::write(fixture.repo.path().join("staged.txt"), "staged changed\n")?; + run_git(fixture.repo.path(), &["add", "staged.txt"])?; + fs::write( + fixture.repo.path().join("unstaged.txt"), + "unstaged changed\n", + )?; + fs::write(fixture.repo.path().join("untracked.txt"), "untracked\n")?; + fs::write(fixture.repo.path().join("ignored.txt"), "ignored\n")?; + + let resolution = codex_worktree::ensure_worktree(WorktreeRequest { + codex_home: fixture.codex_home.path().to_path_buf(), + source_cwd: fixture.repo.path().to_path_buf(), + branch: "move-dirty".to_string(), + base_ref: /*base_ref*/ None, + dirty_policy: DirtyPolicy::MoveAll, + })?; + + assert_eq!( + git( + &resolution.info.worktree_git_root, + &["diff", "--cached", "--name-only"] + )?, + "staged.txt" + ); + assert_eq!( + git(&resolution.info.worktree_git_root, &["diff", "--name-only"])?, + "unstaged.txt" + ); + assert_eq!( + fs::read_to_string(resolution.info.worktree_git_root.join("untracked.txt"))?, + "untracked\n" + ); + assert!( + !resolution + .info + .worktree_git_root + .join("ignored.txt") + .exists() + ); + assert_eq!(git(fixture.repo.path(), &["status", "--short"])?, ""); + assert!(fixture.repo.path().join("ignored.txt").exists()); + Ok(()) +} + #[test] fn refuses_sibling_path_collision_for_different_branch() -> anyhow::Result<()> { let fixture = GitFixture::new()?;