From 32c803dd164a684e0f3e7d26f3cb86fa4b6648ea Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Thu, 25 Dec 2025 22:04:40 -0800 Subject: [PATCH] fix(worktree): Cache git detection results to eliminate slowness (bd-7di) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In git worktrees, any bd command was slow with a 2-3s pause because: - git.IsWorktree() was called 4+ times per command - Each call spawned 2 git processes (git-dir and git-common-dir) - git.GetRepoRoot() and git.GetMainRepoRoot() also called multiple times Fix: Cache results using sync.Once since these values do not change during a single command execution: - IsWorktree() - caches worktree detection - GetRepoRoot() - caches repo root path - GetMainRepoRoot() - caches main repo root for worktrees Added ResetCaches() for test cleanup between subtests that change directories. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- internal/beads/beads_test.go | 8 ++++++++ internal/git/gitdir.go | 30 ++++++++++++++++++++++++++++++ internal/git/worktree_test.go | 12 ++++++++++++ 3 files changed, 50 insertions(+) diff --git a/internal/beads/beads_test.go b/internal/beads/beads_test.go index 140baab8..6ec8c3bb 100644 --- a/internal/beads/beads_test.go +++ b/internal/beads/beads_test.go @@ -5,6 +5,8 @@ import ( "os/exec" "path/filepath" "testing" + + "github.com/steveyegge/beads/internal/git" ) func TestFindDatabasePathEnvVar(t *testing.T) { @@ -821,6 +823,7 @@ func TestFindGitRoot_RegularRepo(t *testing.T) { } t.Chdir(subDir) + git.ResetCaches() // Reset after chdir for caching tests // findGitRoot should return the repo root result := findGitRoot() @@ -897,6 +900,7 @@ func TestFindGitRoot_Worktree(t *testing.T) { // Change to the worktree directory t.Chdir(worktreeDir) + git.ResetCaches() // Reset after chdir for caching tests // findGitRoot should return the WORKTREE root, not the main repo root result := findGitRoot() @@ -927,6 +931,7 @@ func TestFindGitRoot_NotGitRepo(t *testing.T) { defer os.RemoveAll(tmpDir) t.Chdir(tmpDir) + git.ResetCaches() // Reset after chdir for caching tests // findGitRoot should return empty string result := findGitRoot() @@ -1024,6 +1029,7 @@ func TestFindBeadsDir_Worktree(t *testing.T) { // Change to worktree t.Chdir(worktreeDir) + git.ResetCaches() // Reset after chdir for caching tests // FindBeadsDir should prioritize the main repo's .beads for worktrees (bd-de6) result := FindBeadsDir() @@ -1134,6 +1140,7 @@ func TestFindDatabasePath_Worktree(t *testing.T) { t.Fatal(err) } t.Chdir(worktreeSubDir) + git.ResetCaches() // Reset after chdir for caching tests // FindDatabasePath should find the main repo's shared database result := FindDatabasePath() @@ -1241,6 +1248,7 @@ func TestFindDatabasePath_WorktreeNoLocalDB(t *testing.T) { // Change to worktree t.Chdir(worktreeDir) + git.ResetCaches() // Reset after chdir for caching tests // FindDatabasePath should find the main repo's shared database result := FindDatabasePath() diff --git a/internal/git/gitdir.go b/internal/git/gitdir.go index fd965e21..7895dd7c 100644 --- a/internal/git/gitdir.go +++ b/internal/git/gitdir.go @@ -93,6 +93,13 @@ func isWorktreeUncached() bool { return absGit != absCommon } +// mainRepoRootOnce ensures we only get main repo root once per process. +var ( + mainRepoRootOnce sync.Once + mainRepoRootResult string + mainRepoRootErr error +) + // GetMainRepoRoot returns the main repository root directory. // When in a worktree, this returns the main repository root. // Otherwise, it returns the regular repository root. @@ -101,7 +108,16 @@ func isWorktreeUncached() bool { // /project/.worktrees/feature/), this correctly returns the main repo // root (/project/) by using git rev-parse --git-common-dir which always // points to the main repo's .git directory. (GH#509) +// The result is cached after the first call. func GetMainRepoRoot() (string, error) { + mainRepoRootOnce.Do(func() { + mainRepoRootResult, mainRepoRootErr = getMainRepoRootUncached() + }) + return mainRepoRootResult, mainRepoRootErr +} + +// getMainRepoRootUncached performs the actual main repo root lookup without caching. +func getMainRepoRootUncached() (string, error) { // Use --git-common-dir which always returns the main repo's .git directory, // even when running from within a worktree or its subdirectories. // This is the most reliable method for finding the main repo root. @@ -190,4 +206,18 @@ func getGitDirNoError(flag string) string { return "" } return strings.TrimSpace(string(out)) +} + +// ResetCaches resets all cached git information. This is intended for use +// by tests that need to change directory between subtests. +// In production, these caches are safe because the working directory +// doesn't change during a single command execution. +func ResetCaches() { + isWorktreeOnce = sync.Once{} + isWorktreeResult = false + mainRepoRootOnce = sync.Once{} + mainRepoRootResult = "" + mainRepoRootErr = nil + repoRootOnce = sync.Once{} + repoRootResult = "" } \ No newline at end of file diff --git a/internal/git/worktree_test.go b/internal/git/worktree_test.go index 35076d69..544c628f 100644 --- a/internal/git/worktree_test.go +++ b/internal/git/worktree_test.go @@ -864,6 +864,7 @@ func TestCountJSONLIssues(t *testing.T) { // TestGetMainRepoRoot tests the GetMainRepoRoot function for various scenarios func TestGetMainRepoRoot(t *testing.T) { t.Run("returns correct root for regular repo", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -877,6 +878,7 @@ func TestGetMainRepoRoot(t *testing.T) { if err := os.Chdir(repoPath); err != nil { t.Fatalf("Failed to chdir to repo: %v", err) } + ResetCaches() // Reset after chdir root, err := GetMainRepoRoot() if err != nil { @@ -893,6 +895,7 @@ func TestGetMainRepoRoot(t *testing.T) { }) t.Run("returns main repo root from worktree", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -913,6 +916,7 @@ func TestGetMainRepoRoot(t *testing.T) { if err := os.Chdir(worktreePath); err != nil { t.Fatalf("Failed to chdir to worktree: %v", err) } + ResetCaches() // Reset after chdir root, err := GetMainRepoRoot() if err != nil { @@ -929,6 +933,7 @@ func TestGetMainRepoRoot(t *testing.T) { }) t.Run("returns main repo root from nested worktree (GH#509)", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -950,6 +955,7 @@ func TestGetMainRepoRoot(t *testing.T) { if err := os.Chdir(nestedWorktreePath); err != nil { t.Fatalf("Failed to chdir to nested worktree: %v", err) } + ResetCaches() // Reset after chdir root, err := GetMainRepoRoot() if err != nil { @@ -966,6 +972,7 @@ func TestGetMainRepoRoot(t *testing.T) { }) t.Run("returns main repo root from subdirectory of nested worktree", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -993,6 +1000,7 @@ func TestGetMainRepoRoot(t *testing.T) { if err := os.Chdir(subDir); err != nil { t.Fatalf("Failed to chdir to subdir: %v", err) } + ResetCaches() // Reset after chdir root, err := GetMainRepoRoot() if err != nil { @@ -1012,6 +1020,7 @@ func TestGetMainRepoRoot(t *testing.T) { // TestIsWorktree tests the IsWorktree function func TestIsWorktree(t *testing.T) { t.Run("returns false for regular repo", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -1024,6 +1033,7 @@ func TestIsWorktree(t *testing.T) { if err := os.Chdir(repoPath); err != nil { t.Fatalf("Failed to chdir to repo: %v", err) } + ResetCaches() // Reset after chdir if IsWorktree() { t.Error("IsWorktree() should return false for regular repo") @@ -1031,6 +1041,7 @@ func TestIsWorktree(t *testing.T) { }) t.Run("returns true for worktree", func(t *testing.T) { + ResetCaches() // Reset caches from previous subtests repoPath, cleanup := setupTestRepo(t) defer cleanup() @@ -1051,6 +1062,7 @@ func TestIsWorktree(t *testing.T) { t.Fatalf("Failed to chdir to worktree: %v", err) } + ResetCaches() // Reset after chdir to worktree if !IsWorktree() { t.Error("IsWorktree() should return true for worktree") }