diff --git a/cmd/bd/doctor.go b/cmd/bd/doctor.go index d7b3f020..8b4a756f 100644 --- a/cmd/bd/doctor.go +++ b/cmd/bd/doctor.go @@ -63,6 +63,9 @@ var ( // ConfigKeyHintsDoctor is the config key for suppressing doctor hints const ConfigKeyHintsDoctor = "hints.doctor" +// minSyncBranchHookVersion is the minimum hook version that supports sync-branch bypass (issue #532) +const minSyncBranchHookVersion = "0.29.0" + var doctorCmd = &cobra.Command{ Use: "doctor [path]", Short: "Check beads installation health", @@ -473,6 +476,11 @@ func runCheckHealth(path string) { issues = append(issues, issue) } + // Check 3: Sync-branch hook compatibility (issue #532) + if issue := checkSyncBranchHookQuick(path); issue != "" { + issues = append(issues, issue) + } + // If any issues found, print hint if len(issues) > 0 { printCheckHealthHint(issues) @@ -600,6 +608,75 @@ func checkHooksQuick() string { return fmt.Sprintf("Git hooks outdated: %s (%s → %s)", strings.Join(outdatedHooks, ", "), oldestVersion, Version) } +// checkSyncBranchHookQuick does a fast check for sync-branch hook compatibility (issue #532). +// Returns empty string if OK, otherwise returns issue description. +func checkSyncBranchHookQuick(path string) string { + // Check if sync-branch is configured + syncBranch := syncbranch.GetFromYAML() + if syncBranch == "" { + return "" // sync-branch not configured, nothing to check + } + + // Get git directory + cmd := exec.Command("git", "rev-parse", "--git-dir") + cmd.Dir = path + output, err := cmd.Output() + if err != nil { + return "" // Not a git repo, skip + } + gitDir := strings.TrimSpace(string(output)) + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(path, gitDir) + } + + // Find pre-push hook (check shared hooks first) + var hookPath string + hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath") + hooksPathCmd.Dir = path + if hooksPathOutput, err := hooksPathCmd.Output(); err == nil { + sharedHooksDir := strings.TrimSpace(string(hooksPathOutput)) + if !filepath.IsAbs(sharedHooksDir) { + sharedHooksDir = filepath.Join(path, sharedHooksDir) + } + hookPath = filepath.Join(sharedHooksDir, "pre-push") + } else { + hookPath = filepath.Join(gitDir, "hooks", "pre-push") + } + + content, err := os.ReadFile(hookPath) // #nosec G304 - path is controlled + if err != nil { + return "" // No pre-push hook, covered by other checks + } + + // Check if bd hook and extract version + hookStr := string(content) + if !strings.Contains(hookStr, "bd-hooks-version:") { + return "" // Not a bd hook, can't check + } + + var hookVersion string + for _, line := range strings.Split(hookStr, "\n") { + if strings.Contains(line, "bd-hooks-version:") { + parts := strings.SplitN(line, ":", 2) + if len(parts) == 2 { + hookVersion = strings.TrimSpace(parts[1]) + } + break + } + } + + if hookVersion == "" { + return "" // Can't determine version + } + + // Check if version < minSyncBranchHookVersion (when sync-branch bypass was added) + if compareVersions(hookVersion, minSyncBranchHookVersion) < 0 { + return fmt.Sprintf("Pre-push hook (%s) incompatible with sync-branch mode (requires %s+)", hookVersion, minSyncBranchHookVersion) + } + + return "" +} + func runDiagnostics(path string) doctorResult { result := doctorResult{ Path: path, @@ -619,6 +696,13 @@ func runDiagnostics(path string) doctorResult { result.Checks = append(result.Checks, hooksCheck) // Don't fail overall check for missing hooks, just warn + // Check sync-branch hook compatibility (issue #532) + syncBranchHookCheck := checkSyncBranchHookCompatibility(path) + result.Checks = append(result.Checks, syncBranchHookCheck) + if syncBranchHookCheck.Status == statusError { + result.OverallOK = false + } + // If no .beads/, skip remaining checks if installCheck.Status != statusOK { return result @@ -1934,6 +2018,117 @@ func checkGitHooks() doctorCheck { } } +// checkSyncBranchHookCompatibility checks if pre-push hook is compatible with sync-branch mode. +// When sync-branch is configured, the pre-push hook must have the sync-branch bypass logic +// (added in version 0.29.0). Without it, users experience circular "bd sync" failures (issue #532). +func checkSyncBranchHookCompatibility(path string) doctorCheck { + // Check if sync-branch is configured + syncBranch := syncbranch.GetFromYAML() + if syncBranch == "" { + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusOK, + Message: "N/A (sync-branch not configured)", + } + } + + // sync-branch is configured - check pre-push hook version + // Get actual git directory (handles worktrees where .git is a file) + cmd := exec.Command("git", "rev-parse", "--git-dir") + cmd.Dir = path + output, err := cmd.Output() + if err != nil { + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusOK, + Message: "N/A (not a git repository)", + } + } + gitDir := strings.TrimSpace(string(output)) + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(path, gitDir) + } + + // Check for pre-push hook in standard location or shared hooks location + var hookPath string + + // First check if core.hooksPath is configured (shared hooks) + hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath") + hooksPathCmd.Dir = path + if hooksPathOutput, err := hooksPathCmd.Output(); err == nil { + sharedHooksDir := strings.TrimSpace(string(hooksPathOutput)) + if !filepath.IsAbs(sharedHooksDir) { + sharedHooksDir = filepath.Join(path, sharedHooksDir) + } + hookPath = filepath.Join(sharedHooksDir, "pre-push") + } else { + // Use standard .git/hooks location + hookPath = filepath.Join(gitDir, "hooks", "pre-push") + } + + hookContent, err := os.ReadFile(hookPath) // #nosec G304 - path is controlled + if err != nil { + // No pre-push hook installed - different issue, covered by checkGitHooks + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusOK, + Message: "N/A (no pre-push hook installed)", + } + } + + // Check if this is a bd hook and extract version + hookStr := string(hookContent) + if !strings.Contains(hookStr, "bd-hooks-version:") { + // Not a bd hook - can't determine compatibility + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusWarning, + Message: "Pre-push hook is not a bd hook", + Detail: "Cannot verify sync-branch compatibility with custom hooks", + } + } + + // Extract version from hook + var hookVersion string + for _, line := range strings.Split(hookStr, "\n") { + if strings.Contains(line, "bd-hooks-version:") { + parts := strings.SplitN(line, ":", 2) + if len(parts) == 2 { + hookVersion = strings.TrimSpace(parts[1]) + } + break + } + } + + if hookVersion == "" { + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusWarning, + Message: "Could not determine pre-push hook version", + Detail: "Cannot verify sync-branch compatibility", + Fix: "Run 'bd hooks install --force' to update hooks", + } + } + + // minSyncBranchHookVersion added sync-branch bypass logic + // If hook version < minSyncBranchHookVersion, it will cause circular "bd sync" failures + if compareVersions(hookVersion, minSyncBranchHookVersion) < 0 { + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusError, + Message: fmt.Sprintf("Pre-push hook incompatible with sync-branch mode (version %s)", hookVersion), + Detail: fmt.Sprintf("Hook version %s lacks sync-branch bypass (requires %s+). This causes circular 'bd sync' failures during push.", hookVersion, minSyncBranchHookVersion), + Fix: "Run 'bd hooks install --force' to update hooks", + } + } + + return doctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: statusOK, + Message: fmt.Sprintf("Pre-push hook compatible with sync-branch (version %s)", hookVersion), + } +} + func checkSchemaCompatibility(path string) doctorCheck { beadsDir := filepath.Join(path, ".beads") diff --git a/cmd/bd/doctor_test.go b/cmd/bd/doctor_test.go index f6f72c07..a207e755 100644 --- a/cmd/bd/doctor_test.go +++ b/cmd/bd/doctor_test.go @@ -1092,3 +1092,176 @@ func TestExportDiagnosticsInvalidPath(t *testing.T) { t.Error("Expected error for invalid path, got nil") } } + +// TestCheckSyncBranchHookCompatibility tests the sync-branch hook compatibility check (issue #532) +// Note: We use BEADS_SYNC_BRANCH env var to control sync-branch detection because the config +// package reads from the actual beads repo's config.yaml. Only test cases with syncBranchEnv +// set to a non-empty value are reliable. +func TestCheckSyncBranchHookCompatibility(t *testing.T) { + tests := []struct { + name string + syncBranchEnv string // BEADS_SYNC_BRANCH env var (must be non-empty to override config.yaml) + hasGitDir bool + hookVersion string // Empty means no hook, "custom" means non-bd hook + expectedStatus string + }{ + { + name: "sync-branch configured, no git repo", + syncBranchEnv: "beads-sync", + hasGitDir: false, + hookVersion: "", + expectedStatus: statusOK, // N/A case + }, + { + name: "sync-branch configured, no pre-push hook", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "", + expectedStatus: statusOK, // Covered by other check + }, + { + name: "sync-branch configured, custom hook", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "custom", + expectedStatus: statusWarning, + }, + { + name: "sync-branch configured, old hook (0.24.2)", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.24.2", + expectedStatus: statusError, + }, + { + name: "sync-branch configured, old hook (0.28.0)", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.28.0", + expectedStatus: statusError, + }, + { + name: "sync-branch configured, compatible hook (0.29.0)", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.29.0", + expectedStatus: statusOK, + }, + { + name: "sync-branch configured, newer hook (0.30.0)", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.30.0", + expectedStatus: statusOK, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + + // Always set environment variable to control sync-branch detection + // This overrides any config.yaml value in the actual beads repo + t.Setenv("BEADS_SYNC_BRANCH", tc.syncBranchEnv) + + if tc.hasGitDir { + // Initialize a real git repo (git rev-parse needs this) + cmd := exec.Command("git", "init") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatal(err) + } + + // Create pre-push hook if specified + if tc.hookVersion != "" { + hooksDir := filepath.Join(tmpDir, ".git", "hooks") + hookPath := filepath.Join(hooksDir, "pre-push") + var hookContent string + if tc.hookVersion == "custom" { + hookContent = "#!/bin/sh\n# Custom hook\nexit 0\n" + } else { + hookContent = fmt.Sprintf("#!/bin/sh\n# bd-hooks-version: %s\nexit 0\n", tc.hookVersion) + } + if err := os.WriteFile(hookPath, []byte(hookContent), 0755); err != nil { + t.Fatal(err) + } + } + } + + check := checkSyncBranchHookCompatibility(tmpDir) + + if check.Status != tc.expectedStatus { + t.Errorf("Expected status %s, got %s (message: %s)", tc.expectedStatus, check.Status, check.Message) + } + + // Error case should have a fix message + if tc.expectedStatus == statusError && check.Fix == "" { + t.Error("Expected fix message for error status") + } + }) + } +} + +// TestCheckSyncBranchHookQuick tests the quick sync-branch hook check (issue #532) +// Note: We use BEADS_SYNC_BRANCH env var to control sync-branch detection. +func TestCheckSyncBranchHookQuick(t *testing.T) { + tests := []struct { + name string + syncBranchEnv string + hasGitDir bool + hookVersion string + expectIssue bool + }{ + { + name: "old hook with sync-branch", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.24.0", + expectIssue: true, + }, + { + name: "compatible hook with sync-branch", + syncBranchEnv: "beads-sync", + hasGitDir: true, + hookVersion: "0.29.0", + expectIssue: false, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + tmpDir := t.TempDir() + + // Always set environment variable to control sync-branch detection + // This overrides any config.yaml value in the actual beads repo + t.Setenv("BEADS_SYNC_BRANCH", tc.syncBranchEnv) + + if tc.hasGitDir { + // Initialize a real git repo (git rev-parse needs this) + cmd := exec.Command("git", "init") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatal(err) + } + + if tc.hookVersion != "" { + hooksDir := filepath.Join(tmpDir, ".git", "hooks") + hookPath := filepath.Join(hooksDir, "pre-push") + hookContent := fmt.Sprintf("#!/bin/sh\n# bd-hooks-version: %s\nexit 0\n", tc.hookVersion) + if err := os.WriteFile(hookPath, []byte(hookContent), 0755); err != nil { + t.Fatal(err) + } + } + } + + issue := checkSyncBranchHookQuick(tmpDir) + + if tc.expectIssue && issue == "" { + t.Error("Expected issue to be reported, got empty string") + } + if !tc.expectIssue && issue != "" { + t.Errorf("Expected no issue, got: %s", issue) + } + }) + } +}