From 8d9bc475f9018d7e22191fd2c257b8f2fbe37b8d Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 10 Mar 2026 15:37:02 -0400 Subject: [PATCH 01/10] Fix git clone failing when default branch is not master When a user pushes only a non-master branch (e.g. main) to a Keybase git repo, cloning fails with "remote HEAD refers to nonexistent ref, unable to checkout" because gogit.Init() hardcodes HEAD to refs/heads/master. Three fixes: - handleList: validate symref targets exist, rewrite HEAD to best available branch (prefer main > master > alphabetical first) - handlePushBatch: after push, update stored HEAD if it points to a nonexistent ref (skipping delete refspecs) - NewBrowser: resolve HEAD dynamically instead of hardcoding master, with fallback to empty browser for stale HEAD Fixes #28943 --- go/kbfs/kbfsgit/runner.go | 78 ++++++++++++++++++++++++++++++++-- go/kbfs/kbfsgit/runner_test.go | 29 +++++++++++++ go/kbfs/libgit/browser.go | 15 ++++++- 3 files changed, 117 insertions(+), 5 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index f9a60c26d9f0..1fe094d81759 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -709,8 +709,16 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { return err } - var symRefs []string + type symRefInfo struct { + name plumbing.ReferenceName + target plumbing.ReferenceName + } + var symRefs []symRefInfo + hashRefNames := make(map[plumbing.ReferenceName]bool) hashesSeen := false + // Track the best fallback branch for HEAD in case its target + // doesn't exist (prefer main > master > alphabetically first). + var bestBranch plumbing.ReferenceName for { ref, err := refs.Next() if errors.Cause(err) == io.EOF { @@ -725,6 +733,18 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { case plumbing.HashReference: value = ref.Hash().String() hashesSeen = true + hashRefNames[ref.Name()] = true + // Track best branch for fallback HEAD. + if strings.HasPrefix(ref.Name().String(), "refs/heads/") { + switch { + case ref.Name() == "refs/heads/main": + bestBranch = ref.Name() + case ref.Name() == "refs/heads/master" && bestBranch != "refs/heads/main": + bestBranch = ref.Name() + case bestBranch == "" || (bestBranch != "refs/heads/main" && bestBranch != "refs/heads/master" && ref.Name() < bestBranch): + bestBranch = ref.Name() + } + } case plumbing.SymbolicReference: value = "@" + ref.Target().String() default: @@ -737,7 +757,10 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { // cloning an empty repo will result in an error because // the HEAD symbolic ref points to a ref that doesn't // exist. - symRefs = append(symRefs, refStr) + symRefs = append(symRefs, symRefInfo{ + name: ref.Name(), + target: ref.Target(), + }) continue } r.log.CDebugf(ctx, "Listing ref %s", refStr) @@ -748,7 +771,23 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { } if hashesSeen && !forPush { - for _, refStr := range symRefs { + for _, sr := range symRefs { + target := sr.target + // If the symref target doesn't exist among hash refs, + // rewrite it to point to the best available branch. + if !hashRefNames[target] { + if bestBranch == "" { + r.log.CDebugf(ctx, + "Skipping symref %s -> %s (no branches available)", + sr.name, target) + continue + } + r.log.CDebugf(ctx, + "Rewriting symref %s from %s to %s", + sr.name, target, bestBranch) + target = bestBranch + } + refStr := "@" + target.String() + " " + sr.name.String() + "\n" r.log.CDebugf(ctx, "Listing symbolic ref %s", refStr) _, err = r.output.Write([]byte(refStr)) if err != nil { @@ -1869,6 +1908,39 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( } } + // If HEAD points to a nonexistent ref, update it to point to the + // first successfully-pushed branch. + head, headErr := repo.Storer.Reference(plumbing.HEAD) + if headErr == nil && head.Type() == plumbing.SymbolicReference { + _, targetErr := repo.Storer.Reference(head.Target()) + if targetErr == plumbing.ErrReferenceNotFound { + // Find the first successfully-pushed branch ref. + for refspec := range refspecs { + if refspec.IsDelete() { + continue // don't point HEAD at a deleted branch + } + dst := refspec.Dst("") + if results[dst.String()] != nil { + continue // errored + } + if strings.HasPrefix(dst.String(), "refs/heads/") { + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, dst) + if setErr := repo.Storer.SetReference( + newHead); setErr != nil { + r.log.CDebugf(ctx, + "Error updating HEAD to %s: %+v", + dst, setErr) + } else { + r.log.CDebugf(ctx, + "Updated HEAD to point to %s", dst) + } + break + } + } + } + } + err = r.checkGC(ctx) if err != nil { return nil, err diff --git a/go/kbfs/kbfsgit/runner_test.go b/go/kbfs/kbfsgit/runner_test.go index 982994338f1c..5e06f59c6cf0 100644 --- a/go/kbfs/kbfsgit/runner_test.go +++ b/go/kbfs/kbfsgit/runner_test.go @@ -1289,3 +1289,32 @@ func TestRunnerLFS(t *testing.T) { require.NoError(t, err) require.Equal(t, lfsData, buf) } + +// Test that when only a non-master branch (e.g. "main") is pushed, +// HEAD correctly points to that branch instead of the nonexistent +// "refs/heads/master". +func TestRunnerListNonMasterDefault(t *testing.T) { + ctx, config, tempdir := initConfigForRunner(t) + defer func() { _ = os.RemoveAll(tempdir) }() + defer libkbfs.CheckConfigAndShutdown(ctx, t, config) + + gitDir, err := os.MkdirTemp(os.TempDir(), "kbfsgittest") + require.NoError(t, err) + defer func() { _ = os.RemoveAll(gitDir) }() + + makeLocalRepoWithOneFile(t, gitDir, "foo", "hello", "main") + + h, err := tlfhandle.ParseHandle( + ctx, config.KBPKI(), config.MDOps(), config, "user1", tlf.Private) + require.NoError(t, err) + _, err = libgit.CreateRepoAndID(ctx, config, h, "test") + require.NoError(t, err) + + testPush(ctx, t, config, gitDir, + "refs/heads/main:refs/heads/main") + + // List refs and verify HEAD points to refs/heads/main. + heads := testListAndGetHeads(ctx, t, config, gitDir, + []string{"refs/heads/main", "HEAD"}) + require.Equal(t, "@refs/heads/main", heads[1]) +} diff --git a/go/kbfs/libgit/browser.go b/go/kbfs/libgit/browser.go index 8f4c7dd7160c..867561485fd8 100644 --- a/go/kbfs/libgit/browser.go +++ b/go/kbfs/libgit/browser.go @@ -73,6 +73,7 @@ func NewBrowser( } const masterBranch = "refs/heads/master" + branchWasEmpty := gitBranchName == "" if gitBranchName == "" { gitBranchName = masterBranch } else if !strings.HasPrefix(string(gitBranchName), "refs/") { @@ -98,9 +99,19 @@ func NewBrowser( return nil, err } + // If no branch was specified, try to resolve HEAD to find the + // default branch instead of hardcoding master. + if branchWasEmpty { + headRef, headErr := repo.Reference(plumbing.HEAD, false) + if headErr == nil && headRef.Type() == plumbing.SymbolicReference { + gitBranchName = headRef.Target() + } + } + ref, err := repo.Reference(gitBranchName, true) - if err == plumbing.ErrReferenceNotFound && gitBranchName == masterBranch { - // This branch has no commits, so pretend it's empty. + if err == plumbing.ErrReferenceNotFound && (gitBranchName == masterBranch || branchWasEmpty) { + // This branch has no commits (or HEAD points to a + // nonexistent ref), so pretend it's empty. return &Browser{ root: string(gitBranchName), sharedCache: sharedCache, From 5c78925410c4be983468c7d713cd8d37b3669cc5 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 10 Mar 2026 16:16:32 -0400 Subject: [PATCH 02/10] Fix PR #29004 feedback: deterministic HEAD repair, journal flush, and test - Use main > master > alphabetical preference for HEAD repair branch selection instead of nondeterministic map iteration order - Add waitForJournal after HEAD repair so the update is flushed - Add TestBrowserHeadResolution covering non-master HEAD and stale HEAD --- go/kbfs/kbfsgit/runner.go | 52 ++++++++++++++++++--------- go/kbfs/libgit/browser_test.go | 66 ++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 16 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index 1fe094d81759..0659347c14b1 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -1909,38 +1909,58 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( } // If HEAD points to a nonexistent ref, update it to point to the - // first successfully-pushed branch. + // best successfully-pushed branch (prefer main > master > alphabetical). + headUpdated := false head, headErr := repo.Storer.Reference(plumbing.HEAD) if headErr == nil && head.Type() == plumbing.SymbolicReference { _, targetErr := repo.Storer.Reference(head.Target()) if targetErr == plumbing.ErrReferenceNotFound { - // Find the first successfully-pushed branch ref. + var bestBranch plumbing.ReferenceName for refspec := range refspecs { if refspec.IsDelete() { - continue // don't point HEAD at a deleted branch + continue } dst := refspec.Dst("") if results[dst.String()] != nil { continue // errored } - if strings.HasPrefix(dst.String(), "refs/heads/") { - newHead := plumbing.NewSymbolicReference( - plumbing.HEAD, dst) - if setErr := repo.Storer.SetReference( - newHead); setErr != nil { - r.log.CDebugf(ctx, - "Error updating HEAD to %s: %+v", - dst, setErr) - } else { - r.log.CDebugf(ctx, - "Updated HEAD to point to %s", dst) - } - break + if !strings.HasPrefix(dst.String(), "refs/heads/") { + continue + } + switch { + case dst == "refs/heads/main": + bestBranch = dst + case dst == "refs/heads/master" && bestBranch != "refs/heads/main": + bestBranch = dst + case bestBranch == "" || (bestBranch != "refs/heads/main" && bestBranch != "refs/heads/master" && dst < bestBranch): + bestBranch = dst + } + } + if bestBranch != "" { + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, bestBranch) + if setErr := repo.Storer.SetReference( + newHead); setErr != nil { + r.log.CDebugf(ctx, + "Error updating HEAD to %s: %+v", + bestBranch, setErr) + } else { + r.log.CDebugf(ctx, + "Updated HEAD to point to %s", bestBranch) + headUpdated = true } } } } + if headUpdated { + if journalErr := r.waitForJournal(ctx); journalErr != nil { + r.log.CDebugf(ctx, + "Error flushing journal after HEAD update: %+v", + journalErr) + } + } + err = r.checkGC(ctx) if err != nil { return nil, err diff --git a/go/kbfs/libgit/browser_test.go b/go/kbfs/libgit/browser_test.go index 480d9abd8841..5c2120f1333d 100644 --- a/go/kbfs/libgit/browser_test.go +++ b/go/kbfs/libgit/browser_test.go @@ -19,6 +19,7 @@ import ( "github.com/keybase/client/go/protocol/keybase1" "github.com/stretchr/testify/require" gogit "gopkg.in/src-d/go-git.v4" + "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/object" ) @@ -176,3 +177,68 @@ func TestBrowserWithCache(t *testing.T) { require.NoError(t, err) testBrowser(t, cache) } + +func TestBrowserHeadResolution(t *testing.T) { + ctx, config, cancel, tempdir := initConfigForAutogit(t) + defer cancel() + defer func() { _ = os.RemoveAll(tempdir) }() + defer libkbfs.CheckConfigAndShutdown(ctx, t, config) + + h, err := tlfhandle.ParseHandle( + ctx, config.KBPKI(), config.MDOps(), nil, "user1", tlf.Private) + require.NoError(t, err) + rootFS, err := libfs.NewFS( + ctx, config, h, data.MasterBranch, "", "", keybase1.MDPriorityNormal) + require.NoError(t, err) + + t.Log("Init a new repo directly into KBFS.") + dotgitFS, _, err := GetOrCreateRepoAndID(ctx, config, h, "test-head", "") + require.NoError(t, err) + + err = rootFS.MkdirAll("worktree-head", 0o600) + require.NoError(t, err) + worktreeFS, err := rootFS.Chroot("worktree-head") + require.NoError(t, err) + dotgitStorage, err := NewGitConfigWithoutRemotesStorer(dotgitFS) + require.NoError(t, err) + repo, err := gogit.Init(dotgitStorage, worktreeFS) + require.NoError(t, err) + + t.Log("Set HEAD to point to refs/heads/main instead of master.") + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, "refs/heads/main") + err = repo.Storer.SetReference(newHead) + require.NoError(t, err) + + t.Log("Commit a file — go-git creates the main branch since HEAD points there.") + addFileToWorktreeAndCommit( + ctx, t, config, h, repo, worktreeFS, "hello.txt", "world") + + t.Log("NewBrowser with empty branch should resolve HEAD to main.") + b, err := NewBrowser(dotgitFS, config.Clock(), "", noopSharedInBrowserCache{}) + require.NoError(t, err) + require.NotNil(t, b.tree, "browser should have a non-nil tree") + fi, err := b.Stat("hello.txt") + require.NoError(t, err) + require.Equal(t, "hello.txt", fi.Name()) + + f, err := b.Open("hello.txt") + require.NoError(t, err) + defer func() { _ = f.Close() }() + fileData, err := io.ReadAll(f) + require.NoError(t, err) + require.Equal(t, "world", string(fileData)) + + t.Log("Test stale HEAD: point HEAD to a nonexistent ref.") + staleHead := plumbing.NewSymbolicReference( + plumbing.HEAD, "refs/heads/nonexistent") + err = repo.Storer.SetReference(staleHead) + require.NoError(t, err) + + t.Log("NewBrowser should return an empty browser without error.") + b, err = NewBrowser(dotgitFS, config.Clock(), "", noopSharedInBrowserCache{}) + require.NoError(t, err) + fis, err := b.ReadDir("") + require.NoError(t, err) + require.Len(t, fis, 0) +} From 79ee8dbe8549e6fffaa7cc3c91710d55fa98a37c Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 10 Mar 2026 19:52:06 -0400 Subject: [PATCH 03/10] Address PR #29004 feedback: extract helper, fix flush ordering, and harden test - Extract bestBranchFromCandidates helper to deduplicate HEAD selection logic - Move HEAD fixup before waitForJournal so the update is included in the flush - Iterate args slice instead of refspecs map for deterministic branch selection - Propagate journal flush error instead of swallowing it - Use git checkout -B (idempotent) to avoid flakiness with init.defaultBranch - Add assertion verifying the underlying KBFS repo HEAD symref is updated --- go/kbfs/kbfsgit/runner.go | 118 ++++++++++++++++----------------- go/kbfs/kbfsgit/runner_test.go | 13 +++- 2 files changed, 69 insertions(+), 62 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index 0659347c14b1..3aecf8b1fcce 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -687,6 +687,21 @@ func (r *runner) waitForJournal(ctx context.Context) error { // Lists the refs, one per line, in the format " [ // …​]". The value may be a hex sha1 hash, "@" for a symref, or // "?" to indicate that the helper could not get the value of the +// bestBranchFromCandidates selects the best branch for HEAD from a set of +// candidate branch names (prefer main > master > alphabetically first). +func bestBranchFromCandidates(best plumbing.ReferenceName, candidate plumbing.ReferenceName) plumbing.ReferenceName { + switch { + case candidate == "refs/heads/main": + return candidate + case candidate == "refs/heads/master" && best != "refs/heads/main": + return candidate + case best == "" || (best != "refs/heads/main" && best != "refs/heads/master" && candidate < best): + return candidate + default: + return best + } +} + // ref. A space-separated list of attributes follows the name; // unrecognized attributes are ignored. The list ends with a blank // line. @@ -736,14 +751,7 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { hashRefNames[ref.Name()] = true // Track best branch for fallback HEAD. if strings.HasPrefix(ref.Name().String(), "refs/heads/") { - switch { - case ref.Name() == "refs/heads/main": - bestBranch = ref.Name() - case ref.Name() == "refs/heads/master" && bestBranch != "refs/heads/main": - bestBranch = ref.Name() - case bestBranch == "" || (bestBranch != "refs/heads/main" && bestBranch != "refs/heads/master" && ref.Name() < bestBranch): - bestBranch = ref.Name() - } + bestBranch = bestBranchFromCandidates(bestBranch, ref.Name()) } case plumbing.SymbolicReference: value = "@" + ref.Target().String() @@ -1869,6 +1877,47 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( return nil, err } + // If HEAD points to a nonexistent ref, update it to point to the + // best successfully-pushed branch (prefer main > master > alphabetical). + // This must happen before waitForJournal so the HEAD update is + // included in the same flush. + head, headErr := repo.Storer.Reference(plumbing.HEAD) + if headErr == nil && head.Type() == plumbing.SymbolicReference { + _, targetErr := repo.Storer.Reference(head.Target()) + if targetErr == plumbing.ErrReferenceNotFound { + var bestBranch plumbing.ReferenceName + // Iterate over args (not the refspecs map) for + // deterministic branch selection. + for _, push := range args { + refspec := gogitcfg.RefSpec(push[0]) + if refspec.IsDelete() { + continue + } + dst := refspec.Dst("") + if results[dst.String()] != nil { + continue // errored + } + if !strings.HasPrefix(dst.String(), "refs/heads/") { + continue + } + bestBranch = bestBranchFromCandidates(bestBranch, dst) + } + if bestBranch != "" { + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, bestBranch) + if setErr := repo.Storer.SetReference( + newHead); setErr != nil { + r.log.CDebugf(ctx, + "Error updating HEAD to %s: %+v", + bestBranch, setErr) + } else { + r.log.CDebugf(ctx, + "Updated HEAD to point to %s", bestBranch) + } + } + } + } + err = r.waitForJournal(ctx) if err != nil { return nil, err @@ -1908,59 +1957,6 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( } } - // If HEAD points to a nonexistent ref, update it to point to the - // best successfully-pushed branch (prefer main > master > alphabetical). - headUpdated := false - head, headErr := repo.Storer.Reference(plumbing.HEAD) - if headErr == nil && head.Type() == plumbing.SymbolicReference { - _, targetErr := repo.Storer.Reference(head.Target()) - if targetErr == plumbing.ErrReferenceNotFound { - var bestBranch plumbing.ReferenceName - for refspec := range refspecs { - if refspec.IsDelete() { - continue - } - dst := refspec.Dst("") - if results[dst.String()] != nil { - continue // errored - } - if !strings.HasPrefix(dst.String(), "refs/heads/") { - continue - } - switch { - case dst == "refs/heads/main": - bestBranch = dst - case dst == "refs/heads/master" && bestBranch != "refs/heads/main": - bestBranch = dst - case bestBranch == "" || (bestBranch != "refs/heads/main" && bestBranch != "refs/heads/master" && dst < bestBranch): - bestBranch = dst - } - } - if bestBranch != "" { - newHead := plumbing.NewSymbolicReference( - plumbing.HEAD, bestBranch) - if setErr := repo.Storer.SetReference( - newHead); setErr != nil { - r.log.CDebugf(ctx, - "Error updating HEAD to %s: %+v", - bestBranch, setErr) - } else { - r.log.CDebugf(ctx, - "Updated HEAD to point to %s", bestBranch) - headUpdated = true - } - } - } - } - - if headUpdated { - if journalErr := r.waitForJournal(ctx); journalErr != nil { - r.log.CDebugf(ctx, - "Error flushing journal after HEAD update: %+v", - journalErr) - } - } - err = r.checkGC(ctx) if err != nil { return nil, err diff --git a/go/kbfs/kbfsgit/runner_test.go b/go/kbfs/kbfsgit/runner_test.go index 5e06f59c6cf0..3648bb44dbb2 100644 --- a/go/kbfs/kbfsgit/runner_test.go +++ b/go/kbfs/kbfsgit/runner_test.go @@ -27,6 +27,7 @@ import ( "github.com/keybase/client/go/protocol/keybase1" "github.com/stretchr/testify/require" gogitcfg "gopkg.in/src-d/go-git.v4/config" + "gopkg.in/src-d/go-git.v4/plumbing" ) type testErrput struct { @@ -156,7 +157,7 @@ func makeLocalRepoWithOneFileCustomCommitMsg(t *testing.T, gitExec(t, dotgit, gitDir, "init") if branch != "" { - gitExec(t, dotgit, gitDir, "checkout", "-b", branch) + gitExec(t, dotgit, gitDir, "checkout", "-B", branch) } gitExec(t, dotgit, gitDir, "add", filename) @@ -1313,6 +1314,16 @@ func TestRunnerListNonMasterDefault(t *testing.T) { testPush(ctx, t, config, gitDir, "refs/heads/main:refs/heads/main") + // Verify the underlying KBFS repo HEAD symref was actually updated. + fs, _, err := libgit.GetRepoAndID(ctx, config, h, "test", "") + require.NoError(t, err) + storage, err := libgit.NewGitConfigWithoutRemotesStorer(fs) + require.NoError(t, err) + headRef, err := storage.Reference(plumbing.HEAD) + require.NoError(t, err) + require.Equal(t, plumbing.SymbolicReference, headRef.Type()) + require.Equal(t, plumbing.ReferenceName("refs/heads/main"), headRef.Target()) + // List refs and verify HEAD points to refs/heads/main. heads := testListAndGetHeads(ctx, t, config, gitDir, []string{"refs/heads/main", "HEAD"}) From 36555365ca5c780d38a798772168ee91c832922f Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 10 Mar 2026 19:58:10 -0400 Subject: [PATCH 04/10] Use all repo branches (not just pushed refs) for HEAD fixup in handlePushBatch The HEAD fixup was only considering refs from the current push batch when selecting the best branch. This could pick a suboptimal branch if better candidates (e.g. main) already existed in the repo. Now enumerates all repo branches, consistent with how handleList selects the best HEAD target. --- go/kbfs/kbfsgit/runner.go | 33 ++++++++++++++++++--------------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index 3aecf8b1fcce..cbb570e881cc 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -1878,7 +1878,7 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( } // If HEAD points to a nonexistent ref, update it to point to the - // best successfully-pushed branch (prefer main > master > alphabetical). + // best available branch in the repo (prefer main > master > alphabetical). // This must happen before waitForJournal so the HEAD update is // included in the same flush. head, headErr := repo.Storer.Reference(plumbing.HEAD) @@ -1886,21 +1886,24 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( _, targetErr := repo.Storer.Reference(head.Target()) if targetErr == plumbing.ErrReferenceNotFound { var bestBranch plumbing.ReferenceName - // Iterate over args (not the refspecs map) for - // deterministic branch selection. - for _, push := range args { - refspec := gogitcfg.RefSpec(push[0]) - if refspec.IsDelete() { - continue - } - dst := refspec.Dst("") - if results[dst.String()] != nil { - continue // errored - } - if !strings.HasPrefix(dst.String(), "refs/heads/") { - continue + allRefs, refsErr := repo.References() + if refsErr == nil { + for { + ref, nextErr := allRefs.Next() + if errors.Cause(nextErr) == io.EOF { + break + } + if nextErr != nil { + break + } + if ref.Type() != plumbing.HashReference { + continue + } + if !strings.HasPrefix(ref.Name().String(), "refs/heads/") { + continue + } + bestBranch = bestBranchFromCandidates(bestBranch, ref.Name()) } - bestBranch = bestBranchFromCandidates(bestBranch, dst) } if bestBranch != "" { newHead := plumbing.NewSymbolicReference( From 2ef1c19334815f73afae45d77308da3b026738e3 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Tue, 10 Mar 2026 21:57:25 -0400 Subject: [PATCH 05/10] Fix doc comment placement, close reference iterators - Move bestBranchFromCandidates above handleList so its doc comment is not split - Add defer refs.Close() in handleList - Add defer allRefs.Close() in handlePushBatch HEAD fixup --- go/kbfs/kbfsgit/runner.go | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index cbb570e881cc..53875f598423 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -682,11 +682,6 @@ func (r *runner) waitForJournal(ctx context.Context) error { r.printStageEndIfNeeded) } -// handleList: From https://git-scm.com/docs/git-remote-helpers -// -// Lists the refs, one per line, in the format " [ -// …​]". The value may be a hex sha1 hash, "@" for a symref, or -// "?" to indicate that the helper could not get the value of the // bestBranchFromCandidates selects the best branch for HEAD from a set of // candidate branch names (prefer main > master > alphabetically first). func bestBranchFromCandidates(best plumbing.ReferenceName, candidate plumbing.ReferenceName) plumbing.ReferenceName { @@ -702,6 +697,11 @@ func bestBranchFromCandidates(best plumbing.ReferenceName, candidate plumbing.Re } } +// handleList: From https://git-scm.com/docs/git-remote-helpers +// +// Lists the refs, one per line, in the format " [ +// …​]". The value may be a hex sha1 hash, "@" for a symref, or +// "?" to indicate that the helper could not get the value of the // ref. A space-separated list of attributes follows the name; // unrecognized attributes are ignored. The list ends with a blank // line. @@ -723,6 +723,7 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { if err != nil { return err } + defer refs.Close() type symRefInfo struct { name plumbing.ReferenceName @@ -1888,6 +1889,7 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( var bestBranch plumbing.ReferenceName allRefs, refsErr := repo.References() if refsErr == nil { + defer allRefs.Close() for { ref, nextErr := allRefs.Next() if errors.Cause(nextErr) == io.EOF { From 99d9973d01753a1009c5c75407d235ef4b8ccee4 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 11 Mar 2026 08:53:46 -0400 Subject: [PATCH 06/10] Scope symref rewrite to HEAD only, update NewBrowser doc comment --- go/kbfs/kbfsgit/runner.go | 17 +++++++++++++---- go/kbfs/libgit/browser.go | 9 +++++---- 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index 53875f598423..7ea265090c95 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -783,17 +783,24 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { for _, sr := range symRefs { target := sr.target // If the symref target doesn't exist among hash refs, - // rewrite it to point to the best available branch. + // rewrite it to point to the best available branch, + // but only for HEAD. Other symrefs are left as-is. if !hashRefNames[target] { + if sr.name != plumbing.HEAD { + r.log.CDebugf(ctx, + "Skipping non-HEAD symref %s -> %s (unknown target)", + sr.name, target) + continue + } if bestBranch == "" { r.log.CDebugf(ctx, - "Skipping symref %s -> %s (no branches available)", + "Skipping HEAD symref %s -> %s (no branches available)", sr.name, target) continue } r.log.CDebugf(ctx, - "Rewriting symref %s from %s to %s", - sr.name, target, bestBranch) + "Rewriting HEAD symref from %s to %s", + target, bestBranch) target = bestBranch } refStr := "@" + target.String() + " " + sr.name.String() + "\n" @@ -1880,6 +1887,8 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( // If HEAD points to a nonexistent ref, update it to point to the // best available branch in the repo (prefer main > master > alphabetical). + // We intentionally repair HEAD even when the target was explicitly + // deleted in this batch, because a broken HEAD causes clone failures. // This must happen before waitForJournal so the HEAD update is // included in the same flush. head, headErr := repo.Storer.Reference(plumbing.HEAD) diff --git a/go/kbfs/libgit/browser.go b/go/kbfs/libgit/browser.go index 867561485fd8..0cc0f1a83fe6 100644 --- a/go/kbfs/libgit/browser.go +++ b/go/kbfs/libgit/browser.go @@ -57,10 +57,11 @@ type Browser struct { var _ billy.Filesystem = (*Browser)(nil) // NewBrowser makes a new Browser instance, browsing the given branch -// of the given repo. If `gitBranchName` is empty, -// "refs/heads/master" is used. If `gitBranchName` is not empty, but -// it doesn't begin with "refs/", then "refs/heads/" is prepended to -// it. +// of the given repo. If `gitBranchName` is empty, HEAD is resolved +// to determine the default branch; if HEAD is missing or points to a +// nonexistent ref, the repo is treated as empty. If `gitBranchName` +// is not empty but doesn't begin with "refs/", then "refs/heads/" is +// prepended to it. func NewBrowser( repoFS *libfs.FS, clock libkbfs.Clock, gitBranchName plumbing.ReferenceName, From 86719f8f0c77d3b961d7d3dcb9b36acd7cbe6c58 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 11 Mar 2026 09:46:44 -0400 Subject: [PATCH 07/10] Fix NewBrowser doc comment to match fallback behavior --- go/kbfs/libgit/browser.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/go/kbfs/libgit/browser.go b/go/kbfs/libgit/browser.go index 0cc0f1a83fe6..b92e19838a2d 100644 --- a/go/kbfs/libgit/browser.go +++ b/go/kbfs/libgit/browser.go @@ -59,7 +59,8 @@ var _ billy.Filesystem = (*Browser)(nil) // NewBrowser makes a new Browser instance, browsing the given branch // of the given repo. If `gitBranchName` is empty, HEAD is resolved // to determine the default branch; if HEAD is missing or points to a -// nonexistent ref, the repo is treated as empty. If `gitBranchName` +// nonexistent ref, NewBrowser falls back to "refs/heads/master" if it +// exists, otherwise the repo is treated as empty. If `gitBranchName` // is not empty but doesn't begin with "refs/", then "refs/heads/" is // prepended to it. func NewBrowser( From d3eb4166f5478e87d48869385bee08152e53affe Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 11 Mar 2026 11:04:58 -0400 Subject: [PATCH 08/10] Fall back to master when HEAD points to a stale ref in NewBrowser --- go/kbfs/libgit/browser.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/go/kbfs/libgit/browser.go b/go/kbfs/libgit/browser.go index b92e19838a2d..4b3b9fb5bf22 100644 --- a/go/kbfs/libgit/browser.go +++ b/go/kbfs/libgit/browser.go @@ -111,9 +111,13 @@ func NewBrowser( } ref, err := repo.Reference(gitBranchName, true) + if err == plumbing.ErrReferenceNotFound && branchWasEmpty && gitBranchName != masterBranch { + // HEAD points to a nonexistent ref; fall back to master. + gitBranchName = masterBranch + ref, err = repo.Reference(gitBranchName, true) + } if err == plumbing.ErrReferenceNotFound && (gitBranchName == masterBranch || branchWasEmpty) { - // This branch has no commits (or HEAD points to a - // nonexistent ref), so pretend it's empty. + // No commits on this branch, so pretend it's empty. return &Browser{ root: string(gitBranchName), sharedCache: sharedCache, From 6f10d3dc698f34a2441f8a65ffc8bec071c55e14 Mon Sep 17 00:00:00 2001 From: chrisnojima Date: Wed, 11 Mar 2026 12:07:22 -0400 Subject: [PATCH 09/10] Emit non-HEAD symrefs unchanged when their targets are unknown --- go/kbfs/kbfsgit/runner.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index 7ea265090c95..a364189feb23 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -784,24 +784,24 @@ func (r *runner) handleList(ctx context.Context, args []string) (err error) { target := sr.target // If the symref target doesn't exist among hash refs, // rewrite it to point to the best available branch, - // but only for HEAD. Other symrefs are left as-is. + // but only for HEAD. Other symrefs are emitted as-is. if !hashRefNames[target] { - if sr.name != plumbing.HEAD { + if sr.name == plumbing.HEAD { + if bestBranch == "" { + r.log.CDebugf(ctx, + "Skipping HEAD symref %s -> %s (no branches available)", + sr.name, target) + continue + } r.log.CDebugf(ctx, - "Skipping non-HEAD symref %s -> %s (unknown target)", - sr.name, target) - continue - } - if bestBranch == "" { + "Rewriting HEAD symref from %s to %s", + target, bestBranch) + target = bestBranch + } else { r.log.CDebugf(ctx, - "Skipping HEAD symref %s -> %s (no branches available)", + "Emitting non-HEAD symref %s -> %s with unknown target", sr.name, target) - continue } - r.log.CDebugf(ctx, - "Rewriting HEAD symref from %s to %s", - target, bestBranch) - target = bestBranch } refStr := "@" + target.String() + " " + sr.name.String() + "\n" r.log.CDebugf(ctx, "Listing symbolic ref %s", refStr) From 508ab13a03b14bc1999145c8880215d7e36a6579 Mon Sep 17 00:00:00 2001 From: Chris Nojima Date: Mon, 16 Mar 2026 14:02:26 -0400 Subject: [PATCH 10/10] Move bestBranch declaration before error check in handlePushBatch --- go/kbfs/kbfsgit/runner.go | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/go/kbfs/kbfsgit/runner.go b/go/kbfs/kbfsgit/runner.go index a364189feb23..c4551957f393 100644 --- a/go/kbfs/kbfsgit/runner.go +++ b/go/kbfs/kbfsgit/runner.go @@ -1894,8 +1894,8 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( head, headErr := repo.Storer.Reference(plumbing.HEAD) if headErr == nil && head.Type() == plumbing.SymbolicReference { _, targetErr := repo.Storer.Reference(head.Target()) + var bestBranch plumbing.ReferenceName if targetErr == plumbing.ErrReferenceNotFound { - var bestBranch plumbing.ReferenceName allRefs, refsErr := repo.References() if refsErr == nil { defer allRefs.Close() @@ -1916,18 +1916,18 @@ func (r *runner) handlePushBatch(ctx context.Context, args [][]string) ( bestBranch = bestBranchFromCandidates(bestBranch, ref.Name()) } } - if bestBranch != "" { - newHead := plumbing.NewSymbolicReference( - plumbing.HEAD, bestBranch) - if setErr := repo.Storer.SetReference( - newHead); setErr != nil { - r.log.CDebugf(ctx, - "Error updating HEAD to %s: %+v", - bestBranch, setErr) - } else { - r.log.CDebugf(ctx, - "Updated HEAD to point to %s", bestBranch) - } + } + if bestBranch != "" { + newHead := plumbing.NewSymbolicReference( + plumbing.HEAD, bestBranch) + if setErr := repo.Storer.SetReference( + newHead); setErr != nil { + r.log.CDebugf(ctx, + "Error updating HEAD to %s: %+v", + bestBranch, setErr) + } else { + r.log.CDebugf(ctx, + "Updated HEAD to point to %s", bestBranch) } } }