From 452051f0c1908e4e16a5ba49a3a298c8016e365d Mon Sep 17 00:00:00 2001 From: Gregory Gauthier Date: Tue, 31 Mar 2026 10:40:33 +0100 Subject: [PATCH] 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 +}