diff --git a/cmd/bd/doctor/fix/hooks.go b/cmd/bd/doctor/fix/hooks.go index 789ee0cf..bddf5487 100644 --- a/cmd/bd/doctor/fix/hooks.go +++ b/cmd/bd/doctor/fix/hooks.go @@ -79,6 +79,7 @@ type HookIntegrationStatus struct { HooksWithoutBd []string // Hooks configured but without bd integration HooksNotInConfig []string // Recommended hooks not in config at all Configured bool // Whether any bd integration was found + DetectionOnly bool // True if we detected the manager but can't verify its config } // bdHookPattern matches the recommended bd hooks run pattern with word boundaries @@ -330,10 +331,11 @@ func CheckExternalHookManagerIntegration(path string) *HookIntegrationStatus { } } - // Return basic status for unsupported managers + // Return basic status for unsupported managers (detection only, can't verify config) return &HookIntegrationStatus{ - Manager: ManagerNames(managers), - Configured: false, + Manager: ManagerNames(managers), + Configured: false, + DetectionOnly: true, } } diff --git a/cmd/bd/doctor/fix/hooks_test.go b/cmd/bd/doctor/fix/hooks_test.go index 9e92de70..9afac6b7 100644 --- a/cmd/bd/doctor/fix/hooks_test.go +++ b/cmd/bd/doctor/fix/hooks_test.go @@ -608,11 +608,12 @@ func slicesEqual(a, b []string) bool { func TestCheckExternalHookManagerIntegration(t *testing.T) { tests := []struct { - name string - setup func(dir string) error - expectNil bool - expectManager string - expectConfigured bool + name string + setup func(dir string) error + expectNil bool + expectManager string + expectConfigured bool + expectDetectionOnly bool }{ { name: "no managers", @@ -631,9 +632,10 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) { ` return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644) }, - expectNil: false, - expectManager: "lefthook", - expectConfigured: true, + expectNil: false, + expectManager: "lefthook", + expectConfigured: true, + expectDetectionOnly: false, }, { name: "husky with bd integration", @@ -644,18 +646,20 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) { } return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("#!/bin/sh\nbd hooks run pre-commit\n"), 0755) }, - expectNil: false, - expectManager: "husky", - expectConfigured: true, + expectNil: false, + expectManager: "husky", + expectConfigured: true, + expectDetectionOnly: false, }, { name: "unsupported manager (pre-commit framework)", setup: func(dir string) error { return os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644) }, - expectNil: false, - expectManager: "pre-commit", - expectConfigured: false, + expectNil: false, + expectManager: "pre-commit", + expectConfigured: false, + expectDetectionOnly: true, }, { name: "lefthook without bd", @@ -667,9 +671,10 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) { ` return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644) }, - expectNil: false, - expectManager: "lefthook", - expectConfigured: false, + expectNil: false, + expectManager: "lefthook", + expectConfigured: false, + expectDetectionOnly: false, // lefthook IS supported, we can verify its config }, } @@ -700,6 +705,10 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) { if result.Configured != tt.expectConfigured { t.Errorf("Configured: expected %v, got %v", tt.expectConfigured, result.Configured) } + + if result.DetectionOnly != tt.expectDetectionOnly { + t.Errorf("DetectionOnly: expected %v, got %v", tt.expectDetectionOnly, result.DetectionOnly) + } }) } } diff --git a/cmd/bd/doctor/git.go b/cmd/bd/doctor/git.go index b328b9b2..0f61a3e1 100644 --- a/cmd/bd/doctor/git.go +++ b/cmd/bd/doctor/git.go @@ -67,26 +67,38 @@ func CheckGitHooks() DoctorCheck { if len(externalManagers) > 0 { // External manager detected - check if it's configured to call bd integration := fix.CheckExternalHookManagerIntegration(repoRoot) - if integration != nil && integration.Configured { - // Check if any hooks are missing bd integration - if len(integration.HooksWithoutBd) > 0 { + if integration != nil { + // Detection-only managers - we can't verify their config + if integration.DetectionOnly { return DoctorCheck{ Name: "Git Hooks", - Status: StatusWarning, - Message: fmt.Sprintf("%s hooks not calling bd", integration.Manager), - Detail: fmt.Sprintf("Missing bd: %s", strings.Join(integration.HooksWithoutBd, ", ")), - Fix: "Add or upgrade to 'bd hooks run '. See " + hooksUpgradeURL, + Status: StatusOK, + Message: fmt.Sprintf("%s detected (cannot verify bd integration)", integration.Manager), + Detail: "Ensure your hook config calls 'bd hooks run '", } } - // All hooks calling bd - success - return DoctorCheck{ - Name: "Git Hooks", - Status: StatusOK, - Message: fmt.Sprintf("All hooks via %s", integration.Manager), - Detail: fmt.Sprintf("bd hooks run: %s", strings.Join(integration.HooksWithBd, ", ")), + if integration.Configured { + // Check if any hooks are missing bd integration + if len(integration.HooksWithoutBd) > 0 { + return DoctorCheck{ + Name: "Git Hooks", + Status: StatusWarning, + Message: fmt.Sprintf("%s hooks not calling bd", integration.Manager), + Detail: fmt.Sprintf("Missing bd: %s", strings.Join(integration.HooksWithoutBd, ", ")), + Fix: "Add or upgrade to 'bd hooks run '. See " + hooksUpgradeURL, + } + } + + // All hooks calling bd - success + return DoctorCheck{ + Name: "Git Hooks", + Status: StatusOK, + Message: fmt.Sprintf("All hooks via %s", integration.Manager), + Detail: fmt.Sprintf("bd hooks run: %s", strings.Join(integration.HooksWithBd, ", ")), + } } - } else { + // External manager exists but doesn't call bd at all return DoctorCheck{ Name: "Git Hooks", @@ -350,37 +362,49 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck { // Check if external manager has bd integration integration := fix.CheckExternalHookManagerIntegration(path) - if integration != nil && integration.Configured { - // Has bd integration - check if pre-push is covered - hasPrepush := false - for _, h := range integration.HooksWithBd { - if h == "pre-push" { - hasPrepush = true - break - } - } - - if hasPrepush { - var detail string - // Only report hooks that ARE in config but lack bd integration - if len(integration.HooksWithoutBd) > 0 { - detail = fmt.Sprintf("Hooks without bd: %s", strings.Join(integration.HooksWithoutBd, ", ")) - } + if integration != nil { + // Detection-only managers - we can't verify their config + if integration.DetectionOnly { return DoctorCheck{ Name: "Sync Branch Hook Compatibility", Status: StatusOK, - Message: fmt.Sprintf("Managed by %s with bd integration", integration.Manager), - Detail: detail, + Message: fmt.Sprintf("Managed by %s (cannot verify bd integration)", names), + Detail: "Ensure pre-push hook calls 'bd hooks run pre-push' for sync-branch", } } - // Has bd integration but missing pre-push - return DoctorCheck{ - Name: "Sync Branch Hook Compatibility", - Status: StatusWarning, - Message: fmt.Sprintf("Managed by %s (missing pre-push bd integration)", integration.Manager), - Detail: "pre-push hook needs 'bd hooks run pre-push' for sync-branch", - Fix: fmt.Sprintf("Add or upgrade to 'bd hooks run pre-push' in %s. See %s", integration.Manager, hooksExamplesURL), + if integration.Configured { + // Has bd integration - check if pre-push is covered + hasPrepush := false + for _, h := range integration.HooksWithBd { + if h == "pre-push" { + hasPrepush = true + break + } + } + + if hasPrepush { + var detail string + // Only report hooks that ARE in config but lack bd integration + if len(integration.HooksWithoutBd) > 0 { + detail = fmt.Sprintf("Hooks without bd: %s", strings.Join(integration.HooksWithoutBd, ", ")) + } + return DoctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: StatusOK, + Message: fmt.Sprintf("Managed by %s with bd integration", integration.Manager), + Detail: detail, + } + } + + // Has bd integration but missing pre-push + return DoctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: StatusWarning, + Message: fmt.Sprintf("Managed by %s (missing pre-push bd integration)", integration.Manager), + Detail: "pre-push hook needs 'bd hooks run pre-push' for sync-branch", + Fix: fmt.Sprintf("Add or upgrade to 'bd hooks run pre-push' in %s. See %s", integration.Manager, hooksExamplesURL), + } } }