[minion] Install git hooks using chaining instead of overwriting existing hooks

internal/git/hooks/hooks.go

@@ -2,29 +2,39 @@ package hooks
 
 import (
 	"fmt"
+	"regexp"
 	"strings"
 )
 
-const partioMarker = "# Installed by partio"
+const (
+	beginSentinel = "# BEGIN partio"
+	endSentinel   = "# END partio"
+)
 
 var hookNames = []string{"pre-commit", "post-commit", "pre-push"}
 
-// hookScript returns the bash shim for a given hook name.
-func hookScript(name string) string {
-	return fmt.Sprintf(`#!/bin/bash
-%s
+var partioBlockRe = regexp.MustCompile(`\n?` + beginSentinel + `\n[\s\S]*?` + endSentinel + `\n?`)
+
+// partioBlock returns the partio invocation block for the given hook name.
+func partioBlock(name string) string {
+	return fmt.Sprintf(`%s
 if command -v partio &> /dev/null; then
     partio _hook %s "$@"
     exit_code=$?
     [ $exit_code -ne 0 ] && exit $exit_code
 fi
-# Chain to original hook if backed up
-hooks_dir="$(git rev-parse --git-common-dir)/hooks"
-[ -f "$hooks_dir/%s.partio-backup" ] && exec "$hooks_dir/%s.partio-backup" "$@"
-exit 0
-`, partioMarker, name, name, name)
+%s`, beginSentinel, name, endSentinel)
+}
+
+// newHookScript returns a complete hook script for a new hook file.
+func newHookScript(name string) string {
+	return "#!/bin/bash\n" + partioBlock(name) + "\n"
+}
+
+func hasPartioBlock(content string) bool {
+	return strings.Contains(content, beginSentinel)
 }
 
-func isPartioHook(content string) bool {
-	return strings.Contains(content, partioMarker)
+func removePartioBlock(content string) string {
+	return partioBlockRe.ReplaceAllString(content, "")
 }

internal/git/hooks/hooks_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"strings"
 	"testing"
 )
 
@@ -36,8 +37,8 @@ func TestInstallAndUninstall(t *testing.T) {
 		}
 
 		content := string(data)
