diff --git a/cmd/prdescribe.go b/cmd/prdescribe.go index 7398ada..edc5415 100644 --- a/cmd/prdescribe.go +++ b/cmd/prdescribe.go @@ -2,12 +2,17 @@ package cmd import ( "fmt" + "strings" "github.com/fatih/color" "github.com/spf13/cobra" "gmgauthier.com/grokkit/config" + "gmgauthier.com/grokkit/internal/git" ) +// gitDiff is the mockable entry point (exactly like gitRun used elsewhere). +var gitDiff = git.Diff + var prDescribeCmd = &cobra.Command{ Use: "pr-describe", Short: "Generate full PR description from current branch", @@ -15,30 +20,32 @@ var prDescribeCmd = &cobra.Command{ } func init() { - prDescribeCmd.Flags().StringP("base", "b", "master", "Base branch to compare against") + prDescribeCmd.Flags().StringP("base", "b", "master", "Base branch to compare against (default: master)") } func runPRDescribe(cmd *cobra.Command, _ []string) { base, _ := cmd.Flags().GetString("base") - diff, err := gitRun([]string{"diff", fmt.Sprintf("%s..HEAD", base), "--no-color"}) - if err != nil || diff == "" { - diff, err = gitRun([]string{"diff", fmt.Sprintf("origin/%s..HEAD", base), "--no-color"}) + // Prefer local base, fallback to origin/. + diff, err := gitDiff([]string{"diff", fmt.Sprintf("%s..HEAD", base), "--no-color"}) + if err != nil || strings.TrimSpace(diff) == "" { + diff, err = gitDiff([]string{"diff", fmt.Sprintf("origin/%s..HEAD", base), "--no-color"}) if err != nil { color.Red("Failed to get branch diff: %v", err) return } } - if diff == "" { + if strings.TrimSpace(diff) == "" { color.Yellow("No changes on this branch compared to %s/origin/%s.", base, base) return } + modelFlag, _ := cmd.Flags().GetString("model") model := config.GetModel("prdescribe", modelFlag) client := newGrokClient() messages := buildPRDescribeMessages(diff) - color.Yellow("Writing PR description...") + color.Yellow("Writing PR description (base=%s)...", base) client.Stream(messages, model) } diff --git a/cmd/run_test.go b/cmd/run_test.go index 859f40d..bed9c2a 100644 --- a/cmd/run_test.go +++ b/cmd/run_test.go @@ -47,6 +47,13 @@ func withMockGit(fn func([]string) (string, error)) func() { return func() { gitRun = orig } } +// withMockGitDiff injects a fake for the new git.Diff (mockable var). +func withMockGitDiff(fn func([]string) (string, error)) func() { + orig := gitDiff + gitDiff = fn + return func() { gitDiff = orig } +} + // testCmd returns a minimal cobra command with common flags registered. func testCmd() *cobra.Command { c := &cobra.Command{} @@ -260,7 +267,7 @@ func TestRunPRDescribe(t *testing.T) { t.Run("no changes on branch — skips AI", func(t *testing.T) { mock := &mockStreamer{} defer withMockClient(mock)() - defer withMockGit(func(args []string) (string, error) { + defer withMockGitDiff(func(args []string) (string, error) { return "", nil // both diff calls return empty })() @@ -275,7 +282,7 @@ func TestRunPRDescribe(t *testing.T) { mock := &mockStreamer{response: "## PR Title\n\nDescription"} defer withMockClient(mock)() callCount := 0 - defer withMockGit(func(args []string) (string, error) { + defer withMockGitDiff(func(args []string) (string, error) { callCount++ if callCount == 1 { return "diff --git a/foo.go b/foo.go", nil @@ -294,7 +301,7 @@ func TestRunPRDescribe(t *testing.T) { mock := &mockStreamer{response: "PR description"} defer withMockClient(mock)() callCount := 0 - defer withMockGit(func(args []string) (string, error) { + defer withMockGitDiff(func(args []string) (string, error) { callCount++ if callCount == 2 { return "diff --git a/bar.go b/bar.go", nil @@ -313,7 +320,7 @@ func TestRunPRDescribe(t *testing.T) { mock := &mockStreamer{response: "PR description"} defer withMockClient(mock)() var capturedArgs []string - defer withMockGit(func(args []string) (string, error) { + defer withMockGitDiff(func(args []string) (string, error) { capturedArgs = args return "diff content", nil })() @@ -345,7 +352,7 @@ func TestRunPRDescribe(t *testing.T) { mock := &mockStreamer{response: "PR description"} defer withMockClient(mock)() var capturedArgs []string - defer withMockGit(func(args []string) (string, error) { + defer withMockGitDiff(func(args []string) (string, error) { capturedArgs = args return "diff content", nil })() diff --git a/internal/git/git.go b/internal/git/git.go index 48556ad..b6db72b 100644 --- a/internal/git/git.go +++ b/internal/git/git.go @@ -1,6 +1,7 @@ package git import ( + "errors" "fmt" "os/exec" "strings" @@ -60,3 +61,24 @@ func LogSince(since string) (string, error) { args := []string{"log", "--pretty=format:%s%n%b%n---", since + "..HEAD"} return Run(args) } + +// Diff runs `git diff` and tolerates exit code 1 (differences found). +// This is normal git behavior when there *are* changes — unlike .Output(). +func Diff(args []string) (string, error) { + cmdStr := "git " + strings.Join(args, " ") + logger.Debug("executing git diff", "command", cmdStr) + + // nolint:gosec // intentional git subprocess + cmd := exec.Command("git", args...) + out, err := cmd.CombinedOutput() + if err != nil { + var exitErr *exec.ExitError + if errors.As(err, &exitErr) && exitErr.ExitCode() == 1 { + // legitimate diff (changes exist) + return string(out), nil + } + logger.Error("git diff failed", "command", cmdStr, "error", err) + return "", fmt.Errorf("git diff failed: %w", err) + } + return string(out), nil +}