diff --git a/README.md b/README.md index e9a93ec..f912a50 100644 --- a/README.md +++ b/README.md @@ -20,6 +20,17 @@ Stack Sync never runs `git stash`, `git reset`, `git clean`, `git commit`, or an The branch behavior is derived from the MIT-licensed `hub sync` implementation and is now built directly into Stack Sync. It fetches and prunes the main remote, fast-forwards outdated branches, warns about divergent/unpushed branches, and removes a branch only when its configured upstream was deleted and the branch is already merged into the remote default branch. It prefers a remote named `upstream`, `github`, or `origin`; a sole differently named remote is also accepted. See [third-party notices](THIRD_PARTY_NOTICES.md) for attribution. +## Branch handling + +`hub sync` only ever moved branches that already existed locally, and a branch whose upstream it could not resolve was dropped from the report entirely. Stack Sync keeps hub's safety model but stops the silent behaviour: + +- **Unmatched branches are reported, not hidden.** A local branch with no configured upstream and no same-named branch on the remote cannot be fast-forwarded. `hub` set its internal `remoteBranch` to an empty string and left `gone` false, so neither of its branches ran and the branch vanished without a word; the run then reported "Already up to date" while having done nothing. Such a branch is now reported as `UNTRACKED` with the reason. This makes the outcome visible, but it does not fast-forward the branch — there is no upstream to fast-forward from. +- **The remote default branch is resolved properly.** Which branch a deleted-and-merged branch had been merged into was inferred from `refs/remotes//HEAD`, a local convenience symref that is missing from plenty of real clones. When it is absent Stack Sync now asks the remote directly with `git ls-remote --symref`, so merged branches are still cleaned up instead of being kept with a vague warning. + +By default, like `hub`, Stack Sync never creates a local branch that does not already exist. Pass `--create-missing` to additionally create local branches for branches that exist only on the remote; they are created with tracking configured and reported as `CREATED`, and this never touches the worktree, moves an existing branch, or checks anything out. + +Branch actions reported per repository are `UPDATED`, `DELETED`, `CREATED`, `PROTECTED`, `WARNING`, and `UNTRACKED`. + ## Requirements Stack Sync supports Linux and Windows. It requires: @@ -78,7 +89,13 @@ PowerShell accepts the same options with a Windows path: stack-sync tui --root C:\Users\you\Coding\jwtf ``` -Every eligible repository starts selected. Dirty repositories remain selectable and are labelled `ready (dirty)`; their checked-out branches are protected if an update would affect them. Move with the arrow keys or `j`/`k`, toggle the focused repository with Space, select all with `a`, clear the selection with `n`, and refresh with `r`. Press `s` or Enter to review the branch-deletion warning, then `y` to begin. Blocked repositories cannot be selected. +Every eligible repository starts selected. The list is grouped by what will happen if you start a run now, so problems are visible before you commit to anything: + +- `BLOCKED` — not eligible, and cannot be selected. The reason (detached `HEAD`, merge in progress, no remotes) is shown for the focused row. +- `DIRTY` — eligible, but the checked-out branch will be protected rather than fast-forwarded. +- `READY` — fully eligible. + +Move with the arrow keys or `j`/`k`, toggle the focused repository with Space, select all with `a`, clear the selection with `n`, and refresh with `r`. Press `s` or Enter to review the branch-deletion warning, then `y` to begin. Blocked repositories cannot be selected. During synchronization, the TUI shows overall progress, elapsed time, active repositories, live success/failure/skip totals, and the latest completed result. The final view lists failures and protected or divergent branches, prioritizes and focuses the first failure automatically, and retains per-repository branch details. Press `f` to cycle through every repository that needs attention. @@ -102,6 +119,14 @@ Review the same plan, confirm it, and sync every eligible repository: stack-sync sync --root ~/Coding/jwtf ``` +While a run is in progress, an updating line keeps the workspace totals on screen so you can see what is happening without waiting for the final summary: + +```text + 17/40 · 2 active · 9 changed · 8 updated · 1 deleted · 2 protected · 1 warning · 1 untracked +``` + +That line is drawn only when stdout is a terminal. Redirected output and `--json` never contain cursor control characters, and `--json` is the right interface for scripts. + For automation, suppress the prompt and optionally require the entire workspace to be clean. In strict mode, either a blocked repository or any worktree change aborts the whole run: ```bash @@ -113,6 +138,7 @@ Useful options: ```text --jobs 4 maximum concurrent inspections or syncs --timeout 5m per-repository fetch and sync timeout +--create-missing create local branches that exist only on the remote --exclude temp skip a directory name anywhere in the tree --exclude Org/old skip a root-relative path --json emit structured output diff --git a/main.go b/main.go index 55ffb9f..0089d5f 100644 --- a/main.go +++ b/main.go @@ -16,9 +16,10 @@ import ( "strings" "sync" "time" + "unicode/utf8" ) -const version = "0.3.1" +const version = "0.4.0" var defaultSkippedDirs = map[string]bool{ ".cache": true, ".claude": true, ".codex": true, ".git": true, ".next": true, ".pnpm-store": true, @@ -27,13 +28,14 @@ var defaultSkippedDirs = map[string]bool{ } type options struct { - root string - json bool - yes bool - strict bool - jobs int - timeout time.Duration - exclusions stringList + root string + json bool + yes bool + strict bool + jobs int + timeout time.Duration + exclusions stringList + createMissing bool } type stringList []string @@ -163,7 +165,12 @@ func run(args []string, stdin io.Reader, stdout, stderr io.Writer) int { } } - results := syncAll(root, repos, opts.jobs, opts.timeout, stdout, opts.json, opts.strict) + var liveMu sync.Mutex + live := newLiveProgress(stdout, &liveMu, len(repos)) + results := syncAllWithLive(root, repos, opts.jobs, opts.timeout, stdout, opts.json, opts.strict, opts.createMissing, live) + liveMu.Lock() + live.clear() + liveMu.Unlock() if opts.json { writeJSON(stdout, results) } else { @@ -196,8 +203,16 @@ func parseFlags(command string, args []string, stderr io.Writer) (options, error } if command == "sync" || command == "tui" { fs.DurationVar(&opts.timeout, "timeout", opts.timeout, "timeout for each repository sync") + fs.BoolVar(&opts.createMissing, "create-missing", false, "create local branches that exist only on the remote") + } + fs.Usage = func() { + usage(stderr) + // PrintDefaults was dropped along with the stock usage function, which + // left "use -h for command options" promising flags that + // were never actually listed. + fmt.Fprintf(stderr, "\nOptions for %s:\n", command) + fs.PrintDefaults() } - fs.Usage = func() { usage(stderr) } if err := fs.Parse(args); err != nil { return opts, err } @@ -239,6 +254,13 @@ Safety: repositories without remotes, and in-progress Git operations remain blocked. stack-sync never stashes, resets, cleans, or commits. +Branch handling: + Outdated local branches are fast-forwarded, and a local branch whose upstream + was deleted is removed once it is merged into the remote default branch. + A local branch with no upstream and no matching remote branch cannot be + fast-forwarded, so it is reported as UNTRACKED and left alone. Pass + --create-missing to also create branches that exist only on the remote. + Use "stack-sync -h" for command options.`) } @@ -422,15 +444,20 @@ func gitBytes(path string, args ...string) ([]byte, error) { return cmd.Output() } -func syncAll(root string, repos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict bool) []syncResult { - return syncSelected(root, repos, repos, jobs, timeout, stdout, quiet, strict) +func syncAll(root string, repos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict, createMissing bool) []syncResult { + return syncSelected(root, repos, repos, jobs, timeout, stdout, quiet, strict, createMissing, nil) } -func syncSelected(root string, repos, workspaceRepos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict bool) []syncResult { - return syncSelectedWithProgress(root, repos, workspaceRepos, jobs, timeout, stdout, quiet, strict, nil) +// syncAllWithLive is syncAll with the in-place progress line attached. +func syncAllWithLive(root string, repos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict, createMissing bool, live *liveProgress) []syncResult { + return syncSelected(root, repos, repos, jobs, timeout, stdout, quiet, strict, createMissing, live) } -func syncSelectedWithProgress(root string, repos, workspaceRepos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict bool, progress func(syncProgressEvent)) []syncResult { +func syncSelected(root string, repos, workspaceRepos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict, createMissing bool, live *liveProgress) []syncResult { + return syncSelectedWithProgress(root, repos, workspaceRepos, jobs, timeout, stdout, quiet, strict, createMissing, live, nil) +} + +func syncSelectedWithProgress(root string, repos, workspaceRepos []repo, jobs int, timeout time.Duration, stdout io.Writer, quiet, strict, createMissing bool, live *liveProgress, progress func(syncProgressEvent)) []syncResult { type item struct { index int repo repo @@ -448,6 +475,7 @@ func syncSelectedWithProgress(root string, repos, workspaceRepos []repo, jobs in if progress != nil { progress(syncProgressEvent{Path: result.Path, Result: result}) } + live.update(func() { live.finished(result); live.draw() }) } for range min(jobs, len(repos)) { @@ -475,15 +503,18 @@ func syncSelectedWithProgress(root string, repos, workspaceRepos []repo, jobs in } if !quiet { outputMu.Lock() + live.clear() fmt.Fprintf(stdout, "\nSTART %s (%s)\n", r.RelativePath, r.Branch) outputMu.Unlock() } + live.update(func() { live.started(); live.draw() }) started := time.Now() - report, err := syncRepository(r.Path, timeout, len(fresh.Dirty) > 0, fresh.NestedRepoEntries) + report, err := syncRepositoryWith(r.Path, timeout, len(fresh.Dirty) > 0, fresh.NestedRepoEntries, syncOptions{CreateMissing: createMissing}) message := report.Message duration := time.Since(started).Round(time.Millisecond).String() if !quiet { outputMu.Lock() + live.clear() state := "DONE" if err != nil { state = "FAIL" @@ -536,26 +567,170 @@ func printScan(w io.Writer, root string, repos []repo) { } } +// actionOrder fixes the order actions are summarised in, so the same workspace +// always reads the same way from one run to the next. +var actionOrder = []string{"updated", "deleted", "created", "protected", "warning", "untracked"} + +// liveProgress redraws a single line in place while repositories sync, so +// workspace totals are visible without waiting for the final summary. It is only +// enabled when stdout is a terminal: writing cursor control into a pipe or a +// --json document would corrupt the captured output, so callers get a nil +// *liveProgress in those cases and every method here is a no-op. +type liveProgress struct { + w io.Writer + mu *sync.Mutex + total int + + done int + active int + changed int + failed int + skipped int + counts map[string]int + lastWidth int +} + +func newLiveProgress(w io.Writer, mu *sync.Mutex, total int) *liveProgress { + if !isTerminalWriter(w) { + return nil + } + return &liveProgress{w: w, mu: mu, total: total, counts: make(map[string]int, len(actionOrder))} +} + +// started records that a repository has begun syncing. +func (p *liveProgress) started() { + if p == nil { + return + } + p.active++ +} + +// finished folds a completed repository into the running totals. +func (p *liveProgress) finished(result syncResult) { + if p == nil { + return + } + p.done++ + p.active-- + for _, branch := range result.Branches { + p.counts[branch.Action]++ + } + switch { + case result.Skipped: + p.skipped++ + case !result.Success: + p.failed++ + case repositoryChanged(result): + p.changed++ + } +} + +func (p *liveProgress) text() string { + parts := []string{fmt.Sprintf("%d/%d", p.done, p.total)} + if p.active > 0 { + parts = append(parts, fmt.Sprintf("%d active", p.active)) + } + parts = append(parts, fmt.Sprintf("%d changed", p.changed)) + if line := formatActionCounts(p.counts); line != "" { + parts = append(parts, line) + } + if p.skipped > 0 { + parts = append(parts, fmt.Sprintf("%d skipped", p.skipped)) + } + if p.failed > 0 { + parts = append(parts, fmt.Sprintf("%d FAILED", p.failed)) + } + return " " + strings.Join(parts, " · ") +} + +// draw repaints the line. Callers must already hold p.mu when drawing. +func (p *liveProgress) draw() { + if p == nil { + return + } + line := p.text() + fmt.Fprintf(p.w, "\r%s\r%s", strings.Repeat(" ", p.lastWidth), line) + p.lastWidth = utf8.RuneCountInString(line) +} + +// clear erases the line so ordinary output can be written on top of it. +func (p *liveProgress) clear() { + if p == nil { + return + } + p.lastWidth = 0 +} + +func (p *liveProgress) update(fn func()) { + if p == nil { + return + } + p.mu.Lock() + defer p.mu.Unlock() + fn() +} + func printSummary(w io.Writer, results []syncResult) { - var ok, failed, skipped int + counts := make(map[string]int, len(actionOrder)) + var changed, unchanged, failed, skipped int for _, r := range results { + for _, branch := range r.Branches { + counts[branch.Action]++ + } switch { - case r.Success: - ok++ case r.Skipped: skipped++ - default: + case !r.Success: failed++ + case repositoryChanged(r): + changed++ + default: + unchanged++ } } - fmt.Fprintf(w, "\nSummary: %d synced, %d failed, %d skipped\n", ok, failed, skipped) - for _, r := range results { - if !r.Success && !r.Skipped { - fmt.Fprintf(w, " FAILED %s: %s\n", r.Path, firstLine(r.Message)) + // "synced" counted every repository that ran, including ones where nothing + // was actually touched, so a run that changed nothing still looked busy. + fmt.Fprintf(w, "\nSummary: %d changed, %d unchanged, %d failed, %d skipped\n", changed, unchanged, failed, skipped) + if line := formatActionCounts(counts); line != "" { + fmt.Fprintf(w, " %s\n", line) + } + // Deletions and failures are destructive or unexpected, so they are listed + // in full here rather than left to be found in the scrolling log above. + if failed > 0 || counts["deleted"] > 0 { + fmt.Fprintln(w, "\nNeeds review:") + for _, r := range results { + if !r.Success && !r.Skipped { + fmt.Fprintf(w, " FAILED %s: %s\n", r.Path, firstLine(r.Message)) + } + for _, branch := range r.Branches { + if branch.Action == "deleted" { + fmt.Fprintf(w, " DELETED %s / %s: %s\n", r.Path, branch.Branch, branch.Message) + } + } } } } +func repositoryChanged(r syncResult) bool { + for _, branch := range r.Branches { + switch branch.Action { + case "updated", "deleted", "created": + return true + } + } + return false +} + +func formatActionCounts(counts map[string]int) string { + parts := make([]string, 0, len(actionOrder)) + for _, action := range actionOrder { + if n := counts[action]; n > 0 { + parts = append(parts, fmt.Sprintf("%d %s", n, action)) + } + } + return strings.Join(parts, " · ") +} + func eligibleCount(repos []repo) int { n := 0 for _, r := range repos { @@ -605,3 +780,10 @@ func isTerminal(file *os.File) bool { info, err := file.Stat() return err == nil && info.Mode()&os.ModeCharDevice != 0 } + +// isTerminalWriter reports whether w is a real terminal, so callers can avoid +// emitting cursor control sequences into redirected or buffered output. +func isTerminalWriter(w io.Writer) bool { + file, ok := w.(*os.File) + return ok && isTerminal(file) +} diff --git a/main_test.go b/main_test.go index 628cdf2..106e7d2 100644 --- a/main_test.go +++ b/main_test.go @@ -6,6 +6,8 @@ import ( "os" "os/exec" "path/filepath" + "strings" + "sync" "testing" "time" ) @@ -124,7 +126,7 @@ func TestStrictSyncRechecksAndSkipsNewlyDirtyRepository(t *testing.T) { } var progress []syncProgressEvent - results := syncSelectedWithProgress(root, []repo{planned}, []repo{planned}, 1, time.Minute, io.Discard, true, true, func(event syncProgressEvent) { + results := syncSelectedWithProgress(root, []repo{planned}, []repo{planned}, 1, time.Minute, io.Discard, true, true, false, nil, func(event syncProgressEvent) { progress = append(progress, event) }) if len(results) != 1 || !results[0].Skipped || results[0].Success { @@ -160,3 +162,109 @@ func TestIndentNormalizesGitProgressOutput(t *testing.T) { t.Fatalf("indent() = %q, want %q", got, want) } } + +// The summary must say what actually changed, not just how many repositories +// ran. "synced" previously counted repositories where nothing was touched. +func TestSummaryReportsPerActionCountsAndDeletions(t *testing.T) { + results := []syncResult{ + {Path: "api", Success: true, Branches: []branchSyncResult{ + {Branch: "main", Action: "updated"}, + {Branch: "stale", Action: "deleted", Message: "upstream was deleted and the branch was merged into main"}, + }}, + {Path: "web", Success: true, Branches: []branchSyncResult{ + {Branch: "wip", Action: "protected"}, + }}, + {Path: "cli", Success: true}, + {Path: "docs", Message: "fetch origin: authentication failed"}, + } + + var out bytes.Buffer + printSummary(&out, results) + got := out.String() + + for _, want := range []string{ + "1 changed", "2 unchanged", "1 failed", "0 skipped", + "1 updated · 1 deleted · 1 protected", + "Needs review:", + "DELETED api / stale: upstream was deleted and the branch was merged into main", + "FAILED docs: fetch origin: authentication failed", + } { + if !strings.Contains(got, want) { + t.Fatalf("summary missing %q:\n%s", want, got) + } + } + // A workspace where nothing changed must not claim to have synced anything, + // and must not open a "Needs review" section with nothing to review. + var quiet bytes.Buffer + printSummary(&quiet, []syncResult{{Path: "web", Success: true, Branches: []branchSyncResult{{Branch: "wip", Action: "protected"}}}}) + if strings.Contains(quiet.String(), "1 changed") { + t.Fatalf("a protected-only run was reported as changed:\n%s", quiet.String()) + } + if strings.Contains(quiet.String(), "Needs review") { + t.Fatalf("a run with no failures or deletions opened a review section:\n%s", quiet.String()) + } +} + +// The in-place progress line must never be written to a pipe, a buffer, or a +// --json document, where the cursor control would corrupt the captured output. +func TestLiveProgressIsDisabledForNonTerminalOutput(t *testing.T) { + var mu sync.Mutex + if p := newLiveProgress(&bytes.Buffer{}, &mu, 3); p != nil { + t.Fatal("newLiveProgress() enabled the live line for a buffered writer") + } +} + +func TestLiveProgressRendersRunningTotals(t *testing.T) { + var mu sync.Mutex + buf := &bytes.Buffer{} + p := &liveProgress{w: buf, mu: &mu, total: 3, counts: map[string]int{}} + + p.update(func() { p.started(); p.draw() }) + if !strings.Contains(buf.String(), "0/3") || !strings.Contains(buf.String(), "1 active") { + t.Fatalf("start did not render progress: %q", buf.String()) + } + + buf.Reset() + p.update(func() { + p.finished(syncResult{Path: "api", Success: true, Branches: []branchSyncResult{{Branch: "main", Action: "updated"}}}) + p.draw() + }) + for _, want := range []string{"1/3", "1 changed", "1 updated"} { + if !strings.Contains(buf.String(), want) { + t.Fatalf("totals missing %q: %q", want, buf.String()) + } + } + + buf.Reset() + p.update(func() { + p.finished(syncResult{Path: "web", Message: "fetch origin: authentication failed"}) + p.draw() + }) + for _, want := range []string{"2/3", "1 FAILED"} { + if !strings.Contains(buf.String(), want) { + t.Fatalf("failure not surfaced live: %q", buf.String()) + } + } + if strings.Contains(buf.String(), "2 changed") { + t.Fatalf("a failed repository was counted as changed: %q", buf.String()) + } +} + +// End-to-end guard: a non-interactive run must emit no carriage returns at all. +func TestSyncOutputToBufferHasNoCarriageReturns(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "repo") + makeRepo(t, path) + + var stdout, stderr bytes.Buffer + code := run([]string{"sync", "--root", root, "--yes"}, bytes.NewReader(nil), &stdout, &stderr) + if code > 1 { + t.Fatalf("run() code = %d, stderr = %q", code, stderr.String()) + } + if strings.Contains(stdout.String(), "\r") { + t.Fatalf("buffered output contained cursor control:\n%q", stdout.String()) + } + if !strings.Contains(stdout.String(), "Summary:") { + t.Fatalf("run did not print a summary:\n%q", stdout.String()) + } +} diff --git a/sync.go b/sync.go index 80908c2..93f94fd 100644 --- a/sync.go +++ b/sync.go @@ -31,7 +31,36 @@ type localBranch struct { oid string } +// upstreamState describes what stack-sync could work out about where a local +// branch comes from on the main remote. +type upstreamState int + +const ( + // upstreamTracked means the local branch has a resolvable remote branch. + upstreamTracked upstreamState = iota + // upstreamDeleted means the branch had a configured upstream which the + // fetch has just pruned, so the remote branch is gone. + upstreamDeleted + // upstreamUnmatched means the branch has no configured upstream and no + // same-named branch on the remote. Such a branch cannot be fast-forwarded + // automatically, and it used to be dropped from the report entirely, which + // made the run claim to be "Already up to date" while silently doing + // nothing at all for it. + upstreamUnmatched +) + +// syncOptions tunes a single repository sync. +type syncOptions struct { + // CreateMissing creates a local branch for every remote branch that has no + // local counterpart, so work started on another machine shows up here. + CreateMissing bool +} + func syncRepository(path string, timeout time.Duration, initiallyDirty bool, ignoredNested []string) (repositorySyncReport, error) { + return syncRepositoryWith(path, timeout, initiallyDirty, ignoredNested, syncOptions{}) +} + +func syncRepositoryWith(path string, timeout time.Duration, initiallyDirty bool, ignoredNested []string, opts syncOptions) (repositorySyncReport, error) { ctx, cancel := context.WithTimeout(context.Background(), timeout) defer cancel() @@ -62,7 +91,7 @@ func syncRepository(path string, timeout time.Duration, initiallyDirty bool, ign } for _, branch := range branches { - target, gone, err := upstreamForBranch(ctx, path, remote, branch.name) + target, state, err := upstreamForBranch(ctx, path, remote, branch.name) if err != nil { return failedReport(report, ctx, timeout, err) } @@ -113,7 +142,8 @@ func syncRepository(path string, timeout time.Duration, initiallyDirty bool, ign continue } - if !gone { + if state == upstreamUnmatched { + report.add(branch.name, "untracked", "no branch "+branch.name+" on "+remote+" and no upstream configured; left unchanged") continue } if defaultRef == "" { @@ -162,6 +192,16 @@ func syncRepository(path string, timeout time.Duration, initiallyDirty bool, ign report.add(branch.name, "deleted", "upstream was deleted and the branch was merged into "+defaultBranch) } + if opts.CreateMissing { + created, err := createMissingBranches(ctx, path, remote, branches) + if err != nil { + return failedReport(report, ctx, timeout, err) + } + for _, name := range created { + report.add(name, "created", "no local branch; created from "+remote+"/"+name) + } + } + if len(report.Branches) == 0 { report.Message = "Already up to date." } else { @@ -170,6 +210,69 @@ func syncRepository(path string, timeout time.Duration, initiallyDirty bool, ign return report, nil } +// createMissingBranches creates a local branch for each remote branch that has +// no local counterpart. This never touches the worktree, never moves an +// existing branch, and never checks anything out: it only fills in branches +// that exist on the remote so that a workspace which is "in sync" really is. +func createMissingBranches(ctx context.Context, path, remote string, existing []localBranch) ([]string, error) { + remoteBranches, err := remoteTrackingBranches(ctx, path, remote) + if err != nil { + return nil, err + } + have := make(map[string]bool, len(existing)) + for _, branch := range existing { + have[branch.name] = true + } + var created []string + for _, remoteBranch := range remoteBranches { + if have[remoteBranch.name] { + continue + } + // --track records the upstream, so later runs report and fast-forward + // this branch like any other. + output, err := gitCombinedContext(ctx, path, "branch", "--track", remoteBranch.name, remote+"/"+remoteBranch.name) + if err != nil { + // A branch created between the listing and here is not a failure. + if exists, existsErr := refExists(ctx, path, "refs/heads/"+remoteBranch.name); existsErr == nil && exists { + have[remoteBranch.name] = true + continue + } + return nil, gitCommandError("create branch "+remoteBranch.name+" from "+remote, output, err) + } + created = append(created, remoteBranch.name) + } + return created, nil +} + +// remoteTrackingBranches lists the branches the main remote advertises, as +// resolved into refs/remotes. The remote's symbolic HEAD is not a branch and is +// excluded. +func remoteTrackingBranches(ctx context.Context, path, remote string) ([]localBranch, error) { + prefix := "refs/remotes/" + remote + "/" + output, err := gitBytesContext(ctx, path, "for-each-ref", "--format=%(refname)%00%(refname:short)%00%(objectname)", prefix) + if err != nil { + return nil, fmt.Errorf("list %s branches: %w", remote, err) + } + var branches []localBranch + for _, line := range bytes.Split(bytes.TrimSpace(output), []byte{'\n'}) { + parts := bytes.Split(line, []byte{0}) + if len(parts) != 3 { + continue + } + // Derive the name from the full ref rather than %(refname:short): + // git shortens refs to the shortest unambiguous form, so + // refs/remotes/origin/HEAD is reported as plain "origin" and would + // otherwise be mistaken for a real branch called "origin". + name, ok := strings.CutPrefix(string(parts[0]), prefix) + if !ok || name == "" || name == "HEAD" { + continue + } + branches = append(branches, localBranch{ref: string(parts[0]), name: name, oid: string(parts[2])}) + } + sort.Slice(branches, func(i, j int) bool { return branches[i].name < branches[j].name }) + return branches, nil +} + func (r *repositorySyncReport) add(branch, action, message string) { r.Branches = append(r.Branches, branchSyncResult{Branch: branch, Action: action, Message: message}) } @@ -221,7 +324,7 @@ func localBranches(ctx context.Context, path string) ([]localBranch, error) { return branches, nil } -func upstreamForBranch(ctx context.Context, path, remote, branch string) (target string, gone bool, err error) { +func upstreamForBranch(ctx context.Context, path, remote, branch string) (target string, state upstreamState, err error) { configuredRemote, _ := gitOutputContext(ctx, path, "config", "--get", "branch."+branch+".remote") configuredRemote = strings.TrimSpace(configuredRemote) mergeRef, _ := gitOutputContext(ctx, path, "config", "--get", "branch."+branch+".merge") @@ -230,22 +333,22 @@ func upstreamForBranch(ctx context.Context, path, remote, branch string) (target target = "refs/remotes/" + remote + "/" + strings.TrimPrefix(mergeRef, "refs/heads/") exists, err := refExists(ctx, path, target) if err != nil { - return "", false, err + return "", upstreamTracked, err } if !exists { - return "", true, nil + return "", upstreamDeleted, nil } - return target, false, nil + return target, upstreamTracked, nil } target = "refs/remotes/" + remote + "/" + branch exists, err := refExists(ctx, path, target) if err != nil { - return "", false, err + return "", upstreamTracked, err } if !exists { - return "", false, nil + return "", upstreamUnmatched, nil } - return target, false, nil + return target, upstreamTracked, nil } func remoteDefaultBranch(ctx context.Context, path, remote string) (ref, branch string, err error) { @@ -262,9 +365,61 @@ func remoteDefaultBranch(ctx context.Context, path, remote string) (ref, branch } else if ctx.Err() != nil { return "", "", ctx.Err() } + // refs/remotes//HEAD is a local convenience symref and is absent + // in plenty of real clones. Without it we could not tell which branch a + // deleted-and-merged branch had been merged into, so merged branches were + // kept with a vague "remote default branch is unknown" warning. Ask the + // remote itself instead of giving up. + name, oid, err := remoteHead(ctx, path, remote) + if err != nil { + if ctx.Err() != nil { + return "", "", ctx.Err() + } + // The remote simply did not tell us; fall back to "unknown". + return "", "", nil + } + ref = "refs/remotes/" + remote + "/" + name + if exists, existsErr := refExists(ctx, path, ref); existsErr == nil && exists { + return ref, name, nil + } + // The fetch has just run, so the tracking ref should be there. If it is + // not, the raw object id is still a valid commit for ancestry checks. + if oid != "" { + return oid, name, nil + } return "", "", nil } +// remoteHead asks the remote which branch its HEAD points at. It returns the +// short branch name and, when advertised, the object id of that commit. +func remoteHead(ctx context.Context, path, remote string) (branch, oid string, err error) { + output, err := gitBytesContext(ctx, path, "ls-remote", "--symref", remote, "HEAD") + if err != nil { + return "", "", err + } + for _, line := range strings.Split(string(output), "\n") { + line = strings.TrimSpace(line) + if line == "" { + continue + } + if rest, ok := strings.CutPrefix(line, "ref: "); ok { + name, _, _ := strings.Cut(rest, "\t") + name = strings.TrimSpace(strings.TrimPrefix(strings.TrimSpace(name), "refs/heads/")) + if name != "" { + return name, oid, nil + } + continue + } + if fields := strings.Fields(line); len(fields) == 2 { + oid = fields[0] + } + } + if branch == "" && oid == "" { + return "", "", errors.New("remote did not advertise HEAD") + } + return "", oid, nil +} + func checkedOutBranches(ctx context.Context, path string) (map[string]bool, error) { output, err := gitOutputContext(ctx, path, "worktree", "list", "--porcelain") if err != nil { diff --git a/sync_test.go b/sync_test.go index 051a97b..8c8cddf 100644 --- a/sync_test.go +++ b/sync_test.go @@ -1,6 +1,7 @@ package main import ( + "io" "os" "os/exec" "path/filepath" @@ -210,3 +211,154 @@ func assertBranchAction(t *testing.T, results []branchSyncResult, branch, action } t.Fatalf("missing %s action for branch %s in %+v", action, branch, results) } + +// A local branch with neither a configured upstream nor a same-named branch on +// the remote cannot be fast-forwarded. It used to be dropped from the report +// entirely, so the run claimed "Already up to date" while doing nothing at all. +func TestLocalBranchWithoutUpstreamIsReportedNotSilentlyDropped(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "branch", "stray", "main") + strayBefore := gitText(t, fixture.work, "rev-parse", "stray") + advanceRemoteBranch(t, fixture, "main", "main-remote.txt") + + report, err := syncRepository(fixture.work, time.Minute, false, nil) + if err != nil { + t.Fatalf("syncRepository(): %v\n%s", err, report.Message) + } + assertBranchAction(t, report.Branches, "stray", "untracked") + if report.Message == "Already up to date." { + t.Fatalf("report claimed to be up to date while a branch was left alone: %q", report.Message) + } + // The untracked branch must genuinely be left untouched, even though a + // sibling branch in the same run did move. + if got := gitText(t, fixture.work, "rev-parse", "stray"); got != strayBefore { + t.Fatalf("stray moved: got %s, want %s", got, strayBefore) + } + // A branch it *can* fix is still fixed in the same run. + if got, want := gitText(t, fixture.work, "rev-parse", "main"), gitText(t, fixture.work, "rev-parse", "origin/main"); got != want { + t.Fatalf("inactive main was not updated: got %s, want %s", got, want) + } +} + +func TestRemoteBranchWithoutLocalCounterpartIsCreated(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "push", "-qu", "origin", "main:refs/heads/brand-new") + + report, err := syncRepositoryWith(fixture.work, time.Minute, false, nil, syncOptions{CreateMissing: true}) + if err != nil { + t.Fatalf("syncRepositoryWith(): %v\n%s", err, report.Message) + } + assertBranchAction(t, report.Branches, "brand-new", "created") + if got, want := gitText(t, fixture.work, "rev-parse", "brand-new"), gitText(t, fixture.work, "rev-parse", "origin/main"); got != want { + t.Fatalf("created branch is at the wrong commit: got %s, want %s", got, want) + } + if upstream := gitText(t, fixture.work, "rev-parse", "--abbrev-ref", "brand-new@{upstream}"); upstream != "origin/brand-new" { + t.Fatalf("created branch upstream = %q, want origin/brand-new", upstream) + } + // Creating a branch must never move the branch that is checked out. + if got := gitText(t, fixture.work, "branch", "--show-current"); got != "feature" { + t.Fatalf("checked-out branch = %q, want feature", got) + } +} + +// Like hub, stack-sync does not create remote-only branches unless asked. +func TestRemoteBranchIsNotCreatedByDefault(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "push", "-qu", "origin", "main:refs/heads/brand-new") + + if _, err := syncRepositoryWith(fixture.work, time.Minute, false, nil, syncOptions{CreateMissing: false}); err != nil { + t.Fatalf("syncRepositoryWith(): %v", err) + } + if err := exec.Command("git", "-C", fixture.work, "show-ref", "--verify", "--quiet", "refs/heads/brand-new").Run(); err == nil { + t.Fatal("brand-new was created even though CreateMissing is false") + } + + // The zero value of syncOptions must also mean "do not create". + if _, err := syncRepository(fixture.work, time.Minute, false, nil); err != nil { + t.Fatalf("syncRepository(): %v", err) + } + if err := exec.Command("git", "-C", fixture.work, "show-ref", "--verify", "--quiet", "refs/heads/brand-new").Run(); err == nil { + t.Fatal("brand-new was created by the default syncRepository()") + } +} + +// The flag is opt-in. Guard against the default silently flipping back on, which +// previously happened because the value was resolved before flags were parsed. +func TestCreateMissingFlagIsOptIn(t *testing.T) { + opts, err := parseFlags("sync", nil, io.Discard) + if err != nil { + t.Fatalf("parseFlags(): %v", err) + } + if opts.createMissing { + t.Fatal("--create-missing must default to false") + } + opts, err = parseFlags("sync", []string{"--create-missing"}, io.Discard) + if err != nil { + t.Fatalf("parseFlags(): %v", err) + } + if !opts.createMissing { + t.Fatal("--create-missing did not enable branch creation") + } +} + +// git shortens refs/remotes/origin/HEAD to the shortest unambiguous form, which +// is plain "origin". Treating that as a branch name produced a bogus +// "create branch origin" failure. +func TestRemoteHeadSymrefIsNotListedAsABranch(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "fetch", "-q", "origin") + + branches, err := remoteTrackingBranches(t.Context(), fixture.work, "origin") + if err != nil { + t.Fatalf("remoteTrackingBranches(): %v", err) + } + var names []string + for _, branch := range branches { + names = append(names, branch.name) + if branch.name == "origin" || branch.name == "HEAD" { + t.Fatalf("remote HEAD symref leaked into the branch list as %q: %v", branch.name, names) + } + } + if len(names) == 0 { + t.Fatal("expected the remote's real branches to be listed") + } +} + +// refs/remotes//HEAD is a local convenience symref and is missing from +// plenty of real clones. Without resolving the default branch another way, merged +// branches with a deleted upstream were kept with a vague warning. +func TestRemoteDefaultBranchFallsBackToAskingTheRemote(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "remote", "set-head", "origin", "-d") + if err := exec.Command("git", "-C", fixture.work, "symbolic-ref", "--quiet", "--verify", "refs/remotes/origin/HEAD").Run(); err == nil { + t.Skip("git kept the remote HEAD symref; fallback not exercised") + } + + ref, branch, err := remoteDefaultBranch(t.Context(), fixture.work, "origin") + if err != nil { + t.Fatalf("remoteDefaultBranch(): %v", err) + } + if branch != "main" { + t.Fatalf("default branch = %q, want main", branch) + } + if ref == "" { + t.Fatal("remoteDefaultBranch() returned no ref") + } +} + +func TestMergedBranchIsDeletedWhenRemoteHeadSymrefIsMissing(t *testing.T) { + fixture := makeSyncFixture(t) + git(t, fixture.work, "branch", "old", "main") + git(t, fixture.work, "push", "-qu", "origin", "old") + git(t, fixture.work, "push", "-q", "origin", "--delete", "old") + git(t, fixture.work, "remote", "set-head", "origin", "-d") + + report, err := syncRepository(fixture.work, time.Minute, false, nil) + if err != nil { + t.Fatalf("syncRepository(): %v\n%s", err, report.Message) + } + if err := exec.Command("git", "-C", fixture.work, "show-ref", "--verify", "--quiet", "refs/heads/old").Run(); err == nil { + t.Fatalf("merged branch was kept without origin/HEAD: %s", report.Message) + } + assertBranchAction(t, report.Branches, "old", "deleted") +} diff --git a/tui.go b/tui.go index 1b74803..b77ee48 100644 --- a/tui.go +++ b/tui.go @@ -75,7 +75,52 @@ func runTUI(root string, repos []repo, opts options) error { return err } +// repoGroup is the outcome a repository will have if a run starts right now. +// The selection screen is grouped by it so problems are visible before the user +// commits to anything, rather than only after a run. +type repoGroup int + +const ( + groupBlocked repoGroup = iota + groupDirty + groupReady +) + +func groupOf(r repo) repoGroup { + switch { + case !r.Eligible: + return groupBlocked + case len(r.Dirty) > 0: + return groupDirty + default: + return groupReady + } +} + +func groupLabel(g repoGroup) string { + switch g { + case groupBlocked: + return "BLOCKED · not eligible, fix by hand" + case groupDirty: + return "DIRTY · checked-out branch protected" + default: + return "READY" + } +} + +// sortReposByOutcome puts the repositories that need attention first, then +// groups equal outcomes together in a stable, name-ordered list. +func sortReposByOutcome(repos []repo) { + sort.SliceStable(repos, func(i, j int) bool { + if gi, gj := groupOf(repos[i]), groupOf(repos[j]); gi != gj { + return gi < gj + } + return repos[i].RelativePath < repos[j].RelativePath + }) +} + func newTUIModel(root string, repos []repo, opts options) tuiModel { + sortReposByOutcome(repos) selected := make(map[string]bool) for _, r := range repos { selected[r.Path] = r.Eligible @@ -103,6 +148,7 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { return m, nil } m.repos = msg.repos + sortReposByOutcome(m.repos) m.reconcileSelection() m.status = "Workspace refreshed" m.clampViewport() @@ -136,18 +182,20 @@ func (m tuiModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) { } m.repos = msg.repos m.reconcileSelection() - var ok, failed, skipped int + var ok, unchanged, failed, skipped int for _, result := range msg.results { switch { - case result.Success: - ok++ case result.Skipped: skipped++ - default: + case !result.Success: failed++ + case repositoryChanged(result): + ok++ + default: + unchanged++ } } - m.status = fmt.Sprintf("Finished: %d synced, %d failed, %d safety-skipped", ok, failed, skipped) + m.status = fmt.Sprintf("Finished: %d changed, %d unchanged, %d failed, %d safety-skipped", ok, unchanged, failed, skipped) if path := firstAttentionPath(msg.results); path != "" { m.focusPath(path) m.status += "; focused first issue" @@ -263,6 +311,10 @@ func (m tuiModel) View() string { b.WriteString("\n") for i := visible.start; i < visible.end; i++ { r := m.repos[i] + if i == 0 || groupOf(r) != groupOf(m.repos[i-1]) { + b.WriteString(mutedStyle.Render(fmt.Sprintf(" %s", groupLabel(groupOf(r))))) + b.WriteString("\n") + } pointer := " " if i == m.cursor { pointer = "›" @@ -318,19 +370,50 @@ func (m tuiModel) View() string { type rowRange struct{ start, end int } -func (m tuiModel) visibleRows() rowRange { +// listHeight is how many screen rows the repository list may occupy, leaving +// room for the header, detail pane, and footer. +func (m tuiModel) listHeight() int { reserved := 15 - if m.mode == modeSyncing { - reserved = 20 - } else if len(m.lastResult) > 0 { + if m.mode == modeSyncing || len(m.lastResult) > 0 { reserved = 20 } - available := m.height - reserved - if available < 4 { - available = 4 + return max(4, m.height-reserved) +} + +// groupHeaderRows counts the group headings rendered inside [start,end). A +// heading appears wherever the group changes, so scrolling into a new group +// labels it. +func (m tuiModel) groupHeaderRows(start, end int) int { + rows := 0 + for i := max(0, start); i < end; i++ { + if i == 0 || groupOf(m.repos[i]) != groupOf(m.repos[i-1]) { + rows++ + } } - end := min(len(m.repos), m.offset+available) - return rowRange{start: min(m.offset, end), end: end} + return rows +} + +// windowRows is the number of screen rows the slice occupies, headings included. +func (m tuiModel) windowRows(start, end int) int { + if start >= end { + return 0 + } + return end - start + m.groupHeaderRows(start, end) +} + +func (m tuiModel) visibleRows() rowRange { + available := m.listHeight() + start := min(m.offset, len(m.repos)) + end := start + for end < len(m.repos) && m.windowRows(start, end+1) <= available { + end++ + } + // Reclaiming rows above the offset keeps a short list from scrolling the + // top out of view when the offset no longer reflects a full window. + for start > 0 && m.windowRows(start-1, end) <= available { + start-- + } + return rowRange{start: start, end: end} } func (m *tuiModel) clampViewport() { @@ -339,16 +422,17 @@ func (m *tuiModel) clampViewport() { return } m.cursor = max(0, min(m.cursor, len(m.repos)-1)) - reserved := 15 - if m.mode == modeSyncing || len(m.lastResult) > 0 { - reserved = 20 - } - available := max(4, m.height-reserved) - if m.cursor < m.offset { + if m.offset > m.cursor { m.offset = m.cursor } - if m.cursor >= m.offset+available { - m.offset = m.cursor - available + 1 + // Headings share the budget, so the window can be shorter than the row + // count alone suggests. Step forward until the cursor is on screen; the + // loop always terminates because an offset equal to the cursor renders it. + for m.offset < m.cursor { + if m.cursor < m.visibleRows().end { + break + } + m.offset++ } } @@ -435,7 +519,7 @@ func (m tuiModel) scanCmd() tea.Cmd { func (m tuiModel) startSyncCmd(selected []repo, events chan tea.Msg) tea.Cmd { return func() tea.Msg { go func() { - results := syncSelectedWithProgress(m.root, selected, m.repos, m.opts.jobs, m.opts.timeout, io.Discard, true, false, func(event syncProgressEvent) { + results := syncSelectedWithProgress(m.root, selected, m.repos, m.opts.jobs, m.opts.timeout, io.Discard, true, false, m.opts.createMissing, nil, func(event syncProgressEvent) { if event.Started { events <- syncStartedMsg{path: event.Path} } else { @@ -503,35 +587,54 @@ func (m tuiModel) syncProgressView(width int) string { } func (m tuiModel) lastRunView(width int) string { - var ok, failed, skipped, protected, warnings int + var changed, unchanged, failed, skipped, protected, warnings, untracked, deleted, updated, created int var critical, caution []string for _, result := range m.lastResult { switch { - case result.Success: - ok++ case result.Skipped: skipped++ critical = append(critical, "↷ "+result.Path+": "+firstLine(result.Message)) - default: + case !result.Success: failed++ critical = append(critical, "✗ "+result.Path+": "+firstLine(result.Message)) + case repositoryChanged(result): + changed++ + default: + unchanged++ } for _, branch := range result.Branches { switch branch.Action { + case "updated": + updated++ + case "created": + created++ + case "deleted": + deleted++ + critical = append(critical, "✗ "+result.Path+" / "+branch.Branch+" was deleted: "+branch.Message) case "protected": protected++ caution = append(caution, "◆ "+result.Path+" / "+branch.Branch+": "+branch.Message) case "warning": warnings++ caution = append(caution, "! "+result.Path+" / "+branch.Branch+": "+branch.Message) + case "untracked": + untracked++ + caution = append(caution, "◦ "+result.Path+" / "+branch.Branch+": "+branch.Message) } } } details := append(critical, caution...) - header := titleStyle.Render("LAST RUN") + fmt.Sprintf(" %s %s %s", readyStyle.Render(fmt.Sprintf("%d synced", ok)), blockStyle.Render(fmt.Sprintf("%d failed", failed)), mutedStyle.Render(fmt.Sprintf("%d skipped", skipped))) - if protected+warnings > 0 { - header += titleStyle.Render(fmt.Sprintf(" %d protected · %d warnings", protected, warnings)) + header := titleStyle.Render("LAST RUN") + fmt.Sprintf(" %s %s %s %s", + readyStyle.Render(fmt.Sprintf("%d changed", changed)), + mutedStyle.Render(fmt.Sprintf("%d unchanged", unchanged)), + blockStyle.Render(fmt.Sprintf("%d failed", failed)), + mutedStyle.Render(fmt.Sprintf("%d skipped", skipped))) + if updated+deleted+created > 0 { + header += titleStyle.Render(fmt.Sprintf(" %d updated · %d deleted · %d created", updated, deleted, created)) + } + if protected+warnings+untracked > 0 { + header += titleStyle.Render(fmt.Sprintf(" %d protected · %d warnings · %d untracked", protected, warnings, untracked)) } lines := []string{header} shown := min(3, len(details)) @@ -559,7 +662,7 @@ func resultNeedsAttention(result syncResult) bool { return true } for _, branch := range result.Branches { - if branch.Action == "warning" || branch.Action == "protected" { + if branch.Action == "warning" || branch.Action == "protected" || branch.Action == "untracked" { return true } } @@ -579,7 +682,7 @@ func firstAttentionPath(results []syncResult) string { } for _, result := range results { for _, branch := range result.Branches { - if branch.Action == "warning" || branch.Action == "protected" { + if branch.Action == "warning" || branch.Action == "protected" || branch.Action == "untracked" { return result.Path } } diff --git a/tui_test.go b/tui_test.go index 8e3298c..90d53f7 100644 --- a/tui_test.go +++ b/tui_test.go @@ -1,6 +1,8 @@ package main import ( + "fmt" + "reflect" "strings" "testing" "time" @@ -22,7 +24,7 @@ func TestTUIOnlySelectsEligibleRepositories(t *testing.T) { t.Fatalf("selected repositories = %d, want 2", got) } - m.cursor = 2 + m = focusRepo(t, m, "blocked") updated, _ := m.Update(tea.KeyMsg{Type: tea.KeySpace}) m = updated.(tuiModel) if got := len(m.selectedRepos()); got != 2 { @@ -47,7 +49,8 @@ func TestTUIRequiresExplicitConfirmation(t *testing.T) { func TestTUIViewExplainsBlockedRepository(t *testing.T) { m := newTUIModel("/workspace", testTUIRepos(), options{}) - m.width, m.height, m.cursor = 100, 30, 2 + m.width, m.height = 100, 30 + m = focusRepo(t, m, "blocked") view := m.View() for _, want := range []string{"STACK SYNC", "detached HEAD", "1 blocked"} { if !strings.Contains(view, want) { @@ -56,9 +59,25 @@ func TestTUIViewExplainsBlockedRepository(t *testing.T) { } } +// focusRepo points the cursor at a repository by name. The selection screen is +// grouped by outcome, so list positions are not stable across changes to the +// grouping rules and tests must not depend on them. +func focusRepo(t *testing.T, m tuiModel, name string) tuiModel { + t.Helper() + for i, r := range m.repos { + if r.RelativePath == name { + m.cursor = i + return m + } + } + t.Fatalf("repository %q not found in %+v", name, m.repos) + return m +} + func TestTUIViewExplainsDirtyBranchProtection(t *testing.T) { m := newTUIModel("/workspace", testTUIRepos(), options{}) - m.width, m.height, m.cursor = 100, 30, 1 + m.width, m.height = 100, 30 + m = focusRepo(t, m, "dirty") view := m.View() for _, want := range []string{"ready (dirty)", "CHECKED-OUT BRANCH PROTECTED", "file.txt"} { if !strings.Contains(view, want) { @@ -71,6 +90,7 @@ func TestTUIViewShowsLastSyncOutputForFocusedRepository(t *testing.T) { m := newTUIModel("/workspace", testTUIRepos(), options{}) m.width, m.height = 100, 30 m.lastResult = []syncResult{{Path: "clean", Success: true, Message: "Updated branch main", Duration: "120ms"}} + m = focusRepo(t, m, "clean") view := m.View() for _, want := range []string{"LAST SYNC", "120ms", "Updated branch main"} { if !strings.Contains(view, want) { @@ -100,6 +120,7 @@ func TestTUISyncProgressShowsLiveCountsAndLatestResult(t *testing.T) { t.Fatalf("finish event did not update progress: done=%d ok=%d active=%+v", m.syncDone, m.syncOK, m.syncActive) } view := m.View() + // The live progress view counts finished repositories, not changed ones. for _, want := range []string{"1/3", "1 synced", "0 failed", "Latest", "clean", "up to date"} { if !strings.Contains(view, want) { t.Fatalf("progress view missing %q:\n%s", want, view) @@ -122,7 +143,9 @@ func TestTUICompletionSummarizesAndFocusesFirstIssue(t *testing.T) { t.Fatalf("completion did not focus the first issue: mode=%v cursor=%d status=%q", m.mode, m.cursor, m.status) } view := m.View() - for _, want := range []string{"LAST RUN", "1 synced", "1 failed", "dirty", "authentication failed", "f next"} { + // "clean" only had a protected branch, so under the changed/unchanged + // accounting it is unchanged rather than changed. + for _, want := range []string{"LAST RUN", "0 changed", "1 unchanged", "1 failed", "dirty", "authentication failed", "f next"} { if !strings.Contains(view, want) { t.Fatalf("completion view missing %q:\n%s", want, view) } @@ -135,7 +158,7 @@ func TestTUIFocusNextIssueCyclesThroughAttentionResults(t *testing.T) { {Path: "clean", Success: true, Branches: []branchSyncResult{{Branch: "main", Action: "protected", Message: "checked out elsewhere"}}}, {Path: "dirty", Message: "fetch failed"}, } - m.cursor = 0 + m = focusRepo(t, m, "clean") updated, _ := m.Update(tea.KeyMsg{Type: tea.KeyRunes, Runes: []rune{'f'}}) m = updated.(tuiModel) @@ -154,3 +177,67 @@ func TestTruncateMiddle(t *testing.T) { t.Fatalf("truncateMiddle() = %q", got) } } + +// Problems must be visible on the selection screen without starting a run. +func TestSelectionScreenGroupsRepositoriesByOutcome(t *testing.T) { + repos := []repo{ + {Path: "/w/zeta", RelativePath: "zeta", Branch: "main", Eligible: true}, + {Path: "/w/dirty-b", RelativePath: "dirty-b", Branch: "main", Dirty: []string{" M a"}, Eligible: true}, + {Path: "/w/blocked", RelativePath: "blocked", BlockReason: "detached HEAD"}, + {Path: "/w/alpha", RelativePath: "alpha", Branch: "main", Eligible: true}, + {Path: "/w/dirty-a", RelativePath: "dirty-a", Branch: "main", Dirty: []string{" M b"}, Eligible: true}, + } + m := newTUIModel("/w", repos, options{}) + m.width, m.height = 100, 40 + + var order []string + for _, r := range m.repos { + order = append(order, r.RelativePath) + } + want := []string{"blocked", "dirty-a", "dirty-b", "alpha", "zeta"} + if !reflect.DeepEqual(order, want) { + t.Fatalf("grouped order = %v, want %v", order, want) + } + + view := m.View() + for _, heading := range []string{"BLOCKED ·", "DIRTY ·", "READY"} { + if !strings.Contains(view, heading) { + t.Fatalf("view missing group heading %q:\n%s", heading, view) + } + } + // Headings must precede the repositories they describe. + if strings.Index(view, "DIRTY ·") > strings.Index(view, "dirty-a") { + t.Fatalf("DIRTY heading rendered after its repositories:\n%s", view) + } +} + +// Group headings share the list height, so a tall list must still keep the +// cursor on screen and must not loop or stall. +func TestGroupedViewportKeepsCursorVisible(t *testing.T) { + var repos []repo + for i := range 40 { + relative := fmt.Sprintf("repo-%02d", i) + r := repo{Path: "/w/" + relative, RelativePath: relative, Branch: "main", Eligible: true} + if i%5 == 0 { + r.Dirty = []string{" M file"} + } + repos = append(repos, r) + } + m := newTUIModel("/w", repos, options{}) + m.width, m.height = 100, 24 + + for _, cursor := range []int{0, 1, 7, 19, 39} { + m.cursor = cursor + m.clampViewport() + rows := m.visibleRows() + if m.cursor < rows.start || m.cursor >= rows.end { + t.Fatalf("cursor %d outside window %+v", m.cursor, rows) + } + if got := m.windowRows(rows.start, rows.end); got > m.listHeight() { + t.Fatalf("window uses %d rows, only %d available", got, m.listHeight()) + } + if !strings.Contains(m.View(), m.repos[m.cursor].RelativePath) { + t.Fatalf("cursor %d (%s) not rendered", m.cursor, m.repos[m.cursor].RelativePath) + } + } +}