Quiet, threshold-gated pre-push checkpoint progress (supersedes #1684)

cmd/entire/cli/checkpoint/remote/git.go

@@ -1,10 +1,12 @@
 package remote
 
 import (
+	"bytes"
 	"context"
 	"encoding/base64"
 	"errors"
 	"fmt"
+	"io"
 	"log/slog"
 	"os"
 	"os/exec"
@@ -403,7 +405,8 @@ func FetchBlobs(ctx context.Context, remote string, hashes []string) error {
 
 // PushResult holds raw porcelain output from git push.
 type PushResult struct {
-	Output string
+	Output string // stdout (porcelain status lines)
+	Stderr string // stderr (git --progress transfer lines)
 }
 
 // PushOptions configures a git push operation.
@@ -412,6 +415,14 @@ type PushOptions struct {
 	RefSpecs  []string
 	ExtraArgs []string // additional flags before remote
 	Dir       string
+	// ProgressWriter, when non-nil, receives a live copy of git's --progress
+	// stderr bytes as they are written, in addition to the buffered capture
+	// that always populates PushResult.Stderr. This lets a caller stream
+	// transfer progress (counting/compressing/writing) to a UI while the push
+	// is still running, without affecting the post-push Stderr capture used
+	// for file logging. checkpoint/remote does not parse these bytes itself —
+	// that stays in the strategy package to avoid an import cycle.
+	ProgressWriter io.Writer
 }
 
 // Push runs git push --no-verify --porcelain with token injection.
@@ -431,7 +442,7 @@ func PushWithOptions(ctx context.Context, opts PushOptions) (PushResult, error)
 		return PushResult{}, fmt.Errorf("resolve push target: %w", err)
 	}
 
-	args := []string{"push", "--no-verify", "--porcelain"}
+	args := []string{"push", "--no-verify", "--porcelain", "--progress"}
 	args = append(args, opts.ExtraArgs...)
 	args = append(args, pushTarget)
 	args = append(args, opts.RefSpecs...)
@@ -441,11 +452,31 @@ func PushWithOptions(ctx context.Context, opts PushOptions) (PushResult, error)
 		cmd.Dir = opts.Dir
 	}
 	disableTerminalPrompt(cmd)
-	output, err := cmd.CombinedOutput()
+
+	var stdout, stderr bytes.Buffer
+	cmd.Stdout = &stdout
+	if opts.ProgressWriter != nil {
+		cmd.Stderr = io.MultiWriter(&stderr, opts.ProgressWriter)
+	} else {
+		cmd.Stderr = &stderr
+	}
+	err = cmd.Run()
+	result := PushResult{
+		Output: stdout.String(),
+		Stderr: stderr.String(),
+	}
 	if err != nil {
-		return PushResult{Output: string(output)}, fmt.Errorf("git push: %w", err)
+		combined := stdout.String()
+		if stderr.Len() > 0 {
+			if combined != "" {
+				combined += "\n"
+			}
+			combined += stderr.String()
+		}
+		result.Output = combined
+		return result, fmt.Errorf("git push: %w", err)
 	}
-	return PushResult{Output: string(output)}, nil
+	return result, nil
 }
 
 // LsRemoteInDir is like LsRemote but runs in a specific directory.

cmd/entire/cli/strategy/common.go

@@ -227,7 +227,7 @@ func replayLocalCommits(ctx context.Context, repo *git.Repository, localRefName
 		return setRefHash(repo, localRefName, targetHash)
 	}
 
-	newTip, err := cherryPickOnto(ctx, repo, targetHash, localCommits, shallow)
+	newTip, err := cherryPickOnto(ctx, repo, targetHash, localCommits, shallow, nil)
 	if err != nil {
 		return fmt.Errorf("failed to replay local commits for %s: %w", localRefName, err)
 	}

cmd/entire/cli/strategy/manual_commit_push.go

@@ -9,14 +9,18 @@ import (
 	"os"
 	"os/exec"
 	"strings"
+	"time"
 
 	git "github.com/go-git/go-git/v6"
 	"github.com/go-git/go-git/v6/plumbing"
 
 	"github.com/entireio/cli/cmd/entire/cli/checkpoint"
 	checkpointremote "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote"
+	"github.com/entireio/cli/cmd/entire/cli/interactive"
 	"github.com/entireio/cli/cmd/entire/cli/logging"
+	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/settings"
+	"github.com/entireio/cli/cmd/entire/cli/trailers"
 	"github.com/entireio/cli/perf"
 	"github.com/entireio/cli/redact"
 )
