From 5eb7ef100a0d44ca46c9a6b72639c518121c0103 Mon Sep 17 00:00:00 2001 From: Victor Gutierrez Calderon Date: Fri, 31 Jul 2026 13:51:34 +0200 Subject: [PATCH 1/5] feat(push): quiet, threshold-gated pre-push checkpoint progress Rework of the pre-push checkpoint-sync progress UX so it is useful to a human at a terminal but silent for the agents and CI that actually run most `git push`es. Now that git-refs is the default checkpoint backend, the old path printed an unconditional "[entire] Pushing N checkpoint ref(s)..." line plus a dot spinner to stderr that nothing downstream reads. - New pushReporter: presence-gated on IsTerminalWriter (non-TTY writes zero bytes), reveals a single in-place line only after a ~2s threshold, and clears it on completion (no scrollback residue). - git-refs default path (flushCheckpointRefsQueue) rewired to the reporter; removed the now-dead startProgressDots. - git --progress transfer detail routed to .entire/logs/ (operational metadata only) on BOTH backends, regardless of TTY. - Legacy git-branch path also silenced for non-TTY; error/actionable-hint lines still print unconditionally on both paths. - Push semantics unchanged (display only, fast-forward-only, never blocks the user's push). Builds on and supersedes #1684. Co-Authored-By: snowingfox Entire-Checkpoint: 01KYW08431VR6EKA854NP1A0SR --- cmd/entire/cli/checkpoint/remote/git.go | 28 +- cmd/entire/cli/strategy/common.go | 2 +- cmd/entire/cli/strategy/manual_commit_push.go | 93 +++- .../cli/strategy/manual_commit_push_test.go | 38 ++ cmd/entire/cli/strategy/metadata_reconcile.go | 12 +- cmd/entire/cli/strategy/push_common.go | 146 +++--- cmd/entire/cli/strategy/push_common_test.go | 123 ++++- cmd/entire/cli/strategy/push_progress.go | 429 ++++++++++++++++++ cmd/entire/cli/strategy/push_progress_test.go | 308 +++++++++++++ cmd/entire/cli/strategy/push_reporter.go | 135 ++++++ cmd/entire/cli/strategy/push_reporter_test.go | 70 +++ 11 files changed, 1280 insertions(+), 104 deletions(-) create mode 100644 cmd/entire/cli/strategy/push_progress.go create mode 100644 cmd/entire/cli/strategy/push_progress_test.go create mode 100644 cmd/entire/cli/strategy/push_reporter.go create mode 100644 cmd/entire/cli/strategy/push_reporter_test.go diff --git a/cmd/entire/cli/checkpoint/remote/git.go b/cmd/entire/cli/checkpoint/remote/git.go index b80534a4ef..35746f9e35 100644 --- a/cmd/entire/cli/checkpoint/remote/git.go +++ b/cmd/entire/cli/checkpoint/remote/git.go @@ -1,6 +1,7 @@ package remote import ( + "bytes" "context" "encoding/base64" "errors" @@ -403,7 +404,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. @@ -431,7 +433,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 +443,27 @@ 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 + 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. diff --git a/cmd/entire/cli/strategy/common.go b/cmd/entire/cli/strategy/common.go index cdce76a108..575e7c70b1 100644 --- a/cmd/entire/cli/strategy/common.go +++ b/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) } diff --git a/cmd/entire/cli/strategy/manual_commit_push.go b/cmd/entire/cli/strategy/manual_commit_push.go index 5ab8070ca6..a0c39b8c03 100644 --- a/cmd/entire/cli/strategy/manual_commit_push.go +++ b/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.IsTerminalWriter(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,30 @@ 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. + // surface it via the threshold-gated reporter instead of leaving the user's + // git push apparently hung. TTY-gated and threshold-delayed: agents and CI + // (non-TTY stderr) see zero progress bytes. displayTarget := displayPushTarget(pushTarget) - fmt.Fprintf(os.Stderr, "[entire] Pushing %d checkpoint ref(s) to %s...", len(existing), displayTarget) - stop := startProgressDots(os.Stderr) + rep := newPushReporter(ctx, os.Stderr, interactive.IsTerminalWriter(os.Stderr), 2*time.Second) + rep.phase(fmt.Sprintf("syncing %d checkpoint(s) to %s", len(existing), displayTarget)) // 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) 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,8 +397,7 @@ 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.phase(fmt.Sprintf("resolving %d diverged checkpoint(s)", len(existing))) pushed := make([]plumbing.ReferenceName, 0, len(existing)) var firstErr error for _, ref := range existing { @@ -404,7 +414,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 +440,66 @@ func cleanupPushedShadowBranches(ctx context.Context) { ) } } + +// 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 + logFormat := "%H|%s|%(trailers:key=" + trailers.SessionTrailerKey + ",valueonly,separator=%x20)|%aI" + + var logOutput string + isNewBranch := false + + if !checkpointremote.IsURL(target) { + rangeSpec := "refs/remotes/" + remoteName + "/" + branchName + "..refs/heads/" + branchName + firstOut, firstErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, rangeSpec) + if firstErr == nil && strings.TrimSpace(firstOut) != "" { + logOutput = firstOut + } else { + fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, "refs/heads/"+branchName) + if fallbackErr == nil && strings.TrimSpace(fallbackOut) != "" { + logOutput = fallbackOut + isNewBranch = true + } + } + } else { + fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, "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")) + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: totalCommits, + IsNewBranch: isNewBranch, + }) + w := pushProgressOutput() + for _, line := range lines { + fmt.Fprintln(w, line) + } +} + +func runPushSummaryGitLog(ctx context.Context, repoRoot, format, rangeSpec string) (string, error) { + cmd := exec.CommandContext(ctx, "git", "log", "--format="+format, rangeSpec) + cmd.Dir = repoRoot + out, err := cmd.Output() + if err != nil { + return "", fmt.Errorf("git log: %w", err) + } + return string(out), nil +} diff --git a/cmd/entire/cli/strategy/manual_commit_push_test.go b/cmd/entire/cli/strategy/manual_commit_push_test.go index 75485f2ed4..44dcc1679e 100644 --- a/cmd/entire/cli/strategy/manual_commit_push_test.go +++ b/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,38 @@ 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") } + +// 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) { + 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") +} diff --git a/cmd/entire/cli/strategy/metadata_reconcile.go b/cmd/entire/cli/strategy/metadata_reconcile.go index 35e70f4a2a..9befd25db5 100644 --- a/cmd/entire/cli/strategy/metadata_reconcile.go +++ b/cmd/entire/cli/strategy/metadata_reconcile.go @@ -194,7 +194,7 @@ func ReconcileDisconnectedMetadataRef( fmt.Fprintf(w, "[entire] Cherry-picking %d local checkpoint(s) onto remote...\n", len(dataCommits)) - newTip, err := cherryPickOnto(ctx, repo, remoteHash, dataCommits, shallow) + newTip, err := cherryPickOnto(ctx, repo, remoteHash, dataCommits, shallow, nil) if err != nil { return fmt.Errorf("failed to cherry-pick local commits onto remote: %w", err) } @@ -305,6 +305,9 @@ func loadShallowHashes(ctx context.Context, repoPath string) (map[plumbing.Hash] return set, nil } +// cherryPickProgress reports cherry-pick replay progress (current/total commits). +type cherryPickProgress func(current, total int) + // cherryPickOnto applies each commit's delta onto base, building a linear chain. // For each commit, it computes the full diff from its parent (additions, modifications, // and deletions), then applies that delta onto the current tip's tree. @@ -314,8 +317,9 @@ func loadShallowHashes(ctx context.Context, repoPath string) (map[plumbing.Hash] // a shallow-boundary commit would be diffed against a stale parent tree whose // objects live in the local pack but no longer represent the actual checkpoint // history — producing nonsense changes when replayed onto the remote tip. -func cherryPickOnto(ctx context.Context, repo *git.Repository, base plumbing.Hash, commits []*object.Commit, shallow map[plumbing.Hash]bool) (plumbing.Hash, error) { +func cherryPickOnto(ctx context.Context, repo *git.Repository, base plumbing.Hash, commits []*object.Commit, shallow map[plumbing.Hash]bool, onProgress cherryPickProgress) (plumbing.Hash, error) { currentTip := base + processed := 0 for _, commit := range commits { changes, err := treeChangesForCherryPick(ctx, repo, commit, shallow) @@ -343,6 +347,10 @@ func cherryPickOnto(ctx context.Context, repo *git.Repository, base plumbing.Has } currentTip = newHash + processed++ + if onProgress != nil { + onProgress(processed, len(commits)) + } } return currentTip, nil diff --git a/cmd/entire/cli/strategy/push_common.go b/cmd/entire/cli/strategy/push_common.go index 05372ab766..4d86b338e1 100644 --- a/cmd/entire/cli/strategy/push_common.go +++ b/cmd/entire/cli/strategy/push_common.go @@ -13,6 +13,7 @@ import ( "time" "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/settings" "github.com/entireio/cli/perf" @@ -63,7 +64,9 @@ func batchPushRefs(ctx context.Context, target string, refs []plumbing.Reference for _, ref := range refs { refSpecs = append(refSpecs, ref.String()+":"+ref.String()) } - if _, err := remote.PushWithOptions(ctx, remote.PushOptions{Remote: target, RefSpecs: refSpecs}); err != nil { + result, err := remote.PushWithOptions(ctx, remote.PushOptions{Remote: target, RefSpecs: refSpecs}) + logGitProgress(ctx, result.Stderr) + if err != nil { return fmt.Errorf("push %d checkpoint refs: %w", len(refs), err) } return nil @@ -88,7 +91,7 @@ func pushCheckpointRefWithRecovery(ctx context.Context, target string, ref plumb if err := batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}); err == nil { return nil } - if err := fetchAndRebaseRefCommon(ctx, target, ref); err != nil { + if err := fetchAndRebaseRefCommon(ctx, target, ref, io.Discard); err != nil { return fmt.Errorf("sync diverged checkpoint ref %s: %w", ref, err) } return batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}) @@ -146,6 +149,22 @@ func displayPushTarget(target string) string { return target } +// pushProgressOutput returns the writer for checkpoint push progress lines +// (the git-branch legacy backend's phased "Pushing/Syncing/done" output and +// the session-tree summary). Always resolves os.Stderr at call time so tests +// can redirect stderr. Resolves to io.Discard when stderr isn't a real +// terminal so agents/CI on this legacy backend don't get spammed — mirrors +// the git-refs default path's non-TTY silence. Actionable errors and hints +// (protected-ref blocks, push/sync warnings, ssh-agent/checkpoint-remote +// hints) must NOT be routed through this writer; they write to os.Stderr +// directly so they still print on non-TTY. +func pushProgressOutput() io.Writer { + if !interactive.IsTerminalWriter(os.Stderr) { + return io.Discard + } + return os.Stderr +} + // checkpointPushBudget is one shared deadline across the initial push, // fetch+rebase, and retry — per-attempt timeouts can stack to ~3x. var so tests // can shrink it. @@ -157,21 +176,23 @@ func doPushRef(ctx context.Context, target string, ref plumbing.ReferenceName) e ctx, cancel := context.WithTimeout(ctx, checkpointPushBudget) defer cancel() + w := pushProgressOutput() + styles := pushProgressStylesFor(w, false) displayTarget := displayPushTarget(target) refLabel := refDisplayName(ref) - fmt.Fprintf(os.Stderr, "[entire] Pushing %s to %s...", refLabel, displayTarget) - stop := startProgressDots(os.Stderr) + fmt.Fprintf(w, "%s Pushing %s to %s...\n", styles.dim("[entire]"), refLabel, displayTarget) + pushStart := time.Now() - // Try pushing first result, err := tryPushRefCommon(ctx, target, ref) if err == nil { - finishPush(ctx, stop, result, target) + writePushFinishLine(ctx, w, result, pushStart, target) return nil } - stop("") - // Protected refs cannot be fixed by syncing and retrying. + // Protected refs cannot be fixed by syncing and retrying. This is an + // actionable error, not progress — always print it, even on non-TTY + // stderr, so it bypasses the gated pushProgressOutput() writer. var protectedErr *protectedRefError if errors.As(err, &protectedErr) { printProtectedRefBlock(os.Stderr, refLabel, target) @@ -181,6 +202,7 @@ func doPushRef(ctx context.Context, target string, ref plumbing.ReferenceName) e // Non-interactive SSH (pre-push BatchMode): auth failures cannot be fixed by // fetch+rebase, and retrying would just reprint the same opaque error. // Surface an actionable ssh-agent hint and skip recovery (issue #1523). + // Actionable warning/hint — bypass the gated writer, print unconditionally. if nonInteractiveSSHAuthFailure(ctx, err) { fmt.Fprintf(os.Stderr, "[entire] Warning: couldn't push %s: %v\n", refLabel, err) printNonInteractiveSSHAuthHint() @@ -188,18 +210,19 @@ func doPushRef(ctx context.Context, target string, ref plumbing.ReferenceName) e return nil } - // Push failed - likely non-fast-forward. Try to fetch and rebase. + writePushConflictLine(w, pushStart) + + // Push failed — try to fetch and rebase. // Spanned (with the network fetch as a child) so the trace distinguishes // "the raw push is slow" from "we keep hitting contention and re-syncing". - fmt.Fprintf(os.Stderr, "[entire] Syncing %s with remote...", refLabel) - stop = startProgressDots(os.Stderr) + fmt.Fprintf(w, "%s Syncing with remote...\n", styles.dim("[entire]")) frCtx, fetchRebaseSpan := perf.Start(ctx, "fetch_and_rebase") - syncErr := fetchAndRebaseRefCommon(frCtx, target, ref) + syncErr := fetchAndRebaseRefCommon(frCtx, target, ref, w) fetchRebaseSpan.RecordError(syncErr) fetchRebaseSpan.End() if syncErr != nil { - stop("") + // Actionable warning — bypass the gated writer, print unconditionally. fmt.Fprintf(os.Stderr, "[entire] Warning: couldn't sync %s: %v\n", refLabel, syncErr) if nonInteractiveSSHAuthFailure(ctx, syncErr) { printNonInteractiveSSHAuthHint() @@ -207,21 +230,19 @@ func doPushRef(ctx context.Context, target string, ref plumbing.ReferenceName) e printCheckpointRemoteHint(target) return nil // Don't fail the main push } - stop(" done") - // Try pushing again after rebase - fmt.Fprintf(os.Stderr, "[entire] Pushing %s to %s...", refLabel, displayTarget) - stop = startProgressDots(os.Stderr) + fmt.Fprintf(w, "%s Pushing %s to %s...\n", styles.dim("[entire]"), refLabel, displayTarget) + retryStart := time.Now() if result, err := tryPushRefCommon(ctx, target, ref); err != nil { - stop("") + // Actionable warning — bypass the gated writer, print unconditionally. fmt.Fprintf(os.Stderr, "[entire] Warning: failed to push %s after sync: %v\n", refLabel, err) if nonInteractiveSSHAuthFailure(ctx, err) { printNonInteractiveSSHAuthHint() } printCheckpointRemoteHint(target) } else { - finishPush(ctx, stop, result, target) + writePushFinishLine(ctx, w, result, retryStart, target) } return nil @@ -329,18 +350,6 @@ func parsePushResult(output string) pushResult { return pushResult{upToDate: false} } -// finishPush stops the progress dots and prints "already up-to-date" or "done" -// depending on the push result. Only prints the settings commit hint when new -// content was actually pushed. -func finishPush(ctx context.Context, stop func(string), result pushResult, target string) { - if result.upToDate { - stop(" already up-to-date") - } else { - stop(" done") - printSettingsCommitHint(ctx, target) - } -} - // tryPushRefCommon attempts to push a ref. No timeout of its own — // runs under doPushRef's shared budget. Branch refs use a bare branch-name // refSpec so existing remote-tracking works; non-branch refs use an explicit @@ -355,17 +364,20 @@ func tryPushRefCommon(ctx context.Context, remoteName string, ref plumbing.Refer refSpec = ref.String() + ":" + ref.String() } - // Span the actual `git push` subprocess: on a slow remote (e.g. a custom - // git transport) this is typically where pre-push time is spent. Called once - // per push attempt, so a retry after fetch+rebase shows up as a second - // git_push step (git_push~1) in the trace. A rejected first push records an - // error flag, which signals the recovery path was taken. _, pushSpan := perf.Start(ctx, "git_push") - result, err := remote.Push(ctx, remoteName, refSpec) + pushOut, err := remote.Push(ctx, remoteName, refSpec) pushSpan.RecordError(err) pushSpan.End() - outputStr := result.Output + displayGitProgress(pushProgressOutput(), pushOut.Stderr) + // File-log transfer detail unconditionally, matching the git-refs backend + // (batchPushRefs): displayGitProgress's writer is gated to io.Discard on + // non-TTY, so without this the legacy git-branch path would drop + // --progress transfer detail entirely instead of recording it to + // .entire/logs/. + logGitProgress(ctx, pushOut.Stderr) + + outputStr := pushOut.Output if err != nil { return pushResult{}, classifyPushFailure(ctx, outputStr, err) } @@ -455,7 +467,10 @@ func printProtectedRefBlock(w io.Writer, ref, target string) { // of the remote tip. Since checkpoint shards use unique paths, rebases always // apply cleanly. // The target can be a remote name or a URL. -func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.ReferenceName) error { +func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.ReferenceName, w io.Writer) error { + styles := pushProgressStylesFor(w, false) + displayTarget := displayPushTarget(target) + // No timeout: runs under doPushRef's shared budget. fetchTarget, err := remote.ResolveFetchTarget(ctx, target) if err != nil { @@ -487,6 +502,7 @@ func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.Re // Span the fetch separately so a slow sync can be attributed to the network // fetch versus the local reconcile/rebase that follows it. _, fetchSpan := perf.Start(ctx, "git_fetch") + fetchStart := time.Now() fetchOutput, fetchErr := remote.Fetch(ctx, remote.FetchOptions{ Remote: fetchTarget, RefSpecs: []string{refSpec}, @@ -497,6 +513,10 @@ func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.Re if fetchErr != nil { return fmt.Errorf("fetch failed: %s", fetchOutput) } + fmt.Fprintf(w, " %s %s %s\n", + styles.dim(fmt.Sprintf("fetching from %s...", displayTarget)), + styles.green("done"), + styles.dim("("+elapsedPushSec(fetchStart)+")")) repo, err := OpenRepository(ctx) if err != nil { @@ -510,7 +530,7 @@ func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.Re // this cherry-picks local commits onto remote tip, updating the local ref. // If reconciliation fails, abort — proceeding to rebase on disconnected // refs would silently combine unrelated histories. - if reconcileErr := ReconcileDisconnectedMetadataRef(ctx, repo, ref, fetchedRefName, os.Stderr); reconcileErr != nil { + if reconcileErr := ReconcileDisconnectedMetadataRef(ctx, repo, ref, fetchedRefName, w); reconcileErr != nil { return fmt.Errorf("metadata reconciliation failed: %w", reconcileErr) } @@ -578,10 +598,29 @@ func fetchAndRebaseRefCommon(ctx context.Context, target string, ref plumbing.Re return fmt.Errorf("failed to load shallow boundaries: %w", err) } - newTip, err := cherryPickOnto(ctx, repo, remoteRef.Hash(), localCommits, shallow) + remoteOnlyCommits, remoteOnlyErr := collectCommitsSince(ctx, repo, repoPath, remoteRef.Hash(), localRef.Hash()) + remoteAhead := 0 + if remoteOnlyErr == nil { + remoteAhead = len(remoteOnlyCommits) + } + fmt.Fprintf(w, " %s\n", + styles.dim(fmt.Sprintf("remote is %d commits ahead, rebasing %d local commits...", remoteAhead, len(localCommits)))) + + rebaseStart := time.Now() + onProgress := func(current, total int) { + fmt.Fprintf(w, " %s %s\n", + styles.dim(fmt.Sprintf("rebasing %d/%d...", current, total)), + styles.dim("("+elapsedPushSec(rebaseStart)+")")) + } + + newTip, err := cherryPickOnto(ctx, repo, remoteRef.Hash(), localCommits, shallow, onProgress) if err != nil { return fmt.Errorf("failed to rebase local commits onto remote: %w", err) } + fmt.Fprintf(w, " %s %s %s\n", + styles.dim("rebasing"), + styles.green("done"), + styles.dim("("+elapsedPushSec(rebaseStart)+")")) return advance(newTip) } @@ -646,28 +685,3 @@ func collectCommitsSince(ctx context.Context, repo *git.Repository, repoPath str return commits, nil } - -// startProgressDots prints dots to w every second until the returned stop function -// is called. The stop function prints the given suffix and a newline. -func startProgressDots(w io.Writer) func(suffix string) { - done := make(chan struct{}) - stopped := make(chan struct{}) - go func() { - defer close(stopped) - ticker := time.NewTicker(time.Second) - defer ticker.Stop() - for { - select { - case <-done: - return - case <-ticker.C: - fmt.Fprint(w, ".") - } - } - }() - return func(suffix string) { - close(done) - <-stopped // Wait for goroutine to finish before writing suffix - fmt.Fprintln(w, suffix) - } -} diff --git a/cmd/entire/cli/strategy/push_common_test.go b/cmd/entire/cli/strategy/push_common_test.go index 5178aca529..9ae9c6c829 100644 --- a/cmd/entire/cli/strategy/push_common_test.go +++ b/cmd/entire/cli/strategy/push_common_test.go @@ -14,6 +14,7 @@ import ( "github.com/entireio/cli/cmd/entire/cli/checkpoint" "github.com/entireio/cli/cmd/entire/cli/checkpoint/remote" + "github.com/entireio/cli/cmd/entire/cli/logging" "github.com/entireio/cli/cmd/entire/cli/paths" "github.com/entireio/cli/cmd/entire/cli/testutil" @@ -107,7 +108,14 @@ func setupRepoWithCheckpointBranch(t *testing.T) string { // warning and returns nil (no error). This is the core behavior that ensures a // failing checkpoint remote never blocks the user's main push. // -// Not parallel: uses t.Chdir() (required for OpenRepository in fetchAndRebaseRefCommon). +// It also proves the non-TTY progress gate does not swallow the actionable +// warning: captureStderr's os.Pipe write end is never a terminal, so this +// exercises the exact writer doPushRef sees under a real pre-push hook run by +// an agent/CI. The "Syncing with remote..." progress line must be gated off +// while the "couldn't sync" warning must still reach stderr. +// +// Not parallel: uses t.Chdir() (required for OpenRepository in +// fetchAndRebaseRefCommon) and os.Stderr redirection via captureStderr. func TestDoPushRef_UnreachableTarget_ReturnsNil(t *testing.T) { tmpDir := setupRepoWithCheckpointBranch(t) t.Chdir(tmpDir) @@ -119,8 +127,15 @@ func TestDoPushRef_UnreachableTarget_ReturnsNil(t *testing.T) { // 2. Try to fetch+rebase (fails — can't fetch from non-existent path) // 3. Log warning and return nil (graceful degradation) nonExistentPath := filepath.Join(t.TempDir(), "does-not-exist") + restore := captureStderr(t) err := doPushRef(ctx, nonExistentPath, plumbing.NewBranchReferenceName(paths.MetadataBranchName)) - assert.NoError(t, err, "doPushRef should return nil when target is unreachable (graceful degradation)") + output := restore() + + require.NoError(t, err, "doPushRef should return nil when target is unreachable (graceful degradation)") + assert.Contains(t, output, "[entire] Warning: couldn't sync", + "actionable warning must still print on non-TTY stderr") + assert.NotContains(t, output, "Syncing with remote", + "progress line must be gated off on non-TTY stderr") } // TestPushRefIfNeeded_UnreachableTarget_ReturnsNil exercises the full push path @@ -260,7 +275,7 @@ func TestFetchAndRebase_NonBranchRef(t *testing.T) { t.Chdir(tmpDir) - require.NoError(t, fetchAndRebaseRefCommon(ctx, "file://"+bareDir, customRef), + require.NoError(t, fetchAndRebaseRefCommon(ctx, "file://"+bareDir, customRef, io.Discard), "fetchAndRebaseRefCommon should accept a non-branch ref") // The local ref should remain at the same hash. @@ -329,7 +344,7 @@ func TestFetchAndRebase_NonBranchRefDisconnected(t *testing.T) { t.Chdir(cloneDir) - err := fetchAndRebaseRefCommon(ctx, "file://"+bareDir, customRef) + err := fetchAndRebaseRefCommon(ctx, "file://"+bareDir, customRef, io.Discard) require.NoError(t, err) repo, err := git.PlainOpen(cloneDir) @@ -441,7 +456,7 @@ func TestFetchAndRebase_DivergedBranches(t *testing.T) { // 5. Run fetchAndRebaseRefCommon on clone A (diverged: local has bb, remote has cc) t.Chdir(cloneA) - err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) // 6. Verify results @@ -550,7 +565,7 @@ func TestFetchAndRebase_SharedCloneLocalCommitInAlternate(t *testing.T) { gitRun(remoteWorkDir, "push", "origin", branchName) t.Chdir(cloneDir) - err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) treePaths := gitRun(cloneDir, "ls-tree", "-r", "--name-only", branchName) @@ -623,7 +638,7 @@ func TestFetchAndRebase_LocalBehind(t *testing.T) { // Clone is now behind — fetchAndRebase should fast-forward t.Chdir(cloneDir) - err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) // Verify local now matches remote @@ -736,7 +751,7 @@ func TestFetchAndRebase_MergeBaseOnSecondParent_DoesNotReplayAncestors(t *testin // Rebase local metadata branch onto the updated remote tip. t.Chdir(cloneLocal) - err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) repo, err := git.PlainOpen(cloneLocal) @@ -864,7 +879,7 @@ func TestFetchAndRebase_DoesNotResurrectRemoteOnlyCheckpointFromMerge(t *testing t.Chdir(cloneLocal) - err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err := fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) repo, err := git.PlainOpen(cloneLocal) @@ -963,7 +978,7 @@ func TestFetchAndRebase_NonOriginRemote_ReconcilesFetchedRef(t *testing.T) { t.Chdir(cloneDir) - err = fetchAndRebaseRefCommon(ctx, "backup", plumbing.NewBranchReferenceName(branchName)) + err = fetchAndRebaseRefCommon(ctx, "backup", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) repo, err = git.PlainOpen(cloneDir) @@ -1061,7 +1076,7 @@ func TestFetchAndRebase_URLTarget_ReconcilesFetchedTempRef(t *testing.T) { t.Chdir(cloneDir) - err = fetchAndRebaseRefCommon(ctx, "file://"+bareDir, plumbing.NewBranchReferenceName(branchName)) + err = fetchAndRebaseRefCommon(ctx, "file://"+bareDir, plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) repo, err = git.PlainOpen(cloneDir) @@ -1166,7 +1181,7 @@ func TestFetchAndRebase_FlaggedOriginTarget_UsesTempRef(t *testing.T) { t.Chdir(cloneDir) paths.ClearWorktreeRootCache() - err = fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName)) + err = fetchAndRebaseRefCommon(ctx, "origin", plumbing.NewBranchReferenceName(branchName), io.Discard) require.NoError(t, err) repo, err = git.PlainOpen(cloneDir) @@ -1524,24 +1539,36 @@ func setupBareRemoteWithCheckpointBranch(t *testing.T) (string, string) { } // TestDoPushRef_AlreadyUpToDate verifies that when the remote already has all -// commits, the output says "already up-to-date" instead of "done". +// commits, doPushRef is a true no-op — remote state is unchanged. Progress +// text ("already up-to-date") is a TTY-only affordance: captureStderr's +// os.Pipe write end is never a terminal, so interactive.IsTerminalWriter is +// false for the duration of the call without any extra faking, and this +// confirms the phased output is fully gated off in that case. There is no way +// to simulate a real terminal from within `go test`, so the TTY-visible +// "already up-to-date" line itself is exercised manually rather than here. // // Not parallel: uses t.Chdir() and os.Stderr redirection. func TestDoPushRef_AlreadyUpToDate(t *testing.T) { workDir, bareDir := setupBareRemoteWithCheckpointBranch(t) t.Chdir(workDir) + ref := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + beforeHash := remoteRefHash(t, bareDir, ref) + restore := captureStderr(t) - err := doPushRef(context.Background(), bareDir, plumbing.NewBranchReferenceName(paths.MetadataBranchName)) + err := doPushRef(context.Background(), bareDir, ref) output := restore() require.NoError(t, err) - assert.Contains(t, output, "already up-to-date", "should indicate nothing was pushed") - assert.NotContains(t, output, " done", "should not say 'done' when nothing was pushed") + assert.Empty(t, output, "non-TTY stderr must contain no progress output") + assert.Equal(t, beforeHash, remoteRefHash(t, bareDir, ref), + "remote ref must be unchanged when already up to date") } // TestDoPushRef_NewContent_SaysDone verifies that when there are new commits -// to push, the output says "done". +// to push, doPushRef still lands them on the remote even though the non-TTY +// "done" progress line is gated off. See TestDoPushRef_AlreadyUpToDate for why +// the TTY-visible text itself isn't asserted here. // // Not parallel: uses t.Chdir() and os.Stderr redirection. func TestDoPushRef_NewContent_SaysDone(t *testing.T) { @@ -1556,14 +1583,70 @@ func TestDoPushRef_NewContent_SaysDone(t *testing.T) { require.NoError(t, err, "git init --bare failed: %s", out) t.Chdir(workDir) + ref := plumbing.NewBranchReferenceName(paths.MetadataBranchName) restore := captureStderr(t) - err = doPushRef(context.Background(), bareDir, plumbing.NewBranchReferenceName(paths.MetadataBranchName)) + err = doPushRef(context.Background(), bareDir, ref) output := restore() require.NoError(t, err) - assert.Contains(t, output, " done", "should say 'done' when new content was pushed") - assert.NotContains(t, output, "already up-to-date", "should not say 'already up-to-date' when content was pushed") + assert.Empty(t, output, "non-TTY stderr must contain no progress output") + + localRepo, openErr := git.PlainOpen(workDir) + require.NoError(t, openErr) + localRef, refErr := localRepo.Reference(ref, true) + require.NoError(t, refErr) + assert.Equal(t, localRef.Hash().String(), remoteRefHash(t, bareDir, ref), + "new content should have landed on the remote despite gated progress output") +} + +// TestTryPushRefCommon_FileLogsProgressOnNonTTY verifies that tryPushRefCommon +// (the legacy git-branch backend's push helper) file-logs git --progress +// transfer detail via logGitProgress unconditionally, not just displays it on +// a TTY via displayGitProgress. Without this, a non-TTY caller (agents, CI, +// git hooks) loses transfer detail entirely: displayGitProgress's writer +// resolves to io.Discard off a real terminal, and nothing else recorded it. +// This mirrors the refs-backend guarantee already covered by +// TestLogGitProgress_ParsesWithoutTerminalWrites / batchPushRefs. +// +// Not parallel: uses t.Chdir() and the logging package's global writer state. +func TestTryPushRefCommon_FileLogsProgressOnNonTTY(t *testing.T) { + workDir, bareDir := setupBareRemoteWithCheckpointBranch(t) + t.Chdir(workDir) + + // Add a new commit on the checkpoint branch so the push actually transfers + // objects (an already-up-to-date push emits no --progress transfer lines). + testutil.WriteFile(t, workDir, "g.txt", "more") + testutil.GitAdd(t, workDir, "g.txt") + testutil.GitCommit(t, workDir, "checkpoint update") + repo, err := git.PlainOpen(workDir) + require.NoError(t, err) + head, err := repo.Head() + require.NoError(t, err) + ref := plumbing.NewBranchReferenceName(paths.MetadataBranchName) + require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(ref, head.Hash()))) + + // logGitProgress logs at Debug level; the default log level is INFO, so + // force DEBUG for the duration of this test (must be set before Init). + t.Setenv(logging.LogLevelEnvVar, "DEBUG") + + ctx := context.Background() + require.NoError(t, logging.Init(ctx, "test-session")) + t.Cleanup(logging.Close) + + restore := captureStderr(t) + _, pushErr := tryPushRefCommon(ctx, bareDir, ref) + stderrOutput := restore() + require.NoError(t, pushErr) + assert.Empty(t, stderrOutput, "non-TTY stderr must contain no progress output") + + // Flush the buffered log writer before reading the file back. + logging.Close() + + logData, readErr := os.ReadFile(filepath.Join(workDir, ".entire", "logs", "entire.log")) + require.NoError(t, readErr) + assert.Contains(t, string(logData), "git push transfer", + "tryPushRefCommon must file-log git --progress transfer detail even when the terminal display is gated off") } func TestIsProtectedRefRejection(t *testing.T) { diff --git a/cmd/entire/cli/strategy/push_progress.go b/cmd/entire/cli/strategy/push_progress.go new file mode 100644 index 0000000000..fe671163f0 --- /dev/null +++ b/cmd/entire/cli/strategy/push_progress.go @@ -0,0 +1,429 @@ +package strategy + +import ( + "context" + "fmt" + "io" + "log/slog" + "regexp" + "strconv" + "strings" + "time" + + "charm.land/lipgloss/v2" + + "github.com/entireio/cli/cmd/entire/cli/interactive" + "github.com/entireio/cli/cmd/entire/cli/logging" + "github.com/entireio/cli/cmd/entire/cli/trailers" +) + +const maxDisplayedPushSessions = 5 + +type gitProgressPhase string + +const ( + gitProgressPhaseCounting gitProgressPhase = "counting" + gitProgressPhaseCompressing gitProgressPhase = "compressing" + gitProgressPhaseWriting gitProgressPhase = "writing" +) + +// gitProgressEvent is a parsed line from git push/fetch --progress stderr. +type gitProgressEvent struct { + Phase gitProgressPhase + Percent int + Current int + Total int + Bytes string + Speed string + Done bool +} + +// sessionSummary aggregates unpushed checkpoint commits for one session. +type sessionSummary struct { + SessionID string + CheckpointCount int + CommitCount int + EarliestTime time.Time + LatestTime time.Time +} + +// formatSessionTreeOpts configures formatSessionTree output. +type formatSessionTreeOpts struct { + TotalCommits int + NoColor bool + IsNewBranch bool +} + +type pushProgressStyles struct { + dim func(string) string + green func(string) string + yellow func(string) string +} + +func pushProgressStylesFor(w io.Writer, noColor bool) pushProgressStyles { + if noColor || !interactive.ShouldStyle(w) { + return pushProgressStyles{ + dim: func(s string) string { return s }, + green: func(s string) string { return s }, + yellow: func(s string) string { return s }, + } + } + dimStyle := lipgloss.NewStyle().Faint(true) + greenStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("2")) + yellowStyle := lipgloss.NewStyle().Foreground(lipgloss.Color("3")) + return pushProgressStyles{ + dim: func(s string) string { return dimStyle.Render(s) }, + green: func(s string) string { return greenStyle.Render(s) }, + yellow: func(s string) string { return yellowStyle.Render(s) }, + } +} + +var ( + checkpointMsgRE = regexp.MustCompile(`^Checkpoint: ([0-9a-f]+)`) + finalizeMsgRE = regexp.MustCompile(`^Finalize transcript for Checkpoint: ([0-9a-f]+)`) + updateMsgRE = regexp.MustCompile(`^Update (?:summary|checkpoint summary) for (?:checkpoint )?([0-9a-f]+)`) + sessionTrailerRE = regexp.MustCompile(regexp.QuoteMeta(trailers.SessionTrailerKey) + `: (.+)`) + + gitEnumeratingRE = regexp.MustCompile(`^Enumerating objects:\s*(\d+)`) + gitCountingRE = regexp.MustCompile(`^Counting objects:\s*(\d+)%\s*\((\d+)/(\d+)\)`) + gitCompressingRE = regexp.MustCompile(`^Compressing objects:\s*(\d+)%\s*\((\d+)/(\d+)\)`) + gitWritingRE = regexp.MustCompile(`^Writing objects:\s*(\d+)%\s*\((\d+)/(\d+)\)`) + gitWritingBytesRE = regexp.MustCompile(`(\d+(?:\.\d+)?\s*[KMG]iB)(?:\s*\|\s*(\d+(?:\.\d+)?\s*[KMG]iB/s))?`) +) + +type pushSummaryAccumulator struct { + checkpoints map[string]struct{} + commitCount int + earliest time.Time + latest time.Time +} + +// parsePushSummaryFromLog groups git log lines by Entire-Session trailer. +// Input format: hash|subject|trailers|authorDate (one commit per line). +func parsePushSummaryFromLog(gitLogOutput string) []sessionSummary { + gitLogOutput = strings.TrimSpace(gitLogOutput) + if gitLogOutput == "" { + return nil + } + + checkpointToSession := make(map[string]string) + sessionMap := make(map[string]*pushSummaryAccumulator) + + for _, line := range strings.Split(gitLogOutput, "\n") { + parts := strings.Split(line, "|") + if len(parts) < 4 { + continue + } + subject := parts[1] + body := parts[2] + when, err := time.Parse(time.RFC3339, parts[3]) + if err != nil { + continue + } + + var checkpointID string + if m := checkpointMsgRE.FindStringSubmatch(subject); m != nil { + checkpointID = m[1] + } else if m := finalizeMsgRE.FindStringSubmatch(subject); m != nil { + checkpointID = m[1] + } else if m := updateMsgRE.FindStringSubmatch(subject); m != nil { + checkpointID = m[1] + } + + sessionID := "" + if m := sessionTrailerRE.FindStringSubmatch(body); m != nil { + sessionID = strings.TrimSpace(m[1]) + if checkpointID != "" { + checkpointToSession[checkpointID] = sessionID + } + } else if checkpointID != "" { + sessionID = checkpointToSession[checkpointID] + } + if sessionID == "" { + sessionID = "unknown" + } + + entry, ok := sessionMap[sessionID] + if !ok { + checkpoints := map[string]struct{}{} + if checkpointID != "" { + checkpoints[checkpointID] = struct{}{} + } + sessionMap[sessionID] = &pushSummaryAccumulator{ + checkpoints: checkpoints, + commitCount: 1, + earliest: when, + latest: when, + } + continue + } + + entry.commitCount++ + if checkpointID != "" { + entry.checkpoints[checkpointID] = struct{}{} + } + if when.Before(entry.earliest) { + entry.earliest = when + } + if when.After(entry.latest) { + entry.latest = when + } + } + + results := make([]sessionSummary, 0, len(sessionMap)) + for sessionID, entry := range sessionMap { + results = append(results, sessionSummary{ + SessionID: sessionID, + CheckpointCount: len(entry.checkpoints), + CommitCount: entry.commitCount, + EarliestTime: entry.earliest, + LatestTime: entry.latest, + }) + } + sortSessionSummariesByLatest(results) + return results +} + +func sortSessionSummariesByLatest(results []sessionSummary) { + for i := 0; i < len(results); i++ { + for j := i + 1; j < len(results); j++ { + if results[j].LatestTime.After(results[i].LatestTime) { + results[i], results[j] = results[j], results[i] + } + } + } +} + +// formatSessionTree renders session summaries as indented stderr lines. +func formatSessionTree(summaries []sessionSummary, opts formatSessionTreeOpts) []string { + styles := pushProgressStylesFor(io.Discard, opts.NoColor) + lines := make([]string, 0, len(summaries)+3) + + branchLabel := "" + if opts.IsNewBranch { + branchLabel = "new branch, " + } + header := fmt.Sprintf("%s Checkpoint push: %s%d commits, %d sessions", + styles.dim("[entire]"), branchLabel, opts.TotalCommits, len(summaries)) + lines = append(lines, header) + + displayed := summaries + remaining := 0 + if len(summaries) > maxDisplayedPushSessions { + displayed = summaries[:maxDisplayedPushSessions] + remaining = len(summaries) - maxDisplayedPushSessions + } + + for i, s := range displayed { + isLast := i == len(displayed)-1 && remaining == 0 + connector := "├─" + if isLast { + connector = "└─" + } + cpLabel := fmt.Sprintf("%d checkpoints", s.CheckpointCount) + if s.CheckpointCount == 1 { + cpLabel = "1 checkpoint" + } + timeLabel := formatPushTimeRange(s.EarliestTime, s.LatestTime) + line := fmt.Sprintf(" %s %s %s %s", + styles.dim(connector), s.SessionID, styles.dim(cpLabel), styles.dim("("+timeLabel+")")) + lines = append(lines, line) + } + + if remaining > 0 { + lines = append(lines, fmt.Sprintf(" %s %s", + styles.dim("├─"), styles.dim(fmt.Sprintf("... and %d more sessions", remaining)))) + oldest := summaries[len(summaries)-1] + lines = append(lines, fmt.Sprintf(" %s %s", + styles.dim("└─"), styles.dim("(oldest: "+formatPushDate(oldest.EarliestTime)+")"))) + } + + return lines +} + +func parseGitProgressLine(line string) *gitProgressEvent { + trimmed := strings.TrimSpace(line) + + if m := gitEnumeratingRE.FindStringSubmatch(trimmed); m != nil { + total, err := strconv.Atoi(m[1]) + if err != nil { + return nil + } + return &gitProgressEvent{ + Phase: gitProgressPhaseCounting, + Total: total, + Done: strings.Contains(trimmed, "done"), + } + } + + if m := gitCountingRE.FindStringSubmatch(trimmed); m != nil { + return parsePercentGitProgressEvent(gitProgressPhaseCounting, m, trimmed) + } + if m := gitCompressingRE.FindStringSubmatch(trimmed); m != nil { + return parsePercentGitProgressEvent(gitProgressPhaseCompressing, m, trimmed) + } + if m := gitWritingRE.FindStringSubmatch(trimmed); m != nil { + event := parsePercentGitProgressEvent(gitProgressPhaseWriting, m, trimmed) + if bm := gitWritingBytesRE.FindStringSubmatch(trimmed); bm != nil { + event.Bytes = bm[1] + if len(bm) > 2 { + event.Speed = bm[2] + } + } + return event + } + + return nil +} + +func parsePercentGitProgressEvent(phase gitProgressPhase, m []string, trimmed string) *gitProgressEvent { + percent, err := strconv.Atoi(m[1]) + if err != nil { + return nil + } + current, err := strconv.Atoi(m[2]) + if err != nil { + return nil + } + total, err := strconv.Atoi(m[3]) + if err != nil { + return nil + } + return &gitProgressEvent{ + Phase: phase, + Percent: percent, + Current: current, + Total: total, + Done: strings.Contains(trimmed, "done"), + } +} + +// displayGitProgress writes human-friendly git transfer progress lines to w. +func displayGitProgress(w io.Writer, stderr string) { + styles := pushProgressStylesFor(w, false) + lastPhase := gitProgressPhase("") + for _, line := range strings.FieldsFunc(stderr, func(r rune) bool { return r == '\n' || r == '\r' }) { + event := parseGitProgressLine(line) + if event == nil { + continue + } + if !event.Done && event.Phase == lastPhase { + continue + } + lastPhase = event.Phase + + switch event.Phase { + case gitProgressPhaseCounting: + if event.Done { + fmt.Fprintf(w, " %s\n", styles.dim(fmt.Sprintf("counting objects: %d", event.Total))) + } + case gitProgressPhaseCompressing: + if event.Done { + fmt.Fprintf(w, " %s\n", styles.dim(fmt.Sprintf("compressing: %d/%d", event.Current, event.Total))) + } + case gitProgressPhaseWriting: + if event.Done { + parts := []string{fmt.Sprintf("writing: %d objects", event.Total)} + if event.Bytes != "" { + parts = append(parts, event.Bytes) + } + if event.Speed != "" { + parts = append(parts, event.Speed) + } + fmt.Fprintf(w, " %s %s\n", + styles.dim(strings.Join(parts, ", ")+"..."), + styles.green("done")) + } + } + } +} + +// logGitProgress parses git `--progress` transfer stderr and records each +// phase transition as operational metadata (phase, counts, bytes) to the +// debug log. Unlike displayGitProgress, it writes to NO io.Writer — this is a +// file-logging sink only, used on paths that keep the terminal quiet. +func logGitProgress(ctx context.Context, stderr string) { + lastPhase := gitProgressPhase("") + for _, line := range strings.FieldsFunc(stderr, func(r rune) bool { return r == '\n' || r == '\r' }) { + event := parseGitProgressLine(line) + if event == nil { + continue + } + if !event.Done && event.Phase == lastPhase { + continue + } + lastPhase = event.Phase + + attrs := []any{ + slog.String("phase", string(event.Phase)), + slog.Bool("done", event.Done), + slog.Int("total", event.Total), + } + if event.Percent > 0 { + attrs = append(attrs, slog.Int("percent", event.Percent)) + } + if event.Current > 0 { + attrs = append(attrs, slog.Int("current", event.Current)) + } + if event.Bytes != "" { + attrs = append(attrs, slog.String("bytes", event.Bytes)) + } + if event.Speed != "" { + attrs = append(attrs, slog.String("speed", event.Speed)) + } + logging.Debug(ctx, "git push transfer", attrs...) + } +} + +func formatPushTimeRange(earliest, latest time.Time) string { + if earliest.Equal(latest) { + return formatPushTime(earliest) + } + if formatPushDate(earliest) == formatPushDate(latest) { + return formatPushHM(earliest) + " ~ " + formatPushHM(latest) + } + return formatPushDate(earliest) + " ~ " + formatPushDate(latest) +} + +func formatPushTime(d time.Time) string { + now := time.Now() + diffDays := int(now.Sub(d).Hours() / 24) + if diffDays <= 0 && now.Before(d.Add(24*time.Hour)) { + return formatPushHM(d) + } + if diffDays == 1 { + return "yesterday" + } + if diffDays > 1 { + return fmt.Sprintf("%d days ago", diffDays) + } + return formatPushHM(d) +} + +func formatPushHM(d time.Time) string { + return fmt.Sprintf("%02d:%02d", d.Hour(), d.Minute()) +} + +func formatPushDate(d time.Time) string { + return fmt.Sprintf("%04d-%02d-%02d", d.Year(), int(d.Month()), d.Day()) +} + +func elapsedPushSec(start time.Time) string { + sec := int(time.Since(start).Round(time.Second) / time.Second) + return fmt.Sprintf("%ds", sec) +} + +func writePushFinishLine(ctx context.Context, w io.Writer, result pushResult, start time.Time, target string) { + styles := pushProgressStylesFor(w, false) + if result.upToDate { + fmt.Fprintf(w, " %s\n", styles.dim("already up-to-date")) + return + } + fmt.Fprintf(w, " %s %s\n", styles.green("done"), styles.dim("("+elapsedPushSec(start)+")")) + printSettingsCommitHint(ctx, target) +} + +func writePushConflictLine(w io.Writer, start time.Time) { + styles := pushProgressStylesFor(w, false) + fmt.Fprintf(w, " %s %s\n", styles.yellow("conflict"), styles.dim("("+elapsedPushSec(start)+")")) +} diff --git a/cmd/entire/cli/strategy/push_progress_test.go b/cmd/entire/cli/strategy/push_progress_test.go new file mode 100644 index 0000000000..04519c2782 --- /dev/null +++ b/cmd/entire/cli/strategy/push_progress_test.go @@ -0,0 +1,308 @@ +package strategy + +import ( + "bytes" + "os" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func mustParseRFC3339(t *testing.T, value string) time.Time { + t.Helper() + when, err := time.Parse(time.RFC3339, value) + require.NoError(t, err) + return when +} + +func TestParsePushSummaryFromLog(t *testing.T) { + t.Parallel() + + t.Run("parses multiple commits into per-session aggregates", func(t *testing.T) { + t.Parallel() + gitLogOutput := strings.Join([]string{ + "abc1234|Checkpoint: a3b2c4d5e6f7|Entire-Session: sess-2026-06-12-abc123|2026-06-12T12:30:00+08:00", + "def5678|Finalize transcript for Checkpoint: a3b2c4d5e6f7||2026-06-12T13:00:00+08:00", + "ghi9012|Checkpoint: b4c3d5e6f7a8|Entire-Session: sess-2026-06-12-abc123|2026-06-12T14:05:00+08:00", + "jkl3456|Checkpoint: c5d4e6f7a8b9|Entire-Session: sess-2026-06-11-def456|2026-06-11T10:00:00+08:00", + "mno7890|Finalize transcript for Checkpoint: c5d4e6f7a8b9||2026-06-11T10:30:00+08:00", + }, "\n") + + result := parsePushSummaryFromLog(gitLogOutput) + require.Len(t, result, 2) + + first := result[0] + assert.Equal(t, "sess-2026-06-12-abc123", first.SessionID) + assert.Equal(t, 2, first.CheckpointCount) + assert.Equal(t, 3, first.CommitCount) + assert.Equal(t, mustParseRFC3339(t, "2026-06-12T12:30:00+08:00"), first.EarliestTime) + assert.Equal(t, mustParseRFC3339(t, "2026-06-12T14:05:00+08:00"), first.LatestTime) + + second := result[1] + assert.Equal(t, "sess-2026-06-11-def456", second.SessionID) + assert.Equal(t, 1, second.CheckpointCount) + assert.Equal(t, 2, second.CommitCount) + }) + + t.Run("returns empty array for empty input", func(t *testing.T) { + t.Parallel() + assert.Empty(t, parsePushSummaryFromLog("")) + }) + + t.Run("assigns unknown when session trailer missing", func(t *testing.T) { + t.Parallel() + gitLogOutput := strings.Join([]string{ + "abc1234|Finalize transcript for Checkpoint: a3b2c4d5e6f7||2026-06-12T12:30:00+08:00", + "def5678|Update checkpoint summary for a3b2c4d5e6f7||2026-06-12T12:35:00+08:00", + }, "\n") + + result := parsePushSummaryFromLog(gitLogOutput) + require.Len(t, result, 1) + assert.Equal(t, "unknown", result[0].SessionID) + assert.Equal(t, 2, result[0].CommitCount) + assert.Equal(t, 1, result[0].CheckpointCount) + }) + + t.Run("sorts sessions by latest time descending", func(t *testing.T) { + t.Parallel() + gitLogOutput := strings.Join([]string{ + "a|Checkpoint: aaa|Entire-Session: old-session|2026-06-01T10:00:00+08:00", + "b|Checkpoint: bbb|Entire-Session: new-session|2026-06-12T10:00:00+08:00", + }, "\n") + + result := parsePushSummaryFromLog(gitLogOutput) + require.Len(t, result, 2) + assert.Equal(t, "new-session", result[0].SessionID) + assert.Equal(t, "old-session", result[1].SessionID) + }) +} + +func TestFormatSessionTree(t *testing.T) { + t.Parallel() + + t.Run("formats sessions into a tree with connector characters", func(t *testing.T) { + t.Parallel() + summaries := []sessionSummary{ + { + SessionID: "sess-2026-06-12-abc123", + CheckpointCount: 3, + CommitCount: 5, + EarliestTime: time.Date(2026, 6, 12, 12, 30, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, 12, 14, 5, 0, 0, time.UTC), + }, + { + SessionID: "sess-2026-06-11-def456", + CheckpointCount: 2, + CommitCount: 3, + EarliestTime: time.Date(2026, 6, 11, 10, 0, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, 11, 10, 30, 0, 0, time.UTC), + }, + } + + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: 8, + NoColor: true, + }) + + assert.Contains(t, lines[0], "8 commits, 2 sessions") + assert.Contains(t, lines[1], "├─") + assert.Contains(t, lines[1], "sess-2026-06-12-abc123") + assert.Contains(t, lines[1], "3 checkpoints") + assert.Contains(t, lines[2], "└─") + assert.Contains(t, lines[2], "sess-2026-06-11-def456") + assert.Contains(t, lines[2], "2 checkpoints") + }) + + t.Run("folds sessions beyond max display", func(t *testing.T) { + t.Parallel() + summaries := make([]sessionSummary, 8) + for i := range summaries { + day := 12 - i + summaries[i] = sessionSummary{ + SessionID: "sess-" + string(rune('0'+i)), + CheckpointCount: 1, + CommitCount: 2, + EarliestTime: time.Date(2026, 6, day, 10, 0, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, day, 11, 0, 0, 0, time.UTC), + } + } + + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: 16, + NoColor: true, + }) + + assert.Contains(t, lines[0], "16 commits, 8 sessions") + treeLines := lines[1:] + assert.Len(t, treeLines, 7) + assert.Contains(t, treeLines[5], "... and 3 more sessions") + assert.Contains(t, treeLines[6], "oldest:") + }) + + t.Run("uses singular checkpoint label", func(t *testing.T) { + t.Parallel() + summaries := []sessionSummary{{ + SessionID: "sess-solo", + CheckpointCount: 1, + CommitCount: 1, + EarliestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + }} + + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: 1, + NoColor: true, + }) + assert.Contains(t, lines[1], "1 checkpoint") + assert.NotContains(t, lines[1], "1 checkpoints") + }) + + t.Run("shows new branch label", func(t *testing.T) { + t.Parallel() + summaries := []sessionSummary{{ + SessionID: "sess-a", + CheckpointCount: 1, + CommitCount: 2, + EarliestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + }} + + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: 2, + NoColor: true, + IsNewBranch: true, + }) + assert.Contains(t, lines[0], "new branch") + }) +} + +func TestParseGitProgressLine(t *testing.T) { + t.Parallel() + + t.Run("enumerating objects", func(t *testing.T) { + t.Parallel() + event := parseGitProgressLine("Enumerating objects: 47, done.") + require.NotNil(t, event) + assert.Equal(t, gitProgressPhaseCounting, event.Phase) + assert.Equal(t, 47, event.Total) + assert.True(t, event.Done) + }) + + t.Run("counting objects percentage", func(t *testing.T) { + t.Parallel() + event := parseGitProgressLine("Counting objects: 100% (47/47), done.") + require.NotNil(t, event) + assert.Equal(t, gitProgressPhaseCounting, event.Phase) + assert.Equal(t, 100, event.Percent) + assert.Equal(t, 47, event.Current) + assert.Equal(t, 47, event.Total) + assert.True(t, event.Done) + }) + + t.Run("compressing objects", func(t *testing.T) { + t.Parallel() + event := parseGitProgressLine("Compressing objects: 81% (31/38)") + require.NotNil(t, event) + assert.Equal(t, gitProgressPhaseCompressing, event.Phase) + assert.Equal(t, 81, event.Percent) + assert.Equal(t, 31, event.Current) + assert.Equal(t, 38, event.Total) + assert.False(t, event.Done) + }) + + t.Run("writing objects with speed", func(t *testing.T) { + t.Parallel() + event := parseGitProgressLine("Writing objects: 100% (42/42), 156.23 KiB | 312.00 KiB/s, done.") + require.NotNil(t, event) + assert.Equal(t, gitProgressPhaseWriting, event.Phase) + assert.Equal(t, 100, event.Percent) + assert.Equal(t, 42, event.Current) + assert.Equal(t, 42, event.Total) + assert.Equal(t, "156.23 KiB", event.Bytes) + assert.Equal(t, "312.00 KiB/s", event.Speed) + assert.True(t, event.Done) + }) + + t.Run("writing objects in progress", func(t *testing.T) { + t.Parallel() + event := parseGitProgressLine("Writing objects: 45% (9/20), 78.00 KiB") + require.NotNil(t, event) + assert.Equal(t, gitProgressPhaseWriting, event.Phase) + assert.Equal(t, 45, event.Percent) + assert.Equal(t, 9, event.Current) + assert.Equal(t, 20, event.Total) + assert.Equal(t, "78.00 KiB", event.Bytes) + assert.Empty(t, event.Speed) + assert.False(t, event.Done) + }) + + t.Run("returns nil for unrecognized lines", func(t *testing.T) { + t.Parallel() + assert.Nil(t, parseGitProgressLine("Total 42 (delta 2), reused 0 (delta 0)")) + assert.Nil(t, parseGitProgressLine("")) + assert.Nil(t, parseGitProgressLine("remote: Resolving deltas: 100%")) + assert.Nil(t, parseGitProgressLine("Delta compression using up to 8 threads")) + }) +} + +// captureStdout redirects os.Stdout to a pipe and returns a function that +// restores stdout and returns the captured output. Mirrors captureStderr in +// push_common_test.go. Must be called on the main goroutine (not +// parallel-safe). +func captureStdout(t *testing.T) func() string { + t.Helper() + old := os.Stdout + r, w, err := os.Pipe() + require.NoError(t, err) + os.Stdout = w + + t.Cleanup(func() { + os.Stdout = old + _ = w.Close() + _ = r.Close() + }) + + return func() string { + _ = w.Close() + var buf bytes.Buffer + _, readErr := buf.ReadFrom(r) + require.NoError(t, readErr) + _ = r.Close() + os.Stdout = old + return buf.String() + } +} + +// TestLogGitProgress_ParsesWithoutTerminalWrites guards the "file logging +// only" property of logGitProgress by capturing the real process stdout and +// stderr around the call and asserting nothing landed on either. Not +// parallel: captureStdout/captureStderr redirect process-global os.Stdout +// and os.Stderr. +func TestLogGitProgress_ParsesWithoutTerminalWrites(t *testing.T) { + // Sample `git push --progress` stderr block, reusing the same lines + // exercised by TestParseGitProgressLine above. + stderr := strings.Join([]string{ + "Enumerating objects: 47, done.", + "Counting objects: 100% (47/47), done.", + "Compressing objects: 81% (31/38)", + "Compressing objects: 100% (38/38), done.", + "Writing objects: 100% (42/42), 156.23 KiB | 312.00 KiB/s, done.", + "Total 42 (delta 2), reused 0 (delta 0)", + }, "\n") + + restoreStdout := captureStdout(t) + restoreStderr := captureStderr(t) + + assert.NotPanics(t, func() { + logGitProgress(t.Context(), stderr) + }) + + stdoutOutput := restoreStdout() + stderrOutput := restoreStderr() + + assert.Empty(t, stdoutOutput, "logGitProgress must not write to stdout") + assert.Empty(t, stderrOutput, "logGitProgress must not write to stderr") +} diff --git a/cmd/entire/cli/strategy/push_reporter.go b/cmd/entire/cli/strategy/push_reporter.go new file mode 100644 index 0000000000..83d0ad2ab4 --- /dev/null +++ b/cmd/entire/cli/strategy/push_reporter.go @@ -0,0 +1,135 @@ +package strategy + +import ( + "context" + "fmt" + "io" + "log/slog" + "sync" + "time" + + "github.com/entireio/cli/cmd/entire/cli/logging" +) + +// pushShouldReveal reports whether the pre-push progress line should be shown: +// only on a terminal, and only once the push has run long enough to feel stuck. +func pushShouldReveal(elapsed, threshold time.Duration, isTTY bool) bool { + return isTTY && elapsed >= threshold +} + +// pushReporter is a threshold-gated, single-in-place-line progress reporter +// for the pre-push checkpoint sync. It always logs to file via logging.Debug, +// but only writes to its terminal writer once the push has run long enough to +// feel stuck (and never at all when isTTY is false) — agents and CI must see +// zero bytes. +type pushReporter struct { + ctx context.Context + w io.Writer + isTTY bool + threshold time.Duration + start time.Time + + mu sync.Mutex + revealed bool + text string + + done chan struct{} + stopped chan struct{} +} + +// newPushReporter creates a pushReporter and starts its background reveal +// goroutine. Callers must call finish to stop the goroutine and clear the +// line. +func newPushReporter(ctx context.Context, w io.Writer, isTTY bool, threshold time.Duration) *pushReporter { + r := &pushReporter{ + ctx: ctx, + w: w, + isTTY: isTTY, + threshold: threshold, + start: time.Now(), + done: make(chan struct{}), + stopped: make(chan struct{}), + } + go r.run() + return r +} + +// run waits until the threshold elapses, then ticks ~1s, redrawing the +// in-place line while the reporter is not yet done. +func (r *pushReporter) run() { + defer close(r.stopped) + + timer := time.NewTimer(r.threshold) + defer timer.Stop() + + select { + case <-r.done: + return + case <-timer.C: + } + + r.mu.Lock() + r.maybeReveal() + r.mu.Unlock() + + ticker := time.NewTicker(time.Second) + defer ticker.Stop() + + for { + select { + case <-r.done: + return + case <-ticker.C: + r.mu.Lock() + r.maybeReveal() + r.mu.Unlock() + } + } +} + +// maybeReveal writes the current in-place line when reveal conditions are +// met. Callers must hold r.mu. +func (r *pushReporter) maybeReveal() { + if !pushShouldReveal(time.Since(r.start), r.threshold, r.isTTY) { + return + } + r.revealed = true + r.redrawLocked() +} + +// redrawLocked writes the current in-place line. Callers must hold r.mu. +func (r *pushReporter) redrawLocked() { + if !r.isTTY { + return + } + elapsed := int(time.Since(r.start).Seconds()) + fmt.Fprintf(r.w, "\r[entire] %s (%ds)\033[K", r.text, elapsed) +} + +// phase sets the current phase text. It always logs to file; if the line is +// already revealed, it redraws immediately. +func (r *pushReporter) phase(text string) { + logging.Debug(r.ctx, "checkpoint push progress", slog.String("phase", text)) + + r.mu.Lock() + defer r.mu.Unlock() + r.text = text + if r.revealed { + r.redrawLocked() + } +} + +// finish stops the reveal goroutine, clears the in-place line if it was +// revealed, and logs the final summary to file. +func (r *pushReporter) finish(summary string) { + close(r.done) + <-r.stopped + + r.mu.Lock() + if r.revealed && r.isTTY { + fmt.Fprint(r.w, "\r\033[K") + } + r.mu.Unlock() + + logging.Debug(r.ctx, "checkpoint push finished", slog.String("summary", summary)) +} diff --git a/cmd/entire/cli/strategy/push_reporter_test.go b/cmd/entire/cli/strategy/push_reporter_test.go new file mode 100644 index 0000000000..d8e252523d --- /dev/null +++ b/cmd/entire/cli/strategy/push_reporter_test.go @@ -0,0 +1,70 @@ +package strategy + +import ( + "bytes" + "context" + "strings" + "testing" + "time" +) + +func TestPushShouldReveal(t *testing.T) { + t.Parallel() + tests := []struct { + name string + elapsed, thresh time.Duration + tty, want bool + }{ + {"non-tty never reveals", 10 * time.Second, time.Second, false, false}, + {"tty below threshold stays hidden", 500 * time.Millisecond, time.Second, true, false}, + {"tty at threshold reveals", time.Second, time.Second, true, true}, + {"tty past threshold reveals", 3 * time.Second, time.Second, true, true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + t.Parallel() + if got := pushShouldReveal(tc.elapsed, tc.thresh, tc.tty); got != tc.want { + t.Fatalf("pushShouldReveal(%v,%v,%v)=%v want %v", tc.elapsed, tc.thresh, tc.tty, got, tc.want) + } + }) + } +} + +func TestPushReporter_NonTTY_WritesNothing(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + r := newPushReporter(context.Background(), &buf, false, time.Millisecond) + r.phase("syncing 3 checkpoints") + time.Sleep(10 * time.Millisecond) + r.finish("pushed 3") + if buf.Len() != 0 { + t.Fatalf("non-tty reporter wrote %q, want nothing", buf.String()) + } +} + +func TestPushReporter_TTY_RevealsThenClears(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + r := newPushReporter(context.Background(), &buf, true, time.Millisecond) + r.phase("syncing 3 checkpoints") + time.Sleep(15 * time.Millisecond) // let the reveal goroutine fire + r.finish("pushed 3") + out := buf.String() + if !strings.Contains(out, "syncing 3 checkpoints") { + t.Fatalf("expected revealed phase text, got %q", out) + } + if !strings.HasSuffix(out, "\r\033[K") { + t.Fatalf("expected trailing clear sequence, got %q", out) + } +} + +func TestPushReporter_TTY_FastPush_StaysHidden(t *testing.T) { + t.Parallel() + var buf bytes.Buffer + r := newPushReporter(context.Background(), &buf, true, time.Hour) // never reached + r.phase("syncing 3 checkpoints") + r.finish("pushed 3") + if buf.Len() != 0 { + t.Fatalf("fast push wrote %q, want nothing", buf.String()) + } +} From ac9d5675ed73a49570ca23dd48acfb1c8dd70dea Mon Sep 17 00:00:00 2001 From: Victor Gutierrez Calderon Date: Mon, 3 Aug 2026 11:19:37 +0200 Subject: [PATCH 2/5] fix(push): address PR review comments - session tree: drop trailer valueonly so Entire-Session prefix matches the parser (was bucketing all commits to 'unknown') - gate progress ANSI on ShouldStyle, not IsTerminalWriter (respects NO_COLOR / TERM=cygwin) - style the session tree against the real output writer instead of io.Discard - dedupe the duplicate 'counting objects' line in displayGitProgress - isolate git config in the new push tests (IsolateGitConfigEnv) - make the reporter reveal test deterministic (kill CI flake) Co-Authored-By: snowingfox Entire-Checkpoint: 01KZ3ER62N63BETRDA7QFR0F88 --- cmd/entire/cli/strategy/manual_commit_push.go | 35 +++++++--- .../cli/strategy/manual_commit_push_test.go | 42 ++++++++++++ cmd/entire/cli/strategy/push_common.go | 2 +- cmd/entire/cli/strategy/push_common_test.go | 1 + cmd/entire/cli/strategy/push_progress.go | 26 +++++++- cmd/entire/cli/strategy/push_progress_test.go | 64 +++++++++++++++++++ cmd/entire/cli/strategy/push_reporter_test.go | 41 +++++++++++- 7 files changed, 197 insertions(+), 14 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_push.go b/cmd/entire/cli/strategy/manual_commit_push.go index a0c39b8c03..5d27a3939b 100644 --- a/cmd/entire/cli/strategy/manual_commit_push.go +++ b/cmd/entire/cli/strategy/manual_commit_push.go @@ -112,7 +112,7 @@ 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.IsTerminalWriter(os.Stderr) { + if interactive.ShouldStyle(os.Stderr) { printPushSummary(ctx, remote, ps.pushTarget()) } @@ -366,7 +366,10 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar // git push apparently hung. TTY-gated and threshold-delayed: agents and CI // (non-TTY stderr) see zero progress bytes. displayTarget := displayPushTarget(pushTarget) - rep := newPushReporter(ctx, os.Stderr, interactive.IsTerminalWriter(os.Stderr), 2*time.Second) + // isTTY here means "ANSI-capable per ShouldStyle" (respects NO_COLOR and + // TERM=cygwin), not merely "stderr is a terminal" — see the repo-wide rule + // on gating \r/\033[K writes. + rep := newPushReporter(ctx, os.Stderr, interactive.ShouldStyle(os.Stderr), 2*time.Second) rep.phase(fmt.Sprintf("syncing %d checkpoint(s) to %s", len(existing), displayTarget)) // Fast path: push all refs in one round-trip (fast-forward-only). If every @@ -441,6 +444,17 @@ 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: " 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) { @@ -450,25 +464,24 @@ func printPushSummary(ctx context.Context, remoteName, target string) { } branchName := paths.MetadataBranchName - logFormat := "%H|%s|%(trailers:key=" + trailers.SessionTrailerKey + ",valueonly,separator=%x20)|%aI" var logOutput string isNewBranch := false if !checkpointremote.IsURL(target) { rangeSpec := "refs/remotes/" + remoteName + "/" + branchName + "..refs/heads/" + branchName - firstOut, firstErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, rangeSpec) + firstOut, firstErr := runPushSummaryGitLog(ctx, repoRoot, rangeSpec) if firstErr == nil && strings.TrimSpace(firstOut) != "" { logOutput = firstOut } else { - fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, "refs/heads/"+branchName) + fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, "refs/heads/"+branchName) if fallbackErr == nil && strings.TrimSpace(fallbackOut) != "" { logOutput = fallbackOut isNewBranch = true } } } else { - fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, logFormat, "refs/heads/"+branchName) + fallbackOut, fallbackErr := runPushSummaryGitLog(ctx, repoRoot, "refs/heads/"+branchName) if fallbackErr == nil { logOutput = fallbackOut isNewBranch = strings.TrimSpace(logOutput) != "" @@ -484,18 +497,22 @@ func printPushSummary(ctx context.Context, remoteName, target string) { } 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, }) - w := pushProgressOutput() for _, line := range lines { fmt.Fprintln(w, line) } } -func runPushSummaryGitLog(ctx context.Context, repoRoot, format, rangeSpec string) (string, error) { - cmd := exec.CommandContext(ctx, "git", "log", "--format="+format, rangeSpec) +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 { diff --git a/cmd/entire/cli/strategy/manual_commit_push_test.go b/cmd/entire/cli/strategy/manual_commit_push_test.go index 44dcc1679e..c0542b3b3e 100644 --- a/cmd/entire/cli/strategy/manual_commit_push_test.go +++ b/cmd/entire/cli/strategy/manual_commit_push_test.go @@ -58,6 +58,47 @@ func TestDeferCheckpointPushOnEmptyRemote_UsesLocalTrackingRefs(t *testing.T) { "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 @@ -66,6 +107,7 @@ func TestDeferCheckpointPushOnEmptyRemote_UsesLocalTrackingRefs(t *testing.T) { // // 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() diff --git a/cmd/entire/cli/strategy/push_common.go b/cmd/entire/cli/strategy/push_common.go index 4d86b338e1..60e946f997 100644 --- a/cmd/entire/cli/strategy/push_common.go +++ b/cmd/entire/cli/strategy/push_common.go @@ -159,7 +159,7 @@ func displayPushTarget(target string) string { // hints) must NOT be routed through this writer; they write to os.Stderr // directly so they still print on non-TTY. func pushProgressOutput() io.Writer { - if !interactive.IsTerminalWriter(os.Stderr) { + if !interactive.ShouldStyle(os.Stderr) { return io.Discard } return os.Stderr diff --git a/cmd/entire/cli/strategy/push_common_test.go b/cmd/entire/cli/strategy/push_common_test.go index 9ae9c6c829..532e4c65ca 100644 --- a/cmd/entire/cli/strategy/push_common_test.go +++ b/cmd/entire/cli/strategy/push_common_test.go @@ -1611,6 +1611,7 @@ func TestDoPushRef_NewContent_SaysDone(t *testing.T) { // // Not parallel: uses t.Chdir() and the logging package's global writer state. func TestTryPushRefCommon_FileLogsProgressOnNonTTY(t *testing.T) { + testutil.IsolateGitConfigEnv(t) workDir, bareDir := setupBareRemoteWithCheckpointBranch(t) t.Chdir(workDir) diff --git a/cmd/entire/cli/strategy/push_progress.go b/cmd/entire/cli/strategy/push_progress.go index fe671163f0..218724ba2e 100644 --- a/cmd/entire/cli/strategy/push_progress.go +++ b/cmd/entire/cli/strategy/push_progress.go @@ -52,6 +52,12 @@ type formatSessionTreeOpts struct { TotalCommits int NoColor bool IsNewBranch bool + // Writer is the target the formatted lines will actually be printed to. + // Styling decisions (pushProgressStylesFor) must be computed against this + // writer, not io.Discard, or ANSI styling never applies even when the + // caller prints to a real, style-capable terminal. Defaults to + // io.Discard (no styling) if left unset. + Writer io.Writer } type pushProgressStyles struct { @@ -196,7 +202,11 @@ func sortSessionSummariesByLatest(results []sessionSummary) { // formatSessionTree renders session summaries as indented stderr lines. func formatSessionTree(summaries []sessionSummary, opts formatSessionTreeOpts) []string { - styles := pushProgressStylesFor(io.Discard, opts.NoColor) + w := opts.Writer + if w == nil { + w = io.Discard + } + styles := pushProgressStylesFor(w, opts.NoColor) lines := make([]string, 0, len(summaries)+3) branchLabel := "" @@ -302,6 +312,14 @@ func parsePercentGitProgressEvent(phase gitProgressPhase, m []string, trimmed st func displayGitProgress(w io.Writer, stderr string) { styles := pushProgressStylesFor(w, false) lastPhase := gitProgressPhase("") + // git emits both "Enumerating objects: N, done." and "Counting objects: + // 100% (N/N), done." — parseGitProgressLine maps both to + // gitProgressPhaseCounting with Done=true, so without this guard the loop + // below prints the "counting objects" Done summary twice. printed tracks + // which phases already had their one-time Done summary printed so each + // phase surfaces at most once per call, no matter how many raw git lines + // mapped to it. + printed := make(map[gitProgressPhase]bool) for _, line := range strings.FieldsFunc(stderr, func(r rune) bool { return r == '\n' || r == '\r' }) { event := parseGitProgressLine(line) if event == nil { @@ -311,6 +329,12 @@ func displayGitProgress(w io.Writer, stderr string) { continue } lastPhase = event.Phase + if event.Done { + if printed[event.Phase] { + continue + } + printed[event.Phase] = true + } switch event.Phase { case gitProgressPhaseCounting: diff --git a/cmd/entire/cli/strategy/push_progress_test.go b/cmd/entire/cli/strategy/push_progress_test.go index 04519c2782..79edf36020 100644 --- a/cmd/entire/cli/strategy/push_progress_test.go +++ b/cmd/entire/cli/strategy/push_progress_test.go @@ -105,6 +105,7 @@ func TestFormatSessionTree(t *testing.T) { lines := formatSessionTree(summaries, formatSessionTreeOpts{ TotalCommits: 8, NoColor: true, + Writer: &bytes.Buffer{}, }) assert.Contains(t, lines[0], "8 commits, 2 sessions") @@ -133,6 +134,7 @@ func TestFormatSessionTree(t *testing.T) { lines := formatSessionTree(summaries, formatSessionTreeOpts{ TotalCommits: 16, NoColor: true, + Writer: &bytes.Buffer{}, }) assert.Contains(t, lines[0], "16 commits, 8 sessions") @@ -155,6 +157,7 @@ func TestFormatSessionTree(t *testing.T) { lines := formatSessionTree(summaries, formatSessionTreeOpts{ TotalCommits: 1, NoColor: true, + Writer: &bytes.Buffer{}, }) assert.Contains(t, lines[1], "1 checkpoint") assert.NotContains(t, lines[1], "1 checkpoints") @@ -174,9 +177,43 @@ func TestFormatSessionTree(t *testing.T) { TotalCommits: 2, NoColor: true, IsNewBranch: true, + Writer: &bytes.Buffer{}, }) assert.Contains(t, lines[0], "new branch") }) + + t.Run("styles against the provided writer, not a hardcoded io.Discard", func(t *testing.T) { + t.Parallel() + summaries := []sessionSummary{{ + SessionID: "sess-a", + CheckpointCount: 1, + CommitCount: 1, + EarliestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + LatestTime: time.Date(2026, 6, 12, 10, 0, 0, 0, time.UTC), + }} + + // NoColor is left false so the only thing suppressing ANSI codes is + // pushProgressStylesFor's own interactive.ShouldStyle(w) check against + // whatever writer formatSessionTree actually passes it. A *bytes.Buffer + // is never a terminal, so this must render unstyled regardless of + // which non-terminal writer is threaded through — but it must be + // threaded through opts.Writer, not silently swapped for io.Discard + // (the pre-fix behavior, which produced the same unstyled output for a + // different reason and so would mask a regression back to it). + var buf bytes.Buffer + lines := formatSessionTree(summaries, formatSessionTreeOpts{ + TotalCommits: 1, + Writer: &buf, + }) + require.NotEmpty(t, lines) + assert.NotContains(t, lines[0], "\x1b[", "a non-terminal writer must never be styled") + + // Writer left unset (nil) must fall back to io.Discard-equivalent + // (unstyled) behavior rather than panicking on a nil io.Writer. + assert.NotPanics(t, func() { + formatSessionTree(summaries, formatSessionTreeOpts{TotalCommits: 1}) + }) + }) } func TestParseGitProgressLine(t *testing.T) { @@ -248,6 +285,33 @@ func TestParseGitProgressLine(t *testing.T) { }) } +// TestDisplayGitProgress_DedupsCountingPhase guards against a regression +// where git's "Enumerating objects: N, done." and "Counting objects: 100% +// (N/N), done." lines both parse to gitProgressPhaseCounting with Done=true, +// so the display loop printed two identical "counting objects" lines for a +// single push. Each phase's Done summary must print at most once. +func TestDisplayGitProgress_DedupsCountingPhase(t *testing.T) { + t.Parallel() + stderr := strings.Join([]string{ + "Enumerating objects: 47, done.", + "Counting objects: 100% (47/47), done.", + "Compressing objects: 81% (31/38)", + "Compressing objects: 100% (38/38), done.", + "Writing objects: 100% (42/42), 156.23 KiB | 312.00 KiB/s, done.", + }, "\n") + + var buf bytes.Buffer + displayGitProgress(&buf, stderr) + + out := buf.String() + assert.Equal(t, 1, strings.Count(out, "counting objects"), + "the counting phase's Done summary must print exactly once, got: %q", out) + assert.Equal(t, 1, strings.Count(out, "compressing:"), + "the compressing phase's Done summary must print exactly once, got: %q", out) + assert.Equal(t, 1, strings.Count(out, "writing:"), + "the writing phase's Done summary must print exactly once, got: %q", out) +} + // captureStdout redirects os.Stdout to a pipe and returns a function that // restores stdout and returns the captured output. Mirrors captureStderr in // push_common_test.go. Must be called on the main goroutine (not diff --git a/cmd/entire/cli/strategy/push_reporter_test.go b/cmd/entire/cli/strategy/push_reporter_test.go index d8e252523d..a2f73ca38d 100644 --- a/cmd/entire/cli/strategy/push_reporter_test.go +++ b/cmd/entire/cli/strategy/push_reporter_test.go @@ -4,10 +4,32 @@ import ( "bytes" "context" "strings" + "sync" "testing" "time" ) +// syncBuffer is a mutex-guarded io.Writer for tests that read a buffer +// concurrently with a goroutine writing to it (e.g. pushReporter's reveal +// goroutine). Reading a plain bytes.Buffer while such a goroutine runs is a +// data race; syncBuffer serializes access so polling is race-safe. +type syncBuffer struct { + mu sync.Mutex + buf bytes.Buffer +} + +func (s *syncBuffer) Write(p []byte) (int, error) { + s.mu.Lock() + defer s.mu.Unlock() + return s.buf.Write(p) +} + +func (s *syncBuffer) String() string { + s.mu.Lock() + defer s.mu.Unlock() + return s.buf.String() +} + func TestPushShouldReveal(t *testing.T) { t.Parallel() tests := []struct { @@ -44,10 +66,23 @@ func TestPushReporter_NonTTY_WritesNothing(t *testing.T) { func TestPushReporter_TTY_RevealsThenClears(t *testing.T) { t.Parallel() - var buf bytes.Buffer - r := newPushReporter(context.Background(), &buf, true, time.Millisecond) + buf := &syncBuffer{} + r := newPushReporter(context.Background(), buf, true, time.Millisecond) r.phase("syncing 3 checkpoints") - time.Sleep(15 * time.Millisecond) // let the reveal goroutine fire + + // Poll until the reveal goroutine writes the phase text, rather than + // sleeping a fixed duration: under saturated -race CI load the goroutine + // may not be scheduled within any fixed window. buf.String() is + // mutex-guarded, so polling concurrently with the goroutine's writes is + // race-safe. + deadline := time.Now().Add(2 * time.Second) + for !strings.Contains(buf.String(), "syncing 3 checkpoints") { + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for revealed phase text, got %q", buf.String()) + } + time.Sleep(time.Millisecond) + } + r.finish("pushed 3") out := buf.String() if !strings.Contains(out, "syncing 3 checkpoints") { From f8ee1c476bfa0451eba3ef54df19e06c78e50012 Mon Sep 17 00:00:00 2001 From: Victor Gutierrez Calderon Date: Mon, 3 Aug 2026 12:02:37 +0200 Subject: [PATCH 3/5] feat(push): live transfer progress + count in pre-push checkpoint UX Reveal at ~1s and stream git's --progress transfer stats live into the reporter line (counting/compressing/writing) alongside the checkpoint count, ending with a persistent "pushed N checkpoints" summary. Non-TTY, non-ShouldStyle, and sub-1s pushes stay silent. Co-Authored-By: snowingfox --- cmd/entire/cli/checkpoint/remote/git.go | 15 +- cmd/entire/cli/strategy/manual_commit_push.go | 27 ++- cmd/entire/cli/strategy/push_common.go | 12 +- cmd/entire/cli/strategy/push_progress.go | 67 +++++++ cmd/entire/cli/strategy/push_reporter.go | 71 ++++--- cmd/entire/cli/strategy/push_reporter_test.go | 180 +++++++++++++++--- cmd/entire/cli/strategy/refs_push_test.go | 16 +- 7 files changed, 314 insertions(+), 74 deletions(-) diff --git a/cmd/entire/cli/checkpoint/remote/git.go b/cmd/entire/cli/checkpoint/remote/git.go index 35746f9e35..0b0dc714bf 100644 --- a/cmd/entire/cli/checkpoint/remote/git.go +++ b/cmd/entire/cli/checkpoint/remote/git.go @@ -6,6 +6,7 @@ import ( "encoding/base64" "errors" "fmt" + "io" "log/slog" "os" "os/exec" @@ -414,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. @@ -446,7 +455,11 @@ func PushWithOptions(ctx context.Context, opts PushOptions) (PushResult, error) var stdout, stderr bytes.Buffer cmd.Stdout = &stdout - cmd.Stderr = &stderr + if opts.ProgressWriter != nil { + cmd.Stderr = io.MultiWriter(&stderr, opts.ProgressWriter) + } else { + cmd.Stderr = &stderr + } err = cmd.Run() result := PushResult{ Output: stdout.String(), diff --git a/cmd/entire/cli/strategy/manual_commit_push.go b/cmd/entire/cli/strategy/manual_commit_push.go index 5d27a3939b..8290a41127 100644 --- a/cmd/entire/cli/strategy/manual_commit_push.go +++ b/cmd/entire/cli/strategy/manual_commit_push.go @@ -363,18 +363,26 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar // Progress: pushing many refs over the network can take tens of seconds, so // surface it via the threshold-gated reporter instead of leaving the user's - // git push apparently hung. TTY-gated and threshold-delayed: agents and CI - // (non-TTY stderr) see zero progress bytes. - displayTarget := displayPushTarget(pushTarget) - // isTTY here means "ANSI-capable per ShouldStyle" (respects NO_COLOR and - // TERM=cygwin), not merely "stderr is a terminal" — see the repo-wide rule - // on gating \r/\033[K writes. - rep := newPushReporter(ctx, os.Stderr, interactive.ShouldStyle(os.Stderr), 2*time.Second) - rep.phase(fmt.Sprintf("syncing %d checkpoint(s) to %s", len(existing), displayTarget)) + // 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 { rep.finish(fmt.Sprintf("pushed %d checkpoint(s)", len(existing))) if removeErr := queue.Remove(existing); removeErr != nil { @@ -401,6 +409,7 @@ func flushCheckpointRefsQueue(ctx context.Context, repo *git.Repository, pushTar // (a genuine cherry-pick conflict leaves that ref queued for a later push, // never force-overwriting the remote). rep.phase(fmt.Sprintf("resolving %d diverged checkpoint(s)", len(existing))) + rep.setDetail("") // clear the batch attempt's stale transfer detail pushed := make([]plumbing.ReferenceName, 0, len(existing)) var firstErr error for _, ref := range existing { diff --git a/cmd/entire/cli/strategy/push_common.go b/cmd/entire/cli/strategy/push_common.go index 60e946f997..5c17ecf5b1 100644 --- a/cmd/entire/cli/strategy/push_common.go +++ b/cmd/entire/cli/strategy/push_common.go @@ -56,7 +56,11 @@ func partitionLocalRefs(repo *git.Repository, refs []plumbing.ReferenceName) (ex // remote history with no signal. On rejection the whole push errors; the caller // retries the rejected refs individually with fetch+replay recovery // (pushCheckpointRefWithRecovery). -func batchPushRefs(ctx context.Context, target string, refs []plumbing.ReferenceName) error { +// progressW, when non-nil, receives a live copy of git's --progress stderr +// bytes as the push runs (see remote.PushOptions.ProgressWriter) — typically a +// *gitProgressStreamer feeding a pushReporter's setDetail. Pass nil when no +// caller needs live streaming (e.g. the single-ref recovery retry). +func batchPushRefs(ctx context.Context, target string, refs []plumbing.ReferenceName, progressW io.Writer) error { if len(refs) == 0 { return nil } @@ -64,7 +68,7 @@ func batchPushRefs(ctx context.Context, target string, refs []plumbing.Reference for _, ref := range refs { refSpecs = append(refSpecs, ref.String()+":"+ref.String()) } - result, err := remote.PushWithOptions(ctx, remote.PushOptions{Remote: target, RefSpecs: refSpecs}) + result, err := remote.PushWithOptions(ctx, remote.PushOptions{Remote: target, RefSpecs: refSpecs, ProgressWriter: progressW}) logGitProgress(ctx, result.Stderr) if err != nil { return fmt.Errorf("push %d checkpoint refs: %w", len(refs), err) @@ -88,13 +92,13 @@ func pushCheckpointRefWithRecovery(ctx context.Context, target string, ref plumb ctx, cancel := context.WithTimeout(ctx, checkpointPushBudget) defer cancel() - if err := batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}); err == nil { + if err := batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}, nil); err == nil { return nil } if err := fetchAndRebaseRefCommon(ctx, target, ref, io.Discard); err != nil { return fmt.Errorf("sync diverged checkpoint ref %s: %w", ref, err) } - return batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}) + return batchPushRefs(ctx, target, []plumbing.ReferenceName{ref}, nil) } // pushRefIfNeeded pushes a ref to the given target if it has unpushed changes. diff --git a/cmd/entire/cli/strategy/push_progress.go b/cmd/entire/cli/strategy/push_progress.go index 218724ba2e..49ab8a3abe 100644 --- a/cmd/entire/cli/strategy/push_progress.go +++ b/cmd/entire/cli/strategy/push_progress.go @@ -308,6 +308,73 @@ func parsePercentGitProgressEvent(phase gitProgressPhase, m []string, trimmed st } } +// gitProgressStreamer is an io.Writer that live-parses git `--progress` +// stderr bytes as they arrive, without waiting for the push to finish. git +// emits progress as segments separated by '\r' (in-place updates) or '\n' +// (a completed line, e.g. the final "done." for a phase). Write accumulates +// bytes across calls — a chunk boundary can land mid-line — and parses each +// completed segment as soon as its terminator arrives, invoking onEvent for +// every line that parses to a non-nil *gitProgressEvent. Leftover partial +// bytes (after the last terminator in the chunk) are buffered for the next +// Write. Not safe for concurrent Write calls; callers must serialize writes +// (e.g. a single git stderr pipe copied by one goroutine). +type gitProgressStreamer struct { + buf []byte + onEvent func(*gitProgressEvent) +} + +// Write implements io.Writer. It never returns an error — parse failures are +// simply skipped — so it never causes the underlying io.MultiWriter copy (see +// checkpoint/remote.PushOptions.ProgressWriter) to abort the push. +func (s *gitProgressStreamer) Write(p []byte) (int, error) { + s.buf = append(s.buf, p...) + + start := 0 + for i, b := range s.buf { + if b != '\r' && b != '\n' { + continue + } + segment := string(s.buf[start:i]) + start = i + 1 + if event := parseGitProgressLine(segment); event != nil && s.onEvent != nil { + s.onEvent(event) + } + } + s.buf = s.buf[start:] + + return len(p), nil +} + +// formatPushProgressDetail renders a gitProgressEvent as the short live +// detail string shown next to the pushReporter's prefix (e.g. "writing +// 40/47 objects"). Returns "" for phases/events that don't carry enough +// information to render (e.g. a bare "Enumerating objects: N" line with no +// current count yet). +func formatPushProgressDetail(event *gitProgressEvent) string { + switch event.Phase { + case gitProgressPhaseCounting: + if event.Current > 0 { + return fmt.Sprintf("counting objects: %d", event.Current) + } + if event.Total > 0 { + return fmt.Sprintf("counting objects: %d", event.Total) + } + return "" + case gitProgressPhaseCompressing: + if event.Total == 0 { + return "" + } + return fmt.Sprintf("compressing %d/%d", event.Current, event.Total) + case gitProgressPhaseWriting: + if event.Total == 0 { + return "" + } + return fmt.Sprintf("writing %d/%d objects", event.Current, event.Total) + default: + return "" + } +} + // displayGitProgress writes human-friendly git transfer progress lines to w. func displayGitProgress(w io.Writer, stderr string) { styles := pushProgressStylesFor(w, false) diff --git a/cmd/entire/cli/strategy/push_reporter.go b/cmd/entire/cli/strategy/push_reporter.go index 83d0ad2ab4..a235510725 100644 --- a/cmd/entire/cli/strategy/push_reporter.go +++ b/cmd/entire/cli/strategy/push_reporter.go @@ -12,39 +12,42 @@ import ( ) // pushShouldReveal reports whether the pre-push progress line should be shown: -// only on a terminal, and only once the push has run long enough to feel stuck. -func pushShouldReveal(elapsed, threshold time.Duration, isTTY bool) bool { - return isTTY && elapsed >= threshold +// only when styled (ANSI-capable per interactive.ShouldStyle — respects +// NO_COLOR and TERM=cygwin, not merely "stderr is a terminal"), and only once +// the push has run long enough to feel stuck. +func pushShouldReveal(elapsed, threshold time.Duration, styled bool) bool { + return styled && elapsed >= threshold } // pushReporter is a threshold-gated, single-in-place-line progress reporter // for the pre-push checkpoint sync. It always logs to file via logging.Debug, // but only writes to its terminal writer once the push has run long enough to -// feel stuck (and never at all when isTTY is false) — agents and CI must see +// feel stuck (and never at all when styled is false) — agents and CI must see // zero bytes. type pushReporter struct { ctx context.Context w io.Writer - isTTY bool + styled bool threshold time.Duration start time.Time mu sync.Mutex revealed bool - text string + prefix string + detail string done chan struct{} stopped chan struct{} } // newPushReporter creates a pushReporter and starts its background reveal -// goroutine. Callers must call finish to stop the goroutine and clear the -// line. -func newPushReporter(ctx context.Context, w io.Writer, isTTY bool, threshold time.Duration) *pushReporter { +// goroutine. Callers must call finish to stop the goroutine and print the +// persistent summary line. +func newPushReporter(ctx context.Context, w io.Writer, styled bool, threshold time.Duration) *pushReporter { r := &pushReporter{ ctx: ctx, w: w, - isTTY: isTTY, + styled: styled, threshold: threshold, start: time.Now(), done: make(chan struct{}), @@ -90,7 +93,7 @@ func (r *pushReporter) run() { // maybeReveal writes the current in-place line when reveal conditions are // met. Callers must hold r.mu. func (r *pushReporter) maybeReveal() { - if !pushShouldReveal(time.Since(r.start), r.threshold, r.isTTY) { + if !pushShouldReveal(time.Since(r.start), r.threshold, r.styled) { return } r.revealed = true @@ -99,35 +102,59 @@ func (r *pushReporter) maybeReveal() { // redrawLocked writes the current in-place line. Callers must hold r.mu. func (r *pushReporter) redrawLocked() { - if !r.isTTY { + if !r.styled { return } elapsed := int(time.Since(r.start).Seconds()) - fmt.Fprintf(r.w, "\r[entire] %s (%ds)\033[K", r.text, elapsed) + if r.detail != "" { + fmt.Fprintf(r.w, "\r[entire] %s… %s (%ds)\033[K", r.prefix, r.detail, elapsed) + } else { + fmt.Fprintf(r.w, "\r[entire] %s… (%ds)\033[K", r.prefix, elapsed) + } } -// phase sets the current phase text. It always logs to file; if the line is -// already revealed, it redraws immediately. -func (r *pushReporter) phase(text string) { - logging.Debug(r.ctx, "checkpoint push progress", slog.String("phase", text)) +// phase sets the current prefix text (e.g. "syncing 12 checkpoints"). It +// always logs to file; if the line is already revealed, it redraws +// immediately. +func (r *pushReporter) phase(prefix string) { + logging.Debug(r.ctx, "checkpoint push progress", slog.String("phase", prefix)) + + r.mu.Lock() + defer r.mu.Unlock() + r.prefix = prefix + if r.revealed { + r.redrawLocked() + } +} +// setDetail updates the live transfer detail (e.g. "writing 40/47 objects") +// shown alongside the prefix, redrawing immediately if the line is already +// revealed. Deliberately does not log to file on every call — it is driven by +// git's --progress stream, which ticks far more often than is useful in the +// debug log; logGitProgress records the transfer detail to file once, after +// the push completes. +func (r *pushReporter) setDetail(detail string) { r.mu.Lock() defer r.mu.Unlock() - r.text = text + r.detail = detail if r.revealed { r.redrawLocked() } } -// finish stops the reveal goroutine, clears the in-place line if it was -// revealed, and logs the final summary to file. +// finish stops the reveal goroutine and, if the line was ever revealed, +// prints a PERSISTENT final summary line (trailing newline, so it stays in +// the user's scrollback rather than being erased) and logs the final summary +// to file. Stays completely silent when the line was never revealed — +// agents/CI/fast pushes must see zero bytes. func (r *pushReporter) finish(summary string) { close(r.done) <-r.stopped r.mu.Lock() - if r.revealed && r.isTTY { - fmt.Fprint(r.w, "\r\033[K") + if r.revealed && r.styled { + elapsed := int(time.Since(r.start).Seconds()) + fmt.Fprintf(r.w, "\r[entire] %s (%ds)\033[K\n", summary, elapsed) } r.mu.Unlock() diff --git a/cmd/entire/cli/strategy/push_reporter_test.go b/cmd/entire/cli/strategy/push_reporter_test.go index a2f73ca38d..08bd2fd9f6 100644 --- a/cmd/entire/cli/strategy/push_reporter_test.go +++ b/cmd/entire/cli/strategy/push_reporter_test.go @@ -30,76 +30,196 @@ func (s *syncBuffer) String() string { return s.buf.String() } +// waitForContains polls buf until it contains substr, rather than sleeping a +// fixed duration: under saturated -race CI load the reporter's background +// goroutine may not be scheduled within any fixed window. buf.String() is +// mutex-guarded, so polling concurrently with the goroutine's writes is +// race-safe. +func waitForContains(t *testing.T, buf *syncBuffer, substr string) { + t.Helper() + deadline := time.Now().Add(2 * time.Second) + for { + out := buf.String() + if strings.Contains(out, substr) { + return + } + if time.Now().After(deadline) { + t.Fatalf("timed out waiting for %q, got %q", substr, out) + } + time.Sleep(time.Millisecond) + } +} + func TestPushShouldReveal(t *testing.T) { t.Parallel() tests := []struct { name string elapsed, thresh time.Duration - tty, want bool + styled, want bool }{ - {"non-tty never reveals", 10 * time.Second, time.Second, false, false}, - {"tty below threshold stays hidden", 500 * time.Millisecond, time.Second, true, false}, - {"tty at threshold reveals", time.Second, time.Second, true, true}, - {"tty past threshold reveals", 3 * time.Second, time.Second, true, true}, + {"unstyled never reveals", 10 * time.Second, time.Second, false, false}, + {"styled below threshold stays hidden", 500 * time.Millisecond, time.Second, true, false}, + {"styled at threshold reveals", time.Second, time.Second, true, true}, + {"styled past threshold reveals", 3 * time.Second, time.Second, true, true}, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { t.Parallel() - if got := pushShouldReveal(tc.elapsed, tc.thresh, tc.tty); got != tc.want { - t.Fatalf("pushShouldReveal(%v,%v,%v)=%v want %v", tc.elapsed, tc.thresh, tc.tty, got, tc.want) + if got := pushShouldReveal(tc.elapsed, tc.thresh, tc.styled); got != tc.want { + t.Fatalf("pushShouldReveal(%v,%v,%v)=%v want %v", tc.elapsed, tc.thresh, tc.styled, got, tc.want) } }) } } -func TestPushReporter_NonTTY_WritesNothing(t *testing.T) { +func TestPushReporter_NotStyled_WritesNothing(t *testing.T) { t.Parallel() var buf bytes.Buffer r := newPushReporter(context.Background(), &buf, false, time.Millisecond) r.phase("syncing 3 checkpoints") + r.setDetail("writing 1/2 objects") time.Sleep(10 * time.Millisecond) - r.finish("pushed 3") + r.finish("pushed 3 checkpoints") if buf.Len() != 0 { - t.Fatalf("non-tty reporter wrote %q, want nothing", buf.String()) + t.Fatalf("unstyled reporter wrote %q, want nothing", buf.String()) } } -func TestPushReporter_TTY_RevealsThenClears(t *testing.T) { +// TestPushReporter_Styled_RevealsLiveDetailThenPersistentSummary exercises the +// full reveal -> live setDetail -> finish flow: the prefix appears once +// revealed, setDetail updates the in-place line with live transfer detail +// while the push is still "running", and finish prints a PERSISTENT summary +// line (trailing newline, no ellipsis) rather than clearing the line to +// nothing. +func TestPushReporter_Styled_RevealsLiveDetailThenPersistentSummary(t *testing.T) { t.Parallel() buf := &syncBuffer{} r := newPushReporter(context.Background(), buf, true, time.Millisecond) r.phase("syncing 3 checkpoints") + waitForContains(t, buf, "syncing 3 checkpoints") - // Poll until the reveal goroutine writes the phase text, rather than - // sleeping a fixed duration: under saturated -race CI load the goroutine - // may not be scheduled within any fixed window. buf.String() is - // mutex-guarded, so polling concurrently with the goroutine's writes is - // race-safe. - deadline := time.Now().Add(2 * time.Second) - for !strings.Contains(buf.String(), "syncing 3 checkpoints") { - if time.Now().After(deadline) { - t.Fatalf("timed out waiting for revealed phase text, got %q", buf.String()) - } - time.Sleep(time.Millisecond) - } + r.setDetail("writing 1/2 objects") + waitForContains(t, buf, "writing 1/2 objects") - r.finish("pushed 3") + r.finish("pushed 3 checkpoints") out := buf.String() - if !strings.Contains(out, "syncing 3 checkpoints") { - t.Fatalf("expected revealed phase text, got %q", out) + if !strings.Contains(out, "pushed 3 checkpoints") { + t.Fatalf("expected persistent summary text, got %q", out) + } + if strings.Contains(out, "pushed 3 checkpoints…") { + t.Fatalf("final summary must not carry the in-progress ellipsis, got %q", out) } - if !strings.HasSuffix(out, "\r\033[K") { - t.Fatalf("expected trailing clear sequence, got %q", out) + if !strings.HasSuffix(out, "\033[K\n") { + t.Fatalf("expected persistent final line to end with a newline, got %q", out) } } -func TestPushReporter_TTY_FastPush_StaysHidden(t *testing.T) { +func TestPushReporter_Styled_FastPush_StaysHidden(t *testing.T) { t.Parallel() var buf bytes.Buffer r := newPushReporter(context.Background(), &buf, true, time.Hour) // never reached r.phase("syncing 3 checkpoints") - r.finish("pushed 3") + r.setDetail("writing 1/2 objects") + r.finish("pushed 3 checkpoints") if buf.Len() != 0 { t.Fatalf("fast push wrote %q, want nothing", buf.String()) } } + +// TestGitProgressStreamer_ParsesChunkedProgressStream feeds a realistic +// \r-delimited git --progress byte stream in small chunks that deliberately +// split lines mid-way (simulating a real os/exec stderr pipe copy), and +// asserts onEvent fires with the expected phases/counts as soon as each +// segment completes. +func TestGitProgressStreamer_ParsesChunkedProgressStream(t *testing.T) { + t.Parallel() + + full := "Enumerating objects: 47, done.\r" + + "Counting objects: 10% (5/47)\r" + + "Counting objects: 100% (47/47), done.\n" + + "Compressing objects: 50% (19/38)\r" + + "Compressing objects: 100% (38/38), done.\n" + + "Writing objects: 85% (40/47), 120.00 KiB | 240.00 KiB/s\r" + + "Writing objects: 100% (47/47), 156.23 KiB | 312.00 KiB/s, done.\n" + + var events []*gitProgressEvent + streamer := &gitProgressStreamer{ + onEvent: func(e *gitProgressEvent) { + events = append(events, e) + }, + } + + const chunkSize = 7 + for i := 0; i < len(full); i += chunkSize { + end := min(i+chunkSize, len(full)) + n, err := streamer.Write([]byte(full[i:end])) + if err != nil { + t.Fatalf("Write returned error: %v", err) + } + if n != end-i { + t.Fatalf("Write returned n=%d, want %d", n, end-i) + } + } + + if len(events) == 0 { + t.Fatal("expected at least one parsed event") + } + + var sawWritingInProgress, sawWritingDone bool + for _, e := range events { + if e.Phase != gitProgressPhaseWriting { + continue + } + switch { + case e.Done: + sawWritingDone = true + if e.Current != 47 || e.Total != 47 { + t.Fatalf("writing done event has wrong counts: %+v", e) + } + case e.Current == 40 && e.Total == 47: + sawWritingInProgress = true + if got := formatPushProgressDetail(e); got != "writing 40/47 objects" { + t.Fatalf("formatPushProgressDetail = %q, want %q", got, "writing 40/47 objects") + } + } + } + if !sawWritingInProgress { + t.Fatalf("expected an in-progress writing event (40/47), got %+v", events) + } + if !sawWritingDone { + t.Fatalf("expected a writing-phase done event, got %+v", events) + } +} + +func TestGitProgressStreamer_BuffersLeftoverPartialLine(t *testing.T) { + t.Parallel() + + var events []*gitProgressEvent + streamer := &gitProgressStreamer{ + onEvent: func(e *gitProgressEvent) { + events = append(events, e) + }, + } + + // Split a single line across two Write calls with no terminator in the + // first chunk — the streamer must buffer it rather than parse a partial + // line or drop it. + n1, err := streamer.Write([]byte("Compressing objects: 50% (19")) + if err != nil || n1 != len("Compressing objects: 50% (19") { + t.Fatalf("first Write: n=%d err=%v", n1, err) + } + if len(events) != 0 { + t.Fatalf("expected no event before the line terminator, got %+v", events) + } + + n2, err := streamer.Write([]byte("/38)\r")) + if err != nil || n2 != len("/38)\r") { + t.Fatalf("second Write: n=%d err=%v", n2, err) + } + if len(events) != 1 { + t.Fatalf("expected exactly one event after the terminator arrived, got %+v", events) + } + if events[0].Phase != gitProgressPhaseCompressing || events[0].Current != 19 || events[0].Total != 38 { + t.Fatalf("unexpected event: %+v", events[0]) + } +} diff --git a/cmd/entire/cli/strategy/refs_push_test.go b/cmd/entire/cli/strategy/refs_push_test.go index 85156de24d..400e95619b 100644 --- a/cmd/entire/cli/strategy/refs_push_test.go +++ b/cmd/entire/cli/strategy/refs_push_test.go @@ -79,7 +79,7 @@ func TestBatchPushRefs(t *testing.T) { workDir, bareDir, refs := setupRepoWithCheckpointRefs(t) t.Chdir(workDir) - require.NoError(t, batchPushRefs(context.Background(), bareDir, refs)) + require.NoError(t, batchPushRefs(context.Background(), bareDir, refs, nil)) // All refs now exist on the bare remote. lsCmd := exec.CommandContext(context.Background(), "git", "ls-remote", bareDir) @@ -95,7 +95,7 @@ func TestBatchPushRefs(t *testing.T) { func TestBatchPushRefs_Empty(t *testing.T) { t.Parallel() // No refs → no git invocation, no error. - require.NoError(t, batchPushRefs(context.Background(), "unused-target", nil)) + require.NoError(t, batchPushRefs(context.Background(), "unused-target", nil, nil)) } // TestBatchPushRefs_AllowsFastForward: advancing a checkpoint ref to a descendant @@ -105,7 +105,7 @@ func TestBatchPushRefs_AllowsFastForward(t *testing.T) { t.Chdir(workDir) ctx := context.Background() - require.NoError(t, batchPushRefs(ctx, bareDir, refs)) + require.NoError(t, batchPushRefs(ctx, bareDir, refs, nil)) // Advance refs[0] to a child commit (fast-forward). repo, err := git.PlainOpen(workDir) @@ -117,7 +117,7 @@ func TestBatchPushRefs_AllowsFastForward(t *testing.T) { require.NoError(t, err) require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(refs[0], head2.Hash()))) - require.NoError(t, batchPushRefs(ctx, bareDir, refs[:1]), "fast-forward update should push without force") + require.NoError(t, batchPushRefs(ctx, bareDir, refs[:1], nil), "fast-forward update should push without force") assert.Equal(t, head2.Hash().String(), remoteRefHash(t, bareDir, refs[0]), "remote ref should advance to the descendant commit") } @@ -130,7 +130,7 @@ func TestBatchPushRefs_RejectsNonFastForward(t *testing.T) { t.Chdir(workDir) ctx := context.Background() - require.NoError(t, batchPushRefs(ctx, bareDir, refs)) + require.NoError(t, batchPushRefs(ctx, bareDir, refs, nil)) original := remoteRefHash(t, bareDir, refs[0]) // Point refs[0] at an orphan commit (no parent) — not a descendant of what was @@ -149,7 +149,7 @@ func TestBatchPushRefs_RejectsNonFastForward(t *testing.T) { require.NoError(t, err) require.NoError(t, repo.Storer.SetReference(plumbing.NewHashReference(refs[0], plumbing.NewHash(orphan)))) - err = batchPushRefs(ctx, bareDir, refs[:1]) + err = batchPushRefs(ctx, bareDir, refs[:1], nil) require.Error(t, err, "a non-fast-forward update must be rejected, not force-pushed") assert.Equal(t, original, remoteRefHash(t, bareDir, refs[0]), "remote ref must be unchanged after a rejected non-fast-forward push") @@ -178,14 +178,14 @@ func TestPushCheckpointRefWithRecovery_MergesDivergedRef(t *testing.T) { } c1 := head() - require.NoError(t, batchPushRefs(ctx, bareDir, []plumbing.ReferenceName{ref})) // remote ref = C1 + require.NoError(t, batchPushRefs(ctx, bareDir, []plumbing.ReferenceName{ref}, nil)) // remote ref = C1 // Remote advances: C2 (child of C1) adds b.txt; point the ref at it and push. testutil.WriteFile(t, workDir, "b.txt", "b") testutil.GitAdd(t, workDir, "b.txt") testutil.GitCommit(t, workDir, "add b") setRef(head()) - require.NoError(t, batchPushRefs(ctx, bareDir, []plumbing.ReferenceName{ref})) // remote ref = C2 + require.NoError(t, batchPushRefs(ctx, bareDir, []plumbing.ReferenceName{ref}, nil)) // remote ref = C2 // Local diverges: reset to C1 and make C3 (sibling of C2) adding c.txt. testutil.GitReset(t, workDir, c1.String()) From 1611bd0006fa96b1e595c1f7f124f812944a4fa5 Mon Sep 17 00:00:00 2001 From: Victor Gutierrez Calderon Date: Mon, 3 Aug 2026 12:15:12 +0200 Subject: [PATCH 4/5] fix(push): clear (not print) the progress line on empty-summary finish The SSH-auth-failure path calls finish("") then prints its own error; with the new persistent-summary finish this emitted a content-less '[entire] (Ns)' line. Clear the line instead when summary is empty. Co-Authored-By: snowingfox --- cmd/entire/cli/strategy/push_reporter.go | 11 ++++++++-- cmd/entire/cli/strategy/push_reporter_test.go | 21 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/strategy/push_reporter.go b/cmd/entire/cli/strategy/push_reporter.go index a235510725..a1512033b0 100644 --- a/cmd/entire/cli/strategy/push_reporter.go +++ b/cmd/entire/cli/strategy/push_reporter.go @@ -153,8 +153,15 @@ func (r *pushReporter) finish(summary string) { r.mu.Lock() if r.revealed && r.styled { - elapsed := int(time.Since(r.start).Seconds()) - fmt.Fprintf(r.w, "\r[entire] %s (%ds)\033[K\n", summary, elapsed) + if summary == "" { + // No summary (e.g. an aborted/failed push whose caller prints its + // own error next): just clear the in-place line, don't emit a + // content-less "[entire] (Ns)" persistent line. + fmt.Fprint(r.w, "\r\033[K") + } else { + elapsed := int(time.Since(r.start).Seconds()) + fmt.Fprintf(r.w, "\r[entire] %s (%ds)\033[K\n", summary, elapsed) + } } r.mu.Unlock() diff --git a/cmd/entire/cli/strategy/push_reporter_test.go b/cmd/entire/cli/strategy/push_reporter_test.go index 08bd2fd9f6..91da353bf4 100644 --- a/cmd/entire/cli/strategy/push_reporter_test.go +++ b/cmd/entire/cli/strategy/push_reporter_test.go @@ -114,6 +114,27 @@ func TestPushReporter_Styled_RevealsLiveDetailThenPersistentSummary(t *testing.T } } +// TestPushReporter_Styled_EmptySummaryClearsWithoutGarbage covers the +// aborted/failed-push path (e.g. non-interactive SSH auth failure) where the +// caller calls finish("") and then prints its own error. A revealed line must +// be cleared, NOT replaced by a content-less "[entire] (Ns)" persistent line. +func TestPushReporter_Styled_EmptySummaryClearsWithoutGarbage(t *testing.T) { + t.Parallel() + buf := &syncBuffer{} + r := newPushReporter(context.Background(), buf, true, time.Millisecond) + r.phase("syncing 3 checkpoints") + waitForContains(t, buf, "syncing 3 checkpoints") + + r.finish("") + out := buf.String() + if strings.Contains(out, "[entire] (") { + t.Fatalf("empty-summary finish emitted a content-less persistent line: %q", out) + } + if !strings.HasSuffix(out, "\r\033[K") { + t.Fatalf("expected empty-summary finish to clear the line, got %q", out) + } +} + func TestPushReporter_Styled_FastPush_StaysHidden(t *testing.T) { t.Parallel() var buf bytes.Buffer From 3e737c2f5b520c68d758061f8ed6f1e318afdd7a Mon Sep 17 00:00:00 2001 From: Victor Gutierrez Calderon Date: Mon, 3 Aug 2026 12:39:59 +0200 Subject: [PATCH 5/5] feat(push): show x/total counter and clearer wording for checkpoint sync recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-ref recovery path (batch push rejected as non-fast-forward) is slow — one fetch+replay+push per ref — and 'resolving N diverged checkpoint(s)' was opaque. Show an advancing 'syncing checkpoint i/N with remote' counter so the user can see progress. Co-Authored-By: snowingfox Entire-Checkpoint: 01KZ3KBDTT8MKFZ4J9JZVXF8VS --- cmd/entire/cli/strategy/manual_commit_push.go | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/cmd/entire/cli/strategy/manual_commit_push.go b/cmd/entire/cli/strategy/manual_commit_push.go index 8290a41127..5ce1aeca80 100644 --- a/cmd/entire/cli/strategy/manual_commit_push.go +++ b/cmd/entire/cli/strategy/manual_commit_push.go @@ -408,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). - rep.phase(fmt.Sprintf("resolving %d diverged checkpoint(s)", len(existing))) 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()))