From eb61b563f310740ca580f204730745a5698658a5 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 16 Jul 2026 11:17:08 -0400 Subject: [PATCH 1/6] fix(hooks): install Entire into husky user hooks Husky regenerates .husky/_ on npm prepare, which previously clobbered Entire's wrappers mid-turn so later commits lost Entire-Checkpoint trailers. Install into the parent .husky/ user-hook directory that husky's stubs invoke, so mid-turn commits keep trailers after prepare. Fixes #784 --- .../hook_overwrite_husky_test.go | 133 ++++++++++ cmd/entire/cli/strategy/hook_managers.go | 21 +- cmd/entire/cli/strategy/hook_managers_test.go | 76 +++--- cmd/entire/cli/strategy/hooks.go | 165 +++++++++++- cmd/entire/cli/strategy/hooks_test.go | 250 +++++++++++++++++- 5 files changed, 588 insertions(+), 57 deletions(-) create mode 100644 cmd/entire/cli/integration_test/hook_overwrite_husky_test.go diff --git a/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go new file mode 100644 index 0000000000..8fd0f4dcf4 --- /dev/null +++ b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go @@ -0,0 +1,133 @@ +//go:build integration + +package integration + +import ( + "os" + "os/exec" + "path/filepath" + "testing" + + "github.com/entireio/cli/cmd/entire/cli/strategy" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// huskyHScript is the husky v9 `_/h` dispatcher: git runs `.husky/_/`, +// which sources this script, which then executes `.husky/` if present. +const huskyHScript = `#!/usr/bin/env sh +n=$(basename "$0") +s=$(dirname "$(dirname "$0")")/$n +[ ! -f "$s" ] && exit 0 +sh -e "$s" "$@" +` + +func seedHuskyLayout(t *testing.T, env *TestEnv) (ownedDir, userDir string) { + t.Helper() + repoDir := env.RepoDir + userDir = filepath.Join(repoDir, ".husky") + ownedDir = filepath.Join(userDir, "_") + require.NoError(t, os.MkdirAll(ownedDir, 0o755)) + require.NoError(t, os.WriteFile(filepath.Join(ownedDir, "h"), []byte(huskyHScript), 0o755)) + + cmd := exec.CommandContext(t.Context(), "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = repoDir + require.NoError(t, cmd.Run()) + + // Intentional hooksPath mutation for the husky layout under test. + configData, err := os.ReadFile(env.gitConfigPath()) + require.NoError(t, err) + env.AcceptGitConfigChanges(string(configData)) + + writeHuskyStubs(t, ownedDir) + return ownedDir, userDir +} + +func writeHuskyStubs(t *testing.T, ownedDir string) { + t.Helper() + stub := "#!/usr/bin/env sh\n. \"$(dirname \"$0\")/h\"\n" + for _, hookName := range strategy.ManagedGitHookNames() { + require.NoError(t, os.WriteFile(filepath.Join(ownedDir, hookName), []byte(stub), 0o755)) + } +} + +// TestHusky_MidTurnPrepare_CommitsKeepTrailer covers the real #784 failure mode: +// husky/npm prepare regenerates `.husky/_` mid-turn. Entire must install into +// `.husky/` user hooks so mid-turn commits still get Entire-Checkpoint trailers +// without waiting for the next user-prompt-submit. +func TestHusky_MidTurnPrepare_CommitsKeepTrailer(t *testing.T) { + t.Parallel() + + env := NewFeatureBranchEnv(t) + ownedDir, userDir := seedHuskyLayout(t, env) + + // Point hooks at the shared test binary via absolute path (GUI/agent PATH + // is irrelevant; this matches `entire enable --absolute-git-hook-path`). + env.WriteSettings(map[string]any{ + "enabled": true, + "absolute_git_hook_path": true, + "strategy_options": map[string]any{"filtered_fetches": true, "commit_linking": "always"}, + }) + + sess := env.NewSession() + err := env.SimulateUserPromptSubmitWithPromptAndTranscriptPath( + sess.ID, "Create files A and B", sess.TranscriptPath) + require.NoError(t, err) + + require.True(t, strategy.IsGitHookInstalledInDir(t.Context(), env.RepoDir), + "EnsureSetup should install Entire into .husky/ user hooks") + + for _, hookName := range strategy.ManagedGitHookNames() { + userHook := filepath.Join(userDir, hookName) + data, readErr := os.ReadFile(userHook) + require.NoError(t, readErr, "Entire hook %s should live in .husky/", hookName) + require.Contains(t, string(data), "Entire CLI hooks") + + ownedHook, readErr := os.ReadFile(filepath.Join(ownedDir, hookName)) + require.NoError(t, readErr) + assert.NotContains(t, string(ownedHook), "Entire CLI hooks", + "husky-owned stub %s must remain untouched", hookName) + } + + env.WriteFile("fileA.go", "package main\n\nfunc A() {}\n") + env.WriteFile("fileB.go", "package main\n\nfunc B() {}\n") + sess.CreateTranscript("Create files A and B", []FileChange{ + {Path: "fileA.go", Content: "package main\n\nfunc A() {}\n"}, + {Path: "fileB.go", Content: "package main\n\nfunc B() {}\n"}, + }) + + // Commit 1 via real git so husky stubs → .husky/ Entire hooks run. + env.GitAdd("fileA.go") + commitViaGit(t, env, "Add file A") + cpID1 := env.GetCheckpointIDFromCommitMessage(env.GetHeadHash()) + require.NotEmpty(t, cpID1, "first commit should have checkpoint trailer through husky stubs") + + // Mid-turn: husky prepare regenerates .husky/_ (issue #784). + writeHuskyStubs(t, ownedDir) + require.True(t, strategy.IsGitHookInstalledInDir(t.Context(), env.RepoDir), + "Entire user hooks in .husky/ must survive husky prepare") + + // Commit 2 same turn — must still get a trailer (no next-prompt recovery). + env.GitAdd("fileB.go") + commitViaGit(t, env, "Add file B") + cpID2 := env.GetCheckpointIDFromCommitMessage(env.GetHeadHash()) + require.NotEmpty(t, cpID2, + "second mid-turn commit after husky prepare must still have Entire-Checkpoint trailer") + assert.NotEqual(t, cpID1, cpID2, "each commit should get its own checkpoint id") +} + +func commitViaGit(t *testing.T, env *TestEnv, message string) { + t.Helper() + cmd := exec.CommandContext(t.Context(), "git", "-c", "core.editor=true", "commit", "-m", message) + cmd.Dir = env.RepoDir + cmd.Env = append(os.Environ(), + "GIT_AUTHOR_NAME=Test User", + "GIT_AUTHOR_EMAIL=test@example.com", + "GIT_COMMITTER_NAME=Test User", + "GIT_COMMITTER_EMAIL=test@example.com", + // Agent-style: no interactive TTY for prepare-commit-msg fast path. + "GIT_TERMINAL_PROMPT=0", + ) + out, err := cmd.CombinedOutput() + require.NoError(t, err, "git commit %q failed: %s", message, out) +} diff --git a/cmd/entire/cli/strategy/hook_managers.go b/cmd/entire/cli/strategy/hook_managers.go index c9b4ac0ed5..c41a0de2f0 100644 --- a/cmd/entire/cli/strategy/hook_managers.go +++ b/cmd/entire/cli/strategy/hook_managers.go @@ -64,7 +64,9 @@ func detectHookManagers(repoRoot string) []hookManager { // hookManagerWarning builds a warning string for detected hook managers. // cmdPrefix is the CLI command prefix (e.g., "entire" or "./scripts/entire-dev"). -func hookManagerWarning(managers []hookManager, cmdPrefix string) string { +// huskySafe is true when the current core.hooksPath is a husky `_` dir with the +// dispatcher present (Entire will install into the parent user-hook directory). +func hookManagerWarning(managers []hookManager, cmdPrefix string, huskySafe bool) string { if len(managers) == 0 { return "" } @@ -75,6 +77,17 @@ func hookManagerWarning(managers []hookManager, cmdPrefix string) string { for _, m := range managers { if m.OverwritesHooks { + // Husky v9 safe path: dispatcher present under core.hooksPath=`_/`. + if m.Name == "Husky" && huskySafe { + fmt.Fprintf(&b, "Note: %s detected (%s)\n", m.Name, m.ConfigPath) + fmt.Fprintf(&b, "\n") + fmt.Fprintf(&b, " Entire installs git hooks into the husky user-hook directory\n") + fmt.Fprintf(&b, " (parent of core.hooksPath), which survives husky/npm prepare.\n") + fmt.Fprintf(&b, " Regenerable `_` stubs are left alone.\n") + fmt.Fprintf(&b, "\n") + continue + } + fmt.Fprintf(&b, "Warning: %s detected (%s)\n", m.Name, m.ConfigPath) fmt.Fprintf(&b, "\n") fmt.Fprintf(&b, " %s may overwrite hooks installed by Entire on npm install.\n", m.Name) @@ -138,7 +151,11 @@ func CheckAndWarnHookManagers(ctx context.Context, w io.Writer, localDev, absolu // Best-effort: hook manager warnings are advisory, skip on resolution failure return } - warning := hookManagerWarning(managers, cmdPrefix) + huskySafe := false + if hooksDir, hooksErr := getHooksDirInPath(ctx, repoRoot); hooksErr == nil { + huskySafe = huskyUserHooksDir(hooksDir) != "" + } + warning := hookManagerWarning(managers, cmdPrefix, huskySafe) if warning != "" { fmt.Fprintln(w) fmt.Fprint(w, warning) diff --git a/cmd/entire/cli/strategy/hook_managers_test.go b/cmd/entire/cli/strategy/hook_managers_test.go index 877bb62e56..d826aa2490 100644 --- a/cmd/entire/cli/strategy/hook_managers_test.go +++ b/cmd/entire/cli/strategy/hook_managers_test.go @@ -4,6 +4,7 @@ import ( "bytes" "context" "os" + "os/exec" "path/filepath" "strings" "testing" @@ -317,34 +318,25 @@ func TestHookManagerWarning_Husky(t *testing.T) { {Name: "Husky", ConfigPath: ".husky/", OverwritesHooks: true}, } - warning := hookManagerWarning(managers, "entire") + warning := hookManagerWarning(managers, "entire", true) - // Should contain all 4 hook file references - for _, hook := range gitHookNames { - if !strings.Contains(warning, ".husky/"+hook+":") { - t.Errorf("warning should contain .husky/%s:", hook) - } + // Husky-safe install: advisory note only (hooks live in parent user dir). + if !strings.Contains(warning, "Note: Husky detected") { + t.Error("warning should start with 'Note: Husky detected'") } - - // Should contain the actual command lines from buildHookSpecs - specs := buildHookSpecs("entire") - for _, spec := range specs { - cmdLine := extractCommandLine(spec.content) - if cmdLine == "" { - t.Errorf("failed to extract command line for %s", spec.name) - continue - } - if !strings.Contains(warning, cmdLine) { - t.Errorf("warning should contain command line %q for %s", cmdLine, spec.name) - } + if !strings.Contains(warning, "survive") { + t.Error("warning should mention that Entire hooks survive husky/npm prepare") } - - // Should mention Husky by name and warn about overwriting - if !strings.Contains(warning, "Warning: Husky detected") { - t.Error("warning should start with 'Warning: Husky detected'") + if strings.Contains(warning, "Warning: Husky detected") { + t.Error("Husky should not emit the overwrite Warning after husky-safe install") } - if !strings.Contains(warning, "may overwrite hooks") { - t.Error("warning should mention 'may overwrite hooks'") + if strings.Contains(warning, "may overwrite hooks") { + t.Error("Husky advisory should not claim Entire hooks may be overwritten") + } + + unsafe := hookManagerWarning(managers, "entire", false) + if !strings.Contains(unsafe, "Warning: Husky detected") { + t.Error("Husky without safe hooksPath should emit overwrite Warning") } } @@ -355,7 +347,7 @@ func TestHookManagerWarning_GitHooksManager(t *testing.T) { {Name: "Lefthook", ConfigPath: "lefthook.yml", OverwritesHooks: false}, } - warning := hookManagerWarning(managers, "entire") + warning := hookManagerWarning(managers, "entire", false) // Category B: should be a Note, not a Warning if !strings.Contains(warning, "Note: Lefthook detected") { @@ -374,12 +366,12 @@ func TestHookManagerWarning_GitHooksManager(t *testing.T) { func TestHookManagerWarning_Empty(t *testing.T) { t.Parallel() - warning := hookManagerWarning(nil, "entire") + warning := hookManagerWarning(nil, "entire", false) if warning != "" { t.Errorf("expected empty string for nil managers, got %q", warning) } - warning = hookManagerWarning([]hookManager{}, "entire") + warning = hookManagerWarning([]hookManager{}, "entire", false) if warning != "" { t.Errorf("expected empty string for empty managers, got %q", warning) } @@ -388,13 +380,14 @@ func TestHookManagerWarning_Empty(t *testing.T) { func TestHookManagerWarning_LocalDev(t *testing.T) { t.Parallel() + // Non-Husky overwrite managers still emit command-line guidance with the + // configured cmd prefix. (Husky is Note-only after the husky-safe install.) managers := []hookManager{ - {Name: "Husky", ConfigPath: ".husky/", OverwritesHooks: true}, + {Name: "CustomManager", ConfigPath: ".custom/", OverwritesHooks: true}, } - warning := hookManagerWarning(managers, localDevHookCmdPrefix) + warning := hookManagerWarning(managers, localDevHookCmdPrefix, false) - // Should use the local dev prefix in command lines if !strings.Contains(warning, localDevHookCmdPrefix+" hooks git") { t.Error("warning should use local dev command prefix") } @@ -408,10 +401,10 @@ func TestHookManagerWarning_Multiple(t *testing.T) { {Name: "Lefthook", ConfigPath: "lefthook.yml", OverwritesHooks: false}, } - warning := hookManagerWarning(managers, "entire") + warning := hookManagerWarning(managers, "entire", true) - if !strings.Contains(warning, "Warning: Husky detected") { - t.Error("should contain Husky warning") + if !strings.Contains(warning, "Note: Husky detected") { + t.Error("should contain Husky note") } if !strings.Contains(warning, "Note: Lefthook detected") { t.Error("should contain Lefthook note") @@ -480,16 +473,25 @@ func TestCheckAndWarnHookManagers_WithHusky(t *testing.T) { // Needs t.Chdir (via initHooksTestRepo), cannot be parallel tmpDir, _ := initHooksTestRepo(t) - // Create .husky/_/ directory - if err := os.MkdirAll(filepath.Join(tmpDir, ".husky", "_"), 0o755); err != nil { + // Create husky v9 layout: .husky/_ + dispatcher, core.hooksPath=.husky/_ + huskyOwned := filepath.Join(tmpDir, ".husky", "_") + if err := os.MkdirAll(huskyOwned, 0o755); err != nil { t.Fatalf("failed to create .husky/_/: %v", err) } + if err := os.WriteFile(filepath.Join(huskyOwned, "h"), []byte("#!/usr/bin/env sh\n"), 0o755); err != nil { + t.Fatalf("write husky dispatcher: %v", err) + } + cmd := exec.CommandContext(context.Background(), "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } var buf bytes.Buffer CheckAndWarnHookManagers(context.Background(), &buf, false, false) output := buf.String() - if !strings.Contains(output, "Warning: Husky detected") { - t.Errorf("expected warning output, got %q", output) + if !strings.Contains(output, "Note: Husky detected") { + t.Errorf("expected husky note output, got %q", output) } } diff --git a/cmd/entire/cli/strategy/hooks.go b/cmd/entire/cli/strategy/hooks.go index 98e183711f..7bb40d044a 100644 --- a/cmd/entire/cli/strategy/hooks.go +++ b/cmd/entire/cli/strategy/hooks.go @@ -139,13 +139,50 @@ func getHooksDirInPath(ctx context.Context, dir string) (string, error) { return filepath.Clean(hooksDir), nil } +// huskyForwardingStub is the husky v9 stub content written into core.hooksPath +// (`_/prepare-commit-msg`, etc.). Git executes these; they source `_/h`, which +// runs the sibling user hook in the parent directory. +const huskyForwardingStub = "#!/usr/bin/env sh\n. \"$(dirname \"$0\")/h\"\n" + +// huskyUserHooksDir returns the user-owned Husky hooks directory when hooksDir +// is a regenerable husky `_` directory (core.hooksPath) AND husky's dispatcher +// script (`_/h`) is present. Otherwise "". +// +// Husky v9 sets core.hooksPath to `/_` (usually `.husky/_`) and regenerates +// that directory on `husky` / npm prepare. User hook scripts live in the parent +// and are invoked by the stubs in `_` via `h` — those user scripts survive +// husky reinstall (see https://github.com/entireio/cli/issues/784). +// +// The dispatcher check is required: git only executes files under +// core.hooksPath. Redirecting to the parent without husky's forwarding layout +// would install hooks that never run. +func huskyUserHooksDir(hooksDir string) string { + if filepath.Base(hooksDir) != "_" { + return "" + } + if !fileExists(filepath.Join(hooksDir, "h")) { + return "" + } + return filepath.Dir(hooksDir) +} + +// hookInstallDir returns the directory where Entire should write managed git +// hooks. For Husky (`_/h` present), that is the parent user-hook dir; +// otherwise it is the git hooks directory itself. +func hookInstallDir(hooksDir string) string { + if userDir := huskyUserHooksDir(hooksDir); userDir != "" { + return userDir + } + return hooksDir +} + // IsGitHookInstalled checks if all generic Entire CLI hooks are installed. func IsGitHookInstalled(ctx context.Context) bool { hooksDir, err := GetHooksDir(ctx) if err != nil { return false } - return isGitHookInstalledInHooksDir(hooksDir) + return isGitHookInstalledForHooksDir(hooksDir) } // IsGitHookInstalledInDir checks if all Entire CLI hooks are installed in the given repo directory. @@ -155,7 +192,36 @@ func IsGitHookInstalledInDir(ctx context.Context, repoDir string) bool { if err != nil { return false } - return isGitHookInstalledInHooksDir(hooksDir) + return isGitHookInstalledForHooksDir(hooksDir) +} + +// isGitHookInstalledForHooksDir reports whether Entire's managed hooks are +// installed in the effective install directory, and (for husky-safe installs) +// that forwarding stubs still exist under core.hooksPath so git can reach them. +func isGitHookInstalledForHooksDir(hooksDir string) bool { + installDir := hookInstallDir(hooksDir) + if !isGitHookInstalledInHooksDir(installDir) { + return false + } + if installDir != hooksDir { + return huskyForwardingStubsPresent(hooksDir) + } + return true +} + +// huskyForwardingStubsPresent reports whether each managed hook has a +// non-Entire stub in the husky-owned hooks directory (so git → `_` → parent). +func huskyForwardingStubsPresent(hooksDir string) bool { + for _, hook := range gitHookNames { + data, err := os.ReadFile(filepath.Join(hooksDir, hook)) //nolint:gosec // path from constants + if err != nil { + return false + } + if strings.Contains(string(data), entireHookMarker) { + return false + } + } + return true } // isGitHookInstalledInHooksDir checks if all hooks are installed in the given hooks directory. @@ -288,13 +354,18 @@ func isWindowsAbsoluteHookCommand(cmdPrefix string) bool { // localDev controls whether hooks use "go run" (true) or the "entire" binary (false). // absolutePath embeds the full binary path in hooks for GUI git clients. // Returns the number of hooks that were installed (0 if all already up to date). +// +// When core.hooksPath is Husky's .husky/_ directory, hooks are written to the +// parent `.husky/` user-hook directory (which husky's stubs invoke) rather than +// into `_`, so `npm install` / `husky` prepare cannot clobber them mid-turn. func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (int, error) { hooksDir, err := GetHooksDir(ctx) if err != nil { return 0, err } + installDir := hookInstallDir(hooksDir) - if err := os.MkdirAll(hooksDir, 0o755); err != nil { //nolint:gosec // Git hooks require executable permissions + if err := os.MkdirAll(installDir, 0o755); err != nil { //nolint:gosec // Git hooks require executable permissions return 0, fmt.Errorf("failed to create hooks directory: %w", err) } @@ -306,7 +377,7 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i installedCount := 0 for _, spec := range specs { - hookPath := filepath.Join(hooksDir, spec.name) + hookPath := filepath.Join(installDir, spec.name) backupPath := hookPath + backupSuffix backupExists := fileExists(backupPath) @@ -339,9 +410,22 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i } } + // After parent install succeeds, migrate older Entire wrappers out of + // husky-owned `_` and ensure forwarding stubs exist so git can reach the + // parent user hooks. Doing this after install avoids a window with zero + // Entire hooks. + if installDir != hooksDir { + if err := migrateEntireHooksFromHuskyOwnedDir(hooksDir); err != nil { + return installedCount, fmt.Errorf("failed to migrate hooks out of husky-owned directory: %w", err) + } + } + if !silent { fmt.Println("✓ Installed git hooks (prepare-commit-msg, commit-msg, post-commit, pre-push)") fmt.Println(" Hooks delegate to the current strategy at runtime") + if installDir != hooksDir { + fmt.Println(" Installed into .husky/ (survives husky/npm prepare)") + } } return installedCount, nil @@ -366,17 +450,88 @@ func writeHookFile(path, content string) (bool, error) { // RemoveGitHook removes all Entire CLI git hooks from the repository. // If a .pre-entire backup exists, it is restored. // Returns the number of hooks removed. +// +// For Husky setups, removals target `.husky/` (user hooks). Any legacy Entire +// wrappers left in the regenerable `.husky/_` directory are also cleaned up. func RemoveGitHook(ctx context.Context) (int, error) { hooksDir, err := GetHooksDir(ctx) if err != nil { return 0, err } + installDir := hookInstallDir(hooksDir) + + removed, err := removeEntireHooksFromDir(installDir) + if err != nil { + return removed, err + } + // When husky-safe install is active, also scrub/restore the regenerable `_` + // directory. If the dispatcher was deleted (hookInstallDir falls back to + // `_`), still scrub Entire wrappers from the parent user-hook directory so + // disable does not leave orphan Entire scripts behind. + if installDir != hooksDir { + if cleanErr := migrateEntireHooksFromHuskyOwnedDir(hooksDir); cleanErr != nil { + return removed, cleanErr + } + } else if filepath.Base(hooksDir) == "_" { + parentRemoved, parentErr := removeEntireHooksFromDir(filepath.Dir(hooksDir)) + if parentErr != nil { + return removed, parentErr + } + removed += parentRemoved + } + return removed, nil +} + +// migrateEntireHooksFromHuskyOwnedDir removes legacy Entire wrappers from the +// husky-owned `_` directory, restoring `.pre-entire` backups when present, or +// writing a standard husky forwarding stub when no backup exists. Also fills +// in any missing managed stubs so git can reach parent user hooks. +func migrateEntireHooksFromHuskyOwnedDir(hooksDir string) error { + for _, hook := range gitHookNames { + hookPath := filepath.Join(hooksDir, hook) + backupPath := hookPath + backupSuffix + + data, err := os.ReadFile(hookPath) //nolint:gosec // path is controlled + hookIsOurs := err == nil && strings.Contains(string(data), entireHookMarker) + + switch { + case hookIsOurs && fileExists(backupPath): + if err := os.Remove(hookPath); err != nil { + return fmt.Errorf("remove legacy %s: %w", hook, err) + } + if err := os.Rename(backupPath, hookPath); err != nil { + return fmt.Errorf("restore %s%s: %w", hook, backupSuffix, err) + } + case hookIsOurs: + if err := writeHookFileForced(hookPath, huskyForwardingStub); err != nil { + return fmt.Errorf("replace legacy %s with husky stub: %w", hook, err) + } + case errors.Is(err, os.ErrNotExist): + if err := writeHookFileForced(hookPath, huskyForwardingStub); err != nil { + return fmt.Errorf("write missing husky stub %s: %w", hook, err) + } + } + } + return nil +} + +// writeHookFileForced writes content to path unconditionally (used for husky stubs). +func writeHookFileForced(path, content string) error { + if err := os.WriteFile(path, []byte(content), 0o755); err != nil { //nolint:gosec // Git hooks require executable permissions + return fmt.Errorf("failed to write hook file %s: %w", path, err) + } + return nil +} + +// removeEntireHooksFromDir removes Entire-managed hooks from dir, restoring +// .pre-entire backups when present. Returns the number of Entire hooks removed. +func removeEntireHooksFromDir(dir string) (int, error) { removed := 0 var removeErrors []string for _, hook := range gitHookNames { - hookPath := filepath.Join(hooksDir, hook) + hookPath := filepath.Join(dir, hook) backupPath := hookPath + backupSuffix // Remove the hook if it contains our marker diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index e818fb62ba..426b644941 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -857,12 +857,17 @@ func TestInstallGitHook_CoreHooksPathRelative(t *testing.T) { tmpDir, _ := initHooksTestRepo(t) ctx := context.Background() - // Simulate Husky-style override: hooks live outside .git/hooks. + // Simulate Husky-style override: git invokes stubs in .husky/_, but Entire + // must install into the parent .husky/ user-hook directory so husky prepare + // cannot clobber the wrappers (issue #784). cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") cmd.Dir = tmpDir if err := cmd.Run(); err != nil { t.Fatalf("failed to set core.hooksPath: %v", err) } + // Seed husky-owned stubs + dispatcher the way `husky` would. + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) count, err := InstallGitHook(context.Background(), true, false, false) if err != nil { @@ -872,15 +877,27 @@ func TestInstallGitHook_CoreHooksPathRelative(t *testing.T) { t.Fatal("InstallGitHook() should install hooks when core.hooksPath is set") } - configuredHooksDir := filepath.Join(tmpDir, ".husky", "_") + userHooksDir := filepath.Join(tmpDir, ".husky") for _, hook := range gitHookNames { - hookPath := filepath.Join(configuredHooksDir, hook) + hookPath := filepath.Join(userHooksDir, hook) data, readErr := os.ReadFile(hookPath) if readErr != nil { - t.Fatalf("expected hook %s in core.hooksPath dir: %v", hook, readErr) + t.Fatalf("expected hook %s in husky user-hook dir: %v", hook, readErr) } if !strings.Contains(string(data), entireHookMarker) { - t.Errorf("hook %s in core.hooksPath dir should contain Entire marker", hook) + t.Errorf("hook %s in .husky/ should contain Entire marker", hook) + } + } + + // Must not replace husky-owned stubs in .husky/_. + for _, hook := range gitHookNames { + hookPath := filepath.Join(huskyOwnedDir, hook) + data, readErr := os.ReadFile(hookPath) + if readErr != nil { + t.Fatalf("expected husky stub %s to remain: %v", hook, readErr) + } + if strings.Contains(string(data), entireHookMarker) { + t.Errorf("husky-owned stub %s must not contain Entire marker", hook) } } @@ -894,7 +911,158 @@ func TestInstallGitHook_CoreHooksPathRelative(t *testing.T) { } if !IsGitHookInstalledInDir(context.Background(), tmpDir) { - t.Error("IsGitHookInstalledInDir() should detect hooks installed in core.hooksPath") + t.Error("IsGitHookInstalledInDir() should detect hooks installed in .husky/") + } +} + +func TestInstallGitHook_HuskyPrepareDoesNotClobberUserHooks(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("hooks should be installed before simulated husky prepare") + } + + // Simulate `npx husky` / npm prepare regenerating .husky/_. + seedHuskyOwnedDir(t, huskyOwnedDir) + + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("Entire hooks in .husky/ must survive husky prepare regenerating .husky/_") + } + for _, hook := range gitHookNames { + data, err := os.ReadFile(filepath.Join(tmpDir, ".husky", hook)) + if err != nil { + t.Fatalf("read user hook %s: %v", hook, err) + } + if !strings.Contains(string(data), entireHookMarker) { + t.Errorf("user hook %s lost Entire marker after husky prepare", hook) + } + } +} + +func TestInstallGitHook_MigratesLegacyEntireHooksFromHuskyOwnedDir(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + if err := os.MkdirAll(huskyOwnedDir, 0o755); err != nil { + t.Fatalf("mkdir .husky/_: %v", err) + } + // Legacy layout: Entire wrappers live in `_`, husky stubs backed up beside them. + for _, hook := range gitHookNames { + stub := huskyForwardingStub + if err := os.WriteFile(filepath.Join(huskyOwnedDir, hook+backupSuffix), []byte(stub), 0o755); err != nil { + t.Fatalf("write backup stub %s: %v", hook, err) + } + legacy := "#!/bin/sh\n# " + entireHookMarker + "\nentire hooks git " + hook + "\n" + if err := os.WriteFile(filepath.Join(huskyOwnedDir, hook), []byte(legacy), 0o755); err != nil { + t.Fatalf("write legacy Entire hook %s: %v", hook, err) + } + } + // Dispatcher present so hookInstallDir redirects to parent. + if err := os.WriteFile(filepath.Join(huskyOwnedDir, "h"), []byte("#!/usr/bin/env sh\n"), 0o755); err != nil { + t.Fatalf("write husky dispatcher: %v", err) + } + + if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + + for _, hook := range gitHookNames { + userData, err := os.ReadFile(filepath.Join(tmpDir, ".husky", hook)) + if err != nil { + t.Fatalf("expected migrated hook in .husky/%s: %v", hook, err) + } + if !strings.Contains(string(userData), entireHookMarker) { + t.Errorf(".husky/%s should contain Entire marker after migrate", hook) + } + ownedData, err := os.ReadFile(filepath.Join(huskyOwnedDir, hook)) + if err != nil { + t.Fatalf("expected restored husky stub %s: %v", hook, err) + } + if strings.Contains(string(ownedData), entireHookMarker) { + t.Errorf("legacy Entire wrapper should be removed from .husky/_/%s", hook) + } + if !strings.Contains(string(ownedData), `dirname "$0"`) { + t.Errorf("husky stub should be restored for %s", hook) + } + } +} + +func TestInstallGitHook_MigratesLegacyEntireHooksWithoutBackupWritesStub(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + if err := os.MkdirAll(huskyOwnedDir, 0o755); err != nil { + t.Fatalf("mkdir .husky/_: %v", err) + } + if err := os.WriteFile(filepath.Join(huskyOwnedDir, "h"), []byte("#!/usr/bin/env sh\n"), 0o755); err != nil { + t.Fatalf("write husky dispatcher: %v", err) + } + // Legacy Entire wrappers with no .pre-entire backup. + for _, hook := range gitHookNames { + legacy := "#!/bin/sh\n# " + entireHookMarker + "\nentire hooks git " + hook + "\n" + if err := os.WriteFile(filepath.Join(huskyOwnedDir, hook), []byte(legacy), 0o755); err != nil { + t.Fatalf("write legacy Entire hook %s: %v", hook, err) + } + } + + if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("hooks should be installed after migrate-without-backup") + } + for _, hook := range gitHookNames { + ownedData, err := os.ReadFile(filepath.Join(huskyOwnedDir, hook)) + if err != nil { + t.Fatalf("expected husky stub written for %s: %v", hook, err) + } + if string(ownedData) != huskyForwardingStub { + t.Errorf("stub for %s = %q, want huskyForwardingStub", hook, ownedData) + } + } +} + +// seedHuskyOwnedDir writes husky v9 stubs + the `_/h` dispatcher into ownedDir. +func seedHuskyOwnedDir(t *testing.T, ownedDir string) { + t.Helper() + if err := os.MkdirAll(ownedDir, 0o755); err != nil { + t.Fatalf("mkdir husky owned dir: %v", err) + } + dispatcher := "#!/usr/bin/env sh\nn=$(basename \"$0\")\ns=$(dirname \"$(dirname \"$0\")\")/$n\n[ ! -f \"$s\" ] && exit 0\nsh -e \"$s\" \"$@\"\n" + if err := os.WriteFile(filepath.Join(ownedDir, "h"), []byte(dispatcher), 0o755); err != nil { + t.Fatalf("write husky dispatcher: %v", err) + } + stub := "#!/usr/bin/env sh\n. \"$(dirname \"$0\")/h\"\n" + for _, hook := range gitHookNames { + if err := os.WriteFile(filepath.Join(ownedDir, hook), []byte(stub), 0o755); err != nil { + t.Fatalf("write husky stub %s: %v", hook, err) + } } } @@ -907,6 +1075,7 @@ func TestRemoveGitHook_CoreHooksPathRelative(t *testing.T) { if err := cmd.Run(); err != nil { t.Fatalf("failed to set core.hooksPath: %v", err) } + seedHuskyOwnedDir(t, filepath.Join(tmpDir, ".husky", "_")) installCount, err := InstallGitHook(context.Background(), true, false, false) if err != nil { @@ -916,12 +1085,12 @@ func TestRemoveGitHook_CoreHooksPathRelative(t *testing.T) { t.Fatal("InstallGitHook() should install hooks before removal test") } - // Hooks must be installed in core.hooksPath (not .git/hooks). - configuredHooksDir := filepath.Join(tmpDir, ".husky", "_") + // Hooks must be installed in .husky/ user-hook dir (not regenerable .husky/_). + userHooksDir := filepath.Join(tmpDir, ".husky") for _, hook := range gitHookNames { - hookPath := filepath.Join(configuredHooksDir, hook) + hookPath := filepath.Join(userHooksDir, hook) if _, statErr := os.Stat(hookPath); statErr != nil { - t.Fatalf("expected hook %s in core.hooksPath before removal: %v", hook, statErr) + t.Fatalf("expected hook %s in .husky/ before removal: %v", hook, statErr) } } @@ -934,14 +1103,69 @@ func TestRemoveGitHook_CoreHooksPathRelative(t *testing.T) { } for _, hook := range gitHookNames { - hookPath := filepath.Join(configuredHooksDir, hook) + hookPath := filepath.Join(userHooksDir, hook) if _, statErr := os.Stat(hookPath); !os.IsNotExist(statErr) { - t.Errorf("hook file %s should not exist in core.hooksPath after removal", hook) + t.Errorf("hook file %s should not exist in .husky/ after removal", hook) } } if IsGitHookInstalledInDir(context.Background(), tmpDir) { - t.Error("IsGitHookInstalledInDir() should be false after removing hooks in core.hooksPath") + t.Error("IsGitHookInstalledInDir() should be false after removing hooks in .husky/") + } +} + +func TestHuskyUserHooksDir(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + huskyOwned := filepath.Join(tmp, ".husky", "_") + if err := os.MkdirAll(huskyOwned, 0o755); err != nil { + t.Fatalf("mkdir: %v", err) + } + + // Without dispatcher, do not redirect (git would not forward to parent). + if got := huskyUserHooksDir(huskyOwned); got != "" { + t.Errorf("huskyUserHooksDir without dispatcher = %q, want \"\"", got) + } + if got := hookInstallDir(huskyOwned); got != huskyOwned { + t.Errorf("hookInstallDir without dispatcher = %q, want %q", got, huskyOwned) + } + + if err := os.WriteFile(filepath.Join(huskyOwned, "h"), []byte("#!/usr/bin/env sh\n"), 0o755); err != nil { + t.Fatalf("write dispatcher: %v", err) + } + wantParent := filepath.Join(tmp, ".husky") + if got := huskyUserHooksDir(huskyOwned); got != wantParent { + t.Errorf("huskyUserHooksDir with dispatcher = %q, want %q", got, wantParent) + } + if got := hookInstallDir(huskyOwned); got != wantParent { + t.Errorf("hookInstallDir with dispatcher = %q, want %q", got, wantParent) + } + + // Non-husky shapes never redirect (no `_`+`h` dispatcher layout). + for _, hooksDir := range []string{ + filepath.Join(tmp, ".git", "hooks"), + filepath.Join(tmp, ".husky"), + } { + if got := huskyUserHooksDir(hooksDir); got != "" { + t.Errorf("huskyUserHooksDir(%q) = %q, want \"\"", hooksDir, got) + } + if got := hookInstallDir(hooksDir); got != hooksDir { + t.Errorf("hookInstallDir(%q) = %q, want %q", hooksDir, got, hooksDir) + } + } + + // Custom husky dir (e.g. husky frontend/.husky) still redirects when `_/h` exists. + customOwned := filepath.Join(tmp, "frontend", ".husky", "_") + if err := os.MkdirAll(customOwned, 0o755); err != nil { + t.Fatalf("mkdir custom husky: %v", err) + } + if err := os.WriteFile(filepath.Join(customOwned, "h"), []byte("#!/usr/bin/env sh\n"), 0o755); err != nil { + t.Fatalf("write custom dispatcher: %v", err) + } + wantCustomParent := filepath.Join(tmp, "frontend", ".husky") + if got := huskyUserHooksDir(customOwned); got != wantCustomParent { + t.Errorf("huskyUserHooksDir(custom) = %q, want %q", got, wantCustomParent) } } From 72ff7f41b964c4ce837905bc58607b518248a5fc Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:15:39 -0400 Subject: [PATCH 2/6] fix(hooks): harden husky-safe install for trail 876 Chain pre-existing hooks with sh -e/[ -f ] so mode-0644 husky scripts still run, surface migrate read errors, scrub `_` without backfill on disable, and cover tracked user-hook backup/restore plus stub healing. Co-authored-by: Cursor --- .../hook_overwrite_husky_test.go | 62 ++- cmd/entire/cli/setup.go | 15 +- cmd/entire/cli/setup_test.go | 2 +- cmd/entire/cli/strategy/common.go | 2 +- cmd/entire/cli/strategy/hook_managers.go | 12 +- cmd/entire/cli/strategy/hook_managers_test.go | 24 +- cmd/entire/cli/strategy/hooks.go | 174 ++++++-- cmd/entire/cli/strategy/hooks_test.go | 371 ++++++++++++++++-- 8 files changed, 580 insertions(+), 82 deletions(-) diff --git a/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go index 8fd0f4dcf4..f621af04b1 100644 --- a/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go +++ b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go @@ -13,8 +13,10 @@ import ( "github.com/stretchr/testify/require" ) -// huskyHScript is the husky v9 `_/h` dispatcher: git runs `.husky/_/`, -// which sources this script, which then executes `.husky/` if present. +// huskyHScript is the core of the husky v9 `_/h` dispatcher (HUSKY=0 opt-out, +// ~/.config/husky/init.sh sourcing, and node_modules/.bin PATH prepend trimmed). +// Git runs `.husky/_/`, which sources this script, which then executes +// `.husky/` if present via `sh -e`. const huskyHScript = `#!/usr/bin/env sh n=$(basename "$0") s=$(dirname "$(dirname "$0")")/$n @@ -131,3 +133,59 @@ func commitViaGit(t *testing.T, env *TestEnv, message string) { out, err := cmd.CombinedOutput() require.NoError(t, err, "git commit %q failed: %s", message, out) } + +// TestHusky_PreExistingUserHook_BackedUpAndChained covers tracked `.husky/` +// territory: a pre-existing mode-0644 user hook is renamed to `.pre-entire`, +// Entire's wrapper is installed, and the backup still runs (via sh -e) on commit. +// RemoveGitHook restores the original script. +func TestHusky_PreExistingUserHook_BackedUpAndChained(t *testing.T) { + t.Parallel() + + env := NewFeatureBranchEnv(t) + _, userDir := seedHuskyLayout(t, env) + + sentinel := filepath.Join(env.RepoDir, "user-hook-ran") + custom := "#!/bin/sh\ntouch \"" + sentinel + "\"\n" + userHook := filepath.Join(userDir, "prepare-commit-msg") + require.NoError(t, os.WriteFile(userHook, []byte(custom), 0o644)) + + env.WriteSettings(map[string]any{ + "enabled": true, + "absolute_git_hook_path": true, + "strategy_options": map[string]any{"filtered_fetches": true, "commit_linking": "always"}, + }) + + sess := env.NewSession() + err := env.SimulateUserPromptSubmitWithPromptAndTranscriptPath( + sess.ID, "Touch preexisting husky hook", sess.TranscriptPath) + require.NoError(t, err) + + backupPath := userHook + ".pre-entire" + backupData, err := os.ReadFile(backupPath) + require.NoError(t, err, "preexisting user hook should be backed up to .pre-entire") + require.Equal(t, custom, string(backupData)) + + installed, err := os.ReadFile(userHook) + require.NoError(t, err) + require.Contains(t, string(installed), "Entire CLI hooks") + require.Contains(t, string(installed), "sh -e") + + env.WriteFile("chain.go", "package main\n") + sess.CreateTranscript("Touch preexisting husky hook", []FileChange{ + {Path: "chain.go", Content: "package main\n"}, + }) + env.GitAdd("chain.go") + commitViaGit(t, env, "Exercise chained husky user hook") + + cpID := env.GetCheckpointIDFromCommitMessage(env.GetHeadHash()) + require.NotEmpty(t, cpID, "Entire prepare-commit-msg should still add trailer") + _, err = os.Stat(sentinel) + require.NoError(t, err, "mode-0644 preexisting user hook must run via chain") + + removed, err := strategy.RemoveGitHook(t.Context()) + require.NoError(t, err) + require.Greater(t, removed, 0) + restored, err := os.ReadFile(userHook) + require.NoError(t, err) + require.Equal(t, custom, string(restored)) +} diff --git a/cmd/entire/cli/setup.go b/cmd/entire/cli/setup.go index fd2be8c463..9a3578ee68 100644 --- a/cmd/entire/cli/setup.go +++ b/cmd/entire/cli/setup.go @@ -315,10 +315,11 @@ func updateGlobalSettings(ctx context.Context, cmd *cobra.Command, w io.Writer, } if cmd.Flags().Changed(flagForce) || cmd.Flags().Changed(flagAbsoluteGitHookPath) || cmd.Flags().Changed(flagLocalDev) { - if _, err := strategy.InstallGitHook(ctx, true, s.LocalDev, s.AbsoluteGitHookPath); err != nil { + _, huskySafe, err := strategy.InstallGitHook(ctx, true, s.LocalDev, s.AbsoluteGitHookPath) + if err != nil { return fmt.Errorf("failed to reinstall git hook: %w", err) } - strategy.CheckAndWarnHookManagers(ctx, w, s.LocalDev, s.AbsoluteGitHookPath) + strategy.CheckAndWarnHookManagers(ctx, w, s.LocalDev, s.AbsoluteGitHookPath, huskySafe) fmt.Fprintln(w, " ✓ Reinstalled git hook") } @@ -1274,10 +1275,11 @@ func runEnableInteractive(ctx context.Context, w io.Writer, agents []agent.Agent // Use settings values (merged from existing config + flags) for hook installation // This ensures re-running `entire enable` without flags preserves existing settings - if _, err := strategy.InstallGitHook(ctx, true, settings.LocalDev, settings.AbsoluteGitHookPath); err != nil { + _, huskySafe, err := strategy.InstallGitHook(ctx, true, settings.LocalDev, settings.AbsoluteGitHookPath) + if err != nil { return fmt.Errorf("failed to install git hooks: %w", err) } - strategy.CheckAndWarnHookManagers(ctx, w, settings.LocalDev, settings.AbsoluteGitHookPath) + strategy.CheckAndWarnHookManagers(ctx, w, settings.LocalDev, settings.AbsoluteGitHookPath, huskySafe) fmt.Fprintln(w, " ✓ Installed hooks") configDisplay := configDisplayProject @@ -1949,10 +1951,11 @@ func setupAgentHooksNonInteractive(ctx context.Context, w io.Writer, ag agent.Ag hookLocalDev := mergedSettings.LocalDev || opts.LocalDev hookAbsoluteGitHookPath := mergedSettings.AbsoluteGitHookPath || opts.AbsoluteGitHookPath - if _, err := strategy.InstallGitHook(ctx, true, hookLocalDev, hookAbsoluteGitHookPath); err != nil { + _, huskySafe, err := strategy.InstallGitHook(ctx, true, hookLocalDev, hookAbsoluteGitHookPath) + if err != nil { return fmt.Errorf("failed to install git hooks: %w", err) } - strategy.CheckAndWarnHookManagers(ctx, w, hookLocalDev, hookAbsoluteGitHookPath) + strategy.CheckAndWarnHookManagers(ctx, w, hookLocalDev, hookAbsoluteGitHookPath, huskySafe) if installedHooks == 0 { msg := fmt.Sprintf("Hooks for %s already installed", ag.Description()) diff --git a/cmd/entire/cli/setup_test.go b/cmd/entire/cli/setup_test.go index a4cefd0f12..faba4b5c34 100644 --- a/cmd/entire/cli/setup_test.go +++ b/cmd/entire/cli/setup_test.go @@ -1249,7 +1249,7 @@ func TestRunUninstall_Force_RemovesGitHooks(t *testing.T) { writeSettings(t, testSettingsEnabled) // Install git hooks - if _, err := strategy.InstallGitHook(context.Background(), true, false, false); err != nil { + if _, _, err := strategy.InstallGitHook(context.Background(), true, false, false); err != nil { t.Fatalf("InstallGitHook() error = %v", err) } diff --git a/cmd/entire/cli/strategy/common.go b/cmd/entire/cli/strategy/common.go index cdce76a108..7a082a2440 100644 --- a/cmd/entire/cli/strategy/common.go +++ b/cmd/entire/cli/strategy/common.go @@ -109,7 +109,7 @@ func EnsureSetup(ctx context.Context) error { // Install generic hooks (they delegate to strategy at runtime) if !IsGitHookInstalled(ctx) { localDev, absoluteHookPath := hookSettingsFromConfig(ctx) - if _, err := InstallGitHook(ctx, true, localDev, absoluteHookPath); err != nil { + if _, _, err := InstallGitHook(ctx, true, localDev, absoluteHookPath); err != nil { return fmt.Errorf("failed to install git hooks: %w", err) } } diff --git a/cmd/entire/cli/strategy/hook_managers.go b/cmd/entire/cli/strategy/hook_managers.go index c41a0de2f0..aea16bf383 100644 --- a/cmd/entire/cli/strategy/hook_managers.go +++ b/cmd/entire/cli/strategy/hook_managers.go @@ -83,7 +83,7 @@ func hookManagerWarning(managers []hookManager, cmdPrefix string, huskySafe bool fmt.Fprintf(&b, "\n") fmt.Fprintf(&b, " Entire installs git hooks into the husky user-hook directory\n") fmt.Fprintf(&b, " (parent of core.hooksPath), which survives husky/npm prepare.\n") - fmt.Fprintf(&b, " Regenerable `_` stubs are left alone.\n") + fmt.Fprintf(&b, " Regenerable `_` stubs are never replaced with Entire hooks.\n") fmt.Fprintf(&b, "\n") continue } @@ -135,7 +135,9 @@ func extractCommandLine(hookContent string) string { // to w if any are found. // localDev controls whether the warning references "go run" or the "entire" binary. // absolutePath embeds the full binary path for GUI git clients. -func CheckAndWarnHookManagers(ctx context.Context, w io.Writer, localDev, absolutePath bool) { +// huskySafeInstall must match InstallGitHook's second return value so the Husky +// advisory cannot disagree with where hooks were just installed. +func CheckAndWarnHookManagers(ctx context.Context, w io.Writer, localDev, absolutePath, huskySafeInstall bool) { repoRoot, err := paths.WorktreeRoot(ctx) if err != nil { return @@ -151,11 +153,7 @@ func CheckAndWarnHookManagers(ctx context.Context, w io.Writer, localDev, absolu // Best-effort: hook manager warnings are advisory, skip on resolution failure return } - huskySafe := false - if hooksDir, hooksErr := getHooksDirInPath(ctx, repoRoot); hooksErr == nil { - huskySafe = huskyUserHooksDir(hooksDir) != "" - } - warning := hookManagerWarning(managers, cmdPrefix, huskySafe) + warning := hookManagerWarning(managers, cmdPrefix, huskySafeInstall) if warning != "" { fmt.Fprintln(w) fmt.Fprint(w, warning) diff --git a/cmd/entire/cli/strategy/hook_managers_test.go b/cmd/entire/cli/strategy/hook_managers_test.go index d826aa2490..a38be9047b 100644 --- a/cmd/entire/cli/strategy/hook_managers_test.go +++ b/cmd/entire/cli/strategy/hook_managers_test.go @@ -462,7 +462,7 @@ func TestCheckAndWarnHookManagers_NoManagers(t *testing.T) { initHooksTestRepo(t) var buf bytes.Buffer - CheckAndWarnHookManagers(context.Background(), &buf, false, false) + CheckAndWarnHookManagers(context.Background(), &buf, false, false, false) if buf.Len() != 0 { t.Errorf("expected no output, got %q", buf.String()) @@ -488,10 +488,30 @@ func TestCheckAndWarnHookManagers_WithHusky(t *testing.T) { } var buf bytes.Buffer - CheckAndWarnHookManagers(context.Background(), &buf, false, false) + CheckAndWarnHookManagers(context.Background(), &buf, false, false, true) output := buf.String() if !strings.Contains(output, "Note: Husky detected") { t.Errorf("expected husky note output, got %q", output) } + if !strings.Contains(output, "never replaced with Entire hooks") { + t.Errorf("expected husky-safe wording, got %q", output) + } +} + +func TestCheckAndWarnHookManagers_HuskyUnsafeWarns(t *testing.T) { + // Needs t.Chdir (via initHooksTestRepo), cannot be parallel + tmpDir, _ := initHooksTestRepo(t) + + if err := os.MkdirAll(filepath.Join(tmpDir, ".husky"), 0o755); err != nil { + t.Fatalf("failed to create .husky/: %v", err) + } + + var buf bytes.Buffer + CheckAndWarnHookManagers(context.Background(), &buf, false, false, false) + + output := buf.String() + if !strings.Contains(output, "Warning: Husky detected") { + t.Errorf("expected husky warning when huskySafeInstall=false, got %q", output) + } } diff --git a/cmd/entire/cli/strategy/hooks.go b/cmd/entire/cli/strategy/hooks.go index 7bb40d044a..ba47ea691f 100644 --- a/cmd/entire/cli/strategy/hooks.go +++ b/cmd/entire/cli/strategy/hooks.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/settings" ) @@ -19,6 +20,10 @@ const entireHookMarker = "Entire CLI hooks" const backupSuffix = ".pre-entire" const chainComment = "# Chain: run pre-existing hook" + +// preEntireExcludePattern keeps hook backups out of accidental `git add -A` +// commits (critical for husky-safe installs into tracked user-hook dirs). +const preEntireExcludePattern = "**/*" + backupSuffix const missingEntireGitHookWarning = "[entire] Entire CLI is enabled but not installed or not on PATH. Skipping Entire Git hook; continuing. Installation guide: https://docs.entire.io/cli/installation#installation-methods" // localDevHookCmdPrefix is the command prefix used for git hooks in local @@ -139,11 +144,16 @@ func getHooksDirInPath(ctx context.Context, dir string) (string, error) { return filepath.Clean(hooksDir), nil } -// huskyForwardingStub is the husky v9 stub content written into core.hooksPath -// (`_/prepare-commit-msg`, etc.). Git executes these; they source `_/h`, which +// huskyForwardingStub matches the husky v9 stub written into core.hooksPath +// (`_/prepare-commit-msg`, etc.), modulo a trailing newline we keep for +// readability. Real husky omits that newline; both forms source `_/h`, which // runs the sibling user hook in the parent directory. const huskyForwardingStub = "#!/usr/bin/env sh\n. \"$(dirname \"$0\")/h\"\n" +// huskyStubDispatchMarker is the sourcing line husky stubs use to reach `_/h`. +// Detection requires this (not merely "exists and lacks Entire marker"). +const huskyStubDispatchMarker = `. "$(dirname "$0")/h"` + // huskyUserHooksDir returns the user-owned Husky hooks directory when hooksDir // is a regenerable husky `_` directory (core.hooksPath) AND husky's dispatcher // script (`_/h`) is present. Otherwise "". @@ -153,6 +163,10 @@ const huskyForwardingStub = "#!/usr/bin/env sh\n. \"$(dirname \"$0\")/h\"\n" // and are invoked by the stubs in `_` via `h` — those user scripts survive // husky reinstall (see https://github.com/entireio/cli/issues/784). // +// Hooks installed in the parent run through husky's dispatcher: HUSKY=0 skips +// them, ~/.config/husky/init.sh is sourced first, node_modules/.bin is +// prepended to PATH, and execution is under `sh -e`. +// // The dispatcher check is required: git only executes files under // core.hooksPath. Redirecting to the parent without husky's forwarding layout // would install hooks that never run. @@ -197,7 +211,9 @@ func IsGitHookInstalledInDir(ctx context.Context, repoDir string) bool { // isGitHookInstalledForHooksDir reports whether Entire's managed hooks are // installed in the effective install directory, and (for husky-safe installs) -// that forwarding stubs still exist under core.hooksPath so git can reach them. +// that each regenerable `_` stub still sources `_/h` (husky's forwarder). A +// non-Entire file that does not source `h` is treated as not installed so +// EnsureSetup can heal. func isGitHookInstalledForHooksDir(hooksDir string) bool { installDir := hookInstallDir(hooksDir) if !isGitHookInstalledInHooksDir(installDir) { @@ -209,15 +225,19 @@ func isGitHookInstalledForHooksDir(hooksDir string) bool { return true } -// huskyForwardingStubsPresent reports whether each managed hook has a -// non-Entire stub in the husky-owned hooks directory (so git → `_` → parent). +// huskyForwardingStubsPresent reports whether each managed hook under the +// husky-owned directory is a non-Entire stub that sources `_/h`. func huskyForwardingStubsPresent(hooksDir string) bool { for _, hook := range gitHookNames { data, err := os.ReadFile(filepath.Join(hooksDir, hook)) //nolint:gosec // path from constants if err != nil { return false } - if strings.Contains(string(data), entireHookMarker) { + content := string(data) + if strings.Contains(content, entireHookMarker) { + return false + } + if !strings.Contains(content, huskyStubDispatchMarker) { return false } } @@ -358,23 +378,33 @@ func isWindowsAbsoluteHookCommand(cmdPrefix string) bool { // When core.hooksPath is Husky's .husky/_ directory, hooks are written to the // parent `.husky/` user-hook directory (which husky's stubs invoke) rather than // into `_`, so `npm install` / `husky` prepare cannot clobber them mid-turn. -func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (int, error) { +// +// For husky-safe installs the parent user-hook directory is often tracked +// (unlike regenerable `_`). Existing non-Entire scripts there are renamed to +// `.pre-entire` and chained after Entire's wrapper. +// +// The second return value is true when install targeted a husky user-hook +// directory (parent of core.hooksPath=`_/`). Callers that print husky advisories +// should use that flag so the message cannot disagree with where hooks landed. +func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (int, bool, error) { hooksDir, err := GetHooksDir(ctx) if err != nil { - return 0, err + return 0, false, err } installDir := hookInstallDir(hooksDir) + huskySafe := installDir != hooksDir if err := os.MkdirAll(installDir, 0o755); err != nil { //nolint:gosec // Git hooks require executable permissions - return 0, fmt.Errorf("failed to create hooks directory: %w", err) + return 0, huskySafe, fmt.Errorf("failed to create hooks directory: %w", err) } cmdPrefix, err := hookCmdPrefix(localDev, absolutePath) if err != nil { - return 0, err + return 0, huskySafe, err } specs := buildHookSpecs(cmdPrefix) installedCount := 0 + backedUpUserHook := false for _, spec := range specs { hookPath := filepath.Join(installDir, spec.name) @@ -386,11 +416,13 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i if existingErr == nil && !strings.Contains(string(existing), entireHookMarker) { if !backupExists { if err := os.Rename(hookPath, backupPath); err != nil { - return installedCount, fmt.Errorf("failed to back up %s: %w", spec.name, err) + return installedCount, huskySafe, fmt.Errorf("failed to back up %s: %w", spec.name, err) } fmt.Fprintf(os.Stderr, "[entire] Backed up existing %s to %s%s\n", spec.name, spec.name, backupSuffix) + backedUpUserHook = true } else { fmt.Fprintf(os.Stderr, "[entire] Warning: replacing %s (backup %s%s already exists from a previous install)\n", spec.name, spec.name, backupSuffix) + backedUpUserHook = true } backupExists = true } @@ -403,7 +435,7 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i written, err := writeHookFile(hookPath, content) if err != nil { - return installedCount, fmt.Errorf("failed to install %s hook: %w", spec.name, err) + return installedCount, huskySafe, fmt.Errorf("failed to install %s hook: %w", spec.name, err) } if written { installedCount++ @@ -414,21 +446,73 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i // husky-owned `_` and ensure forwarding stubs exist so git can reach the // parent user hooks. Doing this after install avoids a window with zero // Entire hooks. - if installDir != hooksDir { + if huskySafe { if err := migrateEntireHooksFromHuskyOwnedDir(hooksDir); err != nil { - return installedCount, fmt.Errorf("failed to migrate hooks out of husky-owned directory: %w", err) + return installedCount, huskySafe, fmt.Errorf("failed to migrate hooks out of husky-owned directory: %w", err) + } + // Keep .pre-entire backups out of accidental commits when we shadow + // tracked user hooks under the husky parent directory. + if backedUpUserHook { + if exclErr := ensurePreEntireExcluded(ctx); exclErr != nil { + fmt.Fprintf(os.Stderr, "[entire] Warning: could not exclude %s backups from git: %v\n", backupSuffix, exclErr) + } + fmt.Fprintf(os.Stderr, "[entire] Note: existing hooks in %s were backed up to *%s and chained; commit the Entire wrappers intentionally and keep backups local\n", filepath.Base(installDir), backupSuffix) } } if !silent { - fmt.Println("✓ Installed git hooks (prepare-commit-msg, commit-msg, post-commit, pre-push)") + fmt.Println("✓ Installed git hooks (prepare-commit-msg, commit-msg, post-commit, post-rewrite, pre-push)") fmt.Println(" Hooks delegate to the current strategy at runtime") - if installDir != hooksDir { - fmt.Println(" Installed into .husky/ (survives husky/npm prepare)") + if huskySafe { + relInstall := installDir + if root, rootErr := paths.WorktreeRoot(ctx); rootErr == nil { + if rel, relErr := filepath.Rel(root, installDir); relErr == nil { + relInstall = rel + } + } + fmt.Printf(" Installed into %s/ (survives husky/npm prepare)\n", filepath.ToSlash(relInstall)) } } - return installedCount, nil + return installedCount, huskySafe, nil +} + +// ensurePreEntireExcluded appends preEntireExcludePattern to .git/info/exclude +// so husky-safe backups are not half-committed beside Entire wrappers. +func ensurePreEntireExcluded(ctx context.Context) error { + gitDir, err := GetGitDir(ctx) + if err != nil { + return err + } + infoDir := filepath.Join(gitDir, "info") + if err := os.MkdirAll(infoDir, 0o755); err != nil { //nolint:gosec // matches git's info dir mode + return fmt.Errorf("create git info dir: %w", err) + } + excludePath := filepath.Join(infoDir, "exclude") + existing, err := os.ReadFile(excludePath) //nolint:gosec // path from git dir + if err != nil && !errors.Is(err, os.ErrNotExist) { + return fmt.Errorf("read git exclude: %w", err) + } + content := string(existing) + for _, line := range strings.Split(content, "\n") { + if strings.TrimSpace(line) == preEntireExcludePattern { + return nil + } + } + var b strings.Builder + b.WriteString(content) + if len(content) > 0 && !strings.HasSuffix(content, "\n") { + b.WriteByte('\n') + } + if !strings.Contains(content, "# Entire CLI") { + b.WriteString("# Entire CLI\n") + } + b.WriteString(preEntireExcludePattern) + b.WriteByte('\n') + if err := os.WriteFile(excludePath, []byte(b.String()), 0o644); err != nil { //nolint:gosec // git exclude is not executable + return fmt.Errorf("write git exclude: %w", err) + } + return nil } // writeHookFile writes a hook file if it doesn't exist or has different content. @@ -451,8 +535,10 @@ func writeHookFile(path, content string) (bool, error) { // If a .pre-entire backup exists, it is restored. // Returns the number of hooks removed. // -// For Husky setups, removals target `.husky/` (user hooks). Any legacy Entire -// wrappers left in the regenerable `.husky/_` directory are also cleaned up. +// For Husky setups, removals target the user-hook directory. Legacy Entire +// wrappers in regenerable `_` are scrubbed (restore backup or replace with a +// stub) but missing stubs are not backfilled — disable must not recreate +// husky's regenerable layout. func RemoveGitHook(ctx context.Context) (int, error) { hooksDir, err := GetHooksDir(ctx) if err != nil { @@ -470,9 +556,11 @@ func RemoveGitHook(ctx context.Context) (int, error) { // `_`), still scrub Entire wrappers from the parent user-hook directory so // disable does not leave orphan Entire scripts behind. if installDir != hooksDir { - if cleanErr := migrateEntireHooksFromHuskyOwnedDir(hooksDir); cleanErr != nil { + scrubbed, cleanErr := scrubEntireHooksFromHuskyOwnedDir(hooksDir) + if cleanErr != nil { return removed, cleanErr } + removed += scrubbed } else if filepath.Base(hooksDir) == "_" { parentRemoved, parentErr := removeEntireHooksFromDir(filepath.Dir(hooksDir)) if parentErr != nil { @@ -488,6 +576,21 @@ func RemoveGitHook(ctx context.Context) (int, error) { // writing a standard husky forwarding stub when no backup exists. Also fills // in any missing managed stubs so git can reach parent user hooks. func migrateEntireHooksFromHuskyOwnedDir(hooksDir string) error { + _, err := rewriteHuskyOwnedHooks(hooksDir, true) + return err +} + +// scrubEntireHooksFromHuskyOwnedDir removes Entire wrappers from the husky-owned +// `_` directory without creating stubs for hooks that were already absent. +// Returns how many Entire wrappers were restored or replaced. +func scrubEntireHooksFromHuskyOwnedDir(hooksDir string) (int, error) { + return rewriteHuskyOwnedHooks(hooksDir, false) +} + +// rewriteHuskyOwnedHooks migrates (backfillMissing=true) or scrubs +// (backfillMissing=false) Entire wrappers under the husky-owned hooks directory. +func rewriteHuskyOwnedHooks(hooksDir string, backfillMissing bool) (int, error) { + changed := 0 for _, hook := range gitHookNames { hookPath := filepath.Join(hooksDir, hook) backupPath := hookPath + backupSuffix @@ -498,22 +601,30 @@ func migrateEntireHooksFromHuskyOwnedDir(hooksDir string) error { switch { case hookIsOurs && fileExists(backupPath): if err := os.Remove(hookPath); err != nil { - return fmt.Errorf("remove legacy %s: %w", hook, err) + return changed, fmt.Errorf("remove legacy %s: %w", hookPath, err) } if err := os.Rename(backupPath, hookPath); err != nil { - return fmt.Errorf("restore %s%s: %w", hook, backupSuffix, err) + return changed, fmt.Errorf("restore %s%s: %w", hookPath, backupSuffix, err) } + changed++ case hookIsOurs: if err := writeHookFileForced(hookPath, huskyForwardingStub); err != nil { - return fmt.Errorf("replace legacy %s with husky stub: %w", hook, err) + return changed, fmt.Errorf("replace legacy %s with husky stub: %w", hookPath, err) } + changed++ case errors.Is(err, os.ErrNotExist): + if !backfillMissing { + continue + } if err := writeHookFileForced(hookPath, huskyForwardingStub); err != nil { - return fmt.Errorf("write missing husky stub %s: %w", hook, err) + return changed, fmt.Errorf("write missing husky stub %s: %w", hookPath, err) } + changed++ + case err != nil: + return changed, fmt.Errorf("read husky-owned hook %s: %w", hookPath, err) } } - return nil + return changed, nil } // writeHookFileForced writes content to path unconditionally (used for husky stubs). @@ -568,6 +679,9 @@ func removeEntireHooksFromDir(dir string) (int, error) { // generateChainedContent appends a chain call to the base hook content, // so the pre-existing hook (backed up to .pre-entire) is called after our hook. +// +// Uses `[ -f ]` + `sh -e` (husky's pattern) rather than `[ -x ]` + direct exec: +// husky user hooks are often mode 0644 and are still run via `sh -e`. func generateChainedContent(baseContent, hookName string) string { if hookName == "post-rewrite" { return generatePostRewriteChainedContent(baseContent) @@ -575,8 +689,8 @@ func generateChainedContent(baseContent, hookName string) string { return baseContent + fmt.Sprintf(`%s _entire_hook_dir="$(dirname "$0")" -if [ -x "$_entire_hook_dir/%s%s" ]; then - "$_entire_hook_dir/%s%s" "$@" +if [ -f "$_entire_hook_dir/%s%s" ]; then + sh -e "$_entire_hook_dir/%s%s" "$@" fi `, chainComment, hookName, backupSuffix, hookName, backupSuffix) } @@ -597,8 +711,8 @@ trap 'rm -f "$_entire_stdin"' EXIT return replayPrefix + body + fmt.Sprintf(` %s _entire_hook_dir="$(dirname "$0")" -if [ -x "$_entire_hook_dir/post-rewrite%s" ]; then - "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" +if [ -f "$_entire_hook_dir/post-rewrite%s" ]; then + sh -e "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" fi `, chainComment, backupSuffix, backupSuffix) } diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index 426b644941..197d9f8fe5 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -319,7 +319,7 @@ func TestInstallGitHook_WorktreeInstallsInCommonHooks(t *testing.T) { t.Chdir(worktreeDir) paths.ClearWorktreeRootCache() - count, err := InstallGitHook(context.Background(), true, false, false) + count, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() in worktree failed: %v", err) } @@ -577,7 +577,7 @@ func TestInstallGitHook_Idempotent(t *testing.T) { _, hooksDir := initHooksTestRepo(t) // First install should install hooks - firstCount, err := InstallGitHook(context.Background(), true, false, false) + firstCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("First InstallGitHook() error = %v", err) } @@ -599,7 +599,7 @@ func TestInstallGitHook_Idempotent(t *testing.T) { } // Second install should return 0 (all hooks already up to date) - secondCount, err := InstallGitHook(context.Background(), true, false, false) + secondCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("Second InstallGitHook() error = %v", err) } @@ -623,7 +623,7 @@ func TestInstallGitHook_LocalDevCommandPrefix(t *testing.T) { _, hooksDir := initHooksTestRepo(t) // Install with localDev=true - count, err := InstallGitHook(context.Background(), true, true, false) + count, _, err := InstallGitHook(context.Background(), true, true, false) if err != nil { t.Fatalf("InstallGitHook(localDev=true) error = %v", err) } @@ -646,7 +646,7 @@ func TestInstallGitHook_LocalDevCommandPrefix(t *testing.T) { } // Reinstall with localDev=false — hooks should update to use "entire" prefix - count, err = InstallGitHook(context.Background(), true, false, false) + count, _, err = InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook(localDev=false) error = %v", err) } @@ -754,7 +754,7 @@ func TestInstallGitHook_AbsoluteGitHookPath(t *testing.T) { _, hooksDir := initHooksTestRepo(t) // Install with absolutePath=true - count, err := InstallGitHook(context.Background(), true, false, true) + count, _, err := InstallGitHook(context.Background(), true, false, true) if err != nil { t.Fatalf("InstallGitHook(absolutePath=true) error = %v", err) } @@ -869,7 +869,7 @@ func TestInstallGitHook_CoreHooksPathRelative(t *testing.T) { huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") seedHuskyOwnedDir(t, huskyOwnedDir) - count, err := InstallGitHook(context.Background(), true, false, false) + count, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -927,7 +927,7 @@ func TestInstallGitHook_HuskyPrepareDoesNotClobberUserHooks(t *testing.T) { huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") seedHuskyOwnedDir(t, huskyOwnedDir) - if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { t.Fatalf("InstallGitHook() error = %v", err) } if !IsGitHookInstalledInDir(context.Background(), tmpDir) { @@ -981,7 +981,7 @@ func TestInstallGitHook_MigratesLegacyEntireHooksFromHuskyOwnedDir(t *testing.T) t.Fatalf("write husky dispatcher: %v", err) } - if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1031,7 +1031,7 @@ func TestInstallGitHook_MigratesLegacyEntireHooksWithoutBackupWritesStub(t *test } } - if _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { t.Fatalf("InstallGitHook() error = %v", err) } if !IsGitHookInstalledInDir(context.Background(), tmpDir) { @@ -1077,7 +1077,7 @@ func TestRemoveGitHook_CoreHooksPathRelative(t *testing.T) { } seedHuskyOwnedDir(t, filepath.Join(tmpDir, ".husky", "_")) - installCount, err := InstallGitHook(context.Background(), true, false, false) + installCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1173,7 +1173,7 @@ func TestRemoveGitHook_RemovesInstalledHooks(t *testing.T) { tmpDir, _ := initHooksTestRepo(t) // Install hooks first - installCount, err := InstallGitHook(context.Background(), true, false, false) + installCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1273,7 +1273,7 @@ func TestInstallGitHook_BacksUpCustomHook(t *testing.T) { t.Fatalf("failed to create custom hook: %v", err) } - count, err := InstallGitHook(context.Background(), true, false, false) + count, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1320,7 +1320,7 @@ func TestManagedGitHookNames_IncludesPostRewrite(t *testing.T) { func TestInstallGitHook_InstallsPostRewrite(t *testing.T) { _, hooksDir := initHooksTestRepo(t) - count, err := InstallGitHook(context.Background(), true, false, false) + count, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1515,7 +1515,7 @@ func TestInstallGitHook_DoesNotOverwriteExistingBackup(t *testing.T) { t.Fatalf("failed to create second custom hook: %v", err) } - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1551,7 +1551,7 @@ func TestInstallGitHook_IdempotentWithChaining(t *testing.T) { t.Fatalf("failed to create custom hook: %v", err) } - firstCount, err := InstallGitHook(context.Background(), true, false, false) + firstCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("first InstallGitHook() error = %v", err) } @@ -1560,7 +1560,7 @@ func TestInstallGitHook_IdempotentWithChaining(t *testing.T) { } // Re-install should return 0 (idempotent) - secondCount, err := InstallGitHook(context.Background(), true, false, false) + secondCount, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("second InstallGitHook() error = %v", err) } @@ -1572,7 +1572,7 @@ func TestInstallGitHook_IdempotentWithChaining(t *testing.T) { func TestInstallGitHook_NoBackupWhenNoExistingHook(t *testing.T) { _, hooksDir := initHooksTestRepo(t) - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1610,7 +1610,7 @@ func TestInstallGitHook_MixedHooks(t *testing.T) { } } - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1659,7 +1659,7 @@ func TestRemoveGitHook_RestoresBackup(t *testing.T) { t.Fatalf("failed to create custom hook: %v", err) } - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1698,7 +1698,7 @@ func TestRemoveGitHook_RestoresBackupWhenHookAlreadyGone(t *testing.T) { t.Fatalf("failed to create custom hook: %v", err) } - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1750,16 +1750,16 @@ func TestGenerateChainedContent(t *testing.T) { t.Error("chained content should resolve hook directory from $0") } - // Should check executable permission on backup - expectedCheck := `[ -x "$_entire_hook_dir/pre-push` + backupSuffix + `" ]` + // Should check file presence (not -x) so mode-0644 husky hooks still chain + expectedCheck := `[ -f "$_entire_hook_dir/pre-push` + backupSuffix + `" ]` if !strings.Contains(result, expectedCheck) { - t.Errorf("chained content should check -x on backup, got:\n%s", result) + t.Errorf("chained content should check -f on backup, got:\n%s", result) } - // Should forward all arguments with "$@" - expectedExec := `"$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` + // Should run backup via sh -e (husky-compatible) with all arguments + expectedExec := `sh -e "$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` if !strings.Contains(result, expectedExec) { - t.Errorf("chained content should execute backup with $@, got:\n%s", result) + t.Errorf("chained content should sh -e backup with $@, got:\n%s", result) } } @@ -1778,8 +1778,8 @@ func TestGenerateChainedContent_PostRewritePreservesStdinForBackup(t *testing.T) if !strings.Contains(result, `entire hooks git post-rewrite "$1" < "$_entire_stdin" 2>/dev/null || true`) { t.Fatalf("post-rewrite chained content should replay stdin into Entire handler, got:\n%s", result) } - if !strings.Contains(result, `"$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { - t.Fatalf("post-rewrite chained content should replay stdin into backup hook, got:\n%s", result) + if !strings.Contains(result, `sh -e "$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { + t.Fatalf("post-rewrite chained content should sh -e replay stdin into backup hook, got:\n%s", result) } } @@ -1794,7 +1794,7 @@ func TestInstallGitHook_InstallRemoveReinstall(t *testing.T) { } // Install: should back up and chain - count, err := InstallGitHook(context.Background(), true, false, false) + count, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("first install error: %v", err) } @@ -1823,7 +1823,7 @@ func TestInstallGitHook_InstallRemoveReinstall(t *testing.T) { } // Reinstall: should back up again and chain - count, err = InstallGitHook(context.Background(), true, false, false) + count, _, err = InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("reinstall error: %v", err) } @@ -1856,7 +1856,7 @@ func TestRemoveGitHook_DoesNotOverwriteReplacedHook(t *testing.T) { } // entire enable: backs up A, installs our hook with chain - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1897,7 +1897,7 @@ func TestRemoveGitHook_PermissionDenied(t *testing.T) { tmpDir, _ := initHooksTestRepo(t) // Install hooks first - _, err := InstallGitHook(context.Background(), true, false, false) + _, _, err := InstallGitHook(context.Background(), true, false, false) if err != nil { t.Fatalf("InstallGitHook() error = %v", err) } @@ -1976,3 +1976,308 @@ func TestResolveHookExePath(t *testing.T) { } }) } + +func TestGenerateChainedContent_RunsMode0644Backup(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + backupName := "pre-push" + backupSuffix + backupPath := filepath.Join(tmp, backupName) + sentinel := filepath.Join(tmp, "ran") + backupBody := "#!/bin/sh\ntouch \"" + sentinel + "\"\n" + if err := os.WriteFile(backupPath, []byte(backupBody), 0o644); err != nil { + t.Fatalf("write backup: %v", err) + } + + base := "#!/bin/sh\n# Entire CLI hooks\ntrue\n" + script := generateChainedContent(base, "pre-push") + scriptPath := filepath.Join(tmp, "pre-push") + if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil { + t.Fatalf("write chained hook: %v", err) + } + + cmd := exec.CommandContext(context.Background(), "sh", scriptPath) + cmd.Dir = tmp + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("chained hook failed: %v\n%s", err, out) + } + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("mode-0644 backup should have run via sh -e: %v", err) + } +} + +func TestMigrateEntireHooksFromHuskyOwnedDir_ReadErrorIsLoud(t *testing.T) { + t.Parallel() + + owned := t.TempDir() + // Make prepare-commit-msg a directory so ReadFile fails with a non-ErrNotExist error. + blocked := filepath.Join(owned, "prepare-commit-msg") + if err := os.Mkdir(blocked, 0o755); err != nil { + t.Fatalf("mkdir blocked hook path: %v", err) + } + err := migrateEntireHooksFromHuskyOwnedDir(owned) + if err == nil { + t.Fatal("expected loud error when husky-owned hook path is unreadable") + } + if !strings.Contains(err.Error(), blocked) { + t.Errorf("error should include path %q, got: %v", blocked, err) + } +} + +func TestHuskyForwardingStubsPresent_RequiresDispatchSource(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("hooks should be installed") + } + + // Replace one stub with a non-Entire file that does not source `_/h`. + badStub := filepath.Join(huskyOwnedDir, "pre-push") + if err := os.WriteFile(badStub, []byte("#!/bin/sh\necho noop\n"), 0o755); err != nil { + t.Fatalf("write bad stub: %v", err) + } + if IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("missing husky dispatch source should report hooks not installed so EnsureSetup can heal") + } +} + +func TestRemoveGitHook_DoesNotBackfillMissingHuskyStubs(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + + missing := filepath.Join(huskyOwnedDir, "pre-push") + if err := os.Remove(missing); err != nil { + t.Fatalf("remove stub: %v", err) + } + + removed, err := RemoveGitHook(context.Background()) + if err != nil { + t.Fatalf("RemoveGitHook() error = %v", err) + } + if removed == 0 { + t.Fatal("RemoveGitHook() should count removed Entire user hooks") + } + if _, err := os.Stat(missing); !os.IsNotExist(err) { + t.Fatal("disable must not backfill missing husky stubs under `_`") + } +} + +func TestInstallGitHook_HealsMissingHuskyStub(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + missing := filepath.Join(huskyOwnedDir, "pre-push") + if err := os.Remove(missing); err != nil { + t.Fatalf("remove stub: %v", err) + } + if IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("missing stub should make IsGitHookInstalledInDir false") + } + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("reinstall error: %v", err) + } + data, err := os.ReadFile(missing) + if err != nil { + t.Fatalf("stub should be healed: %v", err) + } + if string(data) != huskyForwardingStub { + t.Errorf("healed stub = %q, want huskyForwardingStub", data) + } + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("hooks should be installed after stub heal") + } +} + +func TestRemoveGitHook_DispatcherDeletedScrubsParentUserHooks(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } + userHook := filepath.Join(tmpDir, ".husky", "prepare-commit-msg") + if data, err := os.ReadFile(userHook); err != nil || !strings.Contains(string(data), entireHookMarker) { + t.Fatalf("expected Entire user hook before dispatcher delete: %v", err) + } + + // Delete dispatcher so hookInstallDir falls back to `_` and removal + // scrubs orphan Entire wrappers from the parent user-hook directory. + if err := os.Remove(filepath.Join(huskyOwnedDir, "h")); err != nil { + t.Fatalf("remove dispatcher: %v", err) + } + + removed, err := RemoveGitHook(context.Background()) + if err != nil { + t.Fatalf("RemoveGitHook() error = %v", err) + } + if removed == 0 { + t.Fatal("should scrub orphan Entire wrappers from parent .husky/") + } + if data, err := os.ReadFile(userHook); err == nil && strings.Contains(string(data), entireHookMarker) { + t.Fatal("orphan Entire user hook must be removed when dispatcher is gone") + } +} + +func TestMigrateEntireHooksFromHuskyOwnedDir_MixedLegacyState(t *testing.T) { + t.Parallel() + + owned := t.TempDir() + withBackup := "prepare-commit-msg" + withoutBackup := "commit-msg" + alreadyStub := "post-commit" + missing := "pre-push" + + legacy := "#!/bin/sh\n# " + entireHookMarker + "\nentire hooks git x\n" + if err := os.WriteFile(filepath.Join(owned, withBackup), []byte(legacy), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(owned, withBackup+backupSuffix), []byte(huskyForwardingStub), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(owned, withoutBackup), []byte(legacy), 0o755); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(owned, alreadyStub), []byte(huskyForwardingStub), 0o755); err != nil { + t.Fatal(err) + } + // post-rewrite left as another missing hook besides pre-push + _ = missing + + if err := migrateEntireHooksFromHuskyOwnedDir(owned); err != nil { + t.Fatalf("migrate mixed state: %v", err) + } + + restored, err := os.ReadFile(filepath.Join(owned, withBackup)) + if err != nil || string(restored) != huskyForwardingStub { + t.Fatalf("with-backup should restore stub, got %q err=%v", restored, err) + } + replaced, err := os.ReadFile(filepath.Join(owned, withoutBackup)) + if err != nil || string(replaced) != huskyForwardingStub { + t.Fatalf("without-backup should become stub, got %q err=%v", replaced, err) + } + unchanged, err := os.ReadFile(filepath.Join(owned, alreadyStub)) + if err != nil || string(unchanged) != huskyForwardingStub { + t.Fatalf("already-stub should remain, got %q err=%v", unchanged, err) + } + for _, hook := range []string{missing, "post-rewrite"} { + healed, err := os.ReadFile(filepath.Join(owned, hook)) + if err != nil || string(healed) != huskyForwardingStub { + t.Fatalf("missing %s should be backfilled, got %q err=%v", hook, healed, err) + } + } +} + +func TestInstallGitHook_ChainsPreExistingHuskyUserHook(t *testing.T) { + tmpDir, _ := initHooksTestRepo(t) + ctx := context.Background() + + cmd := exec.CommandContext(ctx, "git", "config", "core.hooksPath", ".husky/_") + cmd.Dir = tmpDir + if err := cmd.Run(); err != nil { + t.Fatalf("failed to set core.hooksPath: %v", err) + } + huskyOwnedDir := filepath.Join(tmpDir, ".husky", "_") + seedHuskyOwnedDir(t, huskyOwnedDir) + + userHook := filepath.Join(tmpDir, ".husky", "prepare-commit-msg") + custom := "#!/bin/sh\necho 'preexisting user hook'\n" + if err := os.MkdirAll(filepath.Dir(userHook), 0o755); err != nil { + t.Fatalf("mkdir .husky: %v", err) + } + if err := os.WriteFile(userHook, []byte(custom), 0o644); err != nil { + t.Fatalf("write preexisting user hook: %v", err) + } + + if _, huskySafe, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("InstallGitHook() error = %v", err) + } else if !huskySafe { + t.Fatal("expected husky-safe install") + } + + backupPath := userHook + backupSuffix + data, err := os.ReadFile(backupPath) + if err != nil { + t.Fatalf("expected .pre-entire backup of tracked user hook: %v", err) + } + if string(data) != custom { + t.Errorf("backup = %q, want %q", data, custom) + } + installed, err := os.ReadFile(userHook) + if err != nil { + t.Fatalf("read installed user hook: %v", err) + } + content := string(installed) + if !strings.Contains(content, entireHookMarker) { + t.Error("installed hook should contain Entire marker") + } + if !strings.Contains(content, chainComment) || !strings.Contains(content, `sh -e`) { + t.Errorf("installed hook should chain preexisting backup via sh -e, got:\n%s", content) + } + + removed, err := RemoveGitHook(context.Background()) + if err != nil { + t.Fatalf("RemoveGitHook() error = %v", err) + } + if removed == 0 { + t.Fatal("RemoveGitHook() should remove Entire wrappers") + } + restored, err := os.ReadFile(userHook) + if err != nil { + t.Fatalf("expected restored user hook: %v", err) + } + if string(restored) != custom { + t.Errorf("restored hook = %q, want %q", restored, custom) + } + + excludePath := filepath.Join(tmpDir, ".git", "info", "exclude") + excludeData, err := os.ReadFile(excludePath) + if err != nil { + t.Fatalf("expected .git/info/exclude after husky-safe backup: %v", err) + } + if !strings.Contains(string(excludeData), preEntireExcludePattern) { + t.Errorf("exclude should contain %q, got %q", preEntireExcludePattern, excludeData) + } +} From 23b40e2176ab8b95a44a3d4d92545dae94a55d9c Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:19:33 -0400 Subject: [PATCH 3/6] fix(hooks): fix husky chain tests after sh -e switch Keep real sh on PATH in the missing-entire chain test, and drop the CWD-based RemoveGitHook assertion from the parallel husky integration test (restore is covered in the strategy unit suite). Co-authored-by: Cursor --- .../cli/integration_test/hook_overwrite_husky_test.go | 9 +++------ cmd/entire/cli/strategy/hooks_test.go | 4 +++- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go index f621af04b1..92f60f666f 100644 --- a/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go +++ b/cmd/entire/cli/integration_test/hook_overwrite_husky_test.go @@ -182,10 +182,7 @@ func TestHusky_PreExistingUserHook_BackedUpAndChained(t *testing.T) { _, err = os.Stat(sentinel) require.NoError(t, err, "mode-0644 preexisting user hook must run via chain") - removed, err := strategy.RemoveGitHook(t.Context()) - require.NoError(t, err) - require.Greater(t, removed, 0) - restored, err := os.ReadFile(userHook) - require.NoError(t, err) - require.Equal(t, custom, string(restored)) + // Byte-for-byte restore on RemoveGitHook is covered by + // TestInstallGitHook_ChainsPreExistingHuskyUserHook (strategy package; + // GetHooksDir is CWD-based and cannot be used from parallel TestEnv). } diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index 197d9f8fe5..262cbb5efc 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -1455,7 +1455,9 @@ func TestGitHookCommitMsg_MissingEntireStillRunsChainedHook(t *testing.T) { } cmd := exec.CommandContext(context.Background(), shPath, hookPath, msgFile) - cmd.Env = envWithPath(binDir) + // Keep real `sh` on PATH: chained backups run via `sh -e` (husky semantics). + // Fake dirname still wins so $_entire_hook_dir resolves inside tempDir. + cmd.Env = envWithPath(binDir + string(os.PathListSeparator) + filepath.Dir(shPath)) output, err := cmd.CombinedOutput() if err != nil { t.Fatalf("chained commit-msg hook should allow commit when entire is missing: %v\n%s", err, output) From e78e2c1b149018d0877875cd63f02f799f77e2f7 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Tue, 21 Jul 2026 20:24:56 -0400 Subject: [PATCH 4/6] fix(hooks): preserve shebang when chaining executable backups Co-authored-by: Cursor --- cmd/entire/cli/strategy/hooks.go | 17 +++++--- cmd/entire/cli/strategy/hooks_test.go | 62 ++++++++++++++++++++++----- 2 files changed, 63 insertions(+), 16 deletions(-) diff --git a/cmd/entire/cli/strategy/hooks.go b/cmd/entire/cli/strategy/hooks.go index ba47ea691f..1411a786cc 100644 --- a/cmd/entire/cli/strategy/hooks.go +++ b/cmd/entire/cli/strategy/hooks.go @@ -680,8 +680,9 @@ func removeEntireHooksFromDir(dir string) (int, error) { // generateChainedContent appends a chain call to the base hook content, // so the pre-existing hook (backed up to .pre-entire) is called after our hook. // -// Uses `[ -f ]` + `sh -e` (husky's pattern) rather than `[ -x ]` + direct exec: -// husky user hooks are often mode 0644 and are still run via `sh -e`. +// Executable backups are run directly so shebang interpreters (Python, Node, +// …) keep working. Non-executable backups (common for husky mode-0644 hooks) +// fall back to `sh -e`, matching husky's own runner. func generateChainedContent(baseContent, hookName string) string { if hookName == "post-rewrite" { return generatePostRewriteChainedContent(baseContent) @@ -689,10 +690,12 @@ func generateChainedContent(baseContent, hookName string) string { return baseContent + fmt.Sprintf(`%s _entire_hook_dir="$(dirname "$0")" -if [ -f "$_entire_hook_dir/%s%s" ]; then +if [ -x "$_entire_hook_dir/%s%s" ]; then + "$_entire_hook_dir/%s%s" "$@" +elif [ -f "$_entire_hook_dir/%s%s" ]; then sh -e "$_entire_hook_dir/%s%s" "$@" fi -`, chainComment, hookName, backupSuffix, hookName, backupSuffix) +`, chainComment, hookName, backupSuffix, hookName, backupSuffix, hookName, backupSuffix, hookName, backupSuffix) } func generatePostRewriteChainedContent(baseContent string) string { @@ -711,10 +714,12 @@ trap 'rm -f "$_entire_stdin"' EXIT return replayPrefix + body + fmt.Sprintf(` %s _entire_hook_dir="$(dirname "$0")" -if [ -f "$_entire_hook_dir/post-rewrite%s" ]; then +if [ -x "$_entire_hook_dir/post-rewrite%s" ]; then + "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" +elif [ -f "$_entire_hook_dir/post-rewrite%s" ]; then sh -e "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" fi -`, chainComment, backupSuffix, backupSuffix) +`, chainComment, backupSuffix, backupSuffix, backupSuffix, backupSuffix) } // hookCmdPrefix returns the command prefix for hook scripts and warning messages. diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index 262cbb5efc..309769b347 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -1752,16 +1752,22 @@ func TestGenerateChainedContent(t *testing.T) { t.Error("chained content should resolve hook directory from $0") } - // Should check file presence (not -x) so mode-0644 husky hooks still chain - expectedCheck := `[ -f "$_entire_hook_dir/pre-push` + backupSuffix + `" ]` - if !strings.Contains(result, expectedCheck) { - t.Errorf("chained content should check -f on backup, got:\n%s", result) + // Executable backups run directly (preserve shebang); non-exec fall back to sh -e. + expectedExecCheck := `[ -x "$_entire_hook_dir/pre-push` + backupSuffix + `" ]` + if !strings.Contains(result, expectedExecCheck) { + t.Errorf("chained content should check -x on backup, got:\n%s", result) } - - // Should run backup via sh -e (husky-compatible) with all arguments - expectedExec := `sh -e "$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` - if !strings.Contains(result, expectedExec) { - t.Errorf("chained content should sh -e backup with $@, got:\n%s", result) + expectedDirect := `"$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` + if !strings.Contains(result, expectedDirect) { + t.Errorf("chained content should direct-exec executable backup, got:\n%s", result) + } + expectedFileCheck := `elif [ -f "$_entire_hook_dir/pre-push` + backupSuffix + `" ]` + if !strings.Contains(result, expectedFileCheck) { + t.Errorf("chained content should fall back to -f for non-exec backups, got:\n%s", result) + } + expectedSh := `sh -e "$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` + if !strings.Contains(result, expectedSh) { + t.Errorf("chained content should sh -e non-exec backup with $@, got:\n%s", result) } } @@ -1780,8 +1786,14 @@ func TestGenerateChainedContent_PostRewritePreservesStdinForBackup(t *testing.T) if !strings.Contains(result, `entire hooks git post-rewrite "$1" < "$_entire_stdin" 2>/dev/null || true`) { t.Fatalf("post-rewrite chained content should replay stdin into Entire handler, got:\n%s", result) } + if !strings.Contains(result, `[ -x "$_entire_hook_dir/post-rewrite`+backupSuffix+`" ]`) { + t.Fatalf("post-rewrite chained content should check -x on backup, got:\n%s", result) + } + if !strings.Contains(result, `"$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { + t.Fatalf("post-rewrite chained content should direct-exec executable backup with stdin, got:\n%s", result) + } if !strings.Contains(result, `sh -e "$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { - t.Fatalf("post-rewrite chained content should sh -e replay stdin into backup hook, got:\n%s", result) + t.Fatalf("post-rewrite chained content should sh -e replay stdin into non-exec backup, got:\n%s", result) } } @@ -2008,6 +2020,36 @@ func TestGenerateChainedContent_RunsMode0644Backup(t *testing.T) { } } +func TestGenerateChainedContent_RunsExecutableShebangBackup(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + backupName := "pre-push" + backupSuffix + backupPath := filepath.Join(tmp, backupName) + sentinel := filepath.Join(tmp, "ran") + // Non-shell shebang: must be direct-exec'd, not forced through `sh -e`. + backupBody := "#!/usr/bin/env python3\nopen(r'" + sentinel + "', 'w').close()\n" + if err := os.WriteFile(backupPath, []byte(backupBody), 0o755); err != nil { + t.Fatalf("write backup: %v", err) + } + + base := "#!/bin/sh\n# Entire CLI hooks\ntrue\n" + script := generateChainedContent(base, "pre-push") + scriptPath := filepath.Join(tmp, "pre-push") + if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil { + t.Fatalf("write chained hook: %v", err) + } + + cmd := exec.CommandContext(context.Background(), "sh", scriptPath) + cmd.Dir = tmp + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("chained hook failed: %v\n%s", err, out) + } + if _, err := os.Stat(sentinel); err != nil { + t.Fatalf("executable shebang backup should have run via direct exec: %v", err) + } +} + func TestMigrateEntireHooksFromHuskyOwnedDir_ReadErrorIsLoud(t *testing.T) { t.Parallel() From 0c5e377be92945a66616fc1e2b62e7a397d18840 Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:33:29 -0400 Subject: [PATCH 5/6] fix(hooks): heal husky stubs and common-dir info/exclude Co-authored-by: Cursor --- cmd/entire/cli/strategy/hooks.go | 67 +++++++++++++++++++++++---- cmd/entire/cli/strategy/hooks_test.go | 67 +++++++++++++++++++++++++++ 2 files changed, 126 insertions(+), 8 deletions(-) diff --git a/cmd/entire/cli/strategy/hooks.go b/cmd/entire/cli/strategy/hooks.go index 1411a786cc..500dc15f79 100644 --- a/cmd/entire/cli/strategy/hooks.go +++ b/cmd/entire/cli/strategy/hooks.go @@ -451,11 +451,14 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i return installedCount, huskySafe, fmt.Errorf("failed to migrate hooks out of husky-owned directory: %w", err) } // Keep .pre-entire backups out of accidental commits when we shadow - // tracked user hooks under the husky parent directory. - if backedUpUserHook { + // tracked user hooks under the husky parent directory. Heal exclude + // whenever any backup is present (not only on this invocation). + if backedUpUserHook || huskyUserDirHasPreEntireBackups(installDir) { if exclErr := ensurePreEntireExcluded(ctx); exclErr != nil { fmt.Fprintf(os.Stderr, "[entire] Warning: could not exclude %s backups from git: %v\n", backupSuffix, exclErr) } + } + if backedUpUserHook { fmt.Fprintf(os.Stderr, "[entire] Note: existing hooks in %s were backed up to *%s and chained; commit the Entire wrappers intentionally and keep backups local\n", filepath.Base(installDir), backupSuffix) } } @@ -477,19 +480,20 @@ func InstallGitHook(ctx context.Context, silent, localDev, absolutePath bool) (i return installedCount, huskySafe, nil } -// ensurePreEntireExcluded appends preEntireExcludePattern to .git/info/exclude -// so husky-safe backups are not half-committed beside Entire wrappers. +// ensurePreEntireExcluded appends preEntireExcludePattern to info/exclude in the +// shared git common dir so husky-safe backups are not half-committed beside +// Entire wrappers. Uses `git rev-parse --git-path info/exclude` so linked +// worktrees write into the common exclude (not .git/worktrees//info/). func ensurePreEntireExcluded(ctx context.Context) error { - gitDir, err := GetGitDir(ctx) + excludePath, err := gitInfoExcludePath(ctx) if err != nil { return err } - infoDir := filepath.Join(gitDir, "info") + infoDir := filepath.Dir(excludePath) if err := os.MkdirAll(infoDir, 0o755); err != nil { //nolint:gosec // matches git's info dir mode return fmt.Errorf("create git info dir: %w", err) } - excludePath := filepath.Join(infoDir, "exclude") - existing, err := os.ReadFile(excludePath) //nolint:gosec // path from git dir + existing, err := os.ReadFile(excludePath) //nolint:gosec // path from git if err != nil && !errors.Is(err, os.ErrNotExist) { return fmt.Errorf("read git exclude: %w", err) } @@ -515,6 +519,38 @@ func ensurePreEntireExcluded(ctx context.Context) error { return nil } +// gitInfoExcludePath resolves the repo's info/exclude path via git so linked +// worktrees share the common-dir exclude file. +func gitInfoExcludePath(ctx context.Context) (string, error) { + cmd := exec.CommandContext(ctx, "git", "rev-parse", "--git-path", "info/exclude") + cmd.Dir = "." + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("resolve git info/exclude path: %w", err) + } + path := strings.TrimSpace(string(out)) + if path == "" { + return "", errors.New("resolve git info/exclude path: empty") + } + if !filepath.IsAbs(path) { + abs, absErr := filepath.Abs(path) + if absErr != nil { + return "", fmt.Errorf("absolutize info/exclude path: %w", absErr) + } + path = abs + } + return filepath.Clean(path), nil +} + +func huskyUserDirHasPreEntireBackups(installDir string) bool { + for _, hook := range gitHookNames { + if fileExists(filepath.Join(installDir, hook+backupSuffix)) { + return true + } + } + return false +} + // writeHookFile writes a hook file if it doesn't exist or has different content. // Returns true if the file was written, false if it already had the same content. func writeHookFile(path, content string) (bool, error) { @@ -589,6 +625,9 @@ func scrubEntireHooksFromHuskyOwnedDir(hooksDir string) (int, error) { // rewriteHuskyOwnedHooks migrates (backfillMissing=true) or scrubs // (backfillMissing=false) Entire wrappers under the husky-owned hooks directory. +// When backfillMissing is true, non-forwarding files in `_` are replaced with +// huskyForwardingStub so EnsureSetup can heal broken stubs (IsGitHookInstalled +// treats those as not installed). func rewriteHuskyOwnedHooks(hooksDir string, backfillMissing bool) (int, error) { changed := 0 for _, hook := range gitHookNames { @@ -622,6 +661,18 @@ func rewriteHuskyOwnedHooks(hooksDir string, backfillMissing bool) (int, error) changed++ case err != nil: return changed, fmt.Errorf("read husky-owned hook %s: %w", hookPath, err) + default: + // Existing non-Entire file under husky-owned `_`. + if !backfillMissing { + continue + } + if strings.Contains(string(data), huskyStubDispatchMarker) { + continue // already a valid forwarding stub + } + if err := writeHookFileForced(hookPath, huskyForwardingStub); err != nil { + return changed, fmt.Errorf("replace non-forwarding husky stub %s: %w", hookPath, err) + } + changed++ } } return changed, nil diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index 309769b347..9bcd1cf357 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -2095,6 +2095,21 @@ func TestHuskyForwardingStubsPresent_RequiresDispatchSource(t *testing.T) { if IsGitHookInstalledInDir(context.Background(), tmpDir) { t.Fatal("missing husky dispatch source should report hooks not installed so EnsureSetup can heal") } + + // Reinstall must heal the non-forwarding stub so IsGitHookInstalled recovers. + if _, _, err := InstallGitHook(context.Background(), true, false, false); err != nil { + t.Fatalf("reinstall to heal stub: %v", err) + } + if !IsGitHookInstalledInDir(context.Background(), tmpDir) { + t.Fatal("reinstall should replace non-forwarding husky stubs and report installed") + } + healed, err := os.ReadFile(badStub) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(healed), huskyStubDispatchMarker) { + t.Fatalf("healed stub should source _/h, got:\n%s", healed) + } } func TestRemoveGitHook_DoesNotBackfillMissingHuskyStubs(t *testing.T) { @@ -2325,3 +2340,55 @@ func TestInstallGitHook_ChainsPreExistingHuskyUserHook(t *testing.T) { t.Errorf("exclude should contain %q, got %q", preEntireExcludePattern, excludeData) } } + +func TestEnsurePreEntireExcluded_LinkedWorktreeUsesCommonExclude(t *testing.T) { + mainRepo, worktreeDir := initHooksWorktreeRepo(t) + t.Chdir(worktreeDir) + paths.ClearWorktreeRootCache() + + // Create a backup path that check-ignore will evaluate from the worktree. + huskyParent := filepath.Join(mainRepo, ".husky") + if err := os.MkdirAll(huskyParent, 0o755); err != nil { + t.Fatal(err) + } + backupRel := filepath.ToSlash(filepath.Join(".husky", "prepare-commit-msg"+backupSuffix)) + backupAbs := filepath.Join(mainRepo, ".husky", "prepare-commit-msg"+backupSuffix) + if err := os.WriteFile(backupAbs, []byte("#!/bin/sh\necho backup\n"), 0o644); err != nil { + t.Fatal(err) + } + + if err := ensurePreEntireExcluded(context.Background()); err != nil { + t.Fatalf("ensurePreEntireExcluded from linked worktree: %v", err) + } + + commonExclude := filepath.Join(mainRepo, ".git", "info", "exclude") + data, err := os.ReadFile(commonExclude) + if err != nil { + t.Fatalf("common-dir exclude missing: %v", err) + } + if !strings.Contains(string(data), preEntireExcludePattern) { + t.Fatalf("common exclude should contain %q, got %q", preEntireExcludePattern, data) + } + + resolved, err := gitInfoExcludePath(context.Background()) + if err != nil { + t.Fatalf("gitInfoExcludePath: %v", err) + } + commonResolved, err := filepath.EvalSymlinks(commonExclude) + if err != nil { + commonResolved = filepath.Clean(commonExclude) + } + resolvedEval, err := filepath.EvalSymlinks(resolved) + if err != nil { + resolvedEval = filepath.Clean(resolved) + } + if resolvedEval != commonResolved { + t.Fatalf("gitInfoExcludePath from worktree = %q, want common exclude %q", resolvedEval, commonResolved) + } + + check := exec.CommandContext(context.Background(), "git", "check-ignore", "-q", backupRel) + check.Dir = worktreeDir + if err := check.Run(); err != nil { + t.Fatalf("git check-ignore from linked worktree should match %s: %v", backupRel, err) + } +} From 9e32fef4966834703b2541a1c0839429a77a3e8a Mon Sep 17 00:00:00 2001 From: Suhaan Thayyil <257360244+suhaanthayyil@users.noreply.github.com> Date: Thu, 23 Jul 2026 12:57:41 -0400 Subject: [PATCH 6/6] fix(hooks): preserve Entire exit status when chaining backups Co-authored-by: Cursor --- cmd/entire/cli/strategy/hooks.go | 18 ++++++++--- cmd/entire/cli/strategy/hooks_test.go | 44 ++++++++++++++++++++++++--- 2 files changed, 53 insertions(+), 9 deletions(-) diff --git a/cmd/entire/cli/strategy/hooks.go b/cmd/entire/cli/strategy/hooks.go index 500dc15f79..6690ca59fe 100644 --- a/cmd/entire/cli/strategy/hooks.go +++ b/cmd/entire/cli/strategy/hooks.go @@ -734,18 +734,24 @@ func removeEntireHooksFromDir(dir string) (int, error) { // Executable backups are run directly so shebang interpreters (Python, Node, // …) keep working. Non-executable backups (common for husky mode-0644 hooks) // fall back to `sh -e`, matching husky's own runner. +// +// Entire's exit status is preserved: a successful backup must not mask a +// failing pre-push (OPF abort). A failing backup still fails the hook. func generateChainedContent(baseContent, hookName string) string { if hookName == "post-rewrite" { return generatePostRewriteChainedContent(baseContent) } - return baseContent + fmt.Sprintf(`%s + return baseContent + fmt.Sprintf(` +_entire_status=$? +%s _entire_hook_dir="$(dirname "$0")" if [ -x "$_entire_hook_dir/%s%s" ]; then - "$_entire_hook_dir/%s%s" "$@" + "$_entire_hook_dir/%s%s" "$@" || exit $? elif [ -f "$_entire_hook_dir/%s%s" ]; then - sh -e "$_entire_hook_dir/%s%s" "$@" + sh -e "$_entire_hook_dir/%s%s" "$@" || exit $? fi +exit $_entire_status `, chainComment, hookName, backupSuffix, hookName, backupSuffix, hookName, backupSuffix, hookName, backupSuffix) } @@ -763,13 +769,15 @@ trap 'rm -f "$_entire_stdin"' EXIT body = strings.Replace(body, original, replacement, 1) return replayPrefix + body + fmt.Sprintf(` +_entire_status=$? %s _entire_hook_dir="$(dirname "$0")" if [ -x "$_entire_hook_dir/post-rewrite%s" ]; then - "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" + "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" || exit $? elif [ -f "$_entire_hook_dir/post-rewrite%s" ]; then - sh -e "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" + sh -e "$_entire_hook_dir/post-rewrite%s" "$@" < "$_entire_stdin" || exit $? fi +exit $_entire_status `, chainComment, backupSuffix, backupSuffix, backupSuffix, backupSuffix) } diff --git a/cmd/entire/cli/strategy/hooks_test.go b/cmd/entire/cli/strategy/hooks_test.go index 9bcd1cf357..717f395518 100644 --- a/cmd/entire/cli/strategy/hooks_test.go +++ b/cmd/entire/cli/strategy/hooks_test.go @@ -1757,7 +1757,7 @@ func TestGenerateChainedContent(t *testing.T) { if !strings.Contains(result, expectedExecCheck) { t.Errorf("chained content should check -x on backup, got:\n%s", result) } - expectedDirect := `"$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` + expectedDirect := `"$_entire_hook_dir/pre-push` + backupSuffix + `" "$@" || exit $?` if !strings.Contains(result, expectedDirect) { t.Errorf("chained content should direct-exec executable backup, got:\n%s", result) } @@ -1765,10 +1765,13 @@ func TestGenerateChainedContent(t *testing.T) { if !strings.Contains(result, expectedFileCheck) { t.Errorf("chained content should fall back to -f for non-exec backups, got:\n%s", result) } - expectedSh := `sh -e "$_entire_hook_dir/pre-push` + backupSuffix + `" "$@"` + expectedSh := `sh -e "$_entire_hook_dir/pre-push` + backupSuffix + `" "$@" || exit $?` if !strings.Contains(result, expectedSh) { t.Errorf("chained content should sh -e non-exec backup with $@, got:\n%s", result) } + if !strings.Contains(result, "_entire_status=$?") || !strings.Contains(result, "exit $_entire_status") { + t.Errorf("chained content must preserve Entire exit status, got:\n%s", result) + } } func TestGenerateChainedContent_PostRewritePreservesStdinForBackup(t *testing.T) { @@ -1789,12 +1792,15 @@ func TestGenerateChainedContent_PostRewritePreservesStdinForBackup(t *testing.T) if !strings.Contains(result, `[ -x "$_entire_hook_dir/post-rewrite`+backupSuffix+`" ]`) { t.Fatalf("post-rewrite chained content should check -x on backup, got:\n%s", result) } - if !strings.Contains(result, `"$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { + if !strings.Contains(result, `"$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin" || exit $?`) { t.Fatalf("post-rewrite chained content should direct-exec executable backup with stdin, got:\n%s", result) } - if !strings.Contains(result, `sh -e "$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin"`) { + if !strings.Contains(result, `sh -e "$_entire_hook_dir/post-rewrite`+backupSuffix+`" "$@" < "$_entire_stdin" || exit $?`) { t.Fatalf("post-rewrite chained content should sh -e replay stdin into non-exec backup, got:\n%s", result) } + if !strings.Contains(result, "exit $_entire_status") { + t.Fatalf("post-rewrite chained content must preserve Entire exit status, got:\n%s", result) + } } func TestInstallGitHook_InstallRemoveReinstall(t *testing.T) { @@ -1991,6 +1997,36 @@ func TestResolveHookExePath(t *testing.T) { }) } +func TestGenerateChainedContent_PreservesEntireExitStatus(t *testing.T) { + t.Parallel() + + tmp := t.TempDir() + backupName := "pre-push" + backupSuffix + backupPath := filepath.Join(tmp, backupName) + // Successful backup must not mask Entire's failure (OPF abort). + if err := os.WriteFile(backupPath, []byte("#!/bin/sh\nexit 0\n"), 0o644); err != nil { + t.Fatalf("write backup: %v", err) + } + + base := "#!/bin/sh\n# Entire CLI hooks\nfalse\n" + script := generateChainedContent(base, "pre-push") + scriptPath := filepath.Join(tmp, "pre-push") + if err := os.WriteFile(scriptPath, []byte(script), 0o755); err != nil { + t.Fatalf("write chained hook: %v", err) + } + + cmd := exec.CommandContext(context.Background(), "sh", scriptPath) + cmd.Dir = tmp + err := cmd.Run() + if err == nil { + t.Fatal("chained pre-push must exit non-zero when Entire fails even if backup succeeds") + } + var exitErr *exec.ExitError + if !errors.As(err, &exitErr) || exitErr.ExitCode() == 0 { + t.Fatalf("want non-zero exit, got %v", err) + } +} + func TestGenerateChainedContent_RunsMode0644Backup(t *testing.T) { t.Parallel()