Fix post-commit hook backgrounding and add hook upgrade on init (#192)

## Summary
- Remove `&` from post-commit hook enqueue command — backgrounding can
kill the process before it completes when agents spawn git commits
- Add version marker (`v2`) to hook template so `roborev init` can
detect and upgrade outdated hooks while preserving user content
- Warn about outdated hooks on daemon startup and in `roborev status`

## Test plan
- [x] `go test ./...` passes
- [x] Run `roborev init` in a repo with an old hook — verify it prints
"Upgraded post-commit hook"
- [x] Run `roborev init` again — verify it prints "Hook already
installed"
- [x] Run `roborev status` in a repo with an old hook — verify warning
is shown

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>

Author: wesm

cmd/roborev/hook_test.go

@@ -244,19 +244,24 @@ func TestGenerateHookContent(t *testing.T) {
 		}
 	})
 
-	t.Run("enqueue line has quiet and stderr redirect", func(t *testing.T) {
-		// Must have exact enqueue line with --quiet, stderr redirect, and background
+	t.Run("enqueue line has quiet and stderr redirect without background", func(t *testing.T) {
 		found := false
 		for _, line := range lines {
 			if strings.Contains(line, "enqueue --quiet") &&
 				strings.Contains(line, "2>/dev/null") &&
-				strings.HasSuffix(strings.TrimSpace(line), "&") {
+				!strings.HasSuffix(strings.TrimSpace(line), "&") {
 				found = true
 				break
 			}
 		}
 		if !found {
-			t.Error("hook should have enqueue line with --quiet, 2>/dev/null, and & on same line")
+			t.Error("hook should have enqueue line with --quiet and 2>/dev/null but no trailing &")
+		}
+	})
+
+	t.Run("has version marker", func(t *testing.T) {
+		if !strings.Contains(content, "hook v2") {
+			t.Error("hook should contain version marker 'hook v2'")
 		}
 	})
 
@@ -291,3 +296,115 @@ func TestGenerateHookContent(t *testing.T) {
 		}
 	})
 }
