From 5560a4243e676a9d634296e6bc0666cf6c975703 Mon Sep 17 00:00:00 2001 From: Steve Yegge Date: Mon, 29 Dec 2025 13:22:54 -0800 Subject: [PATCH] refactor: Export FollowRedirect and consolidate duplicate implementations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Rename followRedirect to FollowRedirect in internal/beads (export it) - Update doctor/maintenance.go to use beads.FollowRedirect - Update doctor/fix/common.go to use beads.FollowRedirect - Remove 66 lines of duplicated code across 3 implementations This ensures consistent redirect handling with path canonicalization, chain prevention, and proper error warnings. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/doctor/fix/common.go | 37 ++---------------------------------- cmd/bd/doctor/maintenance.go | 37 ++---------------------------------- internal/beads/beads.go | 20 +++++++++---------- internal/beads/beads_test.go | 6 +++--- 4 files changed, 17 insertions(+), 83 deletions(-) diff --git a/cmd/bd/doctor/fix/common.go b/cmd/bd/doctor/fix/common.go index 09517c25..6c60014c 100644 --- a/cmd/bd/doctor/fix/common.go +++ b/cmd/bd/doctor/fix/common.go @@ -113,40 +113,7 @@ func isWithinWorkspace(root, candidate string) bool { // resolveBeadsDir follows .beads/redirect files to find the actual beads directory. // If no redirect exists, returns the original path unchanged. +// This is a wrapper around beads.FollowRedirect for use within the fix package. func resolveBeadsDir(beadsDir string) string { - redirectFile := filepath.Join(beadsDir, beads.RedirectFileName) - data, err := os.ReadFile(redirectFile) //nolint:gosec // redirect file path is constructed from known beadsDir - if err != nil { - // No redirect file - use original path - return beadsDir - } - - // Parse the redirect target - target := strings.TrimSpace(string(data)) - if target == "" { - return beadsDir - } - - // Skip comments - lines := strings.Split(target, "\n") - for _, line := range lines { - line = strings.TrimSpace(line) - if line != "" && !strings.HasPrefix(line, "#") { - target = line - break - } - } - - // Resolve relative paths from the parent of the .beads directory - if !filepath.IsAbs(target) { - projectRoot := filepath.Dir(beadsDir) - target = filepath.Join(projectRoot, target) - } - - // Verify the target exists - if info, err := os.Stat(target); err != nil || !info.IsDir() { - return beadsDir - } - - return target + return beads.FollowRedirect(beadsDir) } diff --git a/cmd/bd/doctor/maintenance.go b/cmd/bd/doctor/maintenance.go index f3fa9ae1..b48a45fa 100644 --- a/cmd/bd/doctor/maintenance.go +++ b/cmd/bd/doctor/maintenance.go @@ -313,42 +313,9 @@ func CheckCompactionCandidates(path string) DoctorCheck { // resolveBeadsDir follows a redirect file if present in the beads directory. // This handles Gas Town's redirect mechanism where .beads/redirect points to // the actual beads directory location. +// This is a wrapper around beads.FollowRedirect for use within the doctor package. func resolveBeadsDir(beadsDir string) string { - redirectFile := filepath.Join(beadsDir, "redirect") - data, err := os.ReadFile(redirectFile) //nolint:gosec // redirect file path is constructed from known beadsDir - if err != nil { - // No redirect file - use original path - return beadsDir - } - - // Parse the redirect target - target := strings.TrimSpace(string(data)) - if target == "" { - return beadsDir - } - - // Skip comments - lines := strings.Split(target, "\n") - for _, line := range lines { - line = strings.TrimSpace(line) - if line != "" && !strings.HasPrefix(line, "#") { - target = line - break - } - } - - // Resolve relative paths from the parent of the .beads directory - if !filepath.IsAbs(target) { - projectRoot := filepath.Dir(beadsDir) - target = filepath.Join(projectRoot, target) - } - - // Verify the target exists - if info, err := os.Stat(target); err != nil || !info.IsDir() { - return beadsDir - } - - return target + return beads.FollowRedirect(beadsDir) } // CheckPersistentMolIssues detects mol- prefixed issues that should have been ephemeral. diff --git a/internal/beads/beads.go b/internal/beads/beads.go index 4ae216ba..4ab092a0 100644 --- a/internal/beads/beads.go +++ b/internal/beads/beads.go @@ -31,7 +31,7 @@ const RedirectFileName = "redirect" // LegacyDatabaseNames are old names that should be migrated var LegacyDatabaseNames = []string{"bd.db", "issues.db", "bugs.db"} -// followRedirect checks if a .beads directory contains a redirect file and follows it. +// FollowRedirect checks if a .beads directory contains a redirect file and follows it. // If a redirect file exists, it returns the target .beads directory path. // If no redirect exists or there's an error, it returns the original path unchanged. // @@ -41,7 +41,7 @@ var LegacyDatabaseNames = []string{"bd.db", "issues.db", "bugs.db"} // // Redirect chains are not followed - only one level of redirection is supported. // This prevents infinite loops and keeps the behavior predictable. -func followRedirect(beadsDir string) string { +func FollowRedirect(beadsDir string) string { redirectFile := filepath.Join(beadsDir, RedirectFileName) data, err := os.ReadFile(redirectFile) if err != nil { @@ -124,7 +124,7 @@ func GetRedirectInfo() RedirectInfo { } // There's a redirect - find the target - targetDir := followRedirect(localBeadsDir) + targetDir := FollowRedirect(localBeadsDir) if targetDir == localBeadsDir { // Redirect file exists but failed to resolve (invalid target) return info @@ -354,7 +354,7 @@ func FindDatabasePath() string { absBeadsDir := utils.CanonicalizePath(beadsDir) // Follow redirect if present - absBeadsDir = followRedirect(absBeadsDir) + absBeadsDir = FollowRedirect(absBeadsDir) // Use helper to find database (no warnings for BEADS_DIR - user explicitly set it) if dbPath := findDatabaseInBeadsDir(absBeadsDir, false); dbPath != "" { @@ -428,7 +428,7 @@ func FindBeadsDir() string { absBeadsDir := utils.CanonicalizePath(beadsDir) // Follow redirect if present - absBeadsDir = followRedirect(absBeadsDir) + absBeadsDir = FollowRedirect(absBeadsDir) if info, err := os.Stat(absBeadsDir); err == nil && info.IsDir() { // Validate directory contains actual project files @@ -447,7 +447,7 @@ func FindBeadsDir() string { beadsDir := filepath.Join(mainRepoRoot, ".beads") if info, err := os.Stat(beadsDir); err == nil && info.IsDir() { // Follow redirect if present - beadsDir = followRedirect(beadsDir) + beadsDir = FollowRedirect(beadsDir) // Validate directory contains actual project files if hasBeadsProjectFiles(beadsDir) { @@ -474,7 +474,7 @@ func FindBeadsDir() string { beadsDir := filepath.Join(dir, ".beads") if info, err := os.Stat(beadsDir); err == nil && info.IsDir() { // Follow redirect if present - beadsDir = followRedirect(beadsDir) + beadsDir = FollowRedirect(beadsDir) // Validate directory contains actual project files if hasBeadsProjectFiles(beadsDir) { @@ -551,7 +551,7 @@ func findDatabaseInTree() string { beadsDir := filepath.Join(mainRepoRoot, ".beads") if info, err := os.Stat(beadsDir); err == nil && info.IsDir() { // Follow redirect if present - beadsDir = followRedirect(beadsDir) + beadsDir = FollowRedirect(beadsDir) // Use helper to find database (with warnings for auto-discovery) if dbPath := findDatabaseInBeadsDir(beadsDir, true); dbPath != "" { @@ -574,7 +574,7 @@ func findDatabaseInTree() string { beadsDir := filepath.Join(dir, ".beads") if info, err := os.Stat(beadsDir); err == nil && info.IsDir() { // Follow redirect if present - beadsDir = followRedirect(beadsDir) + beadsDir = FollowRedirect(beadsDir) // Use helper to find database (with warnings for auto-discovery) if dbPath := findDatabaseInBeadsDir(beadsDir, true); dbPath != "" { @@ -624,7 +624,7 @@ func FindAllDatabases() []DatabaseInfo { beadsDir := filepath.Join(dir, ".beads") if info, err := os.Stat(beadsDir); err == nil && info.IsDir() { // Follow redirect if present - beadsDir = followRedirect(beadsDir) + beadsDir = FollowRedirect(beadsDir) // Found .beads/ directory, look for *.db files matches, err := filepath.Glob(filepath.Join(beadsDir, "*.db")) diff --git a/internal/beads/beads_test.go b/internal/beads/beads_test.go index 6ec8c3bb..91a5470e 100644 --- a/internal/beads/beads_test.go +++ b/internal/beads/beads_test.go @@ -651,7 +651,7 @@ func TestFollowRedirect(t *testing.T) { stubDir, targetDir := tt.setupFunc(t, tmpDir) - result := followRedirect(stubDir) + result := FollowRedirect(stubDir) // Resolve symlinks for comparison (macOS uses /private/var) resultResolved, _ := filepath.EvalSymlinks(result) @@ -660,11 +660,11 @@ func TestFollowRedirect(t *testing.T) { if tt.expectRedirect { targetResolved, _ := filepath.EvalSymlinks(targetDir) if resultResolved != targetResolved { - t.Errorf("followRedirect() = %q, want %q", result, targetDir) + t.Errorf("FollowRedirect() = %q, want %q", result, targetDir) } } else { if resultResolved != stubResolved { - t.Errorf("followRedirect() = %q, want original %q", result, stubDir) + t.Errorf("FollowRedirect() = %q, want original %q", result, stubDir) } } })