@@ -105,6 +109,13 @@ func (s *ManualCommitStrategy) prePush(ctx context.Context, remote string, prote
 		}
 	}
 
+	// Session-tree summary is progress, not an error/hint — skip the git-log
+	// work entirely on non-TTY stderr (agents/CI) instead of computing it only
+	// to have pushProgressOutput() discard it.
+	if interactive.ShouldStyle(os.Stderr) {
+		printPushSummary(ctx, remote, ps.pushTarget())
+	}
+
 	// OPF pre-push rewrite: if OPF is configured, resolve the user's
 	// decision (env > settings > prompt > non-TTY auto-run), then
 	// re-redact unpushed v1 commits with OPF (producing the OPF-applied,
@@ -351,30 +362,41 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar
 	}
 
 	// Progress: pushing many refs over the network can take tens of seconds, so
-	// surface it (matching the v1 path's "[entire] Pushing ..." line) instead of
-	// leaving the user's git push apparently hung. Written to stderr, which git
-	// shows during the pre-push hook.
-	displayTarget := displayPushTarget(pushTarget)
-	fmt.Fprintf(os.Stderr, "[entire] Pushing %d checkpoint ref(s) to %s...", len(existing), displayTarget)
-	stop := startProgressDots(os.Stderr)
+	// surface it via the threshold-gated reporter instead of leaving the user's
+	// git push apparently hung. Styled-gated (interactive.ShouldStyle — respects
+	// NO_COLOR and TERM=cygwin, not merely "stderr is a terminal") and
+	// threshold-delayed: agents and CI (non-TTY stderr) see zero progress bytes.
+	rep := newPushReporter(ctx, os.Stderr, interactive.ShouldStyle(os.Stderr), time.Second)
+	rep.phase(fmt.Sprintf("syncing %d checkpoint(s)", len(existing)))
+
+	// Stream git's own --progress output live into the reporter's detail line
+	// (counting/compressing/writing) as the batch push runs, instead of only
+	// showing it after the push completes.
+	streamer := &gitProgressStreamer{
+		onEvent: func(event *gitProgressEvent) {
+			if detail := formatPushProgressDetail(event); detail != "" {
+				rep.setDetail(detail)
+			}
+		},
+	}
 
 	// Fast path: push all refs in one round-trip (fast-forward-only). If every
 	// ref was up to date or fast-forwarded, we're done.
-	batchErr := batchPushRefs(pushCtx, pushTarget, existing)
+	batchErr := batchPushRefs(pushCtx, pushTarget, existing, streamer)
 	if batchErr == nil {
-		stop(" done")
+		rep.finish(fmt.Sprintf("pushed %d checkpoint(s)", len(existing)))
 		if removeErr := queue.Remove(existing); removeErr != nil {
 			logging.Warn(ctx, "git-refs push: clear pushed refs from queue failed",
 				slog.String("error", removeErr.Error()))
 		}
 		return len(existing), nil
 	}