+
+func TestInitCmdUpgradesOutdatedHook(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test uses shell script stub, skipping on Windows")
+	}
+
+	tmpHome := t.TempDir()
+	origHome := os.Getenv("HOME")
+	os.Setenv("HOME", tmpHome)
+	defer os.Setenv("HOME", origHome)
+
+	repo := testutil.NewTestRepo(t)
+
+	// Write an old-style hook (no version marker, has &)
+	oldHook := "#!/bin/sh\n# roborev post-commit hook - auto-reviews every commit\nROBOREV=\"/usr/local/bin/roborev\"\n\"$ROBOREV\" enqueue --quiet 2>/dev/null &\n"
+	repo.WriteHook(oldHook)
+
+	defer testutil.MockBinaryInPath(t, "roborev", "#!/bin/sh\nexit 0\n")()
+	defer repo.Chdir()()
+
+	initCommand := initCmd()
+	initCommand.SetArgs([]string{"--agent", "test"})
+	err := initCommand.Execute()
+	if err != nil {
+		t.Fatalf("init command failed: %v", err)
+	}
+
+	content, err := os.ReadFile(repo.HookPath)
+	if err != nil {
+		t.Fatalf("Failed to read hook: %v", err)
+	}
+
+	contentStr := string(content)
+	if !strings.Contains(contentStr, "hook v2") {
+		t.Error("upgraded hook should contain version marker")
+	}
+	if strings.Contains(contentStr, "2>/dev/null &") {
+		t.Error("upgraded hook should not have backgrounded enqueue")
+	}
+}
+
+func TestInitCmdPreservesOtherHooksOnUpgrade(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test uses shell script stub, skipping on Windows")
+	}
+
+	tmpHome := t.TempDir()
+	origHome := os.Getenv("HOME")
+	os.Setenv("HOME", tmpHome)
+	defer os.Setenv("HOME", origHome)
+
+	repo := testutil.NewTestRepo(t)
+
+	oldHook := "#!/bin/sh\necho 'my custom hook'\n# roborev post-commit hook - auto-reviews every commit\nROBOREV=\"/usr/local/bin/roborev\"\n\"$ROBOREV\" enqueue --quiet 2>/dev/null &\n"
+	repo.WriteHook(oldHook)
+
+	defer testutil.MockBinaryInPath(t, "roborev", "#!/bin/sh\nexit 0\n")()
+	defer repo.Chdir()()
+
+	initCommand := initCmd()
+	initCommand.SetArgs([]string{"--agent", "test"})
+	err := initCommand.Execute()
+	if err != nil {
+		t.Fatalf("init command failed: %v", err)
+	}
+
+	content, err := os.ReadFile(repo.HookPath)
+	if err != nil {
+		t.Fatalf("Failed to read hook: %v", err)
+	}
+
+	contentStr := string(content)
+	if !strings.Contains(contentStr, "echo 'my custom hook'") {
+		t.Error("upgrade should preserve non-roborev lines")
+	}
+	if !strings.Contains(contentStr, "hook v2") {
+		t.Error("upgrade should contain version marker")
+	}
+}
+
+func TestHookNeedsUpgrade(t *testing.T) {
+	t.Run("outdated hook", func(t *testing.T) {
+		repo := testutil.NewTestRepo(t)
+		repo.WriteHook("#!/bin/sh\n# roborev post-commit hook\nroborev enqueue\n")
+		if !hookNeedsUpgrade(repo.Root) {
+			t.Error("should detect outdated hook")
+		}
+	})
+
+	t.Run("current hook", func(t *testing.T) {
+		repo := testutil.NewTestRepo(t)
+		repo.WriteHook("#!/bin/sh\n# roborev post-commit hook v2 - auto-reviews every commit\nroborev enqueue\n")
+		if hookNeedsUpgrade(repo.Root) {
+			t.Error("should not flag current hook as outdated")
+		}
+	})
+
+	t.Run("no hook", func(t *testing.T) {
+		repo := testutil.NewTestRepo(t)
+		if hookNeedsUpgrade(repo.Root) {
+			t.Error("should not flag missing hook as outdated")
+		}
+	})
+
+	t.Run("non-roborev hook", func(t *testing.T) {
+		repo := testutil.NewTestRepo(t)
+		repo.WriteHook("#!/bin/sh\necho hello\n")
+		if hookNeedsUpgrade(repo.Root) {
+			t.Error("should not flag non-roborev hook as outdated")
+		}
+	})
+}

cmd/roborev/main.go

@@ -337,12 +337,41 @@ func initCmd() *cobra.Command {
 
 			// Check for existing hook
 			if existing, err := os.ReadFile(hookPath); err == nil {
-				if !strings.Contains(string(existing), "roborev") {
+				existingStr := string(existing)
+				if !strings.Contains(strings.ToLower(existingStr), "roborev") {
 					// Append to existing hook
-					hookContent = string(existing) + "\n" + hookContent
-				} else {
+					hookContent = existingStr + "\n" + hookContent
+				} else if strings.Contains(existingStr, hookVersionMarker) {
 					fmt.Println("  Hook already installed")
 					goto startDaemon
+				} else {
+					// Upgrade: strip old roborev lines, append new content
+					lines := strings.Split(existingStr, "\n")
+					var kept []string
+					for _, line := range lines {
+						if !strings.Contains(strings.ToLower(line), "roborev") {
+							kept = append(kept, line)
+						}
+					}
+					// Remove trailing empty lines
+					for len(kept) > 0 && strings.TrimSpace(kept[len(kept)-1]) == "" {
+						kept = kept[:len(kept)-1]
+					}
+					// Strip shebang from new content if prepending to existing content
+					newHook := hookContent
+					if len(kept) > 0 {
+						newHook = strings.TrimPrefix(newHook, "#!/bin/sh\n")
+					}
+					if len(kept) > 0 {
+						hookContent = strings.Join(kept, "\n") + "\n" + newHook
+					} else {
+						hookContent = newHook
+					}
+					if err := os.WriteFile(hookPath, []byte(hookContent), 0755); err != nil {
+						return fmt.Errorf("upgrade hook: %w", err)
+					}
+					fmt.Println("  Upgraded post-commit hook")
+					goto startDaemon
 				}
 			}
 
@@ -1179,6 +1208,14 @@ func statusCmd() *cobra.Command {
 				w.Flush()
 			}
 
+			// Check for outdated hook in current repo
+			if root, err := git.GetRepoRoot("."); err == nil {
+				if hookNeedsUpgrade(root) {
+					fmt.Println()
+					fmt.Println("Warning: post-commit hook is outdated -- run 'roborev init' to upgrade")
+				}
+			}
+
 			return nil
 		},
 	}
