Fix git hooks not working in worktrees (#1126)
Git hooks are shared across all worktrees and live in the common git directory (e.g., /repo/.git/hooks), not the worktree-specific directory (e.g., /repo/.git/worktrees/feature/hooks). The core issue was in GetGitHooksDir() which used GetGitDir() instead of GetGitCommonDir(). This caused hooks to be installed to/read from the wrong location when running in a worktree. Additionally, several places in the codebase manually constructed hooks paths using gitDir + "hooks" instead of calling GetGitHooksDir(). These have been updated to use the proper worktree-aware path. Affected areas: - GetGitHooksDir() now uses GetGitCommonDir() - CheckGitHooks() uses GetGitHooksDir() - installHooks/uninstallHooks use GetGitHooksDir() - runChainedHook() uses GetGitHooksDir() - Doctor checks use git-common-dir for hooks paths - Reset command uses GetGitCommonDir() for hooks and beads-worktrees Symptoms that this fixes: - Chained hooks (pre-commit.old) not running in worktrees - bd hooks install not finding/installing hooks correctly in worktrees - bd hooks list showing incorrect status in worktrees - bd doctor reporting incorrect hooks status in worktrees Co-authored-by: Zain Rizvi <4468967+ZainRizvi@users.noreply.github.com>
This commit is contained in:
@@ -108,20 +108,20 @@ var hookManagerPatterns = []hookManagerPattern{
|
||||
// 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")
|
||||
// Get common git dir (hooks are shared across worktrees)
|
||||
cmd := exec.Command("git", "rev-parse", "--git-common-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)
|
||||
gitCommonDir := strings.TrimSpace(string(output))
|
||||
if !filepath.IsAbs(gitCommonDir) {
|
||||
gitCommonDir = filepath.Join(path, gitCommonDir)
|
||||
}
|
||||
|
||||
// Check for custom hooks path (core.hooksPath)
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
hooksDir := filepath.Join(gitCommonDir, "hooks")
|
||||
hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath")
|
||||
hooksPathCmd.Dir = path
|
||||
if hooksPathOutput, err := hooksPathCmd.Output(); err == nil {
|
||||
|
||||
@@ -30,7 +30,7 @@ var bdHooksRunPattern = regexp.MustCompile(`\bbd\s+hooks\s+run\b`)
|
||||
// CheckGitHooks verifies that recommended git hooks are installed.
|
||||
func CheckGitHooks() DoctorCheck {
|
||||
// Check if we're in a git repository using worktree-aware detection
|
||||
gitDir, err := git.GetGitDir()
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return DoctorCheck{
|
||||
Name: "Git Hooks",
|
||||
@@ -45,8 +45,6 @@ func CheckGitHooks() DoctorCheck {
|
||||
"post-merge": "Imports updated JSONL after git pull/merge",
|
||||
"pre-push": "Exports database to JSONL before push",
|
||||
}
|
||||
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
var missingHooks []string
|
||||
var installedHooks []string
|
||||
|
||||
@@ -60,13 +58,7 @@ 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
|
||||
}
|
||||
}
|
||||
repoRoot := git.GetRepoRoot()
|
||||
|
||||
// Check for external hook managers (lefthook, husky, etc.)
|
||||
externalManagers := fix.DetectExternalHookManagers(repoRoot)
|
||||
@@ -364,8 +356,8 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck {
|
||||
}
|
||||
|
||||
// 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")
|
||||
// Get common git directory for hooks (shared across worktrees)
|
||||
cmd := exec.Command("git", "rev-parse", "--git-common-dir")
|
||||
cmd.Dir = path
|
||||
output, err := cmd.Output()
|
||||
if err != nil {
|
||||
@@ -375,14 +367,13 @@ func CheckSyncBranchHookCompatibility(path string) DoctorCheck {
|
||||
Message: "N/A (not a git repository)",
|
||||
}
|
||||
}
|
||||
gitDir := strings.TrimSpace(string(output))
|
||||
if !filepath.IsAbs(gitDir) {
|
||||
gitDir = filepath.Join(path, gitDir)
|
||||
gitCommonDir := strings.TrimSpace(string(output))
|
||||
if !filepath.IsAbs(gitCommonDir) {
|
||||
gitCommonDir = filepath.Join(path, gitCommonDir)
|
||||
}
|
||||
|
||||
// Use standard .git/hooks location for consistency with CheckGitHooks (issue #799)
|
||||
// Note: core.hooksPath is intentionally NOT checked here to match CheckGitHooks behavior.
|
||||
hookPath := filepath.Join(gitDir, "hooks", "pre-push")
|
||||
// Hooks are shared across worktrees and live in the common git directory
|
||||
hookPath := filepath.Join(gitCommonDir, "hooks", "pre-push")
|
||||
|
||||
hookContent, err := os.ReadFile(hookPath) // #nosec G304 - path is controlled
|
||||
if err != nil {
|
||||
|
||||
@@ -24,12 +24,11 @@ func CheckSyncBranchQuick() string {
|
||||
// Checks all beads hooks: pre-commit, post-merge, pre-push, post-checkout.
|
||||
// cliVersion is the current CLI version to compare against.
|
||||
func CheckHooksQuick(cliVersion string) string {
|
||||
// Get actual git directory (handles worktrees where .git is a file)
|
||||
gitDir, err := git.GetGitDir()
|
||||
// Get hooks directory from common git dir (hooks are shared across worktrees)
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return "" // Not a git repo, skip
|
||||
}
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
|
||||
// Check if hooks dir exists
|
||||
if _, err := os.Stat(hooksDir); os.IsNotExist(err) {
|
||||
@@ -94,19 +93,19 @@ func CheckSyncBranchHookQuick(path string) string {
|
||||
return "" // sync-branch not configured, nothing to check
|
||||
}
|
||||
|
||||
// Get git directory
|
||||
cmd := exec.Command("git", "rev-parse", "--git-dir")
|
||||
// Get common git directory for hooks (shared across worktrees)
|
||||
cmd := exec.Command("git", "rev-parse", "--git-common-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)
|
||||
gitCommonDir := strings.TrimSpace(string(output))
|
||||
if !filepath.IsAbs(gitCommonDir) {
|
||||
gitCommonDir = filepath.Join(path, gitCommonDir)
|
||||
}
|
||||
|
||||
// Find pre-push hook (check shared hooks first)
|
||||
// Find pre-push hook (check shared hooks first via core.hooksPath)
|
||||
var hookPath string
|
||||
hooksPathCmd := exec.Command("git", "config", "--get", "core.hooksPath")
|
||||
hooksPathCmd.Dir = path
|
||||
@@ -117,7 +116,8 @@ func CheckSyncBranchHookQuick(path string) string {
|
||||
}
|
||||
hookPath = filepath.Join(sharedHooksDir, "pre-push")
|
||||
} else {
|
||||
hookPath = filepath.Join(gitDir, "hooks", "pre-push")
|
||||
// Hooks are in the common git directory, not the worktree-specific one
|
||||
hookPath = filepath.Join(gitCommonDir, "hooks", "pre-push")
|
||||
}
|
||||
|
||||
content, err := os.ReadFile(hookPath) // #nosec G304 - path is controlled
|
||||
|
||||
@@ -51,8 +51,8 @@ func CheckGitHooks() []HookStatus {
|
||||
hooks := []string{"pre-commit", "post-merge", "pre-push", "post-checkout", "prepare-commit-msg"}
|
||||
statuses := make([]HookStatus, 0, len(hooks))
|
||||
|
||||
// Get actual git directory (handles worktrees)
|
||||
gitDir, err := git.GetGitDir()
|
||||
// Get hooks directory from common git dir (hooks are shared across worktrees)
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
// Not a git repo - return all hooks as not installed
|
||||
for _, hookName := range hooks {
|
||||
@@ -67,7 +67,7 @@ func CheckGitHooks() []HookStatus {
|
||||
}
|
||||
|
||||
// Check if hook exists
|
||||
hookPath := filepath.Join(gitDir, "hooks", hookName)
|
||||
hookPath := filepath.Join(hooksDir, hookName)
|
||||
versionInfo, err := getHookVersion(hookPath)
|
||||
if err != nil {
|
||||
// Hook doesn't exist or couldn't be read
|
||||
@@ -319,19 +319,17 @@ var hooksListCmd = &cobra.Command{
|
||||
}
|
||||
|
||||
func installHooks(embeddedHooks map[string]string, force bool, shared bool, chain bool) error {
|
||||
// Get actual git directory (handles worktrees where .git is a file)
|
||||
gitDir, err := git.GetGitDir()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
|
||||
var hooksDir string
|
||||
if shared {
|
||||
// Use versioned directory for shared hooks
|
||||
hooksDir = ".beads-hooks"
|
||||
} else {
|
||||
// Use standard .git/hooks directory
|
||||
hooksDir = filepath.Join(gitDir, "hooks")
|
||||
// Use common git directory for hooks (shared across worktrees)
|
||||
var err error
|
||||
hooksDir, err = git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
}
|
||||
|
||||
// Create hooks directory if it doesn't exist
|
||||
@@ -401,12 +399,11 @@ func configureSharedHooksPath() error {
|
||||
}
|
||||
|
||||
func uninstallHooks() error {
|
||||
// Get actual git directory (handles worktrees)
|
||||
gitDir, err := git.GetGitDir()
|
||||
// Get hooks directory from common git dir (hooks are shared across worktrees)
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
hookNames := []string{"pre-commit", "post-merge", "pre-push", "post-checkout", "prepare-commit-msg"}
|
||||
|
||||
for _, hookName := range hookNames {
|
||||
@@ -442,13 +439,13 @@ func uninstallHooks() error {
|
||||
// runChainedHook runs a .old hook if it exists. Returns the exit code.
|
||||
// If the hook doesn't exist, returns 0 (success).
|
||||
func runChainedHook(hookName string, args []string) int {
|
||||
// Get the hooks directory
|
||||
gitDir, err := git.GetGitDir()
|
||||
// Get the hooks directory from common dir (hooks are shared across worktrees)
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return 0 // Not a git repo, nothing to chain
|
||||
}
|
||||
|
||||
oldHookPath := filepath.Join(gitDir, "hooks", hookName+".old")
|
||||
oldHookPath := filepath.Join(hooksDir, hookName+".old")
|
||||
|
||||
// Check if the .old hook exists and is executable
|
||||
info, err := os.Stat(oldHookPath)
|
||||
|
||||
@@ -21,12 +21,12 @@ var preCommitFrameworkPattern = regexp.MustCompile(`(?i)(pre-commit\s+run|prek\s
|
||||
|
||||
// hooksInstalled checks if bd git hooks are installed
|
||||
func hooksInstalled() bool {
|
||||
gitDir, err := git.GetGitDir()
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return false
|
||||
}
|
||||
preCommit := filepath.Join(gitDir, "hooks", "pre-commit")
|
||||
postMerge := filepath.Join(gitDir, "hooks", "post-merge")
|
||||
preCommit := filepath.Join(hooksDir, "pre-commit")
|
||||
postMerge := filepath.Join(hooksDir, "post-merge")
|
||||
|
||||
// Check if both hooks exist
|
||||
_, err1 := os.Stat(preCommit)
|
||||
@@ -81,11 +81,10 @@ type hookInfo struct {
|
||||
|
||||
// detectExistingHooks scans for existing git hooks
|
||||
func detectExistingHooks() []hookInfo {
|
||||
gitDir, err := git.GetGitDir()
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return nil
|
||||
}
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
hooks := []hookInfo{
|
||||
{name: "pre-commit", path: filepath.Join(hooksDir, "pre-commit")},
|
||||
{name: "post-merge", path: filepath.Join(hooksDir, "post-merge")},
|
||||
@@ -137,11 +136,10 @@ func promptHookAction(existingHooks []hookInfo) string {
|
||||
|
||||
// installGitHooks installs git hooks inline (no external dependencies)
|
||||
func installGitHooks() error {
|
||||
gitDir, err := git.GetGitDir()
|
||||
hooksDir, err := git.GetGitHooksDir()
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
|
||||
// Ensure hooks directory exists
|
||||
if err := os.MkdirAll(hooksDir, 0750); err != nil {
|
||||
|
||||
@@ -44,8 +44,8 @@ func runReset(cmd *cobra.Command, args []string) {
|
||||
|
||||
force, _ := cmd.Flags().GetBool("force")
|
||||
|
||||
// Check if we're in a git repo
|
||||
gitDir, err := git.GetGitDir()
|
||||
// Get common git directory (for hooks and beads-worktrees, which are shared across worktrees)
|
||||
gitCommonDir, err := git.GetGitCommonDir()
|
||||
if err != nil {
|
||||
if jsonOutput {
|
||||
outputJSON(map[string]interface{}{
|
||||
@@ -73,7 +73,7 @@ func runReset(cmd *cobra.Command, args []string) {
|
||||
}
|
||||
|
||||
// Collect what would be deleted
|
||||
items := collectResetItems(gitDir, beadsDir)
|
||||
items := collectResetItems(gitCommonDir, beadsDir)
|
||||
|
||||
if !force {
|
||||
// Dry-run mode: show what would be deleted
|
||||
@@ -82,7 +82,7 @@ func runReset(cmd *cobra.Command, args []string) {
|
||||
}
|
||||
|
||||
// Actually perform the reset
|
||||
performReset(items, gitDir, beadsDir)
|
||||
performReset(items, gitCommonDir, beadsDir)
|
||||
}
|
||||
|
||||
type resetItem struct {
|
||||
@@ -91,7 +91,7 @@ type resetItem struct {
|
||||
Description string `json:"description"`
|
||||
}
|
||||
|
||||
func collectResetItems(gitDir, beadsDir string) []resetItem {
|
||||
func collectResetItems(gitCommonDir, beadsDir string) []resetItem {
|
||||
var items []resetItem
|
||||
|
||||
// Check for running daemon
|
||||
@@ -106,9 +106,9 @@ func collectResetItems(gitDir, beadsDir string) []resetItem {
|
||||
}
|
||||
}
|
||||
|
||||
// Check for git hooks
|
||||
// Check for git hooks (hooks are in common git dir, shared across worktrees)
|
||||
hookNames := []string{"pre-commit", "post-merge", "pre-push", "post-checkout"}
|
||||
hooksDir := filepath.Join(gitDir, "hooks")
|
||||
hooksDir := filepath.Join(gitCommonDir, "hooks")
|
||||
for _, hookName := range hookNames {
|
||||
hookPath := filepath.Join(hooksDir, hookName)
|
||||
if _, err := os.Stat(hookPath); err == nil {
|
||||
@@ -141,8 +141,8 @@ func collectResetItems(gitDir, beadsDir string) []resetItem {
|
||||
})
|
||||
}
|
||||
|
||||
// Check for sync branch worktrees
|
||||
worktreesDir := filepath.Join(gitDir, "beads-worktrees")
|
||||
// Check for sync branch worktrees (in common git dir, shared across worktrees)
|
||||
worktreesDir := filepath.Join(gitCommonDir, "beads-worktrees")
|
||||
if info, err := os.Stat(worktreesDir); err == nil && info.IsDir() {
|
||||
items = append(items, resetItem{
|
||||
Type: "worktrees",
|
||||
|
||||
Reference in New Issue
Block a user