fix(syncbranch): support bare repos and worktrees with git-common-dir (#641)

* fix(syncbranch): support bare repos and worktrees with git-common-dir Replace hardcoded .git/beads-worktrees/ path with dynamic detection using git rev-parse --git-common-dir. This correctly handles: - Regular repositories (.git is a directory) - Git worktrees (.git is a file pointing elsewhere) - Bare repositories (no .git directory, repo IS the git dir) - Worktrees of bare repositories The new getBeadsWorktreePath() helper uses git's native API to find the shared git directory, ensuring beads worktrees are created in the correct location regardless of repository structure. Updated functions: - CommitToSyncBranch - PullFromSyncBranch - CheckDivergence - ResetToRemote Fixes #639 * test(syncbranch): add regression tests for getBeadsWorktreePath Add comprehensive tests for the worktree path calculation to ensure proper handling of various repository structures: - Regular repos: uses .git/beads-worktrees path - Bare repos: uses <bare-repo>/beads-worktrees (no .git subdirectory) - Worktrees: uses main repo's .git/beads-worktrees (git-common-dir) - Fallback: legacy behavior when git command fails - Relative paths: ensures absolute path conversion These tests ensure the fix for GH#639 doesn't regress. --------- Co-authored-by: Charles P. Cross <cpdata@users.noreply.github.com>
2025-12-19 20:52:40 -05:00
parent e309b1c1aa
commit 81aa301649
2 changed files with 204 additions and 8 deletions
--- a/internal/syncbranch/worktree.go
+++ b/internal/syncbranch/worktree.go
@@ -77,8 +77,8 @@ func CommitToSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str
 		Branch: syncBranch,
 	}

-	// Worktree path is under .git/beads-worktrees/<branch>
-	worktreePath := filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch)
+	// GH#639: Use git-common-dir for worktree path to support bare repos
+	worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch)

 	// Initialize worktree manager
 	wtMgr := git.NewWorktreeManager(repoRoot)
@@ -240,8 +240,8 @@ func PullFromSyncBranch(ctx context.Context, repoRoot, syncBranch, jsonlPath str
 		JSONLPath: jsonlPath,
 	}

-	// Worktree path is under .git/beads-worktrees/<branch>
-	worktreePath := filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch)
+	// GH#639: Use git-common-dir for worktree path to support bare repos
+	worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch)

 	// Initialize worktree manager
 	wtMgr := git.NewWorktreeManager(repoRoot)
@@ -469,8 +469,8 @@ func CheckDivergence(ctx context.Context, repoRoot, syncBranch string) (*Diverge
 		Branch: syncBranch,
 	}

-	// Worktree path is under .git/beads-worktrees/<branch>
-	worktreePath := filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch)
+	// GH#639: Use git-common-dir for worktree path to support bare repos
+	worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch)

 	// Initialize worktree manager
 	wtMgr := git.NewWorktreeManager(repoRoot)