-		if !isPartioHook(content) {
-			t.Errorf("hook %s missing partio marker", name)
+		if !hasPartioBlock(content) {
+			t.Errorf("hook %s missing partio block", name)
 		}
 
 		// Check executable permission
@@ -52,7 +53,7 @@ func TestInstallAndUninstall(t *testing.T) {
 		t.Fatalf("Uninstall error: %v", err)
 	}
 
-	// Verify hooks are removed
+	// Verify hooks are removed (no pre-existing hook, so file should be gone)
 	for _, name := range hookNames {
 		path := filepath.Join(hooksDir, name)
 		if _, err := os.Stat(path); !os.IsNotExist(err) {
@@ -61,7 +62,7 @@ func TestInstallAndUninstall(t *testing.T) {
 	}
 }
 
-func TestInstallBackupChaining(t *testing.T) {
+func TestInstallChaining(t *testing.T) {
 	dir := initGitRepo(t)
 	hooksDir := filepath.Join(dir, ".git", "hooks")
 
@@ -77,36 +78,40 @@ func TestInstallBackupChaining(t *testing.T) {
 		t.Fatalf("Install error: %v", err)
 	}
 
-	// Original should be backed up
+	// No backup should be created
 	backupPath := hookPath + ".partio-backup"
-	data, err := os.ReadFile(backupPath)
-	if err != nil {
-		t.Fatalf("backup not found: %v", err)
-	}
-	if string(data) != existingHook {
-		t.Errorf("backup content mismatch: %q", string(data))
+	if _, err := os.Stat(backupPath); err == nil {
+		t.Error("backup file should not be created with chaining approach")
 	}
 
-	// New hook should be ours
-	data, err = os.ReadFile(hookPath)
+	// Hook should contain both original content and partio block
+	data, err := os.ReadFile(hookPath)
 	if err != nil {
 		t.Fatalf("reading hook: %v", err)
 	}
-	if !isPartioHook(string(data)) {
-		t.Error("installed hook missing partio marker")
+	content := string(data)
+	if !strings.Contains(content, "echo 'existing hook'") {
+		t.Error("hook missing original content")
+	}
+	if !hasPartioBlock(content) {
+		t.Error("hook missing partio block")
 	}
 
-	// Uninstall should restore original
+	// Uninstall should remove only the partio block, leaving original content
 	if err := Uninstall(dir); err != nil {
 		t.Fatalf("Uninstall error: %v", err)
 	}
 
 	data, err = os.ReadFile(hookPath)
 	if err != nil {
-		t.Fatalf("hook should exist after uninstall (restored): %v", err)
+		t.Fatalf("hook should exist after uninstall (original preserved): %v", err)
+	}
+	restored := string(data)
+	if hasPartioBlock(restored) {
+		t.Error("partio block should be removed after uninstall")
 	}
-	if string(data) != existingHook {
-		t.Errorf("original hook not restored: %q", string(data))
+	if !strings.Contains(restored, "echo 'existing hook'") {
+		t.Error("original hook content should be preserved after uninstall")
 	}
 }
 
@@ -152,26 +157,47 @@ func TestInstallWorktree(t *testing.T) {
 			t.Errorf("hook %s not found in worktree: %v", name, err)
 			continue
 		}
-		if !isPartioHook(string(data)) {
-			t.Errorf("hook %s missing partio marker in worktree", name)
+		if !hasPartioBlock(string(data)) {
+			t.Errorf("hook %s missing partio block in worktree", name)
 		}
 	}
 }
 
-func TestIsPartioHook(t *testing.T) {
+func TestHasPartioBlock(t *testing.T) {
 	tests := []struct {
 		content  string
 		expected bool
 	}{
-		{"#!/bin/bash\n# Installed by partio\npartio _hook pre-commit", true},
+		{"#!/bin/bash\n" + beginSentinel + "\npartio _hook pre-commit\n" + endSentinel, true},
 		{"#!/bin/bash\necho hello", false},
 		{"", false},
-		{partioMarker, true},
+		{beginSentinel, true},
 	}
 
 	for _, tt := range tests {
-		if got := isPartioHook(tt.content); got != tt.expected {
-			t.Errorf("isPartioHook(%q) = %v, want %v", tt.content, got, tt.expected)
+		if got := hasPartioBlock(tt.content); got != tt.expected {
+			t.Errorf("hasPartioBlock(%q) = %v, want %v", tt.content, got, tt.expected)
 		}
 	}
 }
+
+func TestRemovePartioBlock(t *testing.T) {
+	block := partioBlock("pre-commit")
+
+	// Test removal from a file that only has the partio block
+	onlyPartio := "#!/bin/bash\n" + block + "\n"
+	result := removePartioBlock(onlyPartio)
+	if strings.Contains(result, beginSentinel) {
+		t.Errorf("partio block not removed: %q", result)
+	}
+
+	// Test removal from a file that has existing content + partio block
+	withExisting := "#!/bin/bash\necho 'existing'\n\n" + block + "\n"
+	result = removePartioBlock(withExisting)
+	if strings.Contains(result, beginSentinel) {
+		t.Errorf("partio block not removed: %q", result)
+	}
+	if !strings.Contains(result, "echo 'existing'") {
+		t.Errorf("existing content removed: %q", result)
+	}
+}

internal/git/hooks/install.go

@@ -8,7 +8,7 @@ import (
 	"github.com/partio-io/cli/internal/git"
 )
 
-// Install installs partio git hooks into the repository, backing up existing hooks.
+// Install installs partio git hooks into the repository, appending to existing hooks.
 func Install(repoRoot string) error {
 	hooksDir, err := git.HooksDir(repoRoot)
 	if err != nil {
@@ -21,22 +21,35 @@ func Install(repoRoot string) error {
 
 	for _, name := range hookNames {
 		hookPath := filepath.Join(hooksDir, name)
-		backupPath := hookPath + ".partio-backup"
-
-		// If existing hook is not ours, back it up
-		if data, err := os.ReadFile(hookPath); err == nil {
-			content := string(data)
-			if !isPartioHook(content) {
-				if err := os.Rename(hookPath, backupPath); err != nil {
-					return fmt.Errorf("backing up %s hook: %w", name, err)
-				}
+
+		data, err := os.ReadFile(hookPath)
+		if err != nil {
+			// No existing hook — create a new one
+			script := newHookScript(name)
+			if err := os.WriteFile(hookPath, []byte(script), 0o755); err != nil {
+				return fmt.Errorf("writing %s hook: %w", name, err)
 			}
+			continue
+		}
+
+		if hasPartioBlock(string(data)) {
+			// Already installed, skip
+			continue
 		}
 
-		// Write our hook
-		script := hookScript(name)
-		if err := os.WriteFile(hookPath, []byte(script), 0o755); err != nil {
-			return fmt.Errorf("writing %s hook: %w", name, err)
+		// Append partio block to existing hook
+		block := "\n" + partioBlock(name) + "\n"
+		f, err := os.OpenFile(hookPath, os.O_APPEND|os.O_WRONLY, 0o755)
+		if err != nil {
+			return fmt.Errorf("opening %s hook for append: %w", name, err)
+		}
+		_, writeErr := f.WriteString(block)
+		closeErr := f.Close()
+		if writeErr != nil {
+			return fmt.Errorf("appending to %s hook: %w", name, writeErr)
+		}
+		if closeErr != nil {
+			return fmt.Errorf("closing %s hook: %w", name, closeErr)
 		}
 	}
 

internal/git/hooks/uninstall.go

@@ -1,13 +1,15 @@
 package hooks
 
 import (
+	"fmt"
 	"os"
 	"path/filepath"
+	"strings"
 
 	"github.com/partio-io/cli/internal/git"
 )
 
-// Uninstall removes partio git hooks, restoring backups if present.
+// Uninstall removes the partio sentinel block from git hooks, leaving other content intact.
 func Uninstall(repoRoot string) error {
 	hooksDir, err := git.HooksDir(repoRoot)
 	if err != nil {
@@ -16,18 +18,27 @@ func Uninstall(repoRoot string) error {
 
 	for _, name := range hookNames {
 		hookPath := filepath.Join(hooksDir, name)
-		backupPath := hookPath + ".partio-backup"
 
-		// Only remove if it's our hook
-		if data, err := os.ReadFile(hookPath); err == nil {
-			if isPartioHook(string(data)) {
-				_ = os.Remove(hookPath)
-			}
+		data, err := os.ReadFile(hookPath)
+		if err != nil {
+			continue // Hook doesn't exist, skip
+		}
+
+		content := string(data)
+		if !hasPartioBlock(content) {
+			continue // No partio block, skip
 		}
 
-		// Restore backup if present
-		if _, err := os.Stat(backupPath); err == nil {
-			_ = os.Rename(backupPath, hookPath)
+		stripped := strings.TrimSpace(removePartioBlock(content))
+
+		if stripped == "" || stripped == "#!/bin/bash" {
+			// Only partio content remained — remove the file
+			_ = os.Remove(hookPath)
+		} else {
+			// Write back without the partio block
+			if err := os.WriteFile(hookPath, []byte(stripped+"\n"), 0o755); err != nil {
+				return fmt.Errorf("writing %s hook after removal: %w", name, err)
+			}
 		}
 	}