fix(doctor): improve messaging for detection-only hook managers (bd-par1)
When bd doctor detects hook managers we can't fully check (pre-commit,
overcommit, yorkie, simple-git-hooks), it now shows an informational
message instead of a warning:
✓ pre-commit detected (cannot verify bd integration)
└─ Ensure your hook config calls 'bd hooks run <hook>'
Previously it incorrectly warned that these managers were "not calling bd"
when we simply couldn't verify their config.
Changes:
- Add DetectionOnly field to HookIntegrationStatus
- Set DetectionOnly=true for unsupported managers in CheckExternalHookManagerIntegration
- Update CheckGitHooks and CheckSyncBranchHookCompatibility to show
informational message for detection-only managers
- Add test coverage for DetectionOnly behavior
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
@@ -79,6 +79,7 @@ type HookIntegrationStatus struct {
|
|||||||
HooksWithoutBd []string // Hooks configured but without bd integration
|
HooksWithoutBd []string // Hooks configured but without bd integration
|
||||||
HooksNotInConfig []string // Recommended hooks not in config at all
|
HooksNotInConfig []string // Recommended hooks not in config at all
|
||||||
Configured bool // Whether any bd integration was found
|
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
|
// 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{
|
return &HookIntegrationStatus{
|
||||||
Manager: ManagerNames(managers),
|
Manager: ManagerNames(managers),
|
||||||
Configured: false,
|
Configured: false,
|
||||||
|
DetectionOnly: true,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -608,11 +608,12 @@ func slicesEqual(a, b []string) bool {
|
|||||||
|
|
||||||
func TestCheckExternalHookManagerIntegration(t *testing.T) {
|
func TestCheckExternalHookManagerIntegration(t *testing.T) {
|
||||||
tests := []struct {
|
tests := []struct {
|
||||||
name string
|
name string
|
||||||
setup func(dir string) error
|
setup func(dir string) error
|
||||||
expectNil bool
|
expectNil bool
|
||||||
expectManager string
|
expectManager string
|
||||||
expectConfigured bool
|
expectConfigured bool
|
||||||
|
expectDetectionOnly bool
|
||||||
}{
|
}{
|
||||||
{
|
{
|
||||||
name: "no managers",
|
name: "no managers",
|
||||||
@@ -631,9 +632,10 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) {
|
|||||||
`
|
`
|
||||||
return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644)
|
return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644)
|
||||||
},
|
},
|
||||||
expectNil: false,
|
expectNil: false,
|
||||||
expectManager: "lefthook",
|
expectManager: "lefthook",
|
||||||
expectConfigured: true,
|
expectConfigured: true,
|
||||||
|
expectDetectionOnly: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "husky with bd integration",
|
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)
|
return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("#!/bin/sh\nbd hooks run pre-commit\n"), 0755)
|
||||||
},
|
},
|
||||||
expectNil: false,
|
expectNil: false,
|
||||||
expectManager: "husky",
|
expectManager: "husky",
|
||||||
expectConfigured: true,
|
expectConfigured: true,
|
||||||
|
expectDetectionOnly: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "unsupported manager (pre-commit framework)",
|
name: "unsupported manager (pre-commit framework)",
|
||||||
setup: func(dir string) error {
|
setup: func(dir string) error {
|
||||||
return os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644)
|
return os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644)
|
||||||
},
|
},
|
||||||
expectNil: false,
|
expectNil: false,
|
||||||
expectManager: "pre-commit",
|
expectManager: "pre-commit",
|
||||||
expectConfigured: false,
|
expectConfigured: false,
|
||||||
|
expectDetectionOnly: true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "lefthook without bd",
|
name: "lefthook without bd",
|
||||||
@@ -667,9 +671,10 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) {
|
|||||||
`
|
`
|
||||||
return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644)
|
return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644)
|
||||||
},
|
},
|
||||||
expectNil: false,
|
expectNil: false,
|
||||||
expectManager: "lefthook",
|
expectManager: "lefthook",
|
||||||
expectConfigured: false,
|
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 {
|
if result.Configured != tt.expectConfigured {
|
||||||
t.Errorf("Configured: expected %v, got %v", tt.expectConfigured, result.Configured)
|
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)
|
||||||
|
}
|
||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -67,26 +67,38 @@ func CheckGitHooks() DoctorCheck {
|
|||||||
if len(externalManagers) > 0 {
|
if len(externalManagers) > 0 {
|
||||||
// External manager detected - check if it's configured to call bd
|
// External manager detected - check if it's configured to call bd
|
||||||
integration := fix.CheckExternalHookManagerIntegration(repoRoot)
|
integration := fix.CheckExternalHookManagerIntegration(repoRoot)
|
||||||
if integration != nil && integration.Configured {
|
if integration != nil {
|
||||||
// Check if any hooks are missing bd integration
|
// Detection-only managers - we can't verify their config
|
||||||
if len(integration.HooksWithoutBd) > 0 {
|
if integration.DetectionOnly {
|
||||||
return DoctorCheck{
|
return DoctorCheck{
|
||||||
Name: "Git Hooks",
|
Name: "Git Hooks",
|
||||||
Status: StatusWarning,
|
Status: StatusOK,
|
||||||
Message: fmt.Sprintf("%s hooks not calling bd", integration.Manager),
|
Message: fmt.Sprintf("%s detected (cannot verify bd integration)", integration.Manager),
|
||||||
Detail: fmt.Sprintf("Missing bd: %s", strings.Join(integration.HooksWithoutBd, ", ")),
|
Detail: "Ensure your hook config calls 'bd hooks run <hook>'",
|
||||||
Fix: "Add or upgrade to 'bd hooks run <hook>'. See " + hooksUpgradeURL,
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// All hooks calling bd - success
|
if integration.Configured {
|
||||||
return DoctorCheck{
|
// Check if any hooks are missing bd integration
|
||||||
Name: "Git Hooks",
|
if len(integration.HooksWithoutBd) > 0 {
|
||||||
Status: StatusOK,
|
return DoctorCheck{
|
||||||
Message: fmt.Sprintf("All hooks via %s", integration.Manager),
|
Name: "Git Hooks",
|
||||||
Detail: fmt.Sprintf("bd hooks run: %s", strings.Join(integration.HooksWithBd, ", ")),
|
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 <hook>'. 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
|
// External manager exists but doesn't call bd at all
|
||||||
return DoctorCheck{
|
return DoctorCheck{
|
||||||
Name: "Git Hooks",
|
Name: "Git Hooks",
|
||||||
@@ -350,37 +362,49 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck {
|
|||||||
|
|
||||||
// Check if external manager has bd integration
|
// Check if external manager has bd integration
|
||||||
integration := fix.CheckExternalHookManagerIntegration(path)
|
integration := fix.CheckExternalHookManagerIntegration(path)
|
||||||
if integration != nil && integration.Configured {
|
if integration != nil {
|
||||||
// Has bd integration - check if pre-push is covered
|
// Detection-only managers - we can't verify their config
|
||||||
hasPrepush := false
|
if integration.DetectionOnly {
|
||||||
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{
|
return DoctorCheck{
|
||||||
Name: "Sync Branch Hook Compatibility",
|
Name: "Sync Branch Hook Compatibility",
|
||||||
Status: StatusOK,
|
Status: StatusOK,
|
||||||
Message: fmt.Sprintf("Managed by %s with bd integration", integration.Manager),
|
Message: fmt.Sprintf("Managed by %s (cannot verify bd integration)", names),
|
||||||
Detail: detail,
|
Detail: "Ensure pre-push hook calls 'bd hooks run pre-push' for sync-branch",
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Has bd integration but missing pre-push
|
if integration.Configured {
|
||||||
return DoctorCheck{
|
// Has bd integration - check if pre-push is covered
|
||||||
Name: "Sync Branch Hook Compatibility",
|
hasPrepush := false
|
||||||
Status: StatusWarning,
|
for _, h := range integration.HooksWithBd {
|
||||||
Message: fmt.Sprintf("Managed by %s (missing pre-push bd integration)", integration.Manager),
|
if h == "pre-push" {
|
||||||
Detail: "pre-push hook needs 'bd hooks run pre-push' for sync-branch",
|
hasPrepush = true
|
||||||
Fix: fmt.Sprintf("Add or upgrade to 'bd hooks run pre-push' in %s. See %s", integration.Manager, hooksExamplesURL),
|
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),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user