@@ -526,8 +526,8 @@ func CheckDivergence(ctx context.Context, repoRoot, syncBranch string) (*Diverge
 //
 // Returns error if reset fails.
 func ResetToRemote(ctx context.Context, repoRoot, syncBranch, jsonlPath string) error {
-	// Worktree path is under .git/beads-worktrees/<branch>
-	worktreePath := filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch)
+	// GH#639: Use git-common-dir for worktree path to support bare repos
+	worktreePath := getBeadsWorktreePath(ctx, repoRoot, syncBranch)

 	// Initialize worktree manager
 	wtMgr := git.NewWorktreeManager(repoRoot)
@@ -858,6 +858,28 @@ func PushSyncBranch(ctx context.Context, repoRoot, syncBranch string) error {
 	return pushFromWorktree(ctx, worktreePath, syncBranch)
 }

+// getBeadsWorktreePath returns the path where beads worktrees should be stored.
+// GH#639: Uses git rev-parse --git-common-dir to correctly handle bare repos and worktrees.
+// For regular repos, this is typically .git/beads-worktrees/<branch>.
+// For bare repos or worktrees of bare repos, this uses the common git directory.
+func getBeadsWorktreePath(ctx context.Context, repoRoot, syncBranch string) string {
+	// Try to get the git common directory using git's native API
+	// This handles all cases: regular repos, worktrees, bare repos
+	cmd := exec.CommandContext(ctx, "git", "-C", repoRoot, "rev-parse", "--git-common-dir")
+	output, err := cmd.Output()
+	if err == nil {
+		gitCommonDir := strings.TrimSpace(string(output))
+		// Make path absolute if it's relative
+		if !filepath.IsAbs(gitCommonDir) {
+			gitCommonDir = filepath.Join(repoRoot, gitCommonDir)
+		}
+		return filepath.Join(gitCommonDir, "beads-worktrees", syncBranch)
+	}
+
+	// Fallback to legacy behavior for compatibility
+	return filepath.Join(repoRoot, ".git", "beads-worktrees", syncBranch)
+}
+
 // getRemoteForBranch gets the remote name for a branch, defaulting to "origin"
 func getRemoteForBranch(ctx context.Context, worktreePath, branch string) string {
 	remoteCmd := exec.CommandContext(ctx, "git", "-C", worktreePath, "config", "--get", fmt.Sprintf("branch.%s.remote", branch))
--- a/internal/syncbranch/worktree_path_test.go
+++ b/internal/syncbranch/worktree_path_test.go
@@ -0,0 +1,174 @@
+package syncbranch
+
+import (
+	"context"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+// TestGetBeadsWorktreePath tests the worktree path calculation for various repo structures.
+// This is the regression test for GH#639.
+func TestGetBeadsWorktreePath(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ctx := context.Background()
+
+	t.Run("regular repo returns .git/beads-worktrees path", func(t *testing.T) {
+		// Create a regular git repository
+		tmpDir := t.TempDir()
+		runGitCmd(t, tmpDir, "init")
+		runGitCmd(t, tmpDir, "config", "user.email", "test@test.com")
+		runGitCmd(t, tmpDir, "config", "user.name", "Test User")
+
+		// Create initial commit
+		testFile := filepath.Join(tmpDir, "test.txt")
+		if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
+			t.Fatalf("Failed to write test file: %v", err)
+		}
+		runGitCmd(t, tmpDir, "add", ".")
+		runGitCmd(t, tmpDir, "commit", "-m", "initial")
+
+		// Test getBeadsWorktreePath
+		path := getBeadsWorktreePath(ctx, tmpDir, "beads-sync")
+
+		// Should be under .git/beads-worktrees
+		expectedSuffix := filepath.Join(".git", "beads-worktrees", "beads-sync")
+		if !strings.HasSuffix(path, expectedSuffix) {
+			t.Errorf("Expected path to end with %q, got %q", expectedSuffix, path)
+		}
+
+		// Path should be absolute
+		if !filepath.IsAbs(path) {
+			t.Errorf("Expected absolute path, got %q", path)
+		}
+	})
+
+	t.Run("bare repo returns correct worktree path", func(t *testing.T) {
+		// Create a bare repository
+		tmpDir := t.TempDir()
+		bareRepoPath := filepath.Join(tmpDir, "bare.git")
+		runGitCmd(t, tmpDir, "init", "--bare", bareRepoPath)
+
+		// Test getBeadsWorktreePath from bare repo
+		path := getBeadsWorktreePath(ctx, bareRepoPath, "beads-sync")
+
+		// For bare repos, git-common-dir returns the bare repo itself
+		// So the path should be <bare-repo>/beads-worktrees/beads-sync
+		expectedPath := filepath.Join(bareRepoPath, "beads-worktrees", "beads-sync")
+		if path != expectedPath {
+			t.Errorf("Expected path %q, got %q", expectedPath, path)
+		}
+
+		// Path should be absolute
+		if !filepath.IsAbs(path) {
+			t.Errorf("Expected absolute path, got %q", path)
+		}
+
+		// Verify it's NOT trying to create .git/beads-worktrees inside the bare repo
+		// (which would fail since bare repos don't have a .git subdirectory)
+		badPath := filepath.Join(bareRepoPath, ".git", "beads-worktrees", "beads-sync")
+		if path == badPath {
+			t.Errorf("Bare repo should not use .git subdirectory path: %q", path)
+		}
+	})
+
+	t.Run("worktree of regular repo uses common git dir", func(t *testing.T) {
+		// Create a regular repository
+		tmpDir := t.TempDir()
+		mainRepoPath := filepath.Join(tmpDir, "main-repo")
+		if err := os.MkdirAll(mainRepoPath, 0750); err != nil {
+			t.Fatalf("Failed to create main repo dir: %v", err)
+		}
+
+		runGitCmd(t, mainRepoPath, "init")
+		runGitCmd(t, mainRepoPath, "config", "user.email", "test@test.com")
+		runGitCmd(t, mainRepoPath, "config", "user.name", "Test User")
+
+		// Create initial commit
+		testFile := filepath.Join(mainRepoPath, "test.txt")
+		if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
+			t.Fatalf("Failed to write test file: %v", err)
+		}
+		runGitCmd(t, mainRepoPath, "add", ".")
+		runGitCmd(t, mainRepoPath, "commit", "-m", "initial")
+
+		// Create a worktree
+		worktreePath := filepath.Join(tmpDir, "feature-worktree")
+		runGitCmd(t, mainRepoPath, "worktree", "add", worktreePath, "-b", "feature")
+
+		// Test getBeadsWorktreePath from the worktree
+		path := getBeadsWorktreePath(ctx, worktreePath, "beads-sync")
+
+		// Should point to the main repo's .git/beads-worktrees, not the worktree's
+		mainGitDir := filepath.Join(mainRepoPath, ".git")
+		expectedPath := filepath.Join(mainGitDir, "beads-worktrees", "beads-sync")
+		if path != expectedPath {
+			t.Errorf("Expected path %q, got %q", expectedPath, path)
+		}
+	})
+
+	t.Run("fallback works when git command fails", func(t *testing.T) {
+		// Test with a non-git directory (should fallback to legacy behavior)
+		tmpDir := t.TempDir()
+
+		path := getBeadsWorktreePath(ctx, tmpDir, "beads-sync")
+
+		// Should fallback to legacy .git/beads-worktrees path
+		expectedPath := filepath.Join(tmpDir, ".git", "beads-worktrees", "beads-sync")
+		if path != expectedPath {
+			t.Errorf("Expected fallback path %q, got %q", expectedPath, path)
+		}
+	})
+}
+
+// TestGetBeadsWorktreePathRelativePath tests that relative paths from git are handled correctly
+func TestGetBeadsWorktreePathRelativePath(t *testing.T) {
+	if testing.Short() {
+		t.Skip("Skipping integration test in short mode")
+	}
+
+	ctx := context.Background()
+
+	// Create a regular git repository
+	tmpDir := t.TempDir()
+	runGitCmd(t, tmpDir, "init")
+	runGitCmd(t, tmpDir, "config", "user.email", "test@test.com")
+	runGitCmd(t, tmpDir, "config", "user.name", "Test User")
+
+	// Create initial commit
+	testFile := filepath.Join(tmpDir, "test.txt")
+	if err := os.WriteFile(testFile, []byte("test"), 0644); err != nil {
+		t.Fatalf("Failed to write test file: %v", err)
+	}
+	runGitCmd(t, tmpDir, "add", ".")
+	runGitCmd(t, tmpDir, "commit", "-m", "initial")
+
+	// Test from a subdirectory
+	subDir := filepath.Join(tmpDir, "subdir")
+	if err := os.MkdirAll(subDir, 0750); err != nil {
+		t.Fatalf("Failed to create subdir: %v", err)
+	}
+
+	// getBeadsWorktreePath should still return an absolute path
+	path := getBeadsWorktreePath(ctx, subDir, "beads-sync")
+
+	if !filepath.IsAbs(path) {
+		t.Errorf("Expected absolute path, got %q", path)
+	}
+}
+
+// runGitCmd is a helper to run git commands
+func runGitCmd(t *testing.T, dir string, args ...string) {
+	t.Helper()
+	cmd := exec.Command("git", args...)
+	cmd.Dir = dir
+	output, err := cmd.CombinedOutput()
+	if err != nil {
+		t.Fatalf("git %v failed: %v\n%s", args, err, output)
+	}
+}