diff --git a/cmd/bd/sync_branch.go b/cmd/bd/sync_branch.go index d632ce56..a82d3afa 100644 --- a/cmd/bd/sync_branch.go +++ b/cmd/bd/sync_branch.go @@ -183,20 +183,56 @@ func mergeSyncBranch(ctx context.Context, dryRun bool) error { // isExternalBeadsDir checks if the beads directory is in a different git repo than cwd. // This is used to detect when BEADS_DIR points to a separate repository. // Contributed by dand-oss (https://github.com/steveyegge/beads/pull/533) +// +// GH#810: Use git-common-dir for comparison instead of repo root. +// For bare repo worktrees, GetRepoRoot returns incorrect values, causing +// local worktrees to be incorrectly identified as "external". Using +// git-common-dir correctly identifies that worktrees of the same repo +// share the same git directory. func isExternalBeadsDir(ctx context.Context, beadsDir string) bool { - // Get repo root of cwd - cwdRepoRoot, err := syncbranch.GetRepoRoot(ctx) + // Get git common dir of cwd + cwdCommonDir, err := getGitCommonDir(ctx, ".") if err != nil { return false // Can't determine, assume local } - // Get repo root of beads dir - beadsRepoRoot, err := getRepoRootFromPath(ctx, beadsDir) + // Get git common dir of beads dir + beadsCommonDir, err := getGitCommonDir(ctx, beadsDir) if err != nil { return false // Can't determine, assume local } - return cwdRepoRoot != beadsRepoRoot + return cwdCommonDir != beadsCommonDir +} + +// getGitCommonDir returns the shared git directory for a path. +// For regular repos, this is the .git directory. +// For worktrees, this returns the shared git directory (common to all worktrees). +// This is the correct way to determine if two paths are in the same git repo, +// especially for bare repos and worktrees. +// GH#810: Added to fix bare repo worktree detection. +func getGitCommonDir(ctx context.Context, path string) (string, error) { + cmd := exec.CommandContext(ctx, "git", "-C", path, "rev-parse", "--git-common-dir") + output, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("failed to get git common dir for %s: %w", path, err) + } + result := strings.TrimSpace(string(output)) + // Make absolute for reliable comparison + if !filepath.IsAbs(result) { + // Resolve the input path to absolute first + absPath, err := filepath.Abs(path) + if err != nil { + return "", fmt.Errorf("failed to get absolute path for %s: %w", path, err) + } + result = filepath.Join(absPath, result) + } + result = filepath.Clean(result) + // Resolve symlinks for consistent comparison (macOS /var -> /private/var) + if resolved, err := filepath.EvalSymlinks(result); err == nil { + result = resolved + } + return result, nil } // getRepoRootFromPath returns the git repository root for a given path. diff --git a/cmd/bd/sync_test.go b/cmd/bd/sync_test.go index 263740b4..42aa2757 100644 --- a/cmd/bd/sync_test.go +++ b/cmd/bd/sync_test.go @@ -746,3 +746,151 @@ func TestResolveNoGitHistoryForFromMain(t *testing.T) { }) } } + +// TestGetGitCommonDir tests that getGitCommonDir correctly returns the shared +// git directory for both regular repos and worktrees. +func TestGetGitCommonDir(t *testing.T) { + ctx := context.Background() + + // Test 1: Regular repo + t.Run("regular repo", func(t *testing.T) { + repoDir, cleanup := setupGitRepo(t) + defer cleanup() + + commonDir, err := getGitCommonDir(ctx, repoDir) + if err != nil { + t.Fatalf("getGitCommonDir failed: %v", err) + } + + // For a regular repo, git-common-dir should point to .git + expectedGitDir := filepath.Join(repoDir, ".git") + // Resolve symlinks for comparison (macOS /var -> /private/var) + if resolved, err := filepath.EvalSymlinks(expectedGitDir); err == nil { + expectedGitDir = resolved + } + if commonDir != expectedGitDir { + t.Errorf("getGitCommonDir = %q, want %q", commonDir, expectedGitDir) + } + }) + + // Test 2: Worktree (non-bare) shares common dir with main repo + t.Run("worktree shares common dir with main repo", func(t *testing.T) { + repoDir, cleanup := setupGitRepo(t) + defer cleanup() + + // Create a branch for the worktree + if err := exec.Command("git", "-C", repoDir, "branch", "test-branch").Run(); err != nil { + t.Fatalf("git branch failed: %v", err) + } + + // Create worktree + worktreeDir := filepath.Join(t.TempDir(), "worktree") + if output, err := exec.Command("git", "-C", repoDir, "worktree", "add", worktreeDir, "test-branch").CombinedOutput(); err != nil { + t.Fatalf("git worktree add failed: %v\n%s", err, output) + } + + // Get common dir for both + mainCommonDir, err := getGitCommonDir(ctx, repoDir) + if err != nil { + t.Fatalf("getGitCommonDir(main) failed: %v", err) + } + + worktreeCommonDir, err := getGitCommonDir(ctx, worktreeDir) + if err != nil { + t.Fatalf("getGitCommonDir(worktree) failed: %v", err) + } + + // Both should return the same common dir + if mainCommonDir != worktreeCommonDir { + t.Errorf("common dirs differ: main=%q, worktree=%q", mainCommonDir, worktreeCommonDir) + } + }) +} + +// TestIsExternalBeadsDir tests that isExternalBeadsDir correctly identifies +// when beads directory is in the same vs different git repo. +// GH#810: This was broken for bare repo worktrees. +func TestIsExternalBeadsDir(t *testing.T) { + ctx := context.Background() + + // Test 1: Same directory - not external + t.Run("same directory is not external", func(t *testing.T) { + repoDir, cleanup := setupGitRepo(t) + defer cleanup() + + beadsDir := filepath.Join(repoDir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatalf("mkdir failed: %v", err) + } + + // Change to the repo directory (isExternalBeadsDir uses cwd) + origDir, _ := os.Getwd() + if err := os.Chdir(repoDir); err != nil { + t.Fatalf("chdir failed: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + if isExternalBeadsDir(ctx, beadsDir) { + t.Error("expected local beads dir to not be external") + } + }) + + // Test 2: Different repo - is external + t.Run("different repo is external", func(t *testing.T) { + repo1Dir, cleanup1 := setupGitRepo(t) + defer cleanup1() + repo2Dir, cleanup2 := setupGitRepo(t) + defer cleanup2() + + beadsDir := filepath.Join(repo2Dir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatalf("mkdir failed: %v", err) + } + + // Change to repo1 + origDir, _ := os.Getwd() + if err := os.Chdir(repo1Dir); err != nil { + t.Fatalf("chdir failed: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + if !isExternalBeadsDir(ctx, beadsDir) { + t.Error("expected beads dir in different repo to be external") + } + }) + + // Test 3: Worktree with beads - not external (GH#810 fix) + t.Run("worktree beads dir is not external", func(t *testing.T) { + repoDir, cleanup := setupGitRepo(t) + defer cleanup() + + // Create a branch for the worktree + if err := exec.Command("git", "-C", repoDir, "branch", "test-branch").Run(); err != nil { + t.Fatalf("git branch failed: %v", err) + } + + // Create worktree + worktreeDir := filepath.Join(t.TempDir(), "worktree") + if output, err := exec.Command("git", "-C", repoDir, "worktree", "add", worktreeDir, "test-branch").CombinedOutput(); err != nil { + t.Fatalf("git worktree add failed: %v\n%s", err, output) + } + + // Create beads dir in worktree + beadsDir := filepath.Join(worktreeDir, ".beads") + if err := os.MkdirAll(beadsDir, 0750); err != nil { + t.Fatalf("mkdir failed: %v", err) + } + + // Change to worktree + origDir, _ := os.Getwd() + if err := os.Chdir(worktreeDir); err != nil { + t.Fatalf("chdir failed: %v", err) + } + defer func() { _ = os.Chdir(origDir) }() + + // Beads dir in same worktree should NOT be external + if isExternalBeadsDir(ctx, beadsDir) { + t.Error("expected beads dir in same worktree to not be external") + } + }) +}