@@ -2657,6 +2694,25 @@ func resolveReasoningWithFast(reasoning string, fast bool, reasoningExplicitlySe
 	return reasoning
 }
 
+// hookVersionMarker is the string that identifies the current hook version.
+// Bump this when the hook template changes to trigger upgrade warnings.
+const hookVersionMarker = "post-commit hook v2"
+
+// hookNeedsUpgrade checks whether a repo's post-commit hook contains roborev
+// but is outdated (missing the current version marker).
+func hookNeedsUpgrade(repoPath string) bool {
+	hooksDir, err := git.GetHooksPath(repoPath)
+	if err != nil {
+		return false
+	}
+	content, err := os.ReadFile(filepath.Join(hooksDir, "post-commit"))
+	if err != nil {
+		return false
+	}
+	s := string(content)
+	return strings.Contains(strings.ToLower(s), "roborev") && !strings.Contains(s, hookVersionMarker)
+}
+
 // generateHookContent creates the post-commit hook script content.
 // It bakes the path to the currently running binary for consistency.
 // Falls back to PATH lookup if the baked path becomes unavailable.
@@ -2678,12 +2734,12 @@ func generateHookContent() string {
 
 	// Prefer baked path (security), fall back to PATH only if baked is missing
 	return fmt.Sprintf(`#!/bin/sh
-# roborev post-commit hook - auto-reviews every commit
+# roborev post-commit hook v2 - auto-reviews every commit
 ROBOREV=%q
 if [ ! -x "$ROBOREV" ]; then
     ROBOREV=$(command -v roborev 2>/dev/null)
     [ -z "$ROBOREV" ] || [ ! -x "$ROBOREV" ] && exit 0
 fi
-"$ROBOREV" enqueue --quiet 2>/dev/null &
+"$ROBOREV" enqueue --quiet 2>/dev/null
 `, roborevPath)
 }

internal/daemon/server.go

@@ -8,6 +8,8 @@ import (
 	"fmt"
 	"log"
 	"net/http"
+	"os"
+	"path/filepath"
 	"strings"
 	"sync"
 	"time"
@@ -132,6 +134,15 @@ func (s *Server) Start(ctx context.Context) error {
 	// Start worker pool
 	s.workerPool.Start()
 
+	// Check for outdated hooks in registered repos
+	if repos, err := s.db.ListRepos(); err == nil {
+		for _, repo := range repos {
+			if hookNeedsUpgrade(repo.RootPath) {
+				log.Printf("Warning: outdated post-commit hook in %s -- run 'roborev init' to upgrade", repo.RootPath)
+			}
+		}
+	}
+
 	// Start HTTP server
 	log.Printf("Starting HTTP server on %s", addr)
 	if err := s.httpServer.ListenAndServe(); err != http.ErrServerClosed {
@@ -142,6 +153,23 @@ func (s *Server) Start(ctx context.Context) error {
 	return nil
 }
 
+// hookVersionMarker identifies the current hook version.
+const hookVersionMarker = "post-commit hook v2"
+
+// hookNeedsUpgrade checks whether a repo's post-commit hook is outdated.
+func hookNeedsUpgrade(repoPath string) bool {
+	hooksDir, err := git.GetHooksPath(repoPath)
+	if err != nil {
+		return false
+	}
+	content, err := os.ReadFile(filepath.Join(hooksDir, "post-commit"))
+	if err != nil {
+		return false
+	}
+	s := string(content)
+	return strings.Contains(strings.ToLower(s), "roborev") && !strings.Contains(s, hookVersionMarker)
+}
+
 // Stop gracefully shuts down the server
 func (s *Server) Stop() error {
 	ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second)