-	stop("")
 
 	// Non-interactive SSH auth failures cannot be fixed by per-ref
 	// fetch+replay. Surface the same actionable hint as the v1 doPushRef path
 	// (issue #1523) instead of only logging to .entire/logs/.
 	if nonInteractiveSSHAuthFailure(pushCtx, batchErr) {
+		rep.finish("")
 		fmt.Fprintf(os.Stderr, "[entire] Warning: couldn't push checkpoint refs: %v\n", batchErr)
 		printNonInteractiveSSHAuthHint()
 		printCheckpointRemoteHint(pushTarget)
@@ -386,11 +408,14 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar
 	// fetch+replay recovery, and remove from the queue only the refs that land
 	// (a genuine cherry-pick conflict leaves that ref queued for a later push,
 	// never force-overwriting the remote).
-	fmt.Fprintf(os.Stderr, "[entire] Some checkpoint refs diverged; syncing %d ref(s) individually...", len(existing))
-	stop = startProgressDots(os.Stderr)
+	rep.setDetail("") // clear the batch attempt's stale transfer detail
 	pushed := make([]plumbing.ReferenceName, 0, len(existing))
 	var firstErr error
-	for _, ref := range existing {
+	for i, ref := range existing {
+		// Recovery is per-ref — fetch the remote's version of the checkpoint,
+		// replay the local commit(s) on top, then push — so it runs one sync
+		// cycle per ref and can take a while. Surface an advancing counter.
+		rep.phase(fmt.Sprintf("syncing checkpoint %d/%d with remote", i+1, len(existing)))
 		if err := pushCheckpointRefWithRecovery(pushCtx, pushTarget, ref); err != nil {
 			logging.Warn(ctx, "git-refs push: checkpoint ref push/sync failed; left queued, not overwritten",
 				slog.String("ref", ref.String()), slog.String("error", err.Error()))
@@ -404,7 +429,7 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar
 		}
 		pushed = append(pushed, ref)
 	}
-	stop(fmt.Sprintf(" pushed %d of %d", len(pushed), len(existing)))
+	rep.finish(fmt.Sprintf("pushed %d of %d checkpoint(s)", len(pushed), len(existing)))
 	if err := queue.Remove(pushed); err != nil {
 		logging.Warn(ctx, "git-refs push: clear pushed refs from queue failed",
 			slog.String("error", err.Error()))
@@ -430,3 +455,80 @@ func cleanupPushedShadowBranches(ctx context.Context) {
 		)
 	}
 }
+
+// pushSummaryLogFormat is the `git log --format=` string runPushSummaryGitLog
+// uses to gather commits for the session-tree summary; parsePushSummaryFromLog
+// parses its output. Deliberately omits `valueonly` on the trailers
+// placeholder: parsePushSummaryFromLog's sessionTrailerRE regex matches
+// against the "Entire-Session: <id>" prefixed form, so stripping the key
+// prefix here would bucket every commit under "unknown". Kept as a
+// package-level const (baked into runPushSummaryGitLog rather than threaded
+// as a parameter) so tests can exercise the exact production format against a
+// real git log — see TestPrintPushSummaryLogFormat_TrailerGroupsUnderSessionID.
+const pushSummaryLogFormat = "%H|%s|%(trailers:key=" + trailers.SessionTrailerKey + ",separator=%x20)|%aI"
+
+// printPushSummary writes a session-level summary of pending checkpoint commits.
+// Silent on any failure — never blocks the push.
+func printPushSummary(ctx context.Context, remoteName, target string) {
+	repoRoot, err := paths.WorktreeRoot(ctx)
+	if err != nil {
+		return
+	}
+
+	branchName := paths.MetadataBranchName
+
+	var logOutput string
+	isNewBranch := false
+
+	if !checkpointremote.IsURL(target) {
+		rangeSpec := "refs/remotes/" + remoteName + "/" + branchName + "..refs/heads/" + branchName
+		firstOut, firstErr := runPushSummaryGitLog(ctx, repoRoot, rangeSpec)
+		if firstErr == nil && strings.TrimSpace(firstOut) != "" {
+			logOutput = firstOut
+		} else {
+			fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, "refs/heads/"+branchName)
+			if fallbackErr == nil && strings.TrimSpace(fallbackOut) != "" {
+				logOutput = fallbackOut
+				isNewBranch = true
+			}
+		}
+	} else {
+		fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, "refs/heads/"+branchName)
+		if fallbackErr == nil {
+			logOutput = fallbackOut
+			isNewBranch = strings.TrimSpace(logOutput) != ""
+		}
+	}
+	if strings.TrimSpace(logOutput) == "" {
+		return
+	}
+
+	summaries := parsePushSummaryFromLog(logOutput)
+	if len(summaries) == 0 {
+		return
+	}
+
+	totalCommits := len(strings.Split(strings.TrimSpace(logOutput), "\n"))
+	w := pushProgressOutput()
+	// Pass the same writer the tree is printed to so formatSessionTree styles
+	// against the real target instead of io.Discard — otherwise ANSI styling
+	// never applies even when w is a real, style-capable terminal.
+	lines := formatSessionTree(summaries, formatSessionTreeOpts{
+		TotalCommits: totalCommits,
+		IsNewBranch:  isNewBranch,
+		Writer:       w,
+	})
+	for _, line := range lines {
+		fmt.Fprintln(w, line)
+	}
+}
+
+func runPushSummaryGitLog(ctx context.Context, repoRoot, rangeSpec string) (string, error) {
+	cmd := exec.CommandContext(ctx, "git", "log", "--format="+pushSummaryLogFormat, rangeSpec)
+	cmd.Dir = repoRoot
+	out, err := cmd.Output()
+	if err != nil {
+		return "", fmt.Errorf("git log: %w", err)
+	}
+	return string(out), nil
+}

cmd/entire/cli/strategy/manual_commit_push_test.go

