Fix post-commit hook to use main repo root in git worktrees (#497)

## Summary

- When the post-commit hook fires from a linked git worktree,
`GetRepoRoot` returns the worktree path rather than the main repo root,
causing the daemon to register the commit under a phantom repo entry
- Fix by using `GetMainRepoRoot` for the `RepoPath` sent to the daemon,
while keeping `GetRepoRoot` (worktree-local path) for rebase detection
and branch/ref resolution, since those states live in the
worktree-specific git dir
- Adds table-driven tests covering both plain repo and worktree cases
for `RepoPath` correctness and rebase suppression during in-progress
rebases

---------

Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-authored-by: Wes McKinney <wesmckinn+git@gmail.com>

Author: 3 people

cmd/roborev/postcommit_test.go

@@ -2,12 +2,22 @@ package main
 
 import (
 	"bytes"
+	"encoding/json"
 	"fmt"
-	"github.com/stretchr/testify/assert"
-	"github.com/stretchr/testify/require"
 	"net/http"
+	"os"
+	"os/exec"
+	"path/filepath"
+	"runtime"
+	"strings"
 	"testing"
 	"time"
+
+	"github.com/roborev-dev/roborev/internal/daemon"
+	"github.com/roborev-dev/roborev/internal/git"
+	"github.com/roborev-dev/roborev/internal/githook"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 )
 
 func executePostCommitCmd(
@@ -196,3 +206,284 @@ func TestEnqueueAliasIsHidden(t *testing.T) {
 	assert.True(t, cmd.Hidden)
 	assert.Contains(t, cmd.Use, "enqueue")
 }
+
+// repoUnderTest holds a repo for post-commit hook tests.
+type repoUnderTest struct {
+	// repo is the directory post-commit runs from (may be a worktree).
+	repo *TestGitRepo
+}
+
+// setupPlainRepo returns a repoUnderTest backed by a plain (non-worktree) repo.
+func setupPlainRepo(t *testing.T) repoUnderTest {
+	t.Helper()
+	repo := newTestGitRepo(t)
+	repo.CommitFile("file.txt", "content", "initial commit")
+	return repoUnderTest{repo: repo}
+}
+
+// setupWorktreeRepo returns a repoUnderTest backed by a linked worktree.
+func setupWorktreeRepo(t *testing.T) repoUnderTest {
+	t.Helper()
+	mainRepo := newTestGitRepo(t)
+	mainRepo.CommitFile("file.txt", "content", "initial commit")
+
+	wtDir := t.TempDir()
+	resolved, err := filepath.EvalSymlinks(wtDir)
+	require.NoError(t, err)
+	mainRepo.Run("worktree", "add", resolved, "-b", "worktree-branch")
+
+	return repoUnderTest{repo: &TestGitRepo{Dir: resolved, t: t}}
+}
+
+// mockEnqueueCapture registers a handler on mux that captures full enqueue
+// requests. The returned channel receives at most one request.
+func mockEnqueueCapture(t *testing.T, mux *http.ServeMux) <-chan daemon.EnqueueRequest {
+	t.Helper()
+	ch := make(chan daemon.EnqueueRequest, 1)
+	mux.HandleFunc("/api/enqueue", func(w http.ResponseWriter, r *http.Request) {
+		var req daemon.EnqueueRequest
+		if err := json.NewDecoder(r.Body).Decode(&req); err != nil {
+			http.Error(w, err.Error(), http.StatusBadRequest)
+			return
+		}
+		select {
+		case ch <- req:
+		default:
+			assert.Fail(t, "mockEnqueueCapture: unexpected extra request")
+			http.Error(w, "duplicate request", http.StatusConflict)
+			return
+		}
+		w.WriteHeader(http.StatusCreated)
+		json.NewEncoder(w).Encode(map[string]any{"id": 1})
+	})
+	return ch
+}
+
+// TestPostCommitSendsLocalRepoPath checks that the RepoPath in the enqueue
+// request is the local (worktree) path in both plain repos and linked
+// worktrees. The daemon canonicalizes to the main repo root itself.
+func TestPostCommitSendsLocalRepoPath(t *testing.T) {
+	tests := []struct {
+		name  string
+		setup func(t *testing.T) repoUnderTest
+	}{
+		{"plain repo", setupPlainRepo},
+		{"worktree", setupWorktreeRepo},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := tt.setup(t)
+			mux := http.NewServeMux()
+			daemonFromHandler(t, mux)
+			reqCh := mockEnqueueCapture(t, mux)
+
+			r.repo.CommitFile("change.txt", "content", "a commit")
+
+			_, _, err := executePostCommitCmd("--repo", r.repo.Dir)
+			require.NoError(t, err)
+
+			req := <-reqCh
+			assert.Equal(t, r.repo.Dir, req.RepoPath)
+		})
+	}
+}
+
+// TestPostCommitSkipsEnqueueDuringRebase exercises the real Go code path
+// (postCommitCmd → git.IsRebaseInProgress) by simulating a rebase state
+// with a synthetic rebase-merge directory. This is the unit-level
+// complement to TestPostCommitDoesNotEnqueueDuringRebase which tests the
+// end-to-end shell hook flow.
+func TestPostCommitSkipsEnqueueDuringRebase(t *testing.T) {
+	tests := []struct {
+		name     string
+		setup    func(t *testing.T) repoUnderTest
+		sentinel string // directory to create inside git dir
+	}{
+		{"plain repo/rebase-merge", setupPlainRepo, "rebase-merge"},
+		{"plain repo/rebase-apply", setupPlainRepo, "rebase-apply"},
+		{"worktree/rebase-merge", setupWorktreeRepo, "rebase-merge"},
+		{"worktree/rebase-apply", setupWorktreeRepo, "rebase-apply"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := tt.setup(t)
+			mux := http.NewServeMux()
+			daemonFromHandler(t, mux)
+
+			mux.HandleFunc("/api/enqueue", func(
+				w http.ResponseWriter, r *http.Request,
+			) {
+				t.Error("enqueue should not be called during rebase")
+				http.Error(w, "unexpected", http.StatusConflict)
+			})
+
+			// Resolve the actual git dir (may differ from .git/ in
+			// linked worktrees where .git is a file).
+			gitDir := strings.TrimSpace(r.repo.Run(
+				"rev-parse", "--git-dir"))
+			if !filepath.IsAbs(gitDir) {
+				gitDir = filepath.Join(r.repo.Dir, gitDir)
+			}
+			require.NoError(t, os.MkdirAll(
+				filepath.Join(gitDir, tt.sentinel), 0755))
+
+			_, _, err := executePostCommitCmd("--repo", r.repo.Dir)
+			require.NoError(t, err)
+		})
+	}
+}
+
+// mockRoborevBinary creates a mock "roborev" shell script in a temp directory
+// and returns the directory (to prepend to PATH). The mock script handles
+// "post-commit" by using roborev's same rebase detection logic: it checks
+// for rebase-merge/rebase-apply in git-dir and writes to the marker file
+// only when NOT rebasing.
+func mockRoborevBinary(t *testing.T, marker string) string {
+	t.Helper()
+	binDir := t.TempDir()
+	script := fmt.Sprintf(`#!/bin/sh
+# Mock roborev binary for testing post-commit hook behavior.
+# Only handles the "post-commit" subcommand.
+case "$1" in
+  post-commit)
+    git_dir=$(git rev-parse --git-dir 2>/dev/null) || exit 0
+    [ -d "$git_dir/rebase-merge" ] && exit 0
+    [ -d "$git_dir/rebase-apply" ] && exit 0
+    echo enqueued >> %q
+    ;;
+esac
+`, marker)
+	require.NoError(t, os.WriteFile(
+		filepath.Join(binDir, "roborev"),
+		[]byte(script), 0755))
+	return binDir
+}
+
+// installMockHook installs the real githook-generated post-commit hook with
+// the ROBOREV= line patched to point at a mock binary.
+func installMockHook(t *testing.T, repoDir, mockBinDir string) {
+	t.Helper()
+	hooksDir, err := git.GetHooksPath(repoDir)
+	require.NoError(t, err)
+	require.NoError(t, os.MkdirAll(hooksDir, 0755))
+
+	hookContent := githook.GeneratePostCommit()
+	mockBin := filepath.Join(mockBinDir, "roborev")
+	lines := strings.Split(hookContent, "\n")
+	for i, line := range lines {
+		if strings.HasPrefix(line, "ROBOREV=") {
+			lines[i] = fmt.Sprintf("ROBOREV=%q", mockBin)
+			break
+		}
+	}
+	require.NoError(t, os.WriteFile(
+		filepath.Join(hooksDir, "post-commit"),
+		[]byte(strings.Join(lines, "\n")), 0755))
+}
+
+// TestPostCommitDoesNotEnqueueDuringRebase runs a real clean git rebase with
+// hooks installed via githook.GeneratePostCommit and a mock roborev binary in
+// PATH. It asserts that roborev's rebase detection prevents any enqueue during
+// replayed commits.
+//
+// The mock binary reimplements the rebase guard in shell using the same
+// git rev-parse --git-dir + rebase-merge/rebase-apply check that
+// git.IsRebaseInProgress uses. TestPostCommitSkipsEnqueueDuringRebase (above)
+// tests the real Go code path (postCommitCmd) via simulated rebase state; this
+// test validates the end-to-end hook installation and invocation flow during
+// an actual git rebase.
+//
+// The "hook before commits" variant installs the hook before the branch
+// topology commits, so the hook fires for every setup commit as well. The
+// "hook after commits" variant installs just before the rebase.
+func TestPostCommitDoesNotEnqueueDuringRebase(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("test uses shell hooks and Unix PATH semantics")
+	}
+	tests := []struct {
+		name            string
+		setup           func(t *testing.T) repoUnderTest
+		hookBeforeSetup bool
+	}{
+		{"plain repo/hook before commits", setupPlainRepo, true},
+		{"plain repo/hook after commits", setupPlainRepo, false},
+		{"worktree/hook before commits", setupWorktreeRepo, true},
+		{"worktree/hook after commits", setupWorktreeRepo, false},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			r := tt.setup(t)
+
+			marker := filepath.Join(r.repo.Dir, "hook-enqueues.log")
+			mockBinDir := mockRoborevBinary(t, marker)
+
+			// Build env with mock roborev first in PATH so the hook finds it.
+			gitEnv := append(os.Environ(),
+				"PATH="+mockBinDir+string(os.PathListSeparator)+os.Getenv("PATH"),
+				"HOME="+r.repo.Dir,
+				"GIT_CONFIG_NOSYSTEM=1",
+				"GIT_AUTHOR_NAME=Test",
+				"GIT_AUTHOR_EMAIL=test@test.com",
+				"GIT_COMMITTER_NAME=Test",
+				"GIT_COMMITTER_EMAIL=test@test.com",
+			)
+
+			if tt.hookBeforeSetup {
+				installMockHook(t, r.repo.Dir, mockBinDir)
+			}
+
+			// Create a branch topology for a clean rebase:
+			//   base: A -- B (base.txt, no conflict)
+			//          \
+			//   current: C -- D -- E (branch files)
+			gitCmd := func(args ...string) {
+				t.Helper()
+				cmd := exec.Command("git", args...)
+				cmd.Dir = r.repo.Dir
+				cmd.Env = gitEnv
+				out, err := cmd.CombinedOutput()
+				require.NoError(t, err, "git %v failed: %s", args, out)
+			}
+			gitCmd("checkout", "-b", "rebase-base")
+			gitCmd("commit", "--allow-empty", "-m", "base commit")
+			gitCmd("checkout", "-")
+			// Create 3 feature commits with actual file changes.
+			for i := 1; i <= 3; i++ {
+				f := filepath.Join(r.repo.Dir, fmt.Sprintf("branch%d.txt", i))
+				require.NoError(t, os.WriteFile(f, fmt.Appendf(nil, "content %d", i), 0644))
+				gitCmd("add", f)
+				gitCmd("commit", "-m", fmt.Sprintf("feature commit %d", i))
+			}
+
+			if !tt.hookBeforeSetup {
+				installMockHook(t, r.repo.Dir, mockBinDir)
+			}
+
+			// Positive control: make a normal commit to prove the hook
+			// fires outside of a rebase. Without this, a broken hook
+			// install would silently pass (0 == 0).
+			gitCmd("commit", "--allow-empty", "-m", "positive control commit")
+			data, err := os.ReadFile(marker)
+			require.NoError(t, err, "hook should have fired on normal commit")
+			preRebaseCount := strings.Count(string(data), "enqueued")
+			require.GreaterOrEqual(t, preRebaseCount, 1,
+				"hook must fire at least once before rebase to prove it works")
+
+			// Run a full clean rebase — all 3 feature commits replay.
+			cmd := exec.Command("git", "rebase", "rebase-base")
+			cmd.Dir = r.repo.Dir
+			cmd.Env = gitEnv
+			out, err := cmd.CombinedOutput()
+			require.NoError(t, err, "rebase should succeed cleanly: %s", out)
+
+			// After the rebase, the marker count should be unchanged.
+			// If the hook enqueued during the rebase, there would be more.
+			data, err = os.ReadFile(marker)
+			require.NoError(t, err)
+			postRebaseCount := strings.Count(string(data), "enqueued")
+			assert.Equal(t, preRebaseCount, postRebaseCount,
+				"hook should not have enqueued during rebase (got %d, want %d)",
+				postRebaseCount, preRebaseCount)
+		})
+	}
+}