diff --git a/CHANGELOG.md b/CHANGELOG.md index ea27be51..c7e9d666 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to the beads project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Added + +- **External hook manager detection** - `bd doctor` now detects lefthook, husky, pre-commit, and other hook managers + - Checks if external managers have `bd hooks run` integration configured + - Reports which hooks have bd integration vs which are missing + - `bd doctor --fix` uses `--chain` flag when external managers detected to preserve existing hooks + - Supports YAML, TOML, and JSON config formats for lefthook + - Detects active manager from git hooks when multiple managers present + ## [0.44.0] - 2026-01-04 ### Added diff --git a/cmd/bd/doctor/fix/hooks.go b/cmd/bd/doctor/fix/hooks.go index d46131b1..789ee0cf 100644 --- a/cmd/bd/doctor/fix/hooks.go +++ b/cmd/bd/doctor/fix/hooks.go @@ -1,12 +1,354 @@ package fix import ( + "encoding/json" "fmt" "os" "os/exec" + "path/filepath" + "regexp" + "strings" + + "github.com/BurntSushi/toml" + "gopkg.in/yaml.v3" ) -// GitHooks fixes missing or broken git hooks by calling bd hooks install +// ExternalHookManager represents a detected external hook management tool. +type ExternalHookManager struct { + Name string // e.g., "lefthook", "husky", "pre-commit" + ConfigFile string // Path to the config file that was detected +} + +// hookManagerConfig pairs a manager name with its possible config files. +type hookManagerConfig struct { + name string + configFiles []string +} + +// hookManagerConfigs defines external hook managers in priority order. +// See https://lefthook.dev/configuration/ for lefthook config options. +var hookManagerConfigs = []hookManagerConfig{ + {"lefthook", []string{ + // YAML variants + "lefthook.yml", ".lefthook.yml", ".config/lefthook.yml", + "lefthook.yaml", ".lefthook.yaml", ".config/lefthook.yaml", + // TOML variants + "lefthook.toml", ".lefthook.toml", ".config/lefthook.toml", + // JSON variants + "lefthook.json", ".lefthook.json", ".config/lefthook.json", + }}, + {"husky", []string{".husky"}}, + {"pre-commit", []string{".pre-commit-config.yaml", ".pre-commit-config.yml"}}, + {"overcommit", []string{".overcommit.yml"}}, + {"yorkie", []string{".yorkie"}}, + {"simple-git-hooks", []string{ + ".simple-git-hooks.cjs", ".simple-git-hooks.js", + "simple-git-hooks.cjs", "simple-git-hooks.js", + }}, +} + +// DetectExternalHookManagers checks for presence of external hook management tools. +// Returns a list of detected managers along with their config file paths. +func DetectExternalHookManagers(path string) []ExternalHookManager { + var managers []ExternalHookManager + + for _, mgr := range hookManagerConfigs { + for _, configFile := range mgr.configFiles { + configPath := filepath.Join(path, configFile) + if info, err := os.Stat(configPath); err == nil { + // For directories like .husky, check if it exists + // For files, check if it's a regular file + if info.IsDir() || info.Mode().IsRegular() { + managers = append(managers, ExternalHookManager{ + Name: mgr.name, + ConfigFile: configFile, + }) + break // Only report each manager once + } + } + } + } + + return managers +} + +// HookIntegrationStatus represents the status of bd integration in an external hook manager. +type HookIntegrationStatus struct { + Manager string // Hook manager name + HooksWithBd []string // Hooks that have bd integration (bd hooks run) + 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 +} + +// bdHookPattern matches the recommended bd hooks run pattern with word boundaries +var bdHookPattern = regexp.MustCompile(`\bbd\s+hooks\s+run\b`) + +// hookManagerPattern pairs a manager name with its detection pattern. +type hookManagerPattern struct { + name string + pattern *regexp.Regexp +} + +// hookManagerPatterns identifies which hook manager installed a git hook (in priority order). +var hookManagerPatterns = []hookManagerPattern{ + {"lefthook", regexp.MustCompile(`(?i)lefthook`)}, + {"husky", regexp.MustCompile(`(?i)(\.husky|husky\.sh)`)}, + {"pre-commit", regexp.MustCompile(`(?i)(pre-commit\s+run|\.pre-commit-config|INSTALL_PYTHON|PRE_COMMIT)`)}, + {"simple-git-hooks", regexp.MustCompile(`(?i)simple-git-hooks`)}, +} + +// DetectActiveHookManager reads the git hooks to determine which manager installed them. +// This is more reliable than just checking for config files when multiple managers exist. +func DetectActiveHookManager(path string) string { + // Get git dir + cmd := exec.Command("git", "rev-parse", "--git-dir") + cmd.Dir = path + output, err := cmd.Output() + if err != nil { + return "" + } + gitDir := strings.TrimSpace(string(output)) + if !filepath.IsAbs(gitDir) { + gitDir = filepath.Join(path, gitDir) + } + + // Check for custom hooks path (core.hooksPath) + hooksDir := filepath.Join(gitDir, "hooks") + hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath") + hooksPathCmd.Dir = path + if hooksPathOutput, err := hooksPathCmd.Output(); err == nil { + customPath := strings.TrimSpace(string(hooksPathOutput)) + if customPath != "" { + if !filepath.IsAbs(customPath) { + customPath = filepath.Join(path, customPath) + } + hooksDir = customPath + } + } + + // Check common hooks for manager signatures + for _, hookName := range []string{"pre-commit", "pre-push", "post-merge"} { + hookPath := filepath.Join(hooksDir, hookName) + content, err := os.ReadFile(hookPath) // #nosec G304 - path is validated + if err != nil { + continue + } + contentStr := string(content) + + // Check each manager pattern (deterministic order) + for _, mp := range hookManagerPatterns { + if mp.pattern.MatchString(contentStr) { + return mp.name + } + } + } + + return "" +} + +// recommendedBdHooks are the hooks that should have bd integration +var recommendedBdHooks = []string{"pre-commit", "post-merge", "pre-push"} + +// lefthookConfigFiles lists lefthook config files (derived from hookManagerConfigs). +// Format is inferred from extension. +var lefthookConfigFiles = hookManagerConfigs[0].configFiles // lefthook is first + +// CheckLefthookBdIntegration parses lefthook config (YAML, TOML, or JSON) and checks if bd hooks are integrated. +// See https://lefthook.dev/configuration/ for supported config file locations. +func CheckLefthookBdIntegration(path string) *HookIntegrationStatus { + // Find first existing config file + var configPath string + for _, name := range lefthookConfigFiles { + p := filepath.Join(path, name) + if _, err := os.Stat(p); err == nil { + configPath = p + break + } + } + if configPath == "" { + return nil + } + + content, err := os.ReadFile(configPath) // #nosec G304 - path is validated + if err != nil { + return nil + } + + // Parse config based on extension + var config map[string]interface{} + ext := filepath.Ext(configPath) + switch ext { + case ".toml": + if _, err := toml.Decode(string(content), &config); err != nil { + return nil + } + case ".json": + if err := json.Unmarshal(content, &config); err != nil { + return nil + } + default: // .yml, .yaml + if err := yaml.Unmarshal(content, &config); err != nil { + return nil + } + } + + status := &HookIntegrationStatus{ + Manager: "lefthook", + Configured: false, + } + + // Check each recommended hook + for _, hookName := range recommendedBdHooks { + hookSection, ok := config[hookName] + if !ok { + // Hook not configured at all in lefthook + status.HooksNotInConfig = append(status.HooksNotInConfig, hookName) + continue + } + + // Walk to commands.*.run to check for bd hooks run + if hasBdInCommands(hookSection) { + status.HooksWithBd = append(status.HooksWithBd, hookName) + status.Configured = true + } else { + // Hook is in config but has no bd integration + status.HooksWithoutBd = append(status.HooksWithoutBd, hookName) + } + } + + return status +} + +// hasBdInCommands checks if any command's "run" field contains bd hooks run. +// Walks the lefthook structure: hookSection.commands.*.run +func hasBdInCommands(hookSection interface{}) bool { + sectionMap, ok := hookSection.(map[string]interface{}) + if !ok { + return false + } + + commands, ok := sectionMap["commands"] + if !ok { + return false + } + + commandsMap, ok := commands.(map[string]interface{}) + if !ok { + return false + } + + for _, cmdConfig := range commandsMap { + cmdMap, ok := cmdConfig.(map[string]interface{}) + if !ok { + continue + } + + runVal, ok := cmdMap["run"] + if !ok { + continue + } + + runStr, ok := runVal.(string) + if !ok { + continue + } + + if bdHookPattern.MatchString(runStr) { + return true + } + } + + return false +} + +// CheckHuskyBdIntegration checks .husky/ scripts for bd integration. +func CheckHuskyBdIntegration(path string) *HookIntegrationStatus { + huskyDir := filepath.Join(path, ".husky") + if _, err := os.Stat(huskyDir); os.IsNotExist(err) { + return nil + } + + status := &HookIntegrationStatus{ + Manager: "husky", + Configured: false, + } + + for _, hookName := range recommendedBdHooks { + hookPath := filepath.Join(huskyDir, hookName) + content, err := os.ReadFile(hookPath) // #nosec G304 - path is validated + if err != nil { + // Hook script doesn't exist in .husky/ + status.HooksNotInConfig = append(status.HooksNotInConfig, hookName) + continue + } + + contentStr := string(content) + + // Check for bd hooks run pattern + if bdHookPattern.MatchString(contentStr) { + status.HooksWithBd = append(status.HooksWithBd, hookName) + status.Configured = true + } else { + status.HooksWithoutBd = append(status.HooksWithoutBd, hookName) + } + } + + return status +} + +// checkManagerBdIntegration checks a specific manager for bd integration. +func checkManagerBdIntegration(name, path string) *HookIntegrationStatus { + switch name { + case "lefthook": + return CheckLefthookBdIntegration(path) + case "husky": + return CheckHuskyBdIntegration(path) + default: + return nil + } +} + +// CheckExternalHookManagerIntegration checks if detected hook managers have bd integration. +func CheckExternalHookManagerIntegration(path string) *HookIntegrationStatus { + managers := DetectExternalHookManagers(path) + if len(managers) == 0 { + return nil + } + + // First, try to detect which manager is actually active from git hooks + if activeManager := DetectActiveHookManager(path); activeManager != "" { + if status := checkManagerBdIntegration(activeManager, path); status != nil { + return status + } + } + + // Fall back to checking detected managers in order + for _, m := range managers { + if status := checkManagerBdIntegration(m.Name, path); status != nil { + return status + } + } + + // Return basic status for unsupported managers + return &HookIntegrationStatus{ + Manager: ManagerNames(managers), + Configured: false, + } +} + +// ManagerNames extracts names from a slice of ExternalHookManager as comma-separated string. +func ManagerNames(managers []ExternalHookManager) string { + names := make([]string, len(managers)) + for i, m := range managers { + names[i] = m.Name + } + return strings.Join(names, ", ") +} + +// GitHooks fixes missing or broken git hooks by calling bd hooks install. +// If external hook managers are detected (lefthook, husky, etc.), it uses +// --chain to preserve existing hooks instead of overwriting them. func GitHooks(path string) error { // Validate workspace if err := validateBeadsWorkspace(path); err != nil { @@ -21,15 +363,26 @@ func GitHooks(path string) error { return fmt.Errorf("not a git repository") } + // Detect external hook managers + externalManagers := DetectExternalHookManagers(path) + // Get bd binary path bdBinary, err := getBdBinary() if err != nil { return err } + // Build command arguments + args := []string{"hooks", "install"} + + // If external hook managers detected, use --chain to preserve them + if len(externalManagers) > 0 { + args = append(args, "--chain") + } + // Run bd hooks install - cmd := newBdCmd(bdBinary, "hooks", "install") - cmd.Dir = path // Set working directory without changing process dir + cmd := newBdCmd(bdBinary, args...) + cmd.Dir = path // Set working directory without changing process dir cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/cmd/bd/doctor/fix/hooks_test.go b/cmd/bd/doctor/fix/hooks_test.go new file mode 100644 index 00000000..9e92de70 --- /dev/null +++ b/cmd/bd/doctor/fix/hooks_test.go @@ -0,0 +1,894 @@ +package fix + +import ( + "os" + "os/exec" + "path/filepath" + "testing" +) + +func TestDetectExternalHookManagers(t *testing.T) { + tests := []struct { + name string + setup func(dir string) error + expected []string // Expected manager names + }{ + { + name: "no hook managers", + setup: func(dir string) error { + return nil + }, + expected: nil, + }, + { + name: "lefthook.yml", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte("pre-commit:\n"), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: "lefthook.yaml", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, "lefthook.yaml"), []byte("pre-commit:\n"), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: "lefthook.toml", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, "lefthook.toml"), []byte("[pre-commit]\n"), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: "lefthook.json", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, "lefthook.json"), []byte(`{"pre-commit":{}}`), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: ".lefthook.yml (hidden)", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, ".lefthook.yml"), []byte("pre-commit:\n"), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: ".config/lefthook.yml", + setup: func(dir string) error { + configDir := filepath.Join(dir, ".config") + if err := os.MkdirAll(configDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(configDir, "lefthook.yml"), []byte("pre-commit:\n"), 0644) + }, + expected: []string{"lefthook"}, + }, + { + name: ".husky directory", + setup: func(dir string) error { + return os.MkdirAll(filepath.Join(dir, ".husky"), 0755) + }, + expected: []string{"husky"}, + }, + { + name: ".pre-commit-config.yaml", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644) + }, + expected: []string{"pre-commit"}, + }, + { + name: ".overcommit.yml", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, ".overcommit.yml"), []byte("PreCommit:\n"), 0644) + }, + expected: []string{"overcommit"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if err := tt.setup(dir); err != nil { + t.Fatalf("setup failed: %v", err) + } + + managers := DetectExternalHookManagers(dir) + + if len(tt.expected) == 0 { + if len(managers) != 0 { + t.Errorf("expected no managers, got %v", managers) + } + return + } + + if len(managers) != len(tt.expected) { + t.Errorf("expected %d managers, got %d", len(tt.expected), len(managers)) + return + } + + for i, exp := range tt.expected { + if managers[i].Name != exp { + t.Errorf("expected manager %q, got %q", exp, managers[i].Name) + } + } + }) + } +} + +func TestCheckLefthookBdIntegration(t *testing.T) { + tests := []struct { + name string + configFile string + configContent string + expectConfigured bool + expectHooksWithBd []string + expectHooksWithoutBd []string + expectNotInConfig []string + }{ + { + name: "no config", + configFile: "", + // No file created + expectConfigured: false, + }, + { + name: "yaml with bd hooks run", + configFile: "lefthook.yml", + configContent: ` +pre-commit: + commands: + bd: + run: bd hooks run pre-commit +post-merge: + commands: + bd: + run: bd hooks run post-merge +pre-push: + commands: + bd: + run: bd hooks run pre-push +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "yaml with partial bd integration", + configFile: "lefthook.yml", + configContent: ` +pre-commit: + commands: + bd: + run: bd hooks run pre-commit + lint: + run: eslint . +post-merge: + commands: + bd: + run: echo "no bd here" +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit"}, + expectHooksWithoutBd: []string{"post-merge"}, + expectNotInConfig: []string{"pre-push"}, + }, + { + name: "yaml without bd at all", + configFile: "lefthook.yml", + configContent: ` +pre-commit: + commands: + lint: + run: eslint . +`, + expectConfigured: false, + expectHooksWithoutBd: []string{"pre-commit"}, + expectNotInConfig: []string{"post-merge", "pre-push"}, + }, + { + name: "toml with bd hooks run", + configFile: "lefthook.toml", + configContent: ` +[pre-commit.commands.bd] +run = "bd hooks run pre-commit" + +[post-merge.commands.bd] +run = "bd hooks run post-merge" + +[pre-push.commands.bd] +run = "bd hooks run pre-push" +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "json with bd hooks run", + configFile: "lefthook.json", + configContent: `{ + "pre-commit": { + "commands": { + "bd": { + "run": "bd hooks run pre-commit" + } + } + }, + "post-merge": { + "commands": { + "bd": { + "run": "bd hooks run post-merge" + } + } + }, + "pre-push": { + "commands": { + "bd": { + "run": "bd hooks run pre-push" + } + } + } +}`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "hidden config .lefthook.yml", + configFile: ".lefthook.yml", + configContent: ` +pre-commit: + commands: + bd: + run: bd hooks run pre-commit +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit"}, + expectNotInConfig: []string{"post-merge", "pre-push"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + + if tt.configFile != "" { + configPath := filepath.Join(dir, tt.configFile) + // Handle .config/ subdirectory + if err := os.MkdirAll(filepath.Dir(configPath), 0755); err != nil { + t.Fatalf("failed to create config dir: %v", err) + } + if err := os.WriteFile(configPath, []byte(tt.configContent), 0644); err != nil { + t.Fatalf("failed to write config: %v", err) + } + } + + status := CheckLefthookBdIntegration(dir) + + if tt.configFile == "" { + if status != nil { + t.Errorf("expected nil status for no config, got %+v", status) + } + return + } + + if status == nil { + t.Fatal("expected non-nil status") + } + + if status.Configured != tt.expectConfigured { + t.Errorf("Configured: expected %v, got %v", tt.expectConfigured, status.Configured) + } + + if !slicesEqual(status.HooksWithBd, tt.expectHooksWithBd) { + t.Errorf("HooksWithBd: expected %v, got %v", tt.expectHooksWithBd, status.HooksWithBd) + } + + if !slicesEqual(status.HooksWithoutBd, tt.expectHooksWithoutBd) { + t.Errorf("HooksWithoutBd: expected %v, got %v", tt.expectHooksWithoutBd, status.HooksWithoutBd) + } + + if !slicesEqual(status.HooksNotInConfig, tt.expectNotInConfig) { + t.Errorf("HooksNotInConfig: expected %v, got %v", tt.expectNotInConfig, status.HooksNotInConfig) + } + }) + } +} + +func TestCheckHuskyBdIntegration(t *testing.T) { + tests := []struct { + name string + hooks map[string]string // hookName -> content + expectConfigured bool + expectHooksWithBd []string + expectHooksWithoutBd []string + expectNotInConfig []string + }{ + { + name: "no .husky directory", + hooks: nil, + expectConfigured: false, + }, + { + name: "all hooks with bd", + hooks: map[string]string{ + "pre-commit": "#!/bin/sh\nbd hooks run pre-commit\n", + "post-merge": "#!/bin/sh\nbd hooks run post-merge\n", + "pre-push": "#!/bin/sh\nbd hooks run pre-push\n", + }, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "partial bd integration", + hooks: map[string]string{ + "pre-commit": "#!/bin/sh\nbd hooks run pre-commit\n", + "post-merge": "#!/bin/sh\necho 'no bd'\n", + }, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit"}, + expectHooksWithoutBd: []string{"post-merge"}, + expectNotInConfig: []string{"pre-push"}, + }, + { + name: "no bd at all", + hooks: map[string]string{ + "pre-commit": "#!/bin/sh\neslint .\n", + }, + expectConfigured: false, + expectHooksWithoutBd: []string{"pre-commit"}, + expectNotInConfig: []string{"post-merge", "pre-push"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + + if tt.hooks != nil { + huskyDir := filepath.Join(dir, ".husky") + if err := os.MkdirAll(huskyDir, 0755); err != nil { + t.Fatalf("failed to create .husky: %v", err) + } + for hookName, content := range tt.hooks { + if err := os.WriteFile(filepath.Join(huskyDir, hookName), []byte(content), 0755); err != nil { + t.Fatalf("failed to write hook %s: %v", hookName, err) + } + } + } + + status := CheckHuskyBdIntegration(dir) + + if tt.hooks == nil { + if status != nil { + t.Errorf("expected nil status for no .husky, got %+v", status) + } + return + } + + if status == nil { + t.Fatal("expected non-nil status") + } + + if status.Configured != tt.expectConfigured { + t.Errorf("Configured: expected %v, got %v", tt.expectConfigured, status.Configured) + } + + if !slicesEqual(status.HooksWithBd, tt.expectHooksWithBd) { + t.Errorf("HooksWithBd: expected %v, got %v", tt.expectHooksWithBd, status.HooksWithBd) + } + + if !slicesEqual(status.HooksWithoutBd, tt.expectHooksWithoutBd) { + t.Errorf("HooksWithoutBd: expected %v, got %v", tt.expectHooksWithoutBd, status.HooksWithoutBd) + } + + if !slicesEqual(status.HooksNotInConfig, tt.expectNotInConfig) { + t.Errorf("HooksNotInConfig: expected %v, got %v", tt.expectNotInConfig, status.HooksNotInConfig) + } + }) + } +} + +func TestBdHookPatternMatching(t *testing.T) { + tests := []struct { + content string + matches bool + }{ + {"bd hooks run pre-commit", true}, + {"bd hooks run pre-commit", true}, + {"bd hooks run post-merge", true}, + {`bd hooks run pre-push "$@"`, true}, + {"if command -v bd; then bd hooks run pre-commit; fi", true}, + {"# bd hooks run is recommended", true}, + // Word boundary tests - should NOT match partial words + {"kbd hooks runner", false}, // 'kbd' contains 'bd' but is different word + {"bd_hooks_run", false}, // underscores make different tokens + {"bd sync", false}, + {"bd export", false}, + {".beads/", false}, + {"eslint .", false}, + {"echo hello", false}, + } + + for _, tt := range tests { + t.Run(tt.content, func(t *testing.T) { + if got := bdHookPattern.MatchString(tt.content); got != tt.matches { + t.Errorf("bdHookPattern.MatchString(%q) = %v, want %v", tt.content, got, tt.matches) + } + }) + } +} + +func TestDetectActiveHookManager(t *testing.T) { + tests := []struct { + name string + hookContent string + expected string + }{ + { + name: "lefthook signature", + hookContent: "#!/bin/sh\n# lefthook\nexec lefthook run pre-commit\n", + expected: "lefthook", + }, + { + name: "husky signature", + hookContent: "#!/bin/sh\n. \"$(dirname \"$0\")/_/husky.sh\"\nnpm test\n", + expected: "husky", + }, + { + name: "pre-commit framework signature", + hookContent: "#!/usr/bin/env bash\n# PRE_COMMIT hook\npre-commit run --all-files\n", + expected: "pre-commit", + }, + { + name: "simple-git-hooks signature", + hookContent: "#!/bin/sh\n# simple-git-hooks\nnpm run lint\n", + expected: "simple-git-hooks", + }, + { + name: "no manager signature", + hookContent: "#!/bin/sh\necho 'custom hook'\n", + expected: "", + }, + { + name: "empty hook", + hookContent: "", + expected: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + + // Initialize real git repo + cmd := exec.Command("git", "init") + cmd.Dir = dir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + // Write hook file + if tt.hookContent != "" { + hooksDir := filepath.Join(dir, ".git", "hooks") + hookPath := filepath.Join(hooksDir, "pre-commit") + if err := os.WriteFile(hookPath, []byte(tt.hookContent), 0755); err != nil { + t.Fatalf("failed to write hook: %v", err) + } + } + + result := DetectActiveHookManager(dir) + if result != tt.expected { + t.Errorf("DetectActiveHookManager() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestDetectActiveHookManager_CustomHooksPath(t *testing.T) { + dir := t.TempDir() + + // Initialize real git repo + cmd := exec.Command("git", "init") + cmd.Dir = dir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to init git repo: %v", err) + } + + // Create custom hooks directory outside .git + customHooksDir := filepath.Join(dir, "my-hooks") + if err := os.MkdirAll(customHooksDir, 0755); err != nil { + t.Fatalf("failed to create custom hooks dir: %v", err) + } + + // Write hook in custom location with lefthook signature + hookContent := "#!/bin/sh\n# lefthook\nexec lefthook run pre-commit\n" + if err := os.WriteFile(filepath.Join(customHooksDir, "pre-commit"), []byte(hookContent), 0755); err != nil { + t.Fatalf("failed to write hook: %v", err) + } + + // Set core.hooksPath via git config + configCmd := exec.Command("git", "config", "core.hooksPath", "my-hooks") + configCmd.Dir = dir + if err := configCmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + + result := DetectActiveHookManager(dir) + if result != "lefthook" { + t.Errorf("DetectActiveHookManager() with core.hooksPath = %q, want %q", result, "lefthook") + } +} + +func TestHasBdInCommands(t *testing.T) { + tests := []struct { + name string + section interface{} + expected bool + }{ + { + name: "bd hooks run in commands", + section: map[string]interface{}{ + "commands": map[string]interface{}{ + "bd": map[string]interface{}{ + "run": "bd hooks run pre-commit", + }, + }, + }, + expected: true, + }, + { + name: "no bd in commands", + section: map[string]interface{}{ + "commands": map[string]interface{}{ + "lint": map[string]interface{}{ + "run": "eslint .", + }, + }, + }, + expected: false, + }, + { + name: "bd mentioned in comment not run field", + section: map[string]interface{}{ + "commands": map[string]interface{}{ + "lint": map[string]interface{}{ + "run": "eslint .", + "tags": "bd hooks run should be added", + }, + }, + }, + expected: false, + }, + { + name: "nil section", + section: nil, + expected: false, + }, + { + name: "non-map section", + section: "string value", + expected: false, + }, + { + name: "no commands key", + section: map[string]interface{}{ + "parallel": true, + }, + expected: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + result := hasBdInCommands(tt.section) + if result != tt.expected { + t.Errorf("hasBdInCommands() = %v, want %v", result, tt.expected) + } + }) + } +} + +// slicesEqual compares two string slices for equality (order-insensitive) +func slicesEqual(a, b []string) bool { + if len(a) != len(b) { + return false + } + aMap := make(map[string]bool) + for _, v := range a { + aMap[v] = true + } + for _, v := range b { + if !aMap[v] { + return false + } + } + return true +} + +func TestCheckExternalHookManagerIntegration(t *testing.T) { + tests := []struct { + name string + setup func(dir string) error + expectNil bool + expectManager string + expectConfigured bool + }{ + { + name: "no managers", + setup: func(dir string) error { + return nil + }, + expectNil: true, + }, + { + name: "lefthook with bd integration", + setup: func(dir string) error { + config := `pre-commit: + commands: + bd: + run: bd hooks run pre-commit +` + return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644) + }, + expectNil: false, + expectManager: "lefthook", + expectConfigured: true, + }, + { + name: "husky with bd integration", + setup: func(dir string) error { + huskyDir := filepath.Join(dir, ".husky") + if err := os.MkdirAll(huskyDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("#!/bin/sh\nbd hooks run pre-commit\n"), 0755) + }, + expectNil: false, + expectManager: "husky", + expectConfigured: true, + }, + { + 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, + }, + { + name: "lefthook without bd", + setup: func(dir string) error { + config := `pre-commit: + commands: + lint: + run: eslint . +` + return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644) + }, + expectNil: false, + expectManager: "lefthook", + expectConfigured: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if err := tt.setup(dir); err != nil { + t.Fatalf("setup failed: %v", err) + } + + result := CheckExternalHookManagerIntegration(dir) + + if tt.expectNil { + if result != nil { + t.Errorf("expected nil, got %+v", result) + } + return + } + + if result == nil { + t.Fatal("expected non-nil result") + } + + if result.Manager != tt.expectManager { + t.Errorf("Manager: expected %q, got %q", tt.expectManager, result.Manager) + } + + if result.Configured != tt.expectConfigured { + t.Errorf("Configured: expected %v, got %v", tt.expectConfigured, result.Configured) + } + }) + } +} + +func TestMultipleManagersDetected(t *testing.T) { + tests := []struct { + name string + setup func(dir string) error + expectManager string + }{ + { + name: "lefthook and husky both present - lefthook wins by priority", + setup: func(dir string) error { + // Create lefthook config + config := `pre-commit: + commands: + bd: + run: bd hooks run pre-commit +` + if err := os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte(config), 0644); err != nil { + return err + } + // Create husky directory + huskyDir := filepath.Join(dir, ".husky") + if err := os.MkdirAll(huskyDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("#!/bin/sh\nbd hooks run pre-commit\n"), 0755) + }, + expectManager: "lefthook", + }, + { + name: "husky only", + setup: func(dir string) error { + huskyDir := filepath.Join(dir, ".husky") + if err := os.MkdirAll(huskyDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("#!/bin/sh\nbd hooks run pre-commit\n"), 0755) + }, + expectManager: "husky", + }, + { + name: "multiple unsupported managers", + setup: func(dir string) error { + // pre-commit and overcommit + if err := os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644); err != nil { + return err + } + return os.WriteFile(filepath.Join(dir, ".overcommit.yml"), []byte("PreCommit:\n"), 0644) + }, + expectManager: "pre-commit, overcommit", // Falls through to basic status + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if err := tt.setup(dir); err != nil { + t.Fatalf("setup failed: %v", err) + } + + result := CheckExternalHookManagerIntegration(dir) + if result == nil { + t.Fatal("expected non-nil result") + } + + if result.Manager != tt.expectManager { + t.Errorf("Manager: expected %q, got %q", tt.expectManager, result.Manager) + } + }) + } +} + +func TestManagerNames(t *testing.T) { + tests := []struct { + managers []ExternalHookManager + expected string + }{ + {nil, ""}, + {[]ExternalHookManager{}, ""}, + {[]ExternalHookManager{{Name: "lefthook"}}, "lefthook"}, + {[]ExternalHookManager{{Name: "lefthook"}, {Name: "husky"}}, "lefthook, husky"}, + {[]ExternalHookManager{{Name: "a"}, {Name: "b"}, {Name: "c"}}, "a, b, c"}, + } + + for _, tt := range tests { + t.Run(tt.expected, func(t *testing.T) { + result := ManagerNames(tt.managers) + if result != tt.expected { + t.Errorf("ManagerNames() = %q, want %q", result, tt.expected) + } + }) + } +} + +func TestCheckManagerBdIntegration(t *testing.T) { + tests := []struct { + name string + managerName string + setup func(dir string) error + expectNil bool + expectManager string + }{ + { + name: "unknown manager returns nil", + managerName: "unknown-manager", + setup: func(dir string) error { return nil }, + expectNil: true, + }, + { + name: "lefthook integration", + managerName: "lefthook", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte("pre-commit:\n commands:\n bd:\n run: bd hooks run pre-commit\n"), 0644) + }, + expectNil: false, + expectManager: "lefthook", + }, + { + name: "husky integration", + managerName: "husky", + setup: func(dir string) error { + huskyDir := filepath.Join(dir, ".husky") + if err := os.MkdirAll(huskyDir, 0755); err != nil { + return err + } + return os.WriteFile(filepath.Join(huskyDir, "pre-commit"), []byte("bd hooks run pre-commit"), 0755) + }, + expectNil: false, + expectManager: "husky", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + if err := tt.setup(dir); err != nil { + t.Fatalf("setup failed: %v", err) + } + + result := checkManagerBdIntegration(tt.managerName, dir) + + if tt.expectNil { + if result != nil { + t.Errorf("expected nil, got %+v", result) + } + return + } + + if result == nil { + t.Fatal("expected non-nil result") + } + + if result.Manager != tt.expectManager { + t.Errorf("Manager: expected %q, got %q", tt.expectManager, result.Manager) + } + }) + } +} + +func TestDetectExternalHookManagers_MultiplePresentInOrder(t *testing.T) { + dir := t.TempDir() + + // Create configs for multiple managers + // lefthook (priority 0) + if err := os.WriteFile(filepath.Join(dir, "lefthook.yml"), []byte("pre-commit:\n"), 0644); err != nil { + t.Fatal(err) + } + // husky (priority 1) + if err := os.MkdirAll(filepath.Join(dir, ".husky"), 0755); err != nil { + t.Fatal(err) + } + // pre-commit (priority 2) + if err := os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte("repos:\n"), 0644); err != nil { + t.Fatal(err) + } + + managers := DetectExternalHookManagers(dir) + + if len(managers) != 3 { + t.Fatalf("expected 3 managers, got %d: %v", len(managers), managers) + } + + // Verify order matches priority + expectedOrder := []string{"lefthook", "husky", "pre-commit"} + for i, exp := range expectedOrder { + if managers[i].Name != exp { + t.Errorf("managers[%d]: expected %q, got %q", i, exp, managers[i].Name) + } + } +} diff --git a/cmd/bd/doctor/git.go b/cmd/bd/doctor/git.go index ca96edd0..b328b9b2 100644 --- a/cmd/bd/doctor/git.go +++ b/cmd/bd/doctor/git.go @@ -16,6 +16,11 @@ import ( "github.com/steveyegge/beads/internal/syncbranch" ) +const ( + hooksExamplesURL = "https://github.com/steveyegge/beads/tree/main/examples/git-hooks" + hooksUpgradeURL = "https://github.com/steveyegge/beads/issues/615" +) + // CheckGitHooks verifies that recommended git hooks are installed. func CheckGitHooks() DoctorCheck { // Check if we're in a git repository using worktree-aware detection @@ -48,6 +53,51 @@ func CheckGitHooks() DoctorCheck { } } + // Get repo root for external manager detection + repoRoot := filepath.Dir(gitDir) + if filepath.Base(gitDir) != ".git" { + // Worktree case - gitDir might be .git file content + if cwd, err := os.Getwd(); err == nil { + repoRoot = cwd + } + } + + // Check for external hook managers (lefthook, husky, etc.) + externalManagers := fix.DetectExternalHookManagers(repoRoot) + 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 { + 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", + Status: StatusWarning, + Message: fmt.Sprintf("%s not calling bd", fix.ManagerNames(externalManagers)), + Detail: "Configure hooks to call bd commands", + Fix: "Add or upgrade to 'bd hooks run '. See " + hooksUpgradeURL, + } + } + } + if len(missingHooks) == 0 { return DoctorCheck{ Name: "Git Hooks", @@ -57,7 +107,7 @@ func CheckGitHooks() DoctorCheck { } } - hookInstallMsg := "Install hooks with 'bd hooks install'. See https://github.com/steveyegge/beads/tree/main/examples/git-hooks for installation instructions" + hookInstallMsg := "Install hooks with 'bd hooks install'. See " + hooksExamplesURL if len(installedHooks) > 0 { return DoctorCheck{ @@ -293,7 +343,58 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck { // 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 + // Not a bd hook - check if it's an external hook manager + externalManagers := fix.DetectExternalHookManagers(path) + if len(externalManagers) > 0 { + names := fix.ManagerNames(externalManagers) + + // 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, ", ")) + } + 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), + } + } + + // External manager detected but no bd integration found + return DoctorCheck{ + Name: "Sync Branch Hook Compatibility", + Status: StatusWarning, + Message: fmt.Sprintf("Managed by %s (no bd integration detected)", names), + Detail: fmt.Sprintf("Pre-push hook managed by %s but no 'bd hooks run' found", names), + Fix: fmt.Sprintf("Add or upgrade to 'bd hooks run ' in %s. See %s", names, hooksExamplesURL), + } + } + + // No external manager - truly custom hook return DoctorCheck{ Name: "Sync Branch Hook Compatibility", Status: StatusWarning,