diff --git a/src/commands/relocate.rs b/src/commands/relocate.rs index 7c3ea3d0a0..5d5680a86b 100644 --- a/src/commands/relocate.rs +++ b/src/commands/relocate.rs @@ -444,7 +444,28 @@ impl<'a> RelocationExecutor<'a> { made_progress = true; } Some(false) => { - // Target occupied by another pending worktree - wait for it to move + // Target occupied by another pending worktree. If that + // occupant is itself blocked it will never vacate, so + // this worktree can never reach its target either — + // propagate the block. Leaving it for `break_cycle` + // would temp-move it and then fail to finalize into the + // still-occupied path, stranding it in the staging dir. + if let Some(occupant_idx) = self.blocked_occupant(i) { + let branch = self.pending[i].branch().to_string(); + let occupant = self.pending[occupant_idx].branch().to_string(); + let msg = cformat!( + "Skipping {branch} (blocked by {occupant}, which can't be relocated)" + ); + eprintln!("{}", warning_message(msg)); + self.blocked.insert(i); + self.skipped_entries.push(SkippedEntry { + branch, + reason: "target_blocked", + }); + made_progress = true; + } + // Otherwise the occupant is still pending and may yet + // move (or forms a cycle `break_cycle` resolves). } None => { // Target unexpectedly blocked (TOCTOU race or same-target conflict) @@ -505,6 +526,29 @@ impl<'a> RelocationExecutor<'a> { .map(|occupant_idx| self.moved.contains(occupant_idx)) } + /// If `idx`'s target is occupied by a worktree we've already classified as + /// blocked (it will never vacate), return that occupant's index. + /// + /// A dependent whose occupant is blocked can never reach its target, so it + /// must be blocked too rather than handed to `break_cycle`. `break_cycle` + /// assumes "no progress ⟹ a cycle," temp-moves the dependent into the + /// staging dir, and `finalize_temp_relocations` then fails moving it into + /// the still-occupied target — erroring out and stranding the worktree in + /// staging. + fn blocked_occupant(&self, idx: usize) -> Option { + // The sole caller reaches here from `is_target_empty`'s `Some(false)` + // arm, which already established the target exists and is a tracked + // worktree — so no existence guard is needed. If the path were somehow + // gone, `canonicalize` falls back to the raw path, which won't match a + // canonical key, and `get` returns `None`. + let expected = &self.pending[idx].expected_path; + let canonical = expected.canonicalize().unwrap_or_else(|_| expected.clone()); + self.current_locations + .get(&canonical) + .copied() + .filter(|occupant_idx| self.blocked.contains(occupant_idx)) + } + /// Move a single worktree to its expected path. fn move_worktree( &mut self, diff --git a/tests/integration_tests/step_relocate.rs b/tests/integration_tests/step_relocate.rs index 7bf147020d..56ee7fa4fc 100644 --- a/tests/integration_tests/step_relocate.rs +++ b/tests/integration_tests/step_relocate.rs @@ -632,6 +632,164 @@ fn test_relocate_swap(repo: TestRepo) { assert!(path_for_beta.exists(), "beta should be at repo.beta"); } +/// A worktree whose target is occupied by a *blocked* worktree must itself be +/// skipped, not temp-moved. +/// +/// Regression: `beta` sits at `alpha`'s target, so `alpha` depends on `beta` +/// vacating — but `beta`'s own target is a plain non-worktree file with no +/// `--clobber`, so `beta` is blocked and never moves. Previously the no-progress +/// branch treated `alpha` as a cycle, temp-moved it into the staging dir, and +/// `finalize` then failed moving it into the still-occupied target — erroring +/// out and stranding `alpha` in `.git/wt/staging/relocate/`. +#[rstest] +fn test_relocate_blocked_occupant_skips_dependent(repo: TestRepo) { + let parent = worktree_parent(&repo); + + // beta occupies alpha's expected path (repo.alpha). + let path_alpha = parent.join("repo.alpha"); + repo.run_git(&[ + "worktree", + "add", + "-b", + "beta", + path_alpha.to_str().unwrap(), + ]); + + // alpha lives at a non-standard location and wants repo.alpha. + let wrong_alpha = parent.join("wrong-alpha"); + repo.run_git(&[ + "worktree", + "add", + "-b", + "alpha", + wrong_alpha.to_str().unwrap(), + ]); + + // Block beta's target (repo.beta) with a plain, non-worktree directory. + let path_beta = parent.join("repo.beta"); + fs::create_dir_all(&path_beta).unwrap(); + fs::write(path_beta.join("blocker.txt"), "blocker").unwrap(); + + // Both are skipped; the command must succeed and strand nothing. + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "step", + &["relocate", "alpha", "beta"], + None + )); + + // alpha stays at its original location (not stranded in staging). + assert!( + wrong_alpha.exists(), + "alpha should remain at its original location: {}", + wrong_alpha.display() + ); + // beta stays where it was (still occupying repo.alpha). + assert!(path_alpha.exists(), "beta should remain at repo.alpha"); + // Nothing left behind in the staging dir. + let stranded = repo.root_path().join(".git/wt/staging/relocate/alpha"); + assert!( + !stranded.exists(), + "alpha must not be stranded in the staging dir: {}", + stranded.display() + ); +} + +/// A blocked occupant must propagate transitively down a chain of dependents, +/// across multiple resolution passes. +/// +/// Extends `test_relocate_blocked_occupant_skips_dependent` to a 3-level chain +/// (`alpha → beta → gamma-blocked`) that specifically exercises the loop's +/// `made_progress` re-drive. `gamma`'s target is a plain non-worktree directory +/// (no `--clobber`), so `gamma` is blocked at construction; `beta` occupies +/// `gamma`'s dependency (sits at repo.beta) and `alpha` occupies `beta`'s (sits +/// at repo.alpha), so the block can only reach `alpha` one pass after it reaches +/// `beta`. +/// +/// The re-drive is only load-bearing when a dependent is *iterated before* its +/// occupant within a pass. Worktrees are processed in `git worktree list` order +/// (git sorts linked worktrees by registration id ≈ path basename), independent +/// of the argument order — so `alpha` is parked at `aaa-alpha`, whose basename +/// sorts before `beta`'s `repo.alpha`. That makes pass 1 visit `alpha` while +/// `beta` is still pending (no block yet → `alpha` stays pending), then block +/// `beta` (occupant `gamma` already blocked). Only the `made_progress` re-drive +/// runs a pass 2 that sees `beta` blocked and blocks `alpha` in turn. Drop the +/// re-drive and pass 1 falls straight into `break_cycle`, which temp-moves the +/// still-pending `alpha` and `finalize` then misplaces it into the occupied +/// `repo.alpha` — the exact bug this guards. (Parking `alpha` at a path that +/// sorts *after* `repo.alpha` collapses the chain into a single pass and no +/// longer tests the re-drive; see the 2-level test.) +#[rstest] +fn test_relocate_blocked_occupant_skips_chain(repo: TestRepo) { + let parent = worktree_parent(&repo); + + // alpha lives at aaa-alpha (basename sorts before repo.alpha, so alpha is + // iterated before its occupant beta) and wants repo.alpha. + let wrong_alpha = parent.join("aaa-alpha"); + repo.run_git(&[ + "worktree", + "add", + "-b", + "alpha", + wrong_alpha.to_str().unwrap(), + ]); + + // beta occupies alpha's expected path (repo.alpha) and wants repo.beta. + let path_alpha = parent.join("repo.alpha"); + repo.run_git(&[ + "worktree", + "add", + "-b", + "beta", + path_alpha.to_str().unwrap(), + ]); + + // gamma occupies beta's expected path (repo.beta) and wants repo.gamma. + let path_beta = parent.join("repo.beta"); + repo.run_git(&[ + "worktree", + "add", + "-b", + "gamma", + path_beta.to_str().unwrap(), + ]); + + // Block gamma's target (repo.gamma) with a plain, non-worktree directory. + let path_gamma = parent.join("repo.gamma"); + fs::create_dir_all(&path_gamma).unwrap(); + fs::write(path_gamma.join("blocker.txt"), "blocker").unwrap(); + + // All three are skipped; the command must succeed and strand nothing. + assert_cmd_snapshot!(make_snapshot_cmd( + &repo, + "step", + &["relocate", "alpha", "beta", "gamma"], + None + )); + + // Every worktree stays put — none stranded in staging or misplaced into an + // occupied target. + assert!( + wrong_alpha.exists(), + "alpha should remain at its original location: {}", + wrong_alpha.display() + ); + assert!(path_alpha.exists(), "beta should remain at repo.alpha"); + assert!(path_beta.exists(), "gamma should remain at repo.beta"); + let stranded = repo.root_path().join(".git/wt/staging/relocate/alpha"); + assert!( + !stranded.exists(), + "alpha must not be stranded in the staging dir: {}", + stranded.display() + ); + let misplaced = path_alpha.join("alpha"); + assert!( + !misplaced.exists(), + "alpha must not be misplaced inside beta's worktree: {}", + misplaced.display() + ); +} + /// Test relocating multiple worktrees shows compact output #[rstest] fn test_relocate_multiple(repo: TestRepo) { diff --git a/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_chain.snap b/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_chain.snap new file mode 100644 index 0000000000..5730dfa395 --- /dev/null +++ b/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_chain.snap @@ -0,0 +1,59 @@ +--- +source: tests/integration_tests/step_relocate.rs +info: + program: wt + args: + - step + - relocate + - alpha + - beta + - gamma + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_CONFIG_GLOBAL: "[TEST_GIT_CONFIG]" + GIT_CONFIG_SYSTEM: /dev/null + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +▲ Skipping gamma (target blocked: _REPO_.gamma) +↳ To backup blocking paths, use --clobber +▲ Skipping beta (blocked by gamma, which can't be relocated) +▲ Skipping alpha (blocked by beta, which can't be relocated) + +○ Relocated 0 worktrees, skipped 3 worktrees diff --git a/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_dependent.snap b/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_dependent.snap new file mode 100644 index 0000000000..7b69c6ceaa --- /dev/null +++ b/tests/snapshots/integration__integration_tests__step_relocate__relocate_blocked_occupant_skips_dependent.snap @@ -0,0 +1,57 @@ +--- +source: tests/integration_tests/step_relocate.rs +info: + program: wt + args: + - step + - relocate + - alpha + - beta + env: + APPDATA: "[TEST_CONFIG_HOME]" + CLAUDE_CONFIG_DIR: "[TEST_CLAUDE_CONFIG]" + CLICOLOR_FORCE: "1" + COLUMNS: "500" + GIT_AUTHOR_DATE: "2025-01-01T00:00:00Z" + GIT_COMMITTER_DATE: "2025-01-01T00:00:00Z" + GIT_CONFIG_GLOBAL: "[TEST_GIT_CONFIG]" + GIT_CONFIG_SYSTEM: /dev/null + GIT_TERMINAL_PROMPT: "0" + HOME: "[TEST_HOME]" + LANG: C + LC_ALL: C + LLVM_PROFILE_FILE: "[LLVM_PROFILE_FILE]" + MOCK_CONFIG_DIR: "[MOCK_CONFIG_DIR]" + OPENCODE_CONFIG_DIR: "[TEST_OPENCODE_CONFIG]" + PATH: "[PATH]" + TERM: alacritty + USERPROFILE: "[TEST_HOME]" + WORKTRUNK_APPROVALS_PATH: "[TEST_APPROVALS]" + WORKTRUNK_CONFIG_PATH: "[TEST_CONFIG]" + WORKTRUNK_SYSTEM_CONFIG_PATH: "[TEST_SYSTEM_CONFIG]" + WORKTRUNK_TEST_BASH_INSTALLED: "0" + WORKTRUNK_TEST_CLAUDE_INSTALLED: "0" + WORKTRUNK_TEST_CODEX_INSTALLED: "0" + WORKTRUNK_TEST_DELAYED_STREAM_MS: "-1" + WORKTRUNK_TEST_EPOCH: "1735776000" + WORKTRUNK_TEST_FISH_INSTALLED: "0" + WORKTRUNK_TEST_GEMINI_INSTALLED: "0" + WORKTRUNK_TEST_NUSHELL_ENV: "0" + WORKTRUNK_TEST_OPENCODE_INSTALLED: "0" + WORKTRUNK_TEST_PARENT_SHELL: "" + WORKTRUNK_TEST_POWERSHELL_ENV: "0" + WORKTRUNK_TEST_POWERSHELL_INSTALLED: "0" + WORKTRUNK_TEST_SKIP_URL_HEALTH_CHECK: "1" + WORKTRUNK_TEST_ZSH_INSTALLED: "0" + XDG_CONFIG_HOME: "[TEST_CONFIG_HOME]" +--- +success: true +exit_code: 0 +----- stdout ----- + +----- stderr ----- +▲ Skipping beta (target blocked: _REPO_.beta) +↳ To backup blocking paths, use --clobber +▲ Skipping alpha (blocked by beta, which can't be relocated) + +○ Relocated 0 worktrees, skipped 2 worktrees