diff --git a/src/commands/worktree/switch.rs b/src/commands/worktree/switch.rs index 2d4e13e450..e9ad3c72ce 100644 --- a/src/commands/worktree/switch.rs +++ b/src/commands/worktree/switch.rs @@ -1219,6 +1219,31 @@ pub(crate) fn spawn_switch_background_hooks( announcer.flush() } +/// Capture the source worktree's branch and root for `{{ base }}` / +/// `{{ base_worktree_path }}` in post-switch hooks. Returns empty strings +/// when recovered from a deleted CWD — the source worktree is gone, and +/// `current_worktree()` would resolve to the recovered ancestor (typically +/// the main worktree), which would misleadingly report main's branch/path +/// as the user's "base". +fn capture_switch_source(repo: &Repository, is_recovered: bool) -> (String, String) { + if is_recovered { + return (String::new(), String::new()); + } + let source_branch = repo + .current_worktree() + .branch() + .ok() + .flatten() + .unwrap_or_default(); + let source_path = repo + .current_worktree() + .root() + .ok() + .map(|p| worktrunk::path::to_posix_path(&p.to_string_lossy())) + .unwrap_or_default(); + (source_branch, source_path) +} + /// Handle the switch command. pub fn run_switch( opts: SwitchOptions<'_>, @@ -1295,18 +1320,7 @@ pub fn run_switch( // Capture source (base) worktree identity BEFORE the switch, so post-switch // hooks can reference where the user came from via {{ base }} / {{ base_worktree_path }}. - let source_branch = repo - .current_worktree() - .branch() - .ok() - .flatten() - .unwrap_or_default(); - let source_path = repo - .current_worktree() - .root() - .ok() - .map(|p| worktrunk::path::to_posix_path(&p.to_string_lossy())) - .unwrap_or_default(); + let (source_branch, source_path) = capture_switch_source(&repo, is_recovered); // Execute the validated plan let (result, branch_info) = execute_switch(&repo, plan, config, yes, hooks_approved)?; @@ -1344,8 +1358,11 @@ pub fn run_switch( // Returns path to display in hooks when user's shell won't be in the worktree // Also shows worktree-path hint on first --create (before shell integration warning) // - // When recovered from a deleted worktree, current_dir() and current_worktree().root() - // both fail — fall back to repo_path() (the main worktree root). + // When the user's CWD has been deleted, `std::env::current_dir()` fails — + // fall back to `repo_path()` (the main worktree root). `current_worktree() + // .root()` resolves against the Repository's discovery path, which is alive + // even after recovery, but we keep the same fallback for any pathological + // case where rev-parse fails. let fallback_path = repo.repo_path()?.to_path_buf(); let cwd = std::env::current_dir().unwrap_or(fallback_path.clone()); let source_root = repo.current_worktree().root().unwrap_or(fallback_path); @@ -1522,3 +1539,31 @@ fn validate_switch_templates( Ok(()) } + +#[cfg(test)] +mod tests { + use super::*; + use worktrunk::testing::TestRepo; + + #[test] + fn capture_switch_source_returns_empty_when_recovered() { + // When recovered from a deleted CWD, post-switch hooks must see empty + // `{{ base }}` / `{{ base_worktree_path }}` rather than the recovered + // ancestor's identity (typically the main worktree's branch/path). + let test = TestRepo::with_initial_commit(); + let (branch, path) = capture_switch_source(&test.repo, true); + assert_eq!(branch, ""); + assert_eq!(path, ""); + } + + #[test] + fn capture_switch_source_returns_branch_and_path_normally() { + // When not recovered, the helper reports the current worktree's + // identity. This guards against accidental regressions to the + // `is_recovered` gate (e.g., always returning empty). + let test = TestRepo::with_initial_commit(); + let (branch, path) = capture_switch_source(&test.repo, false); + assert_eq!(branch, "main"); + assert!(!path.is_empty(), "source_path should be the worktree root"); + } +} diff --git a/src/git/repository/config.rs b/src/git/repository/config.rs index 370b3eb50f..2b73e3ad77 100644 --- a/src/git/repository/config.rs +++ b/src/git/repository/config.rs @@ -465,15 +465,7 @@ impl Repository { // - Linked worktrees: HEAD points to CURRENT branch, so skip this heuristic // - Normal repos: HEAD points to CURRENT branch, so skip this heuristic let is_bare = self.is_bare()?; - // Anchor the linked-worktree probe to this Repository's own - // discovery path rather than the process CWD (`current_worktree()`). - // The two are identical for `Repository::current()`, but - // `Repository::at(p)` is also used for tests and for non-CWD callers - // like `wt list`'s pipeline; using CWD there leaks process state - // into a query that should be answered relative to `self` (#2624). - let in_linked_worktree = self - .worktree_at(self.discovery_path().to_path_buf()) - .is_linked()?; + let in_linked_worktree = self.current_worktree().is_linked()?; if ((is_bare && !in_linked_worktree) || branches.is_empty()) && let Ok(head_ref) = self.run_command(&["symbolic-ref", "HEAD"]) && let Some(branch) = head_ref.trim().strip_prefix("refs/heads/") diff --git a/src/git/repository/mod.rs b/src/git/repository/mod.rs index 9af55f704e..73a835a7a1 100644 --- a/src/git/repository/mod.rs +++ b/src/git/repository/mod.rs @@ -890,11 +890,16 @@ impl Repository { &self.discovery_path } - /// Get a worktree view at the current directory. - /// - /// This is the primary way to get a [`WorkingTree`] for worktree-specific operations. + /// Get a worktree view at this Repository's discovery path. + /// + /// This is the primary way to get a [`WorkingTree`] for worktree-specific + /// operations. For [`Repository::current`] the discovery path is the + /// process CWD (or `-C` value), so "current" matches its colloquial + /// meaning. For [`Repository::at`], the discovery path is the path passed + /// in, and this returns a worktree there rather than at the process CWD — + /// see #2625 for the bug class that motivated anchoring this to `self`. pub fn current_worktree(&self) -> WorkingTree<'_> { - self.worktree_at(base_path().clone()) + self.worktree_at(self.discovery_path.clone()) } /// Get a worktree view at a specific path. diff --git a/src/git/repository/tests.rs b/src/git/repository/tests.rs index 99078f04d4..d2ff61ec85 100644 --- a/src/git/repository/tests.rs +++ b/src/git/repository/tests.rs @@ -763,3 +763,34 @@ fn worktree_at_path_resolves_symlinked_path() { let (_, branch) = resolved.unwrap(); assert_eq!(branch.as_deref(), Some("feature")); } + +#[test] +fn current_worktree_anchors_to_repository_discovery_path() { + // `Repository::at(p).current_worktree()` must resolve relative to `p`, + // not to the test process's CWD. PR #2625 fixed one symptom of the + // opposite behavior (`infer_default_branch_locally` calling + // `current_worktree().is_linked()` and reaching for process CWD); the + // structural fix anchored `current_worktree` to `self.discovery_path`, + // which this test pins down so the bug class can't regress. + use super::Repository; + use crate::testing::TestRepo; + use dunce::canonicalize; + + let test = TestRepo::with_initial_commit(); + let repo = Repository::at(test.repo.discovery_path().to_path_buf()).unwrap(); + + let wt_path = repo.current_worktree().path().to_path_buf(); + let expected = canonicalize(test.repo.discovery_path()).unwrap(); + assert_eq!( + wt_path, expected, + "current_worktree() should resolve to the Repository's discovery path, not the process CWD", + ); + // The bug we're guarding against: `current_worktree()` previously used + // the process CWD (`base_path()`), which for unit tests is the cargo + // working directory — different from any test repo's tempdir. + let process_cwd = canonicalize(std::env::current_dir().unwrap()).unwrap(); + assert_ne!( + wt_path, process_cwd, + "current_worktree() must not resolve to the process CWD when Repository::at(p) was given a different path", + ); +} diff --git a/src/git/repository/working_tree.rs b/src/git/repository/working_tree.rs index 35a8891c19..a3acc3b5c5 100644 --- a/src/git/repository/working_tree.rs +++ b/src/git/repository/working_tree.rs @@ -106,10 +106,11 @@ impl<'a> WorkingTree<'a> { /// Get the path this WorkingTree was created with. /// - /// Returns the canonicalized form when the input passed to `worktree_at()` / - /// `base_path()` for `current_worktree()` exists on disk; otherwise returns - /// the raw input. So on macOS, a temp path like `/tmp/foo` may surface here - /// (and to hook template variables) as `/private/tmp/foo`. + /// Returns the canonicalized form when the input passed to `worktree_at()` + /// (or the Repository's discovery path, for `current_worktree()`) exists + /// on disk; otherwise returns the raw input. So on macOS, a temp path + /// like `/tmp/foo` may surface here (and to hook template variables) as + /// `/private/tmp/foo`. /// /// For the canonical git-determined root, use [`root()`](Self::root) instead. pub fn path(&self) -> &Path { diff --git a/tests/integration_tests/repository.rs b/tests/integration_tests/repository.rs index 478f182e40..001a10565a 100644 --- a/tests/integration_tests/repository.rs +++ b/tests/integration_tests/repository.rs @@ -1101,7 +1101,6 @@ fn test_branch_returns_none_for_detached_head() { // Create a fresh repository instance to avoid cached result let repository = Repository::at(&root).unwrap(); - // Use worktree_at with explicit path, not current_worktree() which uses base_path() let wt = repository.worktree_at(&root); let result = wt.branch(); @@ -1139,7 +1138,6 @@ fn test_branch_returns_branch_name() { let repo = TestRepo::new(); let root = repo.root_path().to_path_buf(); let repository = Repository::at(&root).unwrap(); - // Use worktree_at with explicit path, not current_worktree() which uses base_path() let wt = repository.worktree_at(&root); let result = wt.branch(); @@ -1156,7 +1154,6 @@ fn test_branch_caches_result() { let repo = TestRepo::new(); let root = repo.root_path().to_path_buf(); let repository = Repository::at(&root).unwrap(); - // Use worktree_at with explicit path, not current_worktree() which uses base_path() let wt = repository.worktree_at(&root); // First call