@@ -5,8 +5,11 @@ import (
 	"os/exec"
 	"testing"
 
+	"github.com/entireio/cli/cmd/entire/cli/paths"
 	"github.com/entireio/cli/cmd/entire/cli/testutil"
 
+	git "github.com/go-git/go-git/v6"
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
 
@@ -54,3 +57,80 @@ func TestDeferCheckpointPushOnEmptyRemote_UsesLocalTrackingRefs(t *testing.T) {
 		deferCheckpointPushOnEmptyRemote(ctx, pushSettings{remote: "origin", checkpointURL: "https://example.invalid/cp.git"}),
 		"a dedicated checkpoint remote is exempt from the guard")
 }
+
+// TestPrintPushSummaryLogFormat_TrailerGroupsUnderSessionID guards against a
+// regression where pushSummaryLogFormat's %(trailers:...) placeholder used
+// `valueonly`, which strips the "Entire-Session: " key prefix that
+// parsePushSummaryFromLog's regex requires. Without the prefix every commit
+// falls back to the "unknown" bucket. This runs the real `git log` (via
+// runPushSummaryGitLog) against a real commit carrying the trailer, so
+// re-adding `valueonly` to pushSummaryLogFormat makes this test fail.
+func TestPrintPushSummaryLogFormat_TrailerGroupsUnderSessionID(t *testing.T) {
+	// No t.Parallel: uses t.Chdir via testutil helpers reading cwd-independent
+	// paths only (repoRoot passed explicitly to runPushSummaryGitLog).
+	dir := t.TempDir()
+	testutil.InitRepo(t, dir)
+	testutil.WriteFile(t, dir, "f.txt", "init")
+	testutil.GitAdd(t, dir, "f.txt")
+	testutil.GitCommit(t, dir, "init")
+
+	run := func(args ...string) {
+		t.Helper()
+		cmd := exec.CommandContext(t.Context(), "git", args...)
+		cmd.Dir = dir
+		require.NoError(t, cmd.Run(), "git %v", args)
+	}
+	// Orphan branch, matching how the manual-commit strategy actually creates
+	// the metadata branch: the checkpoint commit has no ancestor commit
+	// (which would otherwise carry no trailer and legitimately bucket under
+	// "unknown", muddying the assertion below).
+	run("checkout", "--orphan", paths.MetadataBranchName)
+	run("rm", "-rf", "--cached", ".")
+	testutil.WriteFile(t, dir, "checkpoint.txt", "data")
+	testutil.GitAdd(t, dir, "checkpoint.txt")
+	testutil.GitCommit(t, dir, "Checkpoint: abc1234\n\nEntire-Session: sess-real-123")
+
+	out, err := runPushSummaryGitLog(t.Context(), dir, "refs/heads/"+paths.MetadataBranchName)
+	require.NoError(t, err)
+
+	summaries := parsePushSummaryFromLog(out)
+	require.Len(t, summaries, 1)
+	assert.Equal(t, "sess-real-123", summaries[0].SessionID,
+		"a real Entire-Session trailer must group under its session id, not fall back to unknown")
+}
+
+// TestFlushCheckpointRefsQueue_NonTTY_NoProgressOutput verifies the git-refs
+// default pre-push path stays silent on a non-TTY writer while still landing
+// the queued refs. captureStderr's os.Pipe write end is never a terminal, so
+// interactive.IsTerminalWriter(os.Stderr) is false for the duration of the
+// call without any extra faking.
+//
+// Not parallel: uses captureStderr's os.Stderr redirection and t.Chdir.
+func TestFlushCheckpointRefsQueue_NonTTY_NoProgressOutput(t *testing.T) {
+	testutil.IsolateGitConfigEnv(t)
+	workDir, bareDir, refs := setupRepoWithCheckpointRefs(t)
+	t.Chdir(workDir)
+	paths.ClearWorktreeRootCache()
+
+	repo, err := git.PlainOpen(workDir)
+	require.NoError(t, err)
+	queue := enqueueRefs(t, repo, refs)
+
+	restore := captureStderr(t)
+	pushed, pushDisabled, err := PushQueuedCheckpointRefs(context.Background(), repo, bareDir)
+	output := restore()
+
+	require.NoError(t, err)
+	assert.False(t, pushDisabled)
+	assert.Equal(t, len(refs), pushed)
+
+	assert.NotContains(t, output, "[entire] Pushing", "non-TTY stderr must contain no progress banner")
+	assert.NotContains(t, output, "syncing", "non-TTY stderr must contain no progress text")
+
+	for _, ref := range refs {
+		assert.NotEmpty(t, remoteRefHash(t, bareDir, ref), "ref should still land on the remote")
+	}
+	remaining, err := queue.Drain()
+	require.NoError(t, err)
+	assert.Empty(t, remaining, "pushed refs are removed from the queue")
+}