From dff17d684a981660f3c342c7fbe8c4ef7a837641 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Tue, 28 Jul 2026 18:09:28 -0500
Subject: [PATCH 1/2] fix(git): parse clone progress into thinned log lines
 instead of raw passthrough

git clone already runs with --progress (added in #590), but its
\r-delimited "Receiving objects: NN% (x/y)" frames were logged
verbatim, producing a wall of near-duplicate lines for large clones.

progressWriter splits on \r as well as \n (git overwrites its progress
line with \r rather than emitting a new one), logs percentage updates
thinned to one per 10 points, and passes any other line through
unchanged.
---
 pkg/git/progress.go      |  82 +++++++++++++++++++++++++++++
 pkg/git/progress_test.go | 108 +++++++++++++++++++++++++++++++++++++++
 pkg/git/repo.go          |   7 ++-
 3 files changed, 195 insertions(+), 2 deletions(-)
 create mode 100644 pkg/git/progress.go
 create mode 100644 pkg/git/progress_test.go

diff --git a/pkg/git/progress.go b/pkg/git/progress.go
new file mode 100644
index 000000000..4d27643ef
--- /dev/null
+++ b/pkg/git/progress.go
@@ -0,0 +1,82 @@
+package git
+
+import (
+	"bytes"
+	"io"
+	"regexp"
+	"strconv"
+
+	"github.com/devsy-org/devsy/pkg/log"
+)
+
+// progressLine matches git's repeating "<label>: NN% (x/y)" progress
+// updates, e.g. "Receiving objects:  47% (156/333)".
+var progressLine = regexp.MustCompile(`^(.+?):\s+(\d+)% \(\d+/\d+\)`)
+
+// progressWriter splits git's --progress output on '\r' as well as '\n'
+// (git overwrites a line with '\r' rather than emitting a new one), passes
+// plain informational lines to next unchanged, and logs percentage updates
+// thinned to one per 10 points instead of forwarding every frame.
+type progressWriter struct {
+	next    io.Writer
+	buf     bytes.Buffer
+	lastPct map[string]int
+}
+
+func newProgressWriter(next io.Writer) *progressWriter {
+	return &progressWriter{next: next, lastPct: map[string]int{}}
+}
+
+func (w *progressWriter) Write(p []byte) (int, error) {
+	for _, b := range p {
+		if b == '\r' || b == '\n' {
+			if err := w.flush(); err != nil {
+				return len(p), err
+			}
+			continue
+		}
+		w.buf.WriteByte(b)
+	}
+	return len(p), nil
+}
+
+func (w *progressWriter) Close() error {
+	return w.flush()
+}
+
+func (w *progressWriter) flush() error {
+	line := w.buf.String()
+	w.buf.Reset()
+	if line == "" || w.reportProgress(line) {
+		return nil
+	}
+	_, err := w.next.Write(append([]byte(line), '\n'))
+	return err
+}
+
+// reportProgress returns true if line was a percentage update (thinned or
+// not), so the caller knows not to also write it to the log.
+func (w *progressWriter) reportProgress(line string) bool {
+	m := progressLine.FindStringSubmatch(line)
+	if m == nil {
+		return false
+	}
+	label, pct := m[1], atoiOr(m[2], -1)
+	last, seen := w.lastPct[label]
+	w.lastPct[label] = pct
+	if seen && pct == last {
+		return true // exact duplicate frame (e.g. object count didn't change)
+	}
+	if pct%10 == 0 || pct == 100 {
+		log.Infof("%s", line)
+	}
+	return true
+}
+
+func atoiOr(s string, fallback int) int {
+	n, err := strconv.Atoi(s)
+	if err != nil {
+		return fallback
+	}
+	return n
+}
diff --git a/pkg/git/progress_test.go b/pkg/git/progress_test.go
new file mode 100644
index 000000000..9764c3b29
--- /dev/null
+++ b/pkg/git/progress_test.go
@@ -0,0 +1,108 @@
+package git
+
+import (
+	"bytes"
+	"fmt"
+	"strings"
+	"testing"
+)
+
+func TestProgressWriter_SplitsOnCarriageReturn(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	_, err := w.Write([]byte(
+		"Cloning into 'repo'...\n" +
+			"Receiving objects:   0% (1/333)\r" +
+			"Receiving objects:  10% (34/333)\r" +
+			"Receiving objects: 100% (333/333), done.\n",
+	))
+	if err != nil {
+		t.Fatalf("Write: %v", err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatalf("Close: %v", err)
+	}
+
+	// Only the plain informational line reaches the log; percentage frames
+	// are logged separately (via pkg/log, not through `out`).
+	if got := strings.TrimRight(out.String(), "\n"); got != "Cloning into 'repo'..." {
+		t.Errorf("log output = %q", got)
+	}
+}
+
+func TestProgressWriter_ThinsIntermediatePercentages(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	for pct := 0; pct <= 100; pct++ {
+		_, _ = w.Write(fmt.Appendf(nil, "Receiving objects: %3d%% (1/1)\r", pct))
+	}
+	_ = w.Close()
+
+	// Percentage frames never reach `next`, regardless of thinning.
+	if out.Len() != 0 {
+		t.Errorf("expected no log output, got %q", out.String())
+	}
+}
+
+func TestProgressWriter_DropsExactDuplicateFrame(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	_, _ = w.Write([]byte(
+		"Receiving objects:  10% (34/333)\r" +
+			"Receiving objects:  10% (34/333)\r",
+	))
+	_ = w.Close()
+
+	if out.Len() != 0 {
+		t.Errorf("expected no log output, got %q", out.String())
+	}
+}
+
+func TestProgressWriter_TracksLabelsIndependently(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	_, _ = w.Write([]byte(
+		"Receiving objects:  10% (34/333)\r" +
+			"Resolving deltas:  10% (10/100)\r",
+	))
+	_ = w.Close()
+
+	if out.Len() != 0 {
+		t.Errorf("expected no log output, got %q", out.String())
+	}
+}
+
+func TestProgressWriter_PassesThroughNonProgressLines(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	_, _ = w.Write([]byte(
+		"Cloning into 'repo'...\n" +
+			"remote: Enumerating objects: 333, done.\n",
+	))
+	_ = w.Close()
+
+	got := out.String()
+	if !strings.Contains(got, "Cloning into 'repo'...") ||
+		!strings.Contains(got, "remote: Enumerating objects: 333, done.") {
+		t.Errorf("expected both lines passed through unchanged, got %q", got)
+	}
+}
+
+func TestProgressWriter_FlushesTrailingPartialLineOnClose(t *testing.T) {
+	var out bytes.Buffer
+	w := newProgressWriter(&out)
+
+	_, _ = w.Write([]byte("no trailing newline"))
+	if out.Len() != 0 {
+		t.Errorf("expected nothing forwarded before close, got %q", out.String())
+	}
+	_ = w.Close()
+	if got := strings.TrimRight(out.String(), "\n"); got != "no trailing newline" {
+		t.Errorf("got %q", got)
+	}
+}
diff --git a/pkg/git/repo.go b/pkg/git/repo.go
index 553c3128f..2cab1e873 100644
--- a/pkg/git/repo.go
+++ b/pkg/git/repo.go
@@ -262,6 +262,9 @@ func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig)
 	w := log.Writer(log.LevelInfo)
 	defer func() { _ = w.Close() }()
 
+	pw := newProgressWriter(w)
+	defer func() { _ = pw.Close() }()
+
 	env := append([]string{}, r.env...)
 	if smudgeSkippedForClone(c.lfsMode) {
 		env = append(env, "GIT_LFS_SKIP_SMUDGE=1")
@@ -270,8 +273,8 @@ func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig)
 	_, err := r.runner.Run(ctx, RunOptions{
 		Env:    env,
 		Args:   c.args(repository, r.path),
-		Stdout: w,
-		Stderr: w,
+		Stdout: pw,
+		Stderr: pw,
 	})
 	return err
 }

