From 452051f0c1908e4e16a5ba49a3a298c8016e365d Mon Sep 17 00:00:00 2001 From: Gregory Gauthier Date: Tue, 31 Mar 2026 10:40:33 +0100 Subject: [PATCH 1/2] refactor(git): improve diff handling to tolerate exit code 1 - Introduce git.Diff function that uses CombinedOutput and treats exit code 1 as success (indicating changes exist). - Update prdescribe command to use git.Diff and trim whitespace when checking for empty diffs. - Enhance prdescribe output to include base branch in the status message. --- cmd/prdescribe.go | 14 +++++++++----- internal/git/git.go | 22 ++++++++++++++++++++++ 2 files changed, 31 insertions(+), 5 deletions(-) diff --git a/cmd/prdescribe.go b/cmd/prdescribe.go index 7398ada..5494ca8 100644 --- a/cmd/prdescribe.go +++ b/cmd/prdescribe.go @@ -2,10 +2,12 @@ package cmd import ( "fmt" + "strings" "github.com/fatih/color" "github.com/spf13/cobra" "gmgauthier.com/grokkit/config" + "gmgauthier.com/grokkit/internal/git" ) var prDescribeCmd = &cobra.Command{ @@ -21,24 +23,26 @@ func init() { 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 a local base, fallback to origin/. Tolerant of exit code 1. + diff, err := git.Diff([]string{"diff", fmt.Sprintf("%s..HEAD", base), "--no-color"}) + if err != nil || strings.TrimSpace(diff) == "" { + diff, err = git.Diff([]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/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 +} -- 2.39.5 From 403a408f8d2d8412b45a71aa763ee07e96531f51 Mon Sep 17 00:00:00 2001 From: Gregory Gauthier Date: Tue, 31 Mar 2026 10:54:37 +0100 Subject: [PATCH 2/2] refactor(cmd): make git diff mockable in pr-describe command and tests - Introduce `gitDiff` variable as a mockable wrapper for `git.Diff`. - Update tests to use `withMockGitDiff` for injecting diff mocks. - Minor flag description tweak for clarity. --- cmd/prdescribe.go | 11 +++++++---- cmd/run_test.go | 17 ++++++++++++----- 2 files changed, 19 insertions(+), 9 deletions(-) diff --git a/cmd/prdescribe.go b/cmd/prdescribe.go index 5494ca8..edc5415 100644 --- a/cmd/prdescribe.go +++ b/cmd/prdescribe.go @@ -10,6 +10,9 @@ import ( "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", @@ -17,16 +20,16 @@ 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") - // Prefer a local base, fallback to origin/. Tolerant of exit code 1. - diff, err := git.Diff([]string{"diff", fmt.Sprintf("%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 = git.Diff([]string{"diff", fmt.Sprintf("origin/%s..HEAD", base), "--no-color"}) + 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 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 })() -- 2.39.5