Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
32 changes: 20 additions & 12 deletions src/commands/worktree/switch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1295,18 +1295,26 @@ 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();
// Skip when recovered — the source worktree is gone, and `current_worktree()`
// resolves to the recovered ancestor (typically the main worktree), which would
// misleadingly report main's branch/path as the user's "base".
let (source_branch, source_path) = if is_recovered {
(String::new(), String::new())
} else {
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)
};

// Execute the validated plan
let (result, branch_info) = execute_switch(&repo, plan, config, yes, hooks_approved)?;
Expand Down
10 changes: 1 addition & 9 deletions src/git/repository/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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/")
Expand Down
13 changes: 9 additions & 4 deletions src/git/repository/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(p)`] the discovery path is `p`, and this
Comment thread
worktrunk-bot marked this conversation as resolved.
Resolved
/// returns a worktree at `p` 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.
Expand Down
31 changes: 31 additions & 0 deletions src/git/repository/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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",
);
}
9 changes: 5 additions & 4 deletions src/git/repository/working_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
3 changes: 0 additions & 3 deletions tests/integration_tests/repository.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand All @@ -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
Expand Down
Loading