From 93caac09b283eb96c217939c626e7785b4b87016 Mon Sep 17 00:00:00 2001
From: Samuel K <skevetter@pm.me>
Date: Tue, 28 Jul 2026 19:18:45 -0500
Subject: [PATCH 2/2] fix(git): address CodeRabbit review on clone progress
 writer

- progressWriter.Write now returns the byte offset through the
  delimiter it flushed at on a flush error, instead of len(p) --
  satisfies the io.Writer contract that n reflects bytes actually
  consumed when err != nil.
- cloneWith now calls progressWriter.Close() explicitly after Run and
  surfaces its error when the clone itself succeeded, instead of
  discarding a trailing-flush failure via a bare deferred Close.
- Progress-writer tests now capture actual log output (via
  pkg/log.AddSink) and assert the real 0/10/.../100 thinning cadence,
  duplicate-frame suppression, and independent per-label state,
  instead of only asserting that percentage frames don't reach `next`.
---
 pkg/git/progress.go      |  4 +--
 pkg/git/progress_test.go | 63 +++++++++++++++++++++++++++++++++++-----
 pkg/git/repo.go          |  4 ++-
 3 files changed, 61 insertions(+), 10 deletions(-)

diff --git a/pkg/git/progress.go b/pkg/git/progress.go
index 4d27643ef..30f95d4a6 100644
--- a/pkg/git/progress.go
+++ b/pkg/git/progress.go
@@ -28,10 +28,10 @@ func newProgressWriter(next io.Writer) *progressWriter {
 }
 
 func (w *progressWriter) Write(p []byte) (int, error) {
-	for _, b := range p {
+	for i, b := range p {
 		if b == '\r' || b == '\n' {
 			if err := w.flush(); err != nil {
-				return len(p), err
+				return i + 1, err
 			}
 			continue
 		}
diff --git a/pkg/git/progress_test.go b/pkg/git/progress_test.go
index 9764c3b29..ab264be0e 100644
--- a/pkg/git/progress_test.go
+++ b/pkg/git/progress_test.go
@@ -5,8 +5,12 @@ import (
 	"fmt"
 	"strings"
 	"testing"
+
+	"github.com/devsy-org/devsy/pkg/log"
 )
 
+const testLogFormatText = "text"
+
 func TestProgressWriter_SplitsOnCarriageReturn(t *testing.T) {
 	var out bytes.Buffer
 	w := newProgressWriter(&out)
@@ -32,6 +36,11 @@ func TestProgressWriter_SplitsOnCarriageReturn(t *testing.T) {
 }
 
 func TestProgressWriter_ThinsIntermediatePercentages(t *testing.T) {
+	log.Init(log.Config{Verbosity: 2, Format: testLogFormatText})
+	var sink bytes.Buffer
+	remove := log.AddSink(&sink)
+	defer remove()
+
 	var out bytes.Buffer
 	w := newProgressWriter(&out)
 
@@ -39,14 +48,32 @@ func TestProgressWriter_ThinsIntermediatePercentages(t *testing.T) {
 		_, _ = w.Write(fmt.Appendf(nil, "Receiving objects: %3d%% (1/1)\r", pct))
 	}
 	_ = w.Close()
+	_ = log.Sync()
 
 	// Percentage frames never reach `next`, regardless of thinning.
 	if out.Len() != 0 {
-		t.Errorf("expected no log output, got %q", out.String())
+		t.Errorf("expected no log output via next, got %q", out.String())
+	}
+
+	// Only 0, 10, 20, ..., 100 should have been logged, in that order.
+	lines := strings.Split(strings.TrimSpace(sink.String()), "\n")
+	if len(lines) != 11 {
+		t.Fatalf("got %d progress log lines, want 11: %q", len(lines), sink.String())
+	}
+	for i, want := range []int{0, 10, 20, 30, 40, 50, 60, 70, 80, 90, 100} {
+		wantSub := fmt.Sprintf("%d%%", want)
+		if !strings.Contains(lines[i], wantSub) {
+			t.Errorf("line %d = %q, want to contain %q", i, lines[i], wantSub)
+		}
 	}
 }
 
 func TestProgressWriter_DropsExactDuplicateFrame(t *testing.T) {
+	log.Init(log.Config{Verbosity: 2, Format: testLogFormatText})
+	var sink bytes.Buffer
+	remove := log.AddSink(&sink)
+	defer remove()
+
 	var out bytes.Buffer
 	w := newProgressWriter(&out)
 
@@ -55,13 +82,23 @@ func TestProgressWriter_DropsExactDuplicateFrame(t *testing.T) {
 			"Receiving objects:  10% (34/333)\r",
 	))
 	_ = w.Close()
-
-	if out.Len() != 0 {
-		t.Errorf("expected no log output, got %q", out.String())
+	_ = log.Sync()
+
+	lines := strings.Split(strings.TrimSpace(sink.String()), "\n")
+	if len(lines) != 1 {
+		t.Fatalf(
+			"got %d progress log lines, want 1 (duplicate dropped): %q",
+			len(lines), sink.String(),
+		)
 	}
 }
 
 func TestProgressWriter_TracksLabelsIndependently(t *testing.T) {
+	log.Init(log.Config{Verbosity: 2, Format: testLogFormatText})
+	var sink bytes.Buffer
+	remove := log.AddSink(&sink)
+	defer remove()
+
 	var out bytes.Buffer
 	w := newProgressWriter(&out)
 
@@ -70,9 +107,21 @@ func TestProgressWriter_TracksLabelsIndependently(t *testing.T) {
 			"Resolving deltas:  10% (10/100)\r",
 	))
 	_ = w.Close()
-
-	if out.Len() != 0 {
-		t.Errorf("expected no log output, got %q", out.String())
+	_ = log.Sync()
+
+	got := sink.String()
+	lines := strings.Split(strings.TrimSpace(got), "\n")
+	if len(lines) != 2 {
+		t.Fatalf(
+			"got %d progress log lines, want 2 (independent labels): %q",
+			len(lines), got,
+		)
+	}
+	if !strings.Contains(lines[0], "Receiving objects") {
+		t.Errorf("line 0 = %q, want to contain %q", lines[0], "Receiving objects")
+	}
+	if !strings.Contains(lines[1], "Resolving deltas") {
+		t.Errorf("line 1 = %q, want to contain %q", lines[1], "Resolving deltas")
 	}
 }
 
diff --git a/pkg/git/repo.go b/pkg/git/repo.go
index 2cab1e873..d55ae65fa 100644
--- a/pkg/git/repo.go
+++ b/pkg/git/repo.go
@@ -263,7 +263,6 @@ func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig)
 	defer func() { _ = w.Close() }()
 
 	pw := newProgressWriter(w)
-	defer func() { _ = pw.Close() }()
 
 	env := append([]string{}, r.env...)
 	if smudgeSkippedForClone(c.lfsMode) {
@@ -276,6 +275,9 @@ func (r *Repo) cloneWith(ctx context.Context, repository string, c cloneConfig)
 		Stdout: pw,
 		Stderr: pw,
 	})
+	if closeErr := pw.Close(); err == nil {
+		err = closeErr
+	}
 	return err
 }