From 81aa3016495f19a1263e728f98afd9134387f6dd Mon Sep 17 00:00:00 2001 From: "Charles P. Cross" <8572939+cpdata@users.noreply.github.com> Date: Fri, 19 Dec 2025 20:52:40 -0500 Subject: [PATCH] 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 /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 --- internal/syncbranch/worktree.go | 38 ++++- internal/syncbranch/worktree_path_test.go | 174 ++++++++++++++++++++++ 2 files changed, 204 insertions(+), 8 deletions(-) create mode 100644 internal/syncbranch/worktree_path_test.go diff --git a/internal/syncbranch/worktree.go b/internal/syncbranch/worktree.go index 372d0d50..bb76ebd7 100644 --- 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/ - 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/ - 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/ - 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/ - 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/. +// 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)) diff --git a/internal/syncbranch/worktree_path_test.go b/internal/syncbranch/worktree_path_test.go new file mode 100644 index 00000000..edc824c3 --- /dev/null +++ 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 /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) + } +}