Fix hook v1→v2 upgrade: strip & and document install-hook --force (#207)

## Summary

- Fix the v1→v2 hook upgrade path to patch in place: strips trailing `&`
from the enqueue line and adds the v2 version marker
- For hooks already corrupted by a previous buggy upgrade (e.g. stray
`fi`), the recommended fix is `roborev install-hook --force`
- Remove the `removeStrayFi()` auto-repair logic in favor of the simpler
`--force` approach

## Test plan

- [x] `TestInitCmdUpgradesOutdatedHook` — v1 hook gets `&` stripped and
marker added
- [x] `TestInitCmdPreservesOtherHooksOnUpgrade` — non-roborev hook
content preserved during upgrade
- [x] Full test suite passes

🤖 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

@@ -309,8 +309,8 @@ func TestInitCmdUpgradesOutdatedHook(t *testing.T) {
 
 	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"
+	// Write a realistic old-style hook (no version marker, has &, includes if/fi block)
+	oldHook := "#!/bin/sh\n# roborev post-commit hook - auto-reviews every commit\nROBOREV=\"/usr/local/bin/roborev\"\nif [ ! -x \"$ROBOREV\" ]; then\n    ROBOREV=$(command -v roborev 2>/dev/null)\n    [ -z \"$ROBOREV\" ] || [ ! -x \"$ROBOREV\" ] && exit 0\nfi\n\"$ROBOREV\" enqueue --quiet 2>/dev/null &\n"
 	repo.WriteHook(oldHook)
 
 	defer testutil.MockBinaryInPath(t, "roborev", "#!/bin/sh\nexit 0\n")()
@@ -335,6 +335,13 @@ func TestInitCmdUpgradesOutdatedHook(t *testing.T) {
 	if strings.Contains(contentStr, "2>/dev/null &") {
 		t.Error("upgraded hook should not have backgrounded enqueue")
 	}
+	if !strings.Contains(contentStr, "2>/dev/null\n") {
+		t.Error("upgraded hook should still have enqueue line (without &)")
+	}
+	// Verify the if/fi block is preserved intact (no stray fi)
+	if !strings.Contains(contentStr, "if [ ! -x") {
+		t.Error("upgraded hook should preserve the if block")
+	}
 }
 
 func TestInitCmdPreservesOtherHooksOnUpgrade(t *testing.T) {
@@ -349,7 +356,7 @@ func TestInitCmdPreservesOtherHooksOnUpgrade(t *testing.T) {
 
 	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"
+	oldHook := "#!/bin/sh\necho 'my custom hook'\n# roborev post-commit hook - auto-reviews every commit\nROBOREV=\"/usr/local/bin/roborev\"\nif [ ! -x \"$ROBOREV\" ]; then\n    ROBOREV=$(command -v roborev 2>/dev/null)\n    [ -z \"$ROBOREV\" ] || [ ! -x \"$ROBOREV\" ] && exit 0\nfi\n\"$ROBOREV\" enqueue --quiet 2>/dev/null &\n"
 	repo.WriteHook(oldHook)
 
 	defer testutil.MockBinaryInPath(t, "roborev", "#!/bin/sh\nexit 0\n")()
@@ -374,6 +381,9 @@ func TestInitCmdPreservesOtherHooksOnUpgrade(t *testing.T) {
 	if !strings.Contains(contentStr, "hook v2") {
 		t.Error("upgrade should contain version marker")
 	}
+	if strings.Contains(contentStr, "2>/dev/null &") {
+		t.Error("upgrade should remove trailing &")
+	}
 }
 
 func TestHookNeedsUpgrade(t *testing.T) {

cmd/roborev/main.go

@@ -346,29 +346,19 @@ func initCmd() *cobra.Command {
 					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)
+					// Upgrade: try patching in place first (preserves hook structure)
+					// v1 → v2: remove trailing & from enqueue line, add version marker
+					upgraded := existingStr
+					upgraded = strings.Replace(upgraded, "2>/dev/null &", "2>/dev/null", 1)
+					upgraded = strings.Replace(upgraded, "post-commit hook -", "post-commit hook v2 -", 1)
+					if !strings.Contains(upgraded, hookVersionMarker) {
+						// Comment was edited — just append a marker so we don't re-upgrade
+						if !strings.HasSuffix(upgraded, "\n") {
+							upgraded += "\n"
 						}
+						upgraded += "# " + hookVersionMarker + "\n"
 					}
-					// 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 {
+					if err := os.WriteFile(hookPath, []byte(upgraded), 0755); err != nil {
 						return fmt.Errorf("upgrade hook: %w", err)
 					}
 					fmt.Println("  Upgraded post-commit hook")
@@ -2839,9 +2829,6 @@ func hookNeedsUpgrade(repoPath string) bool {
 	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.
 func generateHookContent() string {
 	// Get path to the currently running binary (not just first in PATH)
 	roborevPath, err := os.Executable()