From ee179f5b6d1607d38d9da3d0fb45f9a37110ce1b Mon Sep 17 00:00:00 2001 From: giles Date: Mon, 5 Jan 2026 21:09:36 -0800 Subject: [PATCH] feat(doctor): add deep integration check for pre-commit framework (bd-28r5) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add config parsing for .pre-commit-config.yaml to detect if bd hooks are configured. pre-commit is popular enough to warrant full support alongside lefthook and husky. Changes: - Add CheckPrecommitBdIntegration() function to parse YAML config - Check hooks.*.entry field for 'bd hooks run' pattern - Handle stages field to determine which git hooks are configured - Support legacy stage names (commit -> pre-commit, push -> pre-push) - Add pre-commit to checkManagerBdIntegration switch statement - Add comprehensive tests for all scenarios Now supported managers with deep integration checks: - lefthook (YAML, TOML, JSON) - husky (.husky/ scripts) - pre-commit (.pre-commit-config.yaml) Detection-only managers (cannot verify config): - overcommit, yorkie, simple-git-hooks 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 --- cmd/bd/doctor/fix/hooks.go | 154 +++++++++++++++++++++++++++ cmd/bd/doctor/fix/hooks_test.go | 183 +++++++++++++++++++++++++++++++- 2 files changed, 332 insertions(+), 5 deletions(-) diff --git a/cmd/bd/doctor/fix/hooks.go b/cmd/bd/doctor/fix/hooks.go index bddf5487..f8b04bc2 100644 --- a/cmd/bd/doctor/fix/hooks.go +++ b/cmd/bd/doctor/fix/hooks.go @@ -263,6 +263,158 @@ func hasBdInCommands(hookSection interface{}) bool { return false } +// precommitConfigFiles lists pre-commit config files. +var precommitConfigFiles = []string{".pre-commit-config.yaml", ".pre-commit-config.yml"} + +// CheckPrecommitBdIntegration parses pre-commit config and checks if bd hooks are integrated. +// See https://pre-commit.com/ for config file format. +func CheckPrecommitBdIntegration(path string) *HookIntegrationStatus { + // Find first existing config file + var configPath string + for _, name := range precommitConfigFiles { + 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 YAML config + var config map[string]interface{} + if err := yaml.Unmarshal(content, &config); err != nil { + return nil + } + + status := &HookIntegrationStatus{ + Manager: "pre-commit", + Configured: false, + } + + // Track which hooks have bd integration + hooksWithBd := make(map[string]bool) + + // Parse repos list + repos, ok := config["repos"] + if !ok { + // Empty config, all hooks missing + status.HooksNotInConfig = recommendedBdHooks + return status + } + + reposList, ok := repos.([]interface{}) + if !ok { + status.HooksNotInConfig = recommendedBdHooks + return status + } + + // Walk through repos and hooks + for _, repo := range reposList { + repoMap, ok := repo.(map[string]interface{}) + if !ok { + continue + } + + hooks, ok := repoMap["hooks"] + if !ok { + continue + } + + hooksList, ok := hooks.([]interface{}) + if !ok { + continue + } + + for _, hook := range hooksList { + hookMap, ok := hook.(map[string]interface{}) + if !ok { + continue + } + + // Check if entry contains bd hooks run + entry, ok := hookMap["entry"] + if !ok { + continue + } + + entryStr, ok := entry.(string) + if !ok { + continue + } + + if !bdHookPattern.MatchString(entryStr) { + continue + } + + // Found bd hooks run - determine which hook stage(s) it applies to + stages := getPrecommitStages(hookMap) + for _, stage := range stages { + hooksWithBd[stage] = true + } + } + } + + // Build status based on what we found + for _, hookName := range recommendedBdHooks { + if hooksWithBd[hookName] { + status.HooksWithBd = append(status.HooksWithBd, hookName) + status.Configured = true + } else { + // Hook not configured with bd integration + status.HooksNotInConfig = append(status.HooksNotInConfig, hookName) + } + } + + return status +} + +// getPrecommitStages extracts the stages from a pre-commit hook config. +// Returns the hook stages, defaulting to ["pre-commit"] if not specified. +// Handles both new format (stages: [pre-commit]) and legacy format (stages: [commit]). +func getPrecommitStages(hookMap map[string]interface{}) []string { + stages, ok := hookMap["stages"] + if !ok { + // Default to pre-commit if no stages specified + return []string{"pre-commit"} + } + + stagesList, ok := stages.([]interface{}) + if !ok { + return []string{"pre-commit"} + } + + var result []string + for _, s := range stagesList { + stage, ok := s.(string) + if !ok { + continue + } + // Normalize legacy stage names (pre-3.2.0) + switch stage { + case "commit": + result = append(result, "pre-commit") + case "push": + result = append(result, "pre-push") + case "merge-commit": + result = append(result, "pre-merge-commit") + default: + result = append(result, stage) + } + } + + if len(result) == 0 { + return []string{"pre-commit"} + } + return result +} + // CheckHuskyBdIntegration checks .husky/ scripts for bd integration. func CheckHuskyBdIntegration(path string) *HookIntegrationStatus { huskyDir := filepath.Join(path, ".husky") @@ -305,6 +457,8 @@ func checkManagerBdIntegration(name, path string) *HookIntegrationStatus { return CheckLefthookBdIntegration(path) case "husky": return CheckHuskyBdIntegration(path) + case "pre-commit": + return CheckPrecommitBdIntegration(path) default: return nil } diff --git a/cmd/bd/doctor/fix/hooks_test.go b/cmd/bd/doctor/fix/hooks_test.go index 9afac6b7..c4c240c1 100644 --- a/cmd/bd/doctor/fix/hooks_test.go +++ b/cmd/bd/doctor/fix/hooks_test.go @@ -389,6 +389,152 @@ func TestCheckHuskyBdIntegration(t *testing.T) { } } +func TestCheckPrecommitBdIntegration(t *testing.T) { + tests := []struct { + name string + configContent string + expectNil bool + expectConfigured bool + expectHooksWithBd []string + expectHooksNotInConfig []string + }{ + { + name: "no config", + expectNil: true, + }, + { + name: "all hooks with bd", + configContent: `repos: + - repo: local + hooks: + - id: bd-pre-commit + entry: bd hooks run pre-commit + language: system + stages: [pre-commit] + - id: bd-post-merge + entry: bd hooks run post-merge + language: system + stages: [post-merge] + - id: bd-pre-push + entry: bd hooks run pre-push + language: system + stages: [pre-push] +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "only pre-commit hook", + configContent: `repos: + - repo: local + hooks: + - id: bd-pre-commit + entry: bd hooks run pre-commit + language: system +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit"}, + expectHooksNotInConfig: []string{"post-merge", "pre-push"}, + }, + { + name: "legacy stage names (pre-3.2.0)", + configContent: `repos: + - repo: local + hooks: + - id: bd-commit + entry: bd hooks run pre-commit + language: system + stages: [commit] + - id: bd-push + entry: bd hooks run pre-push + language: system + stages: [push] +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit", "pre-push"}, + expectHooksNotInConfig: []string{"post-merge"}, + }, + { + name: "no bd hooks at all", + configContent: `repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: trailing-whitespace + - id: end-of-file-fixer +`, + expectConfigured: false, + expectHooksNotInConfig: []string{"pre-commit", "post-merge", "pre-push"}, + }, + { + name: "mixed bd and other hooks", + configContent: `repos: + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.5.0 + hooks: + - id: trailing-whitespace + - repo: local + hooks: + - id: bd-pre-commit + entry: bd hooks run pre-commit + language: system +`, + expectConfigured: true, + expectHooksWithBd: []string{"pre-commit"}, + expectHooksNotInConfig: []string{"post-merge", "pre-push"}, + }, + { + name: "empty repos list", + configContent: `repos: [] +`, + expectConfigured: false, + expectHooksNotInConfig: []string{"pre-commit", "post-merge", "pre-push"}, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + dir := t.TempDir() + + if tt.configContent != "" { + configPath := filepath.Join(dir, ".pre-commit-config.yaml") + if err := os.WriteFile(configPath, []byte(tt.configContent), 0644); err != nil { + t.Fatalf("failed to write config: %v", err) + } + } + + status := CheckPrecommitBdIntegration(dir) + + if tt.expectNil { + if status != nil { + t.Errorf("expected nil status, got %+v", status) + } + return + } + + if status == nil { + t.Fatal("expected non-nil status") + } + + if status.Manager != "pre-commit" { + t.Errorf("Manager: expected 'pre-commit', got %q", status.Manager) + } + + 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.HooksNotInConfig, tt.expectHooksNotInConfig) { + t.Errorf("HooksNotInConfig: expected %v, got %v", tt.expectHooksNotInConfig, status.HooksNotInConfig) + } + }) + } +} + func TestBdHookPatternMatching(t *testing.T) { tests := []struct { content string @@ -652,13 +798,40 @@ func TestCheckExternalHookManagerIntegration(t *testing.T) { expectDetectionOnly: false, }, { - name: "unsupported manager (pre-commit framework)", + name: "pre-commit framework without bd", 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, + expectDetectionOnly: false, // pre-commit is now fully supported + }, + { + name: "pre-commit framework with bd integration", + setup: func(dir string) error { + config := `repos: + - repo: local + hooks: + - id: bd-pre-commit + entry: bd hooks run pre-commit + language: system +` + return os.WriteFile(filepath.Join(dir, ".pre-commit-config.yaml"), []byte(config), 0644) + }, + expectNil: false, + expectManager: "pre-commit", + expectConfigured: true, + expectDetectionOnly: false, + }, + { + name: "unsupported manager (overcommit)", + setup: func(dir string) error { + return os.WriteFile(filepath.Join(dir, ".overcommit.yml"), []byte("PreCommit:\n"), 0644) + }, + expectNil: false, + expectManager: "overcommit", + expectConfigured: false, expectDetectionOnly: true, }, { @@ -754,13 +927,13 @@ func TestMultipleManagersDetected(t *testing.T) { { 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 { + // overcommit and yorkie (both unsupported) + if err := os.WriteFile(filepath.Join(dir, ".overcommit.yml"), []byte("PreCommit:\n"), 0644); err != nil { return err } - return os.WriteFile(filepath.Join(dir, ".overcommit.yml"), []byte("PreCommit:\n"), 0644) + return os.MkdirAll(filepath.Join(dir, ".yorkie"), 0755) }, - expectManager: "pre-commit, overcommit", // Falls through to basic status + expectManager: "overcommit, yorkie", // Falls through to basic status }, }