Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 45 additions & 1 deletion src/commands/relocate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 <bold>{branch}</> (blocked by <bold>{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)
Expand Down Expand Up @@ -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<usize> {
// 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,
Expand Down
158 changes: 158 additions & 0 deletions tests/integration_tests/step_relocate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
@@ -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
Loading