diff --git a/cmd/roborev/main.go b/cmd/roborev/main.go index 8b2bc584a..86214790c 100644 --- a/cmd/roborev/main.go +++ b/cmd/roborev/main.go @@ -388,6 +388,12 @@ Examples: roborev enqueue abc123 def456 # Review range from abc123 to def456 (inclusive) `, RunE: func(cmd *cobra.Command, args []string) error { + // In quiet mode, suppress cobra's error output (hook uses &, so exit code doesn't matter) + if quiet { + cmd.SilenceErrors = true + cmd.SilenceUsage = true + } + // Default to current directory if repoPath == "" { repoPath = "." @@ -397,22 +403,22 @@ Examples: root, err := git.GetRepoRoot(repoPath) if err != nil { if quiet { - return nil // Silent exit in quiet mode + return nil // Not a repo - silent exit for hooks } return fmt.Errorf("not a git repository: %w", err) } // Skip during rebase to avoid reviewing every replayed commit if git.IsRebaseInProgress(root) { - return nil // Silent exit + if !quiet { + fmt.Println("Skipping: rebase in progress") + } + return nil // Intentional skip, exit 0 } // Ensure daemon is running if err := ensureDaemon(); err != nil { - if quiet { - return nil - } - return err + return err // Return error (quiet mode silences output, not exit code) } var gitRef string diff --git a/internal/git/git_test.go b/internal/git/git_test.go index ca5c1fa3a..02e461637 100644 --- a/internal/git/git_test.go +++ b/internal/git/git_test.go @@ -98,3 +98,111 @@ func TestGetHooksPath(t *testing.T) { } }) } + +func TestIsRebaseInProgress(t *testing.T) { + // Create a temp git repo + tmpDir := t.TempDir() + + cmd := exec.Command("git", "init") + cmd.Dir = tmpDir + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git init failed: %v\n%s", err, out) + } + + // Configure git user for commits + exec.Command("git", "-C", tmpDir, "config", "user.email", "test@test.com").Run() + exec.Command("git", "-C", tmpDir, "config", "user.name", "Test").Run() + + t.Run("no rebase", func(t *testing.T) { + if IsRebaseInProgress(tmpDir) { + t.Error("expected no rebase in progress") + } + }) + + t.Run("rebase-merge directory", func(t *testing.T) { + // Simulate interactive rebase by creating rebase-merge directory + rebaseMerge := filepath.Join(tmpDir, ".git", "rebase-merge") + if err := os.MkdirAll(rebaseMerge, 0755); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(rebaseMerge) + + if !IsRebaseInProgress(tmpDir) { + t.Error("expected rebase in progress with rebase-merge") + } + }) + + t.Run("rebase-apply directory", func(t *testing.T) { + // Simulate git am / regular rebase by creating rebase-apply directory + rebaseApply := filepath.Join(tmpDir, ".git", "rebase-apply") + if err := os.MkdirAll(rebaseApply, 0755); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(rebaseApply) + + if !IsRebaseInProgress(tmpDir) { + t.Error("expected rebase in progress with rebase-apply") + } + }) + + t.Run("non-repo returns false", func(t *testing.T) { + nonRepo := t.TempDir() + if IsRebaseInProgress(nonRepo) { + t.Error("expected false for non-repo") + } + }) + + t.Run("worktree with rebase", func(t *testing.T) { + // Create initial commit so we can create a worktree + if err := os.WriteFile(filepath.Join(tmpDir, "file.txt"), []byte("content"), 0644); err != nil { + t.Fatal(err) + } + exec.Command("git", "-C", tmpDir, "add", ".").Run() + exec.Command("git", "-C", tmpDir, "commit", "-m", "initial").Run() + + // Create a worktree + worktreeDir := t.TempDir() + cmd := exec.Command("git", "-C", tmpDir, "worktree", "add", worktreeDir, "-b", "test-branch") + if out, err := cmd.CombinedOutput(); err != nil { + t.Fatalf("git worktree add failed: %v\n%s", err, out) + } + defer exec.Command("git", "-C", tmpDir, "worktree", "remove", worktreeDir).Run() + + // Verify worktree has .git file (not directory) + gitPath := filepath.Join(worktreeDir, ".git") + info, err := os.Stat(gitPath) + if err != nil { + t.Fatalf("worktree .git not found: %v", err) + } + if info.IsDir() { + t.Skip("worktree has .git directory instead of file - older git version") + } + + // No rebase in worktree + if IsRebaseInProgress(worktreeDir) { + t.Error("expected no rebase in worktree") + } + + // Get the actual gitdir for the worktree to simulate rebase + gitDirCmd := exec.Command("git", "-C", worktreeDir, "rev-parse", "--git-dir") + gitDirOut, err := gitDirCmd.Output() + if err != nil { + t.Fatalf("git rev-parse --git-dir failed: %v", err) + } + worktreeGitDir := strings.TrimSpace(string(gitDirOut)) + if !filepath.IsAbs(worktreeGitDir) { + worktreeGitDir = filepath.Join(worktreeDir, worktreeGitDir) + } + + // Simulate rebase in worktree + rebaseMerge := filepath.Join(worktreeGitDir, "rebase-merge") + if err := os.MkdirAll(rebaseMerge, 0755); err != nil { + t.Fatal(err) + } + defer os.RemoveAll(rebaseMerge) + + if !IsRebaseInProgress(worktreeDir) { + t.Error("expected rebase in progress in worktree") + } + }) +}