fix(sync): stop reporting success when no branch was actually synced

hub sync silently dropped any local branch whose upstream it could not resolve: it cleared its internal remote branch while leaving the "gone" flag false, so neither the update nor the delete path ran. A run could then print "Already up to date" having done nothing, which is why a workspace-wide sync looked like it worked when it did not. - report unmatched branches as UNTRACKED with the reason, so "Already up to date" is only said when it is true - resolve the remote default branch via `git ls-remote --symref` when refs/remotes/<remote>/HEAD is missing, so merged branches whose upstream was deleted are still cleaned up - make --create-missing opt-in, keeping hub's default of never creating a local branch that does not already exist - summarise runs as changed/unchanged with per-action counts, and list deletions and failures in full instead of leaving them in scrollback - restore per-command flag help, which the custom Usage had dropped - add an in-place CLI progress line, drawn only to a terminal so piped and --json output stay free of cursor control - group the TUI selection screen by outcome so blocked and dirty repositories are visible before a run starts 🤖 Generated with Codebuff Co-Authored-By: Codebuff <noreply@codebuff.com>
2026-09-28 19:57:42 +01:00
parent 4f6e9c9991
commit f4286edff2
7 changed files with 885 additions and 72 deletions
@@ -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/<remote>/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")
+}