From a488d3c752bcc2592f88c9d6aea8b21fdc11150d Mon Sep 17 00:00:00 2001 From: Xe Iaso Date: Sat, 30 May 2026 16:33:03 -0400 Subject: [PATCH] fix(protocol): heal dangling HEAD on load and after push MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Repositories initialized with HEAD -> refs/heads/main (the project default) but populated by pushing a repo with a different default branch (golang/go uses master) end up with HEAD pointing at a nonexistent branch. Git clients cannot check out a worktree and warn: "remote HEAD refers to nonexistent ref, unable to checkout". Fix: repoint HEAD at an existing branch when its symbolic target is missing. Prefer refs/heads/main, then master, then trunk; fall back to the lexicographically smallest branch. Heal is idempotent — a detached HEAD, already-valid HEAD, or branch-less repo is left untouched. Heal on every repository load (fixes repos already in the bucket with no re-push) and after successful receive-pack (fixes new pushes immediately). Both call sites are now guarded by ensureHEAD(), which is no-op when HEAD is already valid. Fixes: #99 (clone checkout abort with dangling HEAD warning) Assisted-by: Claude Opus 4.8 via claude.ai/code --- cmd/objgitd/git_protocol.go | 92 +++++++++++++- cmd/objgitd/head_test.go | 233 ++++++++++++++++++++++++++++++++++++ cmd/objgitd/hooks.go | 22 +++- cmd/objgitd/http.go | 3 +- cmd/objgitd/ssh.go | 5 +- 5 files changed, 345 insertions(+), 10 deletions(-) create mode 100644 cmd/objgitd/head_test.go diff --git a/cmd/objgitd/git_protocol.go b/cmd/objgitd/git_protocol.go index fb5aa71..6ca558b 100644 --- a/cmd/objgitd/git_protocol.go +++ b/cmd/objgitd/git_protocol.go @@ -144,7 +144,7 @@ func (d *daemon) handle(ctx context.Context, conn net.Conn) error { func (d *daemon) serveGit(ctx context.Context, conn net.Conn, r io.ReadCloser, req packp.GitProtoRequest, gitProtocol string) error { switch req.RequestCommand { case transport.UploadPackService: - st, err := d.loader.Load(&url.URL{Path: req.Pathname}) + st, err := d.load(req.Pathname) if err != nil { _, _ = pktline.WriteError(conn, fmt.Errorf("repository %q not found", req.Pathname)) return fmt.Errorf("loading %q: %w", req.Pathname, err) @@ -154,7 +154,7 @@ func (d *daemon) serveGit(ctx context.Context, conn net.Conn, r io.ReadCloser, r }) case transport.UploadArchiveService: - st, err := d.loader.Load(&url.URL{Path: req.Pathname}) + st, err := d.load(req.Pathname) if err != nil { _, _ = pktline.WriteError(conn, fmt.Errorf("repository %q not found", req.Pathname)) return fmt.Errorf("loading %q: %w", req.Pathname, err) @@ -177,11 +177,97 @@ func (d *daemon) serveGit(ctx context.Context, conn net.Conn, r io.ReadCloser, r } } +// load opens the storer for repoPath and heals a dangling HEAD before returning +// it (see ensureHEAD). It preserves the loader's error verbatim — notably +// transport.ErrRepositoryNotFound, which callers map to a 404 — and treats a +// heal failure as non-fatal so a clone is never broken by a transient HEAD write. +func (d *daemon) load(repoPath string) (storage.Storer, error) { + st, err := d.loader.Load(&url.URL{Path: repoPath}) + if err != nil { + return nil, err + } + if err := ensureHEAD(st); err != nil { + slog.Warn("could not repoint dangling HEAD", "path", repoPath, "err", err) + } + return st, nil +} + +// ensureHEAD repoints a repository's HEAD at an existing branch when its symbolic +// target is missing. objgitd initializes every repo with HEAD -> refs/heads/main +// (loadOrInit), but a repo populated by pushing a project whose default branch +// differs — golang/go uses master — leaves HEAD dangling: clients fetch every +// object yet cannot check out a worktree ("remote HEAD refers to nonexistent +// ref"). Git hosts repoint HEAD on push; we heal idempotently on every load and +// after each push, so repos already in the bucket recover on their next clone +// without a re-push. A detached or already-valid HEAD, or a repo with no branches +// yet, is left untouched. +func ensureHEAD(st storage.Storer) error { + head, err := st.Reference(plumbing.HEAD) + if err != nil { + return err + } + if head.Type() != plumbing.SymbolicReference { + return nil // detached HEAD: nothing to repoint + } + if _, err := st.Reference(head.Target()); err == nil { + return nil // target exists: HEAD is already valid + } else if !errors.Is(err, plumbing.ErrReferenceNotFound) { + return err + } + target, err := pickDefaultBranch(st) + if err != nil || target == "" { + return err // no branches yet (target == ""): leave HEAD as-is + } + return st.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, target)) +} + +// pickDefaultBranch chooses a branch for HEAD: prefer refs/heads/main, then +// master, then trunk; otherwise the lexicographically smallest branch so the +// choice is deterministic. Returns "" when the repo has no branches. +func pickDefaultBranch(st storage.Storer) (plumbing.ReferenceName, error) { + iter, err := st.IterReferences() + if err != nil { + return "", err + } + defer iter.Close() + + rank := map[plumbing.ReferenceName]int{ + plumbing.Main: 0, + plumbing.Master: 1, + plumbing.NewBranchReferenceName("trunk"): 2, + } + var ( + first plumbing.ReferenceName + best plumbing.ReferenceName + bestRank = len(rank) + ) + err = iter.ForEach(func(r *plumbing.Reference) error { + if r.Type() != plumbing.HashReference || !r.Name().IsBranch() { + return nil + } + name := r.Name() + if first == "" || name < first { + first = name + } + if rk, ok := rank[name]; ok && rk < bestRank { + best, bestRank = name, rk + } + return nil + }) + if err != nil { + return "", err + } + if best != "" { + return best, nil + } + return first, nil +} + // loadOrInit returns the storer for repoPath, creating an empty bare repository // on demand. Git's daemon never auto-creates; objgitd does, so a first push to // a new path just works. func (d *daemon) loadOrInit(repoPath string) (storage.Storer, error) { - st, err := d.loader.Load(&url.URL{Path: repoPath}) + st, err := d.load(repoPath) if err == nil { return st, nil } diff --git a/cmd/objgitd/head_test.go b/cmd/objgitd/head_test.go new file mode 100644 index 0000000..26a4543 --- /dev/null +++ b/cmd/objgitd/head_test.go @@ -0,0 +1,233 @@ +package main + +import ( + "io" + "net/http" + "net/http/httptest" + "os/exec" + "path/filepath" + "strings" + "testing" + + "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/memfs" + "github.com/go-git/go-git/v6/plumbing" + "github.com/go-git/go-git/v6/plumbing/cache" + "github.com/go-git/go-git/v6/plumbing/transport" + "github.com/go-git/go-git/v6/storage/filesystem" + "tangled.org/xeiaso.net/objgit/internal/auth" +) + +// dummyHash is a stand-in object id for branch refs in unit tests; ensureHEAD +// never dereferences it, it only needs the refs to exist. +var dummyHash = plumbing.NewHash("1111111111111111111111111111111111111111") + +// TestEnsureHEAD exercises the dangling-HEAD heal in isolation: objgitd inits +// every repo with HEAD -> refs/heads/main, so a repo populated by pushing a +// project whose default branch differs (golang/go uses master) leaves HEAD +// pointing at a branch that does not exist, and clients cannot check out a +// worktree. ensureHEAD repoints HEAD at an existing branch. +func TestEnsureHEAD(t *testing.T) { + tests := []struct { + name string + branches []string // branch short names to create + head *plumbing.Reference // initial HEAD + wantTarget plumbing.ReferenceName + wantHash plumbing.Hash // non-zero ⇒ expect HEAD to stay this detached hash + }{ + { + name: "dangling main heals to master", + branches: []string{"master"}, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main), + wantTarget: plumbing.Master, + }, + { + name: "valid head left unchanged", + branches: []string{"master"}, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Master), + wantTarget: plumbing.Master, + }, + { + name: "detached head left unchanged", + branches: []string{"master"}, + head: plumbing.NewHashReference(plumbing.HEAD, dummyHash), + wantHash: dummyHash, + }, + { + name: "no branches leaves head as-is", + branches: nil, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main), + wantTarget: plumbing.Main, + }, + { + name: "prefers main when both present", + branches: []string{"master", "main"}, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.NewBranchReferenceName("trunk")), + wantTarget: plumbing.Main, + }, + { + name: "prefers master over other branches when main absent", + branches: []string{"zzz", "master"}, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main), + wantTarget: plumbing.Master, + }, + { + name: "falls back to lexicographically smallest branch", + branches: []string{"zebra", "alpha", "mango"}, + head: plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main), + wantTarget: plumbing.NewBranchReferenceName("alpha"), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + st := filesystem.NewStorage(memfs.New(), cache.NewObjectLRUDefault()) + for _, b := range tt.branches { + ref := plumbing.NewHashReference(plumbing.NewBranchReferenceName(b), dummyHash) + if err := st.SetReference(ref); err != nil { + t.Fatalf("seed branch %q: %v", b, err) + } + } + if err := st.SetReference(tt.head); err != nil { + t.Fatalf("seed HEAD: %v", err) + } + + if err := ensureHEAD(st); err != nil { + t.Fatalf("ensureHEAD: %v", err) + } + + got, err := st.Reference(plumbing.HEAD) + if err != nil { + t.Fatalf("read HEAD back: %v", err) + } + if !tt.wantHash.IsZero() { + if got.Type() != plumbing.HashReference || got.Hash() != tt.wantHash { + t.Errorf("HEAD = %v %q, want detached hash %s", got.Type(), got.Hash(), tt.wantHash) + } + return + } + if got.Type() != plumbing.SymbolicReference || got.Target() != tt.wantTarget { + t.Errorf("HEAD target = %q (type %v), want %q", got.Target(), got.Type(), tt.wantTarget) + } + }) + } +} + +// TestEnsureHEADIdempotent verifies a second heal is a no-op once HEAD resolves. +func TestEnsureHEADIdempotent(t *testing.T) { + st := filesystem.NewStorage(memfs.New(), cache.NewObjectLRUDefault()) + if err := st.SetReference(plumbing.NewHashReference(plumbing.Master, dummyHash)); err != nil { + t.Fatalf("seed branch: %v", err) + } + if err := st.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main)); err != nil { + t.Fatalf("seed HEAD: %v", err) + } + + for i := range 2 { + if err := ensureHEAD(st); err != nil { + t.Fatalf("ensureHEAD pass %d: %v", i, err) + } + got, err := st.Reference(plumbing.HEAD) + if err != nil { + t.Fatalf("read HEAD pass %d: %v", i, err) + } + if got.Target() != plumbing.Master { + t.Errorf("pass %d: HEAD target = %q, want %q", i, got.Target(), plumbing.Master) + } + } +} + +// TestSmartHTTPHealsDanglingHEAD reproduces the reported bug end-to-end: a repo +// already sitting in the bucket whose HEAD points at a nonexistent branch (the +// init default refs/heads/main, while only master was pushed — as for a mirror +// of golang/go) must heal on the next clone so git checks out a worktree instead +// of printing "remote HEAD refers to nonexistent ref, unable to checkout". +func TestSmartHTTPHealsDanglingHEAD(t *testing.T) { + if _, err := exec.LookPath("git"); err != nil { + t.Skip("git not installed") + } + + fs := memfs.New() + ts := httptest.NewServer(&daemon{ + fs: fs, + loader: transport.NewFilesystemLoader(fs, false), + authz: auth.AllowAnonymous{AllowWrite: true}, + }) + t.Cleanup(ts.Close) + + // Push a single "master" branch (no "main"), like a project whose default + // branch is master. + work := t.TempDir() + runGit(t, work, "init", "-b", "master") + runGit(t, work, "config", "user.email", "test@example.com") + runGit(t, work, "config", "user.name", "Test") + writeFile(t, filepath.Join(work, "README.md"), "hello\n") + runGit(t, work, "add", ".") + runGit(t, work, "commit", "-m", "initial") + if out, err := tryGit(work, "push", ts.URL+"/go.git", "master"); err != nil { + t.Fatalf("push failed: %v\n%s", err, out) + } + + // Re-break HEAD to simulate a repo created before this fix (post-push heal + // would otherwise have already fixed it): point HEAD back at the dangling + // refs/heads/main directly in the backing store. The very next load (this + // clone) must heal it on the way to serving the advertisement. + breakHEAD(t, fs, "/go.git") + + dst := t.TempDir() + out, err := tryGit(dst, "clone", ts.URL+"/go.git", "cloned") + if err != nil { + t.Fatalf("clone failed: %v\n%s", err, out) + } + if strings.Contains(out, "nonexistent ref") { + t.Errorf("clone still warned about nonexistent HEAD ref; output:\n%s", out) + } + + cloned := filepath.Join(dst, "cloned") + if _, err := exec.Command("git", "-C", cloned, "rev-parse", "HEAD").Output(); err != nil { + t.Fatalf("cloned repo has no checked-out HEAD: %v", err) + } + if got := strings.TrimSpace(runGit(t, cloned, "rev-parse", "--abbrev-ref", "HEAD")); got != "master" { + t.Errorf("checked out branch = %q, want master", got) + } + if _, err := exec.Command("git", "-C", cloned, "cat-file", "-e", "HEAD:README.md").Output(); err != nil { + t.Errorf("worktree missing README.md after clone: %v", err) + } + + // After a load-healed clone, the advertisement now carries the symref. + if body := getInfoRefs(t, ts.URL+"/go.git"); !strings.Contains(body, "symref=HEAD:refs/heads/master") { + t.Errorf("expected symref=HEAD:refs/heads/master after heal; advertisement:\n%q", body) + } +} + +// breakHEAD points the bare repo at repoPath's HEAD back at the dangling +// refs/heads/main, simulating a repository created before the heal existed. +func breakHEAD(t *testing.T, fs billy.Filesystem, repoPath string) { + t.Helper() + sub, err := fs.Chroot(repoPath) + if err != nil { + t.Fatalf("chroot %q: %v", repoPath, err) + } + st := filesystem.NewStorage(sub, cache.NewObjectLRUDefault()) + if err := st.SetReference(plumbing.NewSymbolicReference(plumbing.HEAD, plumbing.Main)); err != nil { + t.Fatalf("break HEAD: %v", err) + } + if _, err := st.Reference(plumbing.Main); err == nil { + t.Fatalf("test setup invalid: refs/heads/main exists, HEAD would not dangle") + } +} + +// getInfoRefs fetches the smart-HTTP upload-pack advertisement for repoURL. +func getInfoRefs(t *testing.T, repoURL string) string { + t.Helper() + resp, err := http.Get(repoURL + "/info/refs?service=git-upload-pack") + if err != nil { + t.Fatalf("GET info/refs: %v", err) + } + defer resp.Body.Close() + body, err := io.ReadAll(resp.Body) + if err != nil { + t.Fatalf("read info/refs: %v", err) + } + return string(body) +} diff --git a/cmd/objgitd/hooks.go b/cmd/objgitd/hooks.go index 4a76e31..4ccea4f 100644 --- a/cmd/objgitd/hooks.go +++ b/cmd/objgitd/hooks.go @@ -87,7 +87,9 @@ func diffRefs(before, after map[plumbing.ReferenceName]plumbing.Hash) []refUpdat // a capability-hiding wrapper. func (d *daemon) receivePack(ctx context.Context, st storage.Storer, repoPath string, r io.ReadCloser, w io.WriteCloser, req *transport.ReceivePackRequest) error { if !d.allowHooks { - return receivePackStreaming(ctx, st, r, w, req, nil) + err := receivePackStreaming(ctx, st, r, w, req, nil) + d.healHEADAfterPush(err, st, repoPath) + return err } before, err := snapshotRefs(st) @@ -112,7 +114,23 @@ func (d *daemon) receivePack(ctx context.Context, st storage.Storer, repoPath st d.runHooks(repoPath, "receive-pack", st, updates, progress) } - return receivePackStreaming(ctx, st, r, w, req, onUpdated) + err = receivePackStreaming(ctx, st, r, w, req, onUpdated) + d.healHEADAfterPush(err, st, repoPath) + return err +} + +// healHEADAfterPush repoints a dangling HEAD once a push succeeds, so the first +// push to a repo whose default branch is not main (e.g. golang/go uses master) +// leaves HEAD resolvable for the next clone. The HEAD write thus lands during the +// push rather than during a later clone. No-op when the receive failed or HEAD is +// already valid (see ensureHEAD). +func (d *daemon) healHEADAfterPush(recvErr error, st storage.Storer, repoPath string) { + if recvErr != nil { + return + } + if err := ensureHEAD(st); err != nil { + slog.Warn("could not repoint HEAD after push", "path", repoPath, "err", err) + } } // runHooks executes the receive-pack hook once per non-deleted branch update, diff --git a/cmd/objgitd/http.go b/cmd/objgitd/http.go index 9774ff2..7587245 100644 --- a/cmd/objgitd/http.go +++ b/cmd/objgitd/http.go @@ -7,7 +7,6 @@ import ( "io" "log/slog" "net/http" - "net/url" "strings" "time" @@ -187,7 +186,7 @@ func (d *daemon) resolve(w http.ResponseWriter, r *http.Request, service, repoPa return st, true } - st, err := d.loader.Load(&url.URL{Path: repoPath}) + st, err := d.load(repoPath) if err != nil { if errors.Is(err, transport.ErrRepositoryNotFound) { http.Error(w, "repository not found", http.StatusNotFound) diff --git a/cmd/objgitd/ssh.go b/cmd/objgitd/ssh.go index ed932fe..cea1363 100644 --- a/cmd/objgitd/ssh.go +++ b/cmd/objgitd/ssh.go @@ -8,7 +8,6 @@ import ( "fmt" "io" "log/slog" - "net/url" "os" "path/filepath" "strings" @@ -189,7 +188,7 @@ func (d *daemon) serveSSH(s ssh.Session, service, repoPath string) error { // forwarded yet; v0/v1 is sufficient. See plan. switch service { case transport.UploadPackService: - st, err := d.loader.Load(&url.URL{Path: repoPath}) + st, err := d.load(repoPath) if err != nil { fmt.Fprintf(s.Stderr(), "objgitd: repository %q not found\n", repoPath) _ = s.Exit(1) @@ -201,7 +200,7 @@ func (d *daemon) serveSSH(s ssh.Session, service, repoPath string) error { } case transport.UploadArchiveService: - st, err := d.loader.Load(&url.URL{Path: repoPath}) + st, err := d.load(repoPath) if err != nil { fmt.Fprintf(s.Stderr(), "objgitd: repository %q not found\n", repoPath) _ = s.Exit(1) -- 2.51.2