Fix bd sync failing to copy local changes to beads-sync worktree (GH#810)
The root cause was isExternalBeadsDir() incorrectly identifying bare repo worktrees as "external" repos. This caused bd sync to take the "external beads dir" code path instead of the worktree-based sync branch path. The bug: isExternalBeadsDir() compared syncbranch.GetRepoRoot() (which returns incorrect values for bare repo worktrees) with getRepoRootFromPath() (which uses --show-toplevel). These returned different values for bare repo worktrees, causing local worktrees to be treated as external. The fix: Use git rev-parse --git-common-dir for comparison instead of repo root. This correctly identifies that worktrees of the same repo share the same git directory, regardless of bare repo setup. Also added: - getGitCommonDir() helper function - Tests for both getGitCommonDir and isExternalBeadsDir 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Steve Yegge
@@ -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.
|
||||
|
||||
@@ -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")
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user