Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion cmd/prr/checkout.go
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var checkoutCmd = &cobra.Command{
}
prRef = provider.EnrichPRRef(context.Background(), prRef, prEnricherFactory(), warnf)

service := mirrorServiceFactory()
service := mirrorServiceFactory(cmd.ErrOrStderr())
commonOpts := git.EnsureOptions{
Verbose: verbose || whatIf,
WhatIf: whatIf,
Expand Down
6 changes: 4 additions & 2 deletions cmd/prr/checkout_helpers.go
Original file line number Diff line number Diff line change
@@ -1,14 +1,16 @@
package main

import (
"io"

apperrors "github.com/richardthombs/prr/internal/errors"
"github.com/richardthombs/prr/internal/git"
"github.com/richardthombs/prr/internal/provider"
"github.com/spf13/cobra"
)

var mirrorServiceFactory = func() *git.Service {
return git.NewService(git.NewExecRunner())
var mirrorServiceFactory = func(stderr io.Writer) *git.Service {
return git.NewService(git.NewExecRunnerWithStderr(stderr))
}

var prEnricherFactory = func() provider.CLIRunner {
Expand Down
5 changes: 3 additions & 2 deletions cmd/prr/checkout_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"io"
"strings"
"testing"

Expand Down Expand Up @@ -59,7 +60,7 @@ func TestCheckoutEmitsPipelineEquivalentPayload(t *testing.T) {
return "", nil
}}, t.TempDir())

mirrorServiceFactory = func() *git.Service {
mirrorServiceFactory = func(_ io.Writer) *git.Service {
return service
}

Expand Down Expand Up @@ -118,7 +119,7 @@ func TestCheckoutWhatIfSkipsExternalExecution(t *testing.T) {
}}

service := git.NewServiceWithCacheDir(runner, t.TempDir())
mirrorServiceFactory = func() *git.Service {
mirrorServiceFactory = func(_ io.Writer) *git.Service {
return service
}

Expand Down
2 changes: 1 addition & 1 deletion cmd/prr/diff.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ var diffCmd = &cobra.Command{
return apperrors.WrapConfig("worktree directory is required; provide --work-dir or stdin JSON with workDir", nil)
}

service := mirrorServiceFactory()
service := mirrorServiceFactory(cmd.ErrOrStderr())
diffOutput, err := service.DiffContributionWithOptions(context.Background(), resolvedWorkDir, strings.TrimSpace(input.BaseRef), git.EnsureOptions{
Verbose: verbose || whatIf,
WhatIf: whatIf,
Expand Down
5 changes: 3 additions & 2 deletions cmd/prr/diff_bundle_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"io"
"strings"
"testing"

Expand Down Expand Up @@ -36,7 +37,7 @@ func TestDiffCommandEmitsJSONFromStdinWorkDir(t *testing.T) {
}
}}, t.TempDir())

mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }

stdin := bytes.NewBufferString(`{"prId":12,"repoUrl":"https://github.com/acme/repo","remote":"origin","provider":"github","bareDir":"/tmp/bare","mergeRef":"refs/prr/pull/12/merge","workDir":"/tmp/work/12"}`)
stdout := &bytes.Buffer{}
Expand Down Expand Up @@ -78,7 +79,7 @@ func TestDiffCommandWhatIfLogsAndSkipsExecution(t *testing.T) {
return "", nil
}}
service := git.NewServiceWithCacheDir(runner, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }

stdin := bytes.NewBufferString(`{"workDir":"/tmp/work/99"}`)
stdout := &bytes.Buffer{}
Expand Down
2 changes: 1 addition & 1 deletion cmd/prr/pwd.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ var pwdCmd = &cobra.Command{
return err
}

service := mirrorServiceFactory()
service := mirrorServiceFactory(cmd.ErrOrStderr())

bareDir, err := service.ResolveMirrorDir(prRef.RepoURL)
if err != nil {
Expand Down
3 changes: 2 additions & 1 deletion cmd/prr/review.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ var reviewCmd = &cobra.Command{
prRef = provider.EnrichPRRef(context.Background(), prRef, prEnricherFactory(), warnf)
}

service := mirrorServiceFactory()
service := mirrorServiceFactory(cmd.ErrOrStderr())
commonOpts := git.EnsureOptions{
Verbose: verbose || whatIf,
WhatIf: whatIf,
Expand Down Expand Up @@ -221,6 +221,7 @@ var reviewCmd = &cobra.Command{
return err
}

_, _ = fmt.Fprintf(cmd.ErrOrStderr(), "[prr] starting PR review...\n")
reviewOutput, err := reviewEngineFactory().Review(context.Background(), engine.ReviewInput{
Bundle: bundlePayload,
WorkDir: workDir,
Expand Down
23 changes: 12 additions & 11 deletions cmd/prr/review_render_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import (
"context"
"encoding/json"
"errors"
"io"
"strings"
"testing"

Expand Down Expand Up @@ -44,7 +45,7 @@ func TestReviewCommandEmitsStructuredJSONAndKeepsDiagnosticsOffStdout(t *testing
}
}}, t.TempDir())

mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, name string, args ...string) (string, error) {
if name != "gh" {
Expand Down Expand Up @@ -79,8 +80,8 @@ func TestReviewCommandEmitsStructuredJSONAndKeepsDiagnosticsOffStdout(t *testing
t.Fatalf("expected review command to succeed, got %v", err)
}

if strings.TrimSpace(stderr.String()) != "" {
t.Fatalf("expected no diagnostics on stderr without verbose/what-if, got %q", stderr.String())
if !strings.Contains(stderr.String(), "[prr] starting PR review...") {
t.Fatalf("expected review commencement message on stderr, got %q", stderr.String())
}

var payload map[string]any
Expand Down Expand Up @@ -133,7 +134,7 @@ func TestReviewCommandWhatIfVerbosePrintsCommandsToStderr(t *testing.T) {
return "", nil
}}
service := git.NewServiceWithCacheDir(runner, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -188,7 +189,7 @@ func TestReviewCommandAcceptsPRURLArgument(t *testing.T) {
return "", nil
}
}}, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -245,7 +246,7 @@ func TestReviewCommandAcceptsPipedCheckoutJSONWithoutArgs(t *testing.T) {
return "", nil
}
}}, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -309,7 +310,7 @@ func TestReviewCommandBypassesSetupWithAuthoritativeCheckoutJSON(t *testing.T) {
}}

service := git.NewServiceWithCacheDir(runner, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -367,7 +368,7 @@ func TestReviewCommandPassesModelFlagToEngine(t *testing.T) {
return "", nil
}
}}, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -419,7 +420,7 @@ func TestReviewCommandClassifiesEngineFailures(t *testing.T) {
return "", nil
}
}}, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -460,7 +461,7 @@ func TestReviewCommandEmitsDeterministicJSONShape(t *testing.T) {
t.Fatalf("expected no external command execution in what-if mode")
return "", nil
}}
mirrorServiceFactory = func() *git.Service { return git.NewServiceWithCacheDir(runner, t.TempDir()) }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return git.NewServiceWithCacheDir(runner, t.TempDir()) }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) { return "[]", nil }}
}
Expand Down Expand Up @@ -528,7 +529,7 @@ func TestReviewCommandEmitsDeterministicMarkdown(t *testing.T) {
return "", nil
}
}}, t.TempDir())
mirrorServiceFactory = func() *git.Service { return service }
mirrorServiceFactory = func(_ io.Writer) *git.Service { return service }
issueRunnerFactory = func() provider.CLIRunner {
return stubRunner{runFunc: func(_ context.Context, _ string, _ ...string) (string, error) {
return `{"data":{"repository":{"pullRequest":{"closingIssuesReferences":{"nodes":[{"number":13,"url":"https://github.com/acme/repo/issues/13","title":"Issue title","body":"Issue body","state":"OPEN","labels":{"nodes":[{"name":"bug"}]}}]}}}}}`, nil
Expand Down
25 changes: 21 additions & 4 deletions internal/git/git.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"fmt"
"io"
"os/exec"
"strings"
)
Expand All @@ -12,24 +13,40 @@ type Runner interface {
Run(ctx context.Context, name string, args ...string) (string, error)
}

type ExecRunner struct{}
// ExecRunner executes real git commands. When Stderr is non-nil, git's stderr
// output (e.g. clone/fetch progress) is forwarded to that writer in addition
// to being captured for inclusion in any error messages.
type ExecRunner struct {
Stderr io.Writer
}

func NewExecRunner() Runner {
return ExecRunner{}
}

// NewExecRunnerWithStderr returns an ExecRunner that forwards git stderr output
// to the provided writer. This allows callers to surface clone and fetch progress
// to the user without interfering with stdout.
func NewExecRunnerWithStderr(stderr io.Writer) Runner {
return ExecRunner{Stderr: stderr}
}

func (r ExecRunner) Run(ctx context.Context, name string, args ...string) (string, error) {
cmd := exec.CommandContext(ctx, name, args...)

var stdout bytes.Buffer
var stderr bytes.Buffer
var stderrBuf bytes.Buffer

cmd.Stdout = &stdout
cmd.Stderr = &stderr
if r.Stderr != nil {
cmd.Stderr = io.MultiWriter(&stderrBuf, r.Stderr)
} else {
cmd.Stderr = &stderrBuf
}

err := cmd.Run()
if err != nil {
combined := strings.TrimSpace(strings.Join([]string{stderr.String(), stdout.String()}, "\n"))
combined := strings.TrimSpace(strings.Join([]string{stderrBuf.String(), stdout.String()}, "\n"))
trimmed := strings.TrimSpace(combined)
if trimmed == "" {
return "", err